Skip to content

Commit

Permalink
Stop using the Error struct from acme_common in tacd
Browse files Browse the repository at this point in the history
rel #83
  • Loading branch information
breard-r committed Jul 2, 2023
1 parent e302789 commit e9522df
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 15 deletions.
28 changes: 28 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions tacd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ openssl_vendored = ["crypto_openssl", "acme_common/openssl_vendored"]

[dependencies]
acme_common = { path = "../acme_common" }
anyhow = "1.0"
clap = { version = "4.0", features = ["string"] }
log = "0.4"
openssl = "0.10"
thiserror = "1.0"
21 changes: 13 additions & 8 deletions tacd/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ mod openssl_server;
#[cfg(feature = "crypto_openssl")]
use crate::openssl_server::start as server_start;
use acme_common::crypto::{get_lib_name, get_lib_version, HashFunction, KeyType, X509Certificate};
use acme_common::error::Error;
use acme_common::logs::{set_log_system, DEFAULT_LOG_LEVEL};
use acme_common::{clean_pid_file, to_idna};
use anyhow::{anyhow, Result};
use clap::builder::PossibleValuesParser;
use clap::{Arg, ArgAction, ArgMatches, Command};
use log::{debug, error, info};
Expand All @@ -21,7 +21,7 @@ const DEFAULT_CRT_KEY_TYPE: KeyType = KeyType::EcdsaP256;
const DEFAULT_CRT_DIGEST: HashFunction = HashFunction::Sha256;
const ALPN_ACME_PROTO_NAME: &[u8] = b"\x0aacme-tls/1";

fn read_line(path: Option<&String>) -> Result<String, Error> {
fn read_line(path: Option<&String>) -> Result<String> {
let mut input = String::new();
match path {
Some(p) => File::open(p)?.read_to_string(&mut input)?,
Expand All @@ -31,7 +31,7 @@ fn read_line(path: Option<&String>) -> Result<String, Error> {
Ok(line)
}

fn get_acme_value(cnf: &ArgMatches, opt: &str, opt_file: &str) -> Result<String, Error> {
fn get_acme_value(cnf: &ArgMatches, opt: &str, opt_file: &str) -> Result<String> {
match cnf.get_one::<String>(opt) {
Some(v) => Ok(v.to_string()),
None => {
Expand All @@ -46,27 +46,32 @@ fn get_acme_value(cnf: &ArgMatches, opt: &str, opt_file: &str) -> Result<String,
}
}

fn init(cnf: &ArgMatches) -> Result<(), Error> {
fn init(cnf: &ArgMatches) -> Result<()> {
acme_common::init_server(
cnf.get_flag("foreground"),
cnf.get_one::<String>("pid-file").map(|e| e.as_str()),
);
let domain = get_acme_value(cnf, "domain", "domain-file")?;
let domain = to_idna(&domain)?;
let domain = to_idna(&domain).map_err(|e| anyhow!(e))?;
let ext = get_acme_value(cnf, "acme-ext", "acme-ext-file")?;
let listen_addr = cnf
.get_one::<String>("listen")
.map(|e| e.as_str())
.unwrap_or(DEFAULT_LISTEN_ADDR);
let crt_signature_alg = match cnf.get_one::<&str>("crt-signature-alg") {
Some(alg) => alg.parse()?,
Some(alg) => alg
.parse()
.map_err(|e: acme_common::error::Error| anyhow!(e))?,

This comment has been minimized.

Copy link
@jcgruenhage

jcgruenhage Jul 3, 2023

Contributor

All of these map_err lines with anyhow!(e) should be unnecessary if the error returned implements the Error trait from the stdlib. That'd be easily possible when using thiserror. I've seen you added it to the deps here, but I'm not seeing it used actually?

This comment has been minimized.

Copy link
@breard-r

breard-r Jul 3, 2023

Author Owner

This is a temporary state, all of these map_err have been put on parts that comes from acme_common. Those parts will be replaced and therefore the anyhow!(e) will be gone too.

None => DEFAULT_CRT_KEY_TYPE,
};
let crt_digest = match cnf.get_one::<&str>("crt-digest") {
Some(alg) => alg.parse()?,
Some(alg) => alg
.parse()
.map_err(|e: acme_common::error::Error| anyhow!(e))?,
None => DEFAULT_CRT_DIGEST,
};
let (pk, cert) = X509Certificate::from_acme_ext(&domain, &ext, crt_signature_alg, crt_digest)?;
let (pk, cert) = X509Certificate::from_acme_ext(&domain, &ext, crt_signature_alg, crt_digest)
.map_err(|e| anyhow!(e))?;
info!("starting {APP_NAME} on {listen_addr} for {domain}");
server_start(listen_addr, &cert, &pk)?;
Ok(())
Expand Down
10 changes: 3 additions & 7 deletions tacd/src/openssl_server.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use acme_common::crypto::{KeyPair, X509Certificate};
use acme_common::error::Error;
use anyhow::{bail, Result};
use log::debug;
use openssl::ssl::{self, AlpnError, SslAcceptor, SslMethod};
use std::net::TcpListener;
Expand Down Expand Up @@ -29,11 +29,7 @@ macro_rules! listen_and_accept {
};
}

pub fn start(
listen_addr: &str,
certificate: &X509Certificate,
key_pair: &KeyPair,
) -> Result<(), Error> {
pub fn start(listen_addr: &str, certificate: &X509Certificate, key_pair: &KeyPair) -> Result<()> {
let mut acceptor = SslAcceptor::mozilla_intermediate(SslMethod::tls())?;
acceptor.set_alpn_select_callback(|_, client| {
debug!("ALPN negociation");
Expand All @@ -51,5 +47,5 @@ pub fn start(
debug!("listening on {listen_addr}");
listen_and_accept!(TcpListener, listen_addr, acceptor);
}
Err("main thread loop unexpectedly exited".into())
bail!("main thread loop unexpectedly exited")
}

0 comments on commit e9522df

Please sign in to comment.