Implement thiserror for libcontainer - Part 3

crates/libcontainer/src/apparmor.rs

@@ -7,17 +7,18 @@ use std::{
 #[derive(Debug, thiserror::Error)]
 pub enum AppArmorError {
     #[error("failed to apply AppArmor profile")]
-    ApplyProfile {
+    ActivateProfile {
         path: std::path::PathBuf,
         profile: String,
-        // TODO: fix this after `utils` crate is migrated to `thiserror`
-        source: anyhow::Error,
+        source: std::io::Error,
     },
     #[error("failed to read AppArmor profile: {source} {path}")]
     ReadProfile {
         path: String,
         source: std::io::Error,
     },
+    #[error(transparent)]
+    EnsureProcfs(#[from] utils::EnsureProcfsError),
 }
 
 type Result<T> = std::result::Result<T, AppArmorError>;
@@ -51,12 +52,8 @@ pub fn apply_profile(profile: &str) -> Result<()> {
 }
 
 fn activate_profile(path: &Path, profile: &str) -> Result<()> {
-    utils::ensure_procfs(path).map_err(|err| AppArmorError::ApplyProfile {
-        path: path.to_owned(),
-        profile: profile.to_owned(),
-        source: err,
-    })?;
-    utils::write_file(path, format!("exec {profile}")).map_err(|err| AppArmorError::ApplyProfile {
+    utils::ensure_procfs(path).map_err(AppArmorError::EnsureProcfs)?;
+    fs::write(path, format!("exec {profile}")).map_err(|err| AppArmorError::ActivateProfile {
         path: path.to_owned(),
         profile: profile.to_owned(),
         source: err,

crates/libcontainer/src/seccomp/mod.rs

@@ -1,6 +1,3 @@
-use anyhow::bail;
-use anyhow::Context;
-use anyhow::Result;
 use libseccomp::ScmpAction;
 use libseccomp::ScmpArch;
 use libseccomp::ScmpArgCompare;
@@ -12,8 +9,52 @@ use oci_spec::runtime::LinuxSeccomp;
 use oci_spec::runtime::LinuxSeccompAction;
 use oci_spec::runtime::LinuxSeccompFilterFlag;
 use oci_spec::runtime::LinuxSeccompOperator;
+use std::num::TryFromIntError;
 use std::os::unix::io;
 
+#[derive(Debug, thiserror::Error)]
+pub enum SeccompError {
+    #[error("failed to translate trace action due to failed to convert errno {errno} into i16")]
+    TraceAction { source: TryFromIntError, errno: i32 },
+    #[error("SCMP_ACT_NOTIFY cannot be used as default action")]
+    NotifyAsDefaultAction,
+    #[error("SCMP_ACT_NOTIFY cannot be used for the write syscall")]
+    NotifyWriteSyscall,
+    #[error("failed to add arch to seccomp")]
+    AddArch {
+        source: libseccomp::error::SeccompError,
+        arch: Arch,
+    },
+    #[error("failed to load seccomp context")]
+    LoadContext {
+        source: libseccomp::error::SeccompError,
+    },
+    #[error("failed to get seccomp notify id")]
+    GetNotifyId {
+        source: libseccomp::error::SeccompError,
+    },
+    #[error("failed to add rule to seccomp")]
+    AddRule {
+        source: libseccomp::error::SeccompError,
+    },
+    #[error("failed to create new seccomp filter")]
+    NewFilter {
+        source: libseccomp::error::SeccompError,
+        default: LinuxSeccompAction,
+    },
+    #[error("failed to set filter flag")]
+    SetFilterFlag {
+        source: libseccomp::error::SeccompError,
+        flag: LinuxSeccompFilterFlag,
+    },
+    #[error("failed to set SCMP_FLTATR_CTL_NNP")]
+    SetCtlNnp {
+        source: libseccomp::error::SeccompError,
+    },
+}
+
+type Result<T> = std::result::Result<T, SeccompError>;
+
 fn translate_arch(arch: Arch) -> ScmpArch {
     match arch {
         Arch::ScmpArchNative => ScmpArch::Native,
@@ -42,7 +83,11 @@ fn translate_action(action: LinuxSeccompAction, errno: Option<u32>) -> Result<Sc
         LinuxSeccompAction::ScmpActKill => ScmpAction::KillThread,
         LinuxSeccompAction::ScmpActTrap => ScmpAction::Trap,
         LinuxSeccompAction::ScmpActErrno => ScmpAction::Errno(errno),
-        LinuxSeccompAction::ScmpActTrace => ScmpAction::Trace(errno.try_into()?),
+        LinuxSeccompAction::ScmpActTrace => ScmpAction::Trace(
+            errno
+                .try_into()
+                .map_err(|err| SeccompError::TraceAction { source: err, errno })?,
+        ),
         LinuxSeccompAction::ScmpActAllow => ScmpAction::Allow,
         LinuxSeccompAction::ScmpActKillProcess => ScmpAction::KillProcess,
         LinuxSeccompAction::ScmpActNotify => ScmpAction::Notify,
@@ -76,15 +121,15 @@ fn check_seccomp(seccomp: &LinuxSeccomp) -> Result<()> {
     // handle read/close syscall and allow read and close to proceed as
     // expected.
     if seccomp.default_action() == LinuxSeccompAction::ScmpActNotify {
-        bail!("SCMP_ACT_NOTIFY cannot be used as default action");
+        return Err(SeccompError::NotifyAsDefaultAction);
     }
 
     if let Some(syscalls) = seccomp.syscalls() {
         for syscall in syscalls {
             if syscall.action() == LinuxSeccompAction::ScmpActNotify {
                 for name in syscall.names() {
                     if name == "write" {
-                        bail!("SCMP_ACT_NOTIFY cannot be used for the write syscall");
+                        return Err(SeccompError::NotifyWriteSyscall);
                     }
                 }
             }
@@ -98,25 +143,30 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
     check_seccomp(seccomp)?;
 
     let default_action = translate_action(seccomp.default_action(), seccomp.default_errno_ret())?;
-    let mut ctx = ScmpFilterContext::new_filter(translate_action(
-        seccomp.default_action(),
-        seccomp.default_errno_ret(),
-    )?)?;
+    let mut ctx =
+        ScmpFilterContext::new_filter(default_action).map_err(|err| SeccompError::NewFilter {
+            source: err,
+            default: seccomp.default_action(),
+        })?;
 
     if let Some(flags) = seccomp.flags() {
         for flag in flags {
             match flag {
-                LinuxSeccompFilterFlag::SeccompFilterFlagLog => ctx.set_ctl_log(true)?,
-                LinuxSeccompFilterFlag::SeccompFilterFlagTsync => ctx.set_ctl_tsync(true)?,
-                LinuxSeccompFilterFlag::SeccompFilterFlagSpecAllow => ctx.set_ctl_ssb(true)?,
+                LinuxSeccompFilterFlag::SeccompFilterFlagLog => ctx.set_ctl_log(true),
+                LinuxSeccompFilterFlag::SeccompFilterFlagTsync => ctx.set_ctl_tsync(true),
+                LinuxSeccompFilterFlag::SeccompFilterFlagSpecAllow => ctx.set_ctl_ssb(true),
             }
+            .map_err(|err| SeccompError::SetFilterFlag {
+                source: err,
+                flag: *flag,
+            })?;
         }
     }
 
     if let Some(architectures) = seccomp.architectures() {
         for &arch in architectures {
             ctx.add_arch(translate_arch(arch))
-                .context("failed to add arch to seccomp")?;
+                .map_err(|err| SeccompError::AddArch { source: err, arch })?;
         }
     }
 
@@ -127,7 +177,8 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
     // set it here.  If the seccomp load operation fails without enough
     // privilege, so be it. To prevent this automatic behavior, we unset the
     // value here.
-    ctx.set_ctl_nnp(false)?;
+    ctx.set_ctl_nnp(false)
+        .map_err(|err| SeccompError::SetCtlNnp { source: err })?;
 
     if let Some(syscalls) = seccomp.syscalls() {
         for syscall in syscalls {
@@ -176,17 +227,20 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
                                 arg.value(),
                             );
                             ctx.add_rule_conditional(action, sc, &[cmp])
-                                .with_context(|| {
-                                    format!(
-                                        "failed to add seccomp action: {:?}. Cmp: {:?} Syscall: {name}",
-                                        &action, cmp,
-                                    )
+                                .map_err(|err| {
+                                    log::error!(
+                                        "failed to add seccomp action: {:?}. Cmp: {:?} Syscall: {name}", &action, cmp,
+                                    );
+                                    SeccompError::AddRule {
+                                        source: err,
+                                    }
                                 })?;
                         }
                     }
                     None => {
-                        ctx.add_rule(action, sc).with_context(|| {
-                            format!("failed to add seccomp rule: {:?}. Syscall: {name}", &sc)
+                        ctx.add_rule(action, sc).map_err(|err| {
+                            log::error!("failed to add seccomp rule: {:?}. Syscall: {name}", &sc);
+                            SeccompError::AddRule { source: err }
                         })?;
                     }
                 }
@@ -198,12 +252,13 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<Option<io::RawFd>> {
     // thread must have the CAP_SYS_ADMIN capability in its user namespace, or
     // the thread must already have the no_new_privs bit set.
     // Ref: https://man7.org/linux/man-pages/man2/seccomp.2.html
-    ctx.load().context("failed to load seccomp context")?;
+    ctx.load()
+        .map_err(|err| SeccompError::LoadContext { source: err })?;
 
     let fd = if is_notify(seccomp) {
         Some(
             ctx.get_notify_fd()
-                .context("failed to get seccomp notify fd")?,
+                .map_err(|err| SeccompError::GetNotifyId { source: err })?,
         )
     } else {
         None
@@ -224,7 +279,7 @@ pub fn is_notify(seccomp: &LinuxSeccomp) -> bool {
 mod tests {
     use super::*;
     use crate::utils::test_utils;
-    use anyhow::Result;
+    use anyhow::{bail, Context, Result};
     use oci_spec::runtime::Arch;
     use oci_spec::runtime::{LinuxSeccompBuilder, LinuxSyscallBuilder};
     use serial_test::serial;