From 9968af0f84295706a4d08b6f9c7fa567b781fc97 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Sat, 6 May 2023 03:20:27 +0000 Subject: [PATCH 1/4] Implement TTY error Signed-off-by: yihuaf --- crates/libcontainer/src/tty.rs | 133 ++++++++++++++++++++++++++------- 1 file changed, 104 insertions(+), 29 deletions(-) diff --git a/crates/libcontainer/src/tty.rs b/crates/libcontainer/src/tty.rs index d921e6e06..7a1b3efc7 100644 --- a/crates/libcontainer/src/tty.rs +++ b/crates/libcontainer/src/tty.rs @@ -1,21 +1,75 @@ //! tty (teletype) for user-system interaction +use nix::errno::Errno; +use nix::sys::socket::{self, UnixAddr}; +use nix::unistd::close; +use nix::unistd::dup2; use std::io::IoSlice; use std::os::unix::fs::symlink; use std::os::unix::io::AsRawFd; use std::os::unix::prelude::RawFd; -use std::path::Path; +use std::path::{Path, PathBuf}; -use anyhow::Context; -use anyhow::{bail, Result}; -use nix::errno::Errno; -use nix::sys::socket::{self, UnixAddr}; -use nix::unistd::close; -use nix::unistd::dup2; +use crate::error::UnifiedSyscallError; -const STDIN: i32 = 0; -const STDOUT: i32 = 1; -const STDERR: i32 = 2; +#[derive(Debug)] +pub enum StdIO { + Stdin = 0, + Stdout = 1, + Stderr = 2, +} + +impl From for i32 { + fn from(value: StdIO) -> Self { + match value { + StdIO::Stdin => 0, + StdIO::Stdout => 1, + StdIO::Stderr => 2, + } + } +} + +impl std::fmt::Display for StdIO { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + StdIO::Stdin => write!(f, "stdin"), + StdIO::Stdout => write!(f, "stdout"), + StdIO::Stderr => write!(f, "stderr"), + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum TTYError { + #[error("failed to connect/duplicate {stdio}")] + ConnectStdIO { source: nix::Error, stdio: StdIO }, + #[error("failed to create console socket")] + CreateConsoleSocket { + source: nix::Error, + socket_name: String, + }, + #[error("failed to symlink console socket into container_dir")] + Symlink { + source: std::io::Error, + linked: Box, + console_socket_path: Box, + }, + #[error("invalid socker name: {socket_name:?}")] + InvalidSocketName { + socket_name: String, + source: nix::Error, + }, + #[error("failed to create console socket fd")] + CreateConsoleSocketFd { source: UnifiedSyscallError }, + #[error("could not create pseudo terminal")] + CreatePseudoTerminal { source: nix::Error }, + #[error("failed to send pty master")] + SendPtyMaster { source: nix::Error }, + #[error("could not close console socket")] + CloseConsoleSocket { source: nix::Error }, +} + +type Result = std::result::Result; // TODO: Handling when there isn't console-socket. pub fn setup_console_socket( @@ -24,21 +78,31 @@ pub fn setup_console_socket( socket_name: &str, ) -> Result { let linked = container_dir.join(socket_name); - symlink(console_socket_path, linked)?; + symlink(console_socket_path, &linked).map_err(|err| TTYError::Symlink { + source: err, + linked: linked.to_path_buf().into(), + console_socket_path: console_socket_path.to_path_buf().into(), + })?; let mut csocketfd = socket::socket( socket::AddressFamily::Unix, socket::SockType::Stream, socket::SockFlag::empty(), None, - )?; - csocketfd = match socket::connect(csocketfd, &socket::UnixAddr::new(socket_name)?) { - Err(errno) => { - if !matches!(errno, Errno::ENOENT) { - bail!("failed to open {}", socket_name); - } - -1 - } + ) + .map_err(|err| TTYError::CreateConsoleSocketFd { source: err.into() })?; + csocketfd = match socket::connect( + csocketfd, + &socket::UnixAddr::new(socket_name).map_err(|err| TTYError::InvalidSocketName { + source: err, + socket_name: socket_name.to_string(), + })?, + ) { + Err(Errno::ENOENT) => -1, + Err(errno) => Err(TTYError::CreateConsoleSocket { + source: errno, + socket_name: socket_name.to_string(), + })?, Ok(()) => csocketfd, }; Ok(csocketfd) @@ -47,8 +111,8 @@ pub fn setup_console_socket( pub fn setup_console(console_fd: &RawFd) -> Result<()> { // You can also access pty master, but it is better to use the API. // ref. https://github.com/containerd/containerd/blob/261c107ffc4ff681bc73988f64e3f60c32233b37/vendor/github.com/containerd/go-runc/console.go#L139-L154 - let openpty_result = - nix::pty::openpty(None, None).context("could not create pseudo terminal")?; + let openpty_result = nix::pty::openpty(None, None) + .map_err(|err| TTYError::CreatePseudoTerminal { source: err })?; let pty_name: &[u8] = b"/dev/ptmx"; let iov = [IoSlice::new(pty_name)]; let fds = [openpty_result.master]; @@ -60,23 +124,34 @@ pub fn setup_console(console_fd: &RawFd) -> Result<()> { socket::MsgFlags::empty(), None, ) - .context("failed to send pty master")?; + .map_err(|err| TTYError::SendPtyMaster { source: err })?; if unsafe { libc::ioctl(openpty_result.slave, libc::TIOCSCTTY) } < 0 { log::warn!("could not TIOCSCTTY"); }; let slave = openpty_result.slave; - connect_stdio(&slave, &slave, &slave).context("could not dup tty to stderr")?; - close(console_fd.as_raw_fd()).context("could not close console socket")?; + connect_stdio(&slave, &slave, &slave)?; + close(console_fd.as_raw_fd()).map_err(|err| TTYError::CloseConsoleSocket { source: err })?; + Ok(()) } fn connect_stdio(stdin: &RawFd, stdout: &RawFd, stderr: &RawFd) -> Result<()> { - dup2(stdin.as_raw_fd(), STDIN)?; - dup2(stdout.as_raw_fd(), STDOUT)?; + dup2(stdin.as_raw_fd(), StdIO::Stdin.into()).map_err(|err| TTYError::ConnectStdIO { + source: err, + stdio: StdIO::Stdin, + })?; + dup2(stdout.as_raw_fd(), StdIO::Stdout.into()).map_err(|err| TTYError::ConnectStdIO { + source: err, + stdio: StdIO::Stdout, + })?; // FIXME: Rarely does it fail. // error message: `Error: Resource temporarily unavailable (os error 11)` - dup2(stderr.as_raw_fd(), STDERR)?; + dup2(stderr.as_raw_fd(), StdIO::Stderr.into()).map_err(|err| TTYError::ConnectStdIO { + source: err, + stdio: StdIO::Stderr, + })?; + Ok(()) } @@ -84,13 +159,13 @@ fn connect_stdio(stdin: &RawFd, stdout: &RawFd, stderr: &RawFd) -> Result<()> { mod tests { use super::*; + use anyhow::Result; + use serial_test::serial; use std::env; use std::fs::{self, File}; use std::os::unix::net::UnixListener; use std::path::PathBuf; - use serial_test::serial; - const CONSOLE_SOCKET: &str = "console-socket"; fn setup() -> Result<(tempfile::TempDir, PathBuf, PathBuf)> { From fb4facacb0e69a2d151d90f8fa51338cec544944 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Sat, 6 May 2023 04:26:08 +0000 Subject: [PATCH 2/4] implement util error Signed-off-by: yihuaf --- crates/libcontainer/src/utils.rs | 237 +++++++++++++++++++++++-------- 1 file changed, 180 insertions(+), 57 deletions(-) diff --git a/crates/libcontainer/src/utils.rs b/crates/libcontainer/src/utils.rs index 79322ef89..c9abd120d 100644 --- a/crates/libcontainer/src/utils.rs +++ b/crates/libcontainer/src/utils.rs @@ -1,7 +1,5 @@ //! Utility functionality -use anyhow::Context; -use anyhow::{bail, Result}; use nix::sys::stat::Mode; use nix::sys::statfs; use nix::unistd; @@ -15,24 +13,45 @@ use std::os::unix::fs::DirBuilderExt; use std::os::unix::prelude::{AsRawFd, OsStrExt}; use std::path::{Component, Path, PathBuf}; +#[derive(Debug, thiserror::Error)] +pub enum PathBufExtError { + #[error("relative path cannot be converted to the path in the container")] + RelativePath, + #[error("failed to strip prefix from {path:?}")] + StripPrefix { + path: PathBuf, + source: std::path::StripPrefixError, + }, + #[error("failed to canonicalize path {path:?}")] + Canonicalize { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to get current directory")] + CurrentDir { source: std::io::Error }, +} + pub trait PathBufExt { - fn as_relative(&self) -> Result<&Path>; - fn join_safely>(&self, p: P) -> Result; - fn canonicalize_safely(&self) -> Result; + fn as_relative(&self) -> Result<&Path, PathBufExtError>; + fn join_safely>(&self, p: P) -> Result; + fn canonicalize_safely(&self) -> Result; fn normalize(&self) -> PathBuf; } impl PathBufExt for Path { - fn as_relative(&self) -> Result<&Path> { - if self.is_relative() { - bail!("relative path cannot be converted to the path in the container.") - } else { - self.strip_prefix("/") - .with_context(|| format!("failed to strip prefix from {self:?}")) + fn as_relative(&self) -> Result<&Path, PathBufExtError> { + match self.is_relative() { + true => Err(PathBufExtError::RelativePath), + false => Ok(self + .strip_prefix("/") + .map_err(|e| PathBufExtError::StripPrefix { + path: self.to_path_buf(), + source: e, + })?), } } - fn join_safely>(&self, path: P) -> Result { + fn join_safely>(&self, path: P) -> Result { let path = path.as_ref(); if path.is_relative() { return Ok(self.join(path)); @@ -40,19 +59,25 @@ impl PathBufExt for Path { let stripped = path .strip_prefix("/") - .with_context(|| format!("failed to strip prefix from {}", path.display()))?; + .map_err(|e| PathBufExtError::StripPrefix { + path: self.to_path_buf(), + source: e, + })?; Ok(self.join(stripped)) } /// Canonicalizes existing and not existing paths - fn canonicalize_safely(&self) -> Result { + fn canonicalize_safely(&self) -> Result { if self.exists() { self.canonicalize() - .with_context(|| format!("failed to canonicalize path {self:?}")) + .map_err(|e| PathBufExtError::Canonicalize { + path: self.to_path_buf(), + source: e, + }) } else { if self.is_relative() { let p = std::env::current_dir() - .context("could not get current directory")? + .map_err(|e| PathBufExtError::CurrentDir { source: e })? .join(self); return Ok(p.normalize()); } @@ -120,14 +145,30 @@ pub fn get_user_home(uid: u32) -> Option { } } -pub fn do_exec(path: impl AsRef, args: &[String]) -> Result<()> { - let p = CString::new(path.as_ref().as_os_str().as_bytes()) - .with_context(|| format!("failed to convert path {:?} to cstring", path.as_ref()))?; - let a: Vec = args +#[derive(Debug, thiserror::Error)] +pub enum DoExecError { + #[error("failed to convert path to cstring")] + PathToCString { + source: std::ffi::NulError, + path: PathBuf, + }, + #[error("failed to execvp")] + Execvp { source: nix::Error }, +} + +pub fn do_exec(path: impl AsRef, args: &[String]) -> Result<(), DoExecError> { + let p = CString::new(path.as_ref().as_os_str().as_bytes()).map_err(|e| { + DoExecError::PathToCString { + source: e, + path: path.as_ref().to_path_buf(), + } + })?; + let cArgs: Vec = args .iter() .map(|s| CString::new(s.as_bytes()).unwrap_or_default()) .collect(); - unistd::execvp(&p, &a)?; + unistd::execvp(&p, &cArgs).map_err(|err| DoExecError::Execvp { source: err })?; + Ok(()) } @@ -146,20 +187,56 @@ pub fn get_cgroup_path( } } -pub fn write_file, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()> { - let path = path.as_ref(); - fs::write(path, contents).with_context(|| format!("failed to write to {path:?}"))?; - Ok(()) +#[derive(Debug, thiserror::Error)] +pub enum WrappedIOError { + #[error("failed to read from {path:?}")] + ReadFile { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to write to {path:?}")] + WriteFile { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to open {path:?}")] + Open { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to create directory {path:?}")] + CreateDirAll { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to get metadata")] + GetMetadata { source: std::io::Error }, + #[error("metada doesn't match the expected attributes")] + MetadataMismatch, } -pub fn create_dir_all>(path: P) -> Result<()> { - let path = path.as_ref(); - fs::create_dir_all(path).with_context(|| format!("failed to create directory {path:?}")) +pub fn write_file, C: AsRef<[u8]>>( + path: P, + contents: C, +) -> Result<(), WrappedIOError> { + fs::write(path.as_ref(), contents).map_err(|err| WrappedIOError::WriteFile { + source: err, + path: path.as_ref().to_path_buf(), + }) } -pub fn open>(path: P) -> Result { - let path = path.as_ref(); - File::open(path).with_context(|| format!("failed to open {path:?}")) +pub fn create_dir_all>(path: P) -> Result<(), WrappedIOError> { + fs::create_dir_all(path.as_ref()).map_err(|err| WrappedIOError::CreateDirAll { + source: err, + path: path.as_ref().to_path_buf(), + }) +} + +pub fn open>(path: P) -> Result { + File::open(path.as_ref()).map_err(|err| WrappedIOError::Open { + source: err, + path: path.as_ref().to_path_buf(), + }) } /// Creates the specified directory and all parent directories with the specified mode. Ensures @@ -175,19 +252,26 @@ pub fn open>(path: P) -> Result { /// create_dir_all_with_mode(&path, 1000, Mode::S_IRWXU).unwrap(); /// assert!(path.exists()) /// ``` -pub fn create_dir_all_with_mode>(path: P, owner: u32, mode: Mode) -> Result<()> { +pub fn create_dir_all_with_mode>( + path: P, + owner: u32, + mode: Mode, +) -> Result<(), WrappedIOError> { let path = path.as_ref(); if !path.exists() { DirBuilder::new() .recursive(true) .mode(mode.bits()) .create(path) - .with_context(|| format!("failed to create directory {}", path.display()))?; + .map_err(|err| WrappedIOError::CreateDirAll { + source: err, + path: path.to_path_buf(), + })?; } let metadata = path .metadata() - .with_context(|| format!("failed to get metadata for {}", path.display()))?; + .map_err(|err| WrappedIOError::GetMetadata { source: err })?; if metadata.is_dir() && metadata.st_uid() == owner @@ -195,27 +279,65 @@ pub fn create_dir_all_with_mode>(path: P, owner: u32, mode: Mode) { Ok(()) } else { - bail!( - "metadata for {} does not possess the expected attributes", - path.display() - ); + Err(WrappedIOError::MetadataMismatch) } } +#[derive(Debug, thiserror::Error)] +pub enum EnsureProcfsError { + #[error("failed to open {path:?}")] + OpenProcfs { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to get statfs for {path:?}")] + StatfsProcfs { source: nix::Error, path: PathBuf }, + #[error("{path:?} is not on the procfs")] + NotProcfs { path: PathBuf }, +} + // Make sure a given path is on procfs. This is to avoid the security risk that // /proc path is mounted over. Ref: CVE-2019-16884 -pub fn ensure_procfs(path: &Path) -> Result<()> { - let procfs_fd = fs::File::open(path)?; - let fstat_info = statfs::fstatfs(&procfs_fd.as_raw_fd())?; +pub fn ensure_procfs(path: &Path) -> Result<(), EnsureProcfsError> { + let procfs_fd = fs::File::open(path).map_err(|err| EnsureProcfsError::OpenProcfs { + source: err, + path: path.to_path_buf(), + })?; + let fstat_info = + statfs::fstatfs(&procfs_fd.as_raw_fd()).map_err(|err| EnsureProcfsError::StatfsProcfs { + source: err, + path: path.to_path_buf(), + })?; if fstat_info.filesystem_type() != statfs::PROC_SUPER_MAGIC { - bail!(format!("{path:?} is not on the procfs")); + return Err(EnsureProcfsError::NotProcfs { + path: path.to_path_buf(), + }); } Ok(()) } -pub fn secure_join>(rootfs: P, unsafe_path: P) -> Result { +#[derive(Debug, thiserror::Error)] +pub enum SecureJoinError { + #[error("dereference too many symlinks, may be infinite loop")] + TooManySymlinks, + #[error("unable to obtain symlink metadata")] + SymlinkMetadata { + source: std::io::Error, + path: PathBuf, + }, + #[error("failed to read link")] + ReadLink { + source: std::io::Error, + path: PathBuf, + }, +} + +pub fn secure_join>( + rootfs: P, + unsafe_path: P, +) -> Result { let mut rootfs = rootfs.into(); let mut path = unsafe_path.into(); let mut clean_path = PathBuf::new(); @@ -225,7 +347,7 @@ pub fn secure_join>(rootfs: P, unsafe_path: P) -> Result 255 { - bail!("dereference too many symlinks, may be infinite loop"); + return Err(SecureJoinError::TooManySymlinks); } let part_path = match part.next() { @@ -241,22 +363,22 @@ pub fn secure_join>(rootfs: P, unsafe_path: P) -> Result Some(metadata), - Err(error) => match error.kind() { - // if file does not exists, treat it as normal path - ErrorKind::NotFound => None, - other_error => { - bail!( - "unable to obtain symlink metadata for file {:?}: {:?}", - curr_path, - other_error - ); - } - }, + Err(err) if err.kind() == ErrorKind::NotFound => None, + Err(err) => { + return Err(SecureJoinError::SymlinkMetadata { + source: err, + path: curr_path, + }) + } }; if let Some(metadata) = metadata { if metadata.file_type().is_symlink() { - let link_path = fs::read_link(curr_path)?; + let link_path = + fs::read_link(curr_path).map_err(|err| SecureJoinError::ReadLink { + source: err, + path: curr_path, + })?; path = link_path.join(part.as_path()); part = path.iter(); @@ -290,7 +412,7 @@ pub fn get_executable_path(name: &str, path_var: &str) -> Option { None } -pub fn is_executable(path: &Path) -> Result { +pub fn is_executable(path: &Path) -> Result { use std::os::unix::fs::PermissionsExt; let metadata = path.metadata()?; let permissions = metadata.permissions(); @@ -356,6 +478,7 @@ pub(crate) mod test_utils { #[cfg(test)] mod tests { use super::*; + use anyhow::Result; #[test] pub fn test_get_unix_user() { From a04aebfae298954951c5ea1017818f183f0f3928 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Mon, 8 May 2023 23:49:44 +0000 Subject: [PATCH 3/4] minor fixes Signed-off-by: yihuaf --- crates/libcontainer/src/apparmor.rs | 15 ++++++--------- crates/libcontainer/src/utils.rs | 8 ++++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/libcontainer/src/apparmor.rs b/crates/libcontainer/src/apparmor.rs index b80982438..2defe9f74 100644 --- a/crates/libcontainer/src/apparmor.rs +++ b/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 = std::result::Result; @@ -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, diff --git a/crates/libcontainer/src/utils.rs b/crates/libcontainer/src/utils.rs index c9abd120d..c2b3ae3d6 100644 --- a/crates/libcontainer/src/utils.rs +++ b/crates/libcontainer/src/utils.rs @@ -163,11 +163,11 @@ pub fn do_exec(path: impl AsRef, args: &[String]) -> Result<(), DoExecErro path: path.as_ref().to_path_buf(), } })?; - let cArgs: Vec = args + let c_args: Vec = args .iter() .map(|s| CString::new(s.as_bytes()).unwrap_or_default()) .collect(); - unistd::execvp(&p, &cArgs).map_err(|err| DoExecError::Execvp { source: err })?; + unistd::execvp(&p, &c_args).map_err(|err| DoExecError::Execvp { source: err })?; Ok(()) } @@ -375,9 +375,9 @@ pub fn secure_join>( if let Some(metadata) = metadata { if metadata.file_type().is_symlink() { let link_path = - fs::read_link(curr_path).map_err(|err| SecureJoinError::ReadLink { + fs::read_link(&curr_path).map_err(|err| SecureJoinError::ReadLink { source: err, - path: curr_path, + path: curr_path.to_owned(), })?; path = link_path.join(part.as_path()); part = path.iter(); From 737d8ff410731d7b3f35ea19edca8b6d1c59c516 Mon Sep 17 00:00:00 2001 From: yihuaf Date: Tue, 9 May 2023 01:07:10 +0000 Subject: [PATCH 4/4] implemented seccomp Signed-off-by: yihuaf --- crates/libcontainer/src/seccomp/mod.rs | 105 +++++++++++++++++++------ 1 file changed, 80 insertions(+), 25 deletions(-) diff --git a/crates/libcontainer/src/seccomp/mod.rs b/crates/libcontainer/src/seccomp/mod.rs index c06071d54..91864654d 100644 --- a/crates/libcontainer/src/seccomp/mod.rs +++ b/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 = std::result::Result; + fn translate_arch(arch: Arch) -> ScmpArch { match arch { Arch::ScmpArchNative => ScmpArch::Native, @@ -42,7 +83,11 @@ fn translate_action(action: LinuxSeccompAction, errno: Option) -> Result 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,7 +121,7 @@ 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() { @@ -84,7 +129,7 @@ fn check_seccomp(seccomp: &LinuxSeccomp) -> Result<()> { 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> { 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> { // 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> { 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> { // 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;