Move miette towards edges of BFFH for more structured error reporting

bffhd/actors/mod.rs

@@ -12,9 +12,10 @@ use std::future::Future;
 
 use std::pin::Pin;
 
-use miette::IntoDiagnostic;
+use miette::{Diagnostic, IntoDiagnostic};
 use std::task::{Context, Poll};
 use std::time::Duration;
+use thiserror::Error;
 
 use once_cell::sync::Lazy;
 use rumqttc::ConnectReturnCode::Success;
@@ -111,11 +112,33 @@ static ROOT_CERTS: Lazy<RootCertStore> = Lazy::new(|| {
     store
 });
 
-pub fn load(executor: Executor, config: &Config, resources: ResourcesHandle) -> miette::Result<()> {
+#[derive(Debug, Error, Diagnostic)]
+pub enum ActorError {
+    #[error("failed to parse MQTT url")]
+    UrlParseError(
+        #[from]
+        #[source]
+        url::ParseError,
+    ),
+    #[error("MQTT config is invalid")]
+    InvalidConfig,
+    #[error("MQTT connection failed")]
+    ConnectionError(
+        #[from]
+        #[source]
+        rumqttc::ConnectionError,
+    ),
+}
+
+pub fn load(
+    executor: Executor,
+    config: &Config,
+    resources: ResourcesHandle,
+) -> Result<(), ActorError> {
     let span = tracing::info_span!("loading actors");
     let _guard = span;
 
-    let mqtt_url = Url::parse(config.mqtt_url.as_str()).into_diagnostic()?;
+    let mqtt_url = Url::parse(config.mqtt_url.as_str())?;
     let (transport, default_port) = match mqtt_url.scheme() {
         "mqtts" | "ssl" => (
             rumqttc::Transport::tls_with_config(
@@ -132,12 +155,12 @@ pub fn load(executor: Executor, config: &Config, resources: ResourcesHandle) ->
 
         scheme => {
             tracing::error!(%scheme, "MQTT url uses invalid scheme");
-            miette::bail!("invalid config");
+            return Err(ActorError::InvalidConfig);
         }
     };
     let host = mqtt_url.host_str().ok_or_else(|| {
         tracing::error!("MQTT url must contain a hostname");
-        miette::miette!("invalid config")
+        ActorError::InvalidConfig
     })?;
     let port = mqtt_url.port().unwrap_or(default_port);
 
@@ -168,7 +191,7 @@ pub fn load(executor: Executor, config: &Config, resources: ResourcesHandle) ->
                 }
                 Err(error) => {
                     tracing::error!(?error, "MQTT connection failed");
-                    miette::bail!("mqtt connection failed")
+                    return Err(ActorError::ConnectionError(error));
                 }
             }
 

bffhd/audit.rs

@@ -1,8 +1,10 @@
+use miette::Diagnostic;
 use once_cell::sync::OnceCell;
 use std::fs::{File, OpenOptions};
 use std::io;
 use std::io::{LineWriter, Write};
 use std::sync::Mutex;
+use thiserror::Error;
 
 use crate::Config;
 use serde::{Deserialize, Serialize};
@@ -23,8 +25,13 @@ pub struct AuditLogLine<'a> {
     state: &'a str,
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[error(transparent)]
+#[repr(transparent)]
+pub struct Error(#[from] pub io::Error);
+
 impl AuditLog {
-    pub fn new(config: &Config) -> io::Result<&'static Self> {
+    pub fn new(config: &Config) -> Result<&'static Self, Error> {
         AUDIT.get_or_try_init(|| {
             tracing::debug!(path = %config.auditlog_path.display(), "Initializing audit log");
             let fd = OpenOptions::new()

bffhd/authentication/mod.rs

@@ -66,9 +66,9 @@ impl SessionCallback for Callback {
                         .get_ref::<Password>()
                         .ok_or(ValidationError::MissingRequiredProperty)?;
 
-                    // if authzid.is_some() {
+                    if authzid.is_some() {
-                    //     return Ok(())
+                        return Ok(());
-                    // }
+                    }
 
                     if let Some(user) = self.users.get_user(authcid) {
                         match user.check_password(password) {

bffhd/capnp/mod.rs

@@ -1,5 +1,7 @@
-use async_net::TcpListener;
+use miette::Diagnostic;
+use thiserror::Error;
 
+use async_net::TcpListener;
 use capnp_rpc::rpc_twoparty_capnp::Side;
 use capnp_rpc::twoparty::VatNetwork;
 use capnp_rpc::RpcSystem;
@@ -37,6 +39,10 @@ pub struct APIServer {
     authentication: AuthenticationHandle,
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[error("Reached Void error, this should not be possible")]
+pub enum Error {}
+
 impl APIServer {
     pub fn new(
         executor: Executor<'static>,
@@ -60,7 +66,7 @@ impl APIServer {
         acceptor: TlsAcceptor,
         sessionmanager: SessionManager,
         authentication: AuthenticationHandle,
-    ) -> miette::Result<Self> {
+    ) -> Result<Self, Error> {
         let span = tracing::info_span!("binding API listen sockets");
         let _guard = span.enter();
 

bffhd/db/mod.rs

@@ -13,9 +13,9 @@ pub type ErrorO = lmdb::Error;
 
 pub type Result<T> = std::result::Result<T, Error>;
 
-#[repr(transparent)]
+#[derive(Clone, Debug, PartialEq, Eq, Error)]
-#[derive(Debug, Error)]
 #[error(transparent)]
+#[repr(transparent)]
 pub struct Error(#[from] lmdb::Error);
 
 impl Diagnostic for Error {

bffhd/lib.rs

@@ -8,6 +8,10 @@
 //! This is the capnp component of the FabAccess project.
 //! The entry point of bffhd can be found in [bin/bffhd/main.rs](../bin/bffhd/main.rs)
 
+use miette::Diagnostic;
+use std::io;
+use thiserror::Error;
+
 pub mod config;
 
 /// Internal Databases build on top of LMDB, a mmap()'ed B-tree DB optimized for reads
@@ -82,10 +86,58 @@ impl error::Description for SignalHandlerErr {
     const CODE: &'static str = "signals::new";
 }
 
+#[derive(Debug, Error, Diagnostic)]
+pub enum BFFHError {
+    #[error("DB operation failed")]
+    DBError(
+        #[from]
+        #[source]
+        db::Error,
+    ),
+    #[error("failed to initialize global user store")]
+    UsersError(
+        #[from]
+        #[source]
+        users::Error,
+    ),
+    #[error("failed to initialize state database")]
+    StateDBError(
+        #[from]
+        #[source]
+        resources::state::db::StateDBError,
+    ),
+    #[error("audit log failed")]
+    AuditLogError(
+        #[from]
+        #[source]
+        audit::Error,
+    ),
+    #[error("Failed to initialize signal handler")]
+    SignalsError(#[source] std::io::Error),
+    #[error("error in actor subsystem")]
+    ActorError(
+        #[from]
+        #[source]
+        actors::ActorError,
+    ),
+    #[error("failed to initialize TLS config")]
+    TlsSetup(
+        #[from]
+        #[source]
+        tls::Error,
+    ),
+    #[error("API handler failed")]
+    ApiError(
+        #[from]
+        #[source]
+        capnp::Error,
+    ),
+}
+
 impl Diflouroborane {
     pub fn setup() {}
 
-    pub fn new(config: Config) -> miette::Result<Self> {
+    pub fn new(config: Config) -> Result<Self, BFFHError> {
         let mut server = logging::init(&config.logging);
         let span = tracing::info_span!(
             target: "bffh",
@@ -121,9 +173,7 @@ impl Diflouroborane {
         let users = Users::new(env.clone())?;
         let roles = Roles::new(config.roles.clone());
 
-        let _audit_log = AuditLog::new(&config)
+        let _audit_log = AuditLog::new(&config)?;
-            .into_diagnostic()
-            .wrap_err("Failed to initialize audit log")?;
 
         let resources = ResourcesHandle::new(config.machines.iter().map(|(id, desc)| {
             Resource::new(Arc::new(resources::Inner::new(
@@ -145,10 +195,10 @@ impl Diflouroborane {
         })
     }
 
-    pub fn run(&mut self) -> miette::Result<()> {
+    pub fn run(&mut self) -> Result<(), BFFHError> {
         let _guard = self.span.enter();
         let mut signals = signal_hook_async_std::Signals::new(&[SIGINT, SIGQUIT, SIGTERM])
-            .map_err(|ioerr| error::wrap::<SignalHandlerErr>(ioerr.into()))?;
+            .map_err(BFFHError::SignalsError)?;
 
         let sessionmanager = SessionManager::new(self.users.clone(), self.roles.clone());
         let authentication = AuthenticationHandle::new(self.users.clone());
@@ -162,8 +212,7 @@ impl Diflouroborane {
         );
         actors::load(self.executor.clone(), &self.config, self.resources.clone())?;
 
-        let tlsconfig = TlsConfig::new(self.config.tlskeylog.as_ref(), !self.config.is_quiet())
+        let tlsconfig = TlsConfig::new(self.config.tlskeylog.as_ref(), !self.config.is_quiet())?;
-            .into_diagnostic()?;
         let acceptor = tlsconfig.make_tls_acceptor(&self.config.tlsconfig)?;
 
         let apiserver = self.executor.run(APIServer::bind(

bffhd/resources/state/db.rs

@@ -17,7 +17,7 @@ pub struct StateDB {
     db: DB<AlignedAdapter<State>>,
 }
 
-#[derive(Debug, Error, Diagnostic)]
+#[derive(Clone, Debug, PartialEq, Eq, Error, Diagnostic)]
 pub enum StateDBError {
     #[error("opening the state db environment failed")]
     #[diagnostic(

bffhd/tls.rs

@@ -1,17 +1,19 @@
 use std::fs::File;
 use std::io;
 use std::io::BufReader;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use crate::capnp::TlsListen;
 use futures_rustls::TlsAcceptor;
-use miette::IntoDiagnostic;
+use miette::Diagnostic;
 use rustls::version::{TLS12, TLS13};
 use rustls::{Certificate, PrivateKey, ServerConfig, SupportedCipherSuite};
+use thiserror::Error;
 use tracing::Level;
 
 use crate::keylog::KeyLogFile;
+use crate::tls::Error::KeyLogOpen;
 
 fn lookup_cipher_suite(name: &str) -> Option<SupportedCipherSuite> {
     match name {
@@ -47,8 +49,32 @@ pub struct TlsConfig {
     keylog: Option<Arc<KeyLogFile>>,
 }
 
+#[derive(Debug, Error, Diagnostic)]
+pub enum Error {
+    #[error("failed to open certificate file at path {0}")]
+    OpenCertFile(PathBuf, #[source] io::Error),
+    #[error("failed to open private key file at path {0}")]
+    OpenKeyFile(PathBuf, #[source] io::Error),
+    #[error("failed to read system certs")]
+    SystemCertsFile(#[source] io::Error),
+    #[error("failed to read from key file")]
+    ReadKeyFile(#[source] io::Error),
+    #[error("private key file must contain a single PEM-encoded private key")]
+    KeyFileFormat,
+    #[error("invalid TLS version {0}")]
+    TlsVersion(String),
+    #[error("Initializing TLS context failed")]
+    Builder(
+        #[from]
+        #[source]
+        rustls::Error,
+    ),
+    #[error("failed to initialize key log")]
+    KeyLogOpen(#[source] io::Error),
+}
+
 impl TlsConfig {
-    pub fn new(keylogfile: Option<impl AsRef<Path>>, warn: bool) -> io::Result<Self> {
+    pub fn new(keylogfile: Option<impl AsRef<Path>>, warn: bool) -> Result<Self, Error> {
         let span = tracing::span!(Level::INFO, "tls");
         let _guard = span.enter();
 
@@ -57,7 +83,11 @@ impl TlsConfig {
         }
 
         if let Some(path) = keylogfile {
-            let keylog = Some(KeyLogFile::new(path).map(|ok| Arc::new(ok))?);
+            let keylog = Some(
+                KeyLogFile::new(path)
+                    .map(|ok| Arc::new(ok))
+                    .map_err(KeyLogOpen)?,
+            );
             Ok(Self { keylog })
         } else {
             Ok(Self { keylog: None })
@@ -75,27 +105,31 @@ impl TlsConfig {
         }
     }
 
-    pub fn make_tls_acceptor(&self, config: &TlsListen) -> miette::Result<TlsAcceptor> {
+    pub fn make_tls_acceptor(&self, config: &TlsListen) -> Result<TlsAcceptor, Error> {
         let span = tracing::debug_span!("tls");
         let _guard = span.enter();
 
-        tracing::debug!(path = %config.certfile.as_path().display(), "reading certificates");
+        let path = config.certfile.as_path();
-        let mut certfp = BufReader::new(File::open(config.certfile.as_path()).into_diagnostic()?);
+        tracing::debug!(path = %path.display(), "reading certificates");
+        let mut certfp =
+            BufReader::new(File::open(path).map_err(|e| Error::OpenCertFile(path.into(), e))?);
         let certs = rustls_pemfile::certs(&mut certfp)
-            .into_diagnostic()?
+            .map_err(Error::SystemCertsFile)?
             .into_iter()
             .map(Certificate)
             .collect();
 
-        tracing::debug!(path = %config.keyfile.as_path().display(), "reading private key");
+        let path = config.keyfile.as_path();
-        let mut keyfp = BufReader::new(File::open(config.keyfile.as_path()).into_diagnostic()?);
+        tracing::debug!(path = %path.display(), "reading private key");
-        let key = match rustls_pemfile::read_one(&mut keyfp).into_diagnostic()? {
+        let mut keyfp =
+            BufReader::new(File::open(path).map_err(|err| Error::OpenKeyFile(path.into(), err))?);
+        let key = match rustls_pemfile::read_one(&mut keyfp).map_err(Error::ReadKeyFile)? {
             Some(rustls_pemfile::Item::PKCS8Key(key) | rustls_pemfile::Item::RSAKey(key)) => {
                 PrivateKey(key)
             }
             _ => {
                 tracing::error!("private key file invalid");
-                miette::bail!("private key file must contain a PEM-encoded private key")
+                return Err(Error::KeyFileFormat);
             }
         };
 
@@ -104,20 +138,19 @@ impl TlsConfig {
             .with_safe_default_kx_groups();
 
         let tls_builder = if let Some(ref min) = config.tls_min_version {
-            match min.as_str() {
+            let v = min.to_lowercase();
+            match v.as_str() {
                 "tls12" => tls_builder.with_protocol_versions(&[&TLS12]),
                 "tls13" => tls_builder.with_protocol_versions(&[&TLS13]),
-                x => miette::bail!("TLS version {} is invalid", x),
+                _ => return Err(Error::TlsVersion(v)),
             }
         } else {
             tls_builder.with_safe_default_protocol_versions()
-        }
+        }?;
-        .into_diagnostic()?;
 
         let mut tls_config = tls_builder
             .with_no_client_auth()
-            .with_single_cert(certs, key)
+            .with_single_cert(certs, key)?;
-            .into_diagnostic()?;
 
         if let Some(keylog) = &self.keylog {
             tls_config.key_log = keylog.clone();

bffhd/users/db.rs

@@ -11,6 +11,8 @@ use rkyv::ser::serializers::AllocSerializer;
 use rkyv::ser::Serializer;
 use rkyv::Deserialize;
 
+pub use crate::db::Error;
+
 #[derive(
     Clone,
     PartialEq,

bffhd/users/mod.rs

@@ -11,6 +11,7 @@ use clap::ArgMatches;
 use miette::{Context, Diagnostic, IntoDiagnostic, SourceOffset, SourceSpan};
 use std::path::Path;
 use std::sync::Arc;
+
 use thiserror::Error;
 
 pub mod db;
@@ -69,17 +70,20 @@ pub struct Users {
     userdb: &'static UserDB,
 }
 
+#[derive(Clone, Debug, PartialEq, Eq, Error, Diagnostic)]
+#[error(transparent)]
+#[repr(transparent)]
+pub struct Error(#[from] pub db::Error);
+
 impl Users {
-    pub fn new(env: Arc<Environment>) -> miette::Result<Self> {
+    pub fn new(env: Arc<Environment>) -> Result<Self, Error> {
         let span = tracing::debug_span!("users", ?env, "Creating Users handle");
         let _guard = span.enter();
 
-        let userdb = USERDB
+        let userdb = USERDB.get_or_try_init(|| {
-            .get_or_try_init(|| {
+            tracing::debug!("Global resource not yet initialized, initializing…");
-                tracing::debug!("Global resource not yet initialized, initializing…");
+            unsafe { UserDB::create(env) }
-                unsafe { UserDB::create(env) }
+        })?;
-            })
-            .wrap_err("Failed to open userdb")?;
 
         Ok(Self { userdb })
     }