From 545df0bf2f26d2961b40c36550bc8257a2a8bae3 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Thu, 12 Oct 2023 00:18:42 +0200 Subject: [PATCH] Fix Linux sandbox error handling This fixes an issue with the Linux sandbox where invalid paths would only cause an error during `Birdcage::lock`, at which point the consumer cannot handle or ignore this error anymore since the sandbox creation has already been attempted. --- Cargo.toml | 5 +++++ src/error.rs | 43 +++----------------------------------- src/linux/mod.rs | 11 +++++++++- src/macos.rs | 19 +++++++++-------- tests/missing_exception.rs | 20 ++++++++++++++++++ 5 files changed, 48 insertions(+), 50 deletions(-) create mode 100644 tests/missing_exception.rs diff --git a/Cargo.toml b/Cargo.toml index e55472c..c1c444d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,11 @@ name = "seccomp" path = "tests/seccomp.rs" harness = false +[[test]] +name = "missing_exception" +path = "tests/missing_exception.rs" +harness = false + [target.'cfg(target_os = "linux")'.dependencies] seccompiler = "0.3.0" libc = "0.2.132" diff --git a/src/error.rs b/src/error.rs index 08fa965..f4eb031 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,6 +5,7 @@ use std::error::Error as StdError; use std::ffi::OsString; use std::fmt::{self, Display, Formatter}; use std::io::Error as IoError; +use std::path::PathBuf; use std::result::Result as StdResult; #[cfg(target_os = "linux")] @@ -21,8 +22,7 @@ pub enum Error { Seccomp(SeccompError), /// Invalid sandbox exception path. - #[cfg(target_os = "macos")] - InvalidPath(InvalidPathError), + InvalidPath(PathBuf), /// I/O error. Io(IoError), @@ -38,8 +38,7 @@ impl Display for Error { match self { #[cfg(target_os = "linux")] Self::Seccomp(error) => write!(f, "seccomp error: {error}"), - #[cfg(target_os = "macos")] - Self::InvalidPath(error) => write!(f, "invalid path: {error:?}"), + Self::InvalidPath(path) => write!(f, "invalid path: {path:?}"), Self::Io(error) => write!(f, "input/output error: {error}"), Self::ActivationFailed(error) => { write!(f, "failed to initialize a sufficient sandbox: {error}") @@ -62,44 +61,8 @@ impl From for Error { } } -#[cfg(target_os = "macos")] -impl From for Error { - fn from(error: InvalidPathError) -> Self { - Self::InvalidPath(error) - } -} - impl From for Error { fn from(error: IoError) -> Self { Self::Io(error) } } - -/// Invalid sandbox exception path. -#[cfg(target_os = "macos")] -#[derive(Debug)] -pub struct InvalidPathError(String); - -#[cfg(target_os = "macos")] -impl StdError for InvalidPathError {} - -#[cfg(target_os = "macos")] -impl Display for InvalidPathError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "invalid path: {}", self.0) - } -} - -#[cfg(target_os = "macos")] -impl From for InvalidPathError { - fn from(error: IoError) -> Self { - InvalidPathError(error.to_string()) - } -} - -#[cfg(target_os = "macos")] -impl From for InvalidPathError { - fn from(error: OsString) -> Self { - InvalidPathError(error.to_string_lossy().into_owned()) - } -} diff --git a/src/linux/mod.rs b/src/linux/mod.rs index ca948f9..c0037c3 100644 --- a/src/linux/mod.rs +++ b/src/linux/mod.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use std::io::Error as IoError; use std::path::PathBuf; -use crate::error::Result; +use crate::error::{Error, Result}; use crate::linux::namespaces::MountFlags; use crate::linux::seccomp::SyscallFilter; use crate::{Exception, Sandbox}; @@ -49,6 +49,15 @@ impl Sandbox for LinuxSandbox { } fn add_exception(&mut self, exception: Exception) -> Result<&mut Self> { + // Report error if exception is added for an invalid path. + if let Exception::Read(path) | Exception::Write(path) | Exception::ExecuteAndRead(path) = + &exception + { + if !path.exists() { + return Err(Error::InvalidPath(path.into())); + } + } + match exception { Exception::Read(path) => self.update_bind_mount(path, false, false), Exception::Write(path) => self.update_bind_mount(path, true, false), diff --git a/src/macos.rs b/src/macos.rs index d580922..933d967 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use std::result::Result as StdResult; use std::{fs, ptr}; -use crate::error::{Error, InvalidPathError, Result}; +use crate::error::{Error, Result}; use crate::{Exception, Sandbox}; /// Deny-all fallback rule. @@ -110,19 +110,20 @@ impl Sandbox for MacSandbox { } /// Escape a path: /tt/in\a"x -> "/tt/in\\a\"x" -fn escape_path(path: PathBuf) -> StdResult { +fn escape_path(path: PathBuf) -> StdResult { // Canonicalize the incoming path to support relative paths. // The `subpath` action only allows absolute paths. - let path = fs::canonicalize(path)?; + let canonical_path = fs::canonicalize(&path).map_err(|_| Error::InvalidPath(path))?; - let mut path = path.into_os_string().into_string()?; + let mut path_str = + canonical_path.into_os_string().into_string().map_err(|_| Error::InvalidPath(path))?; // Paths in `subpath` expressions must not end with /. - while path.ends_with('/') && path != "/" { - String::pop(&mut path); + while path_str.ends_with('/') && path_str != "/" { + String::pop(&mut path_str); } - path = path.replace('"', r#"\""#); - path = path.replace('\\', r#"\\"#); - Ok(format!("\"{path}\"")) + path_str = path_str.replace('"', r#"\""#); + path_str = path_str.replace('\\', r#"\\"#); + Ok(format!("\"{path_str}\"")) } extern "C" { diff --git a/tests/missing_exception.rs b/tests/missing_exception.rs new file mode 100644 index 0000000..e28c2e8 --- /dev/null +++ b/tests/missing_exception.rs @@ -0,0 +1,20 @@ +use std::path::PathBuf; + +use birdcage::error::Error; +use birdcage::{Birdcage, Exception, Sandbox}; + +fn main() { + let mut birdcage = Birdcage::new(); + + // Add a path that doesn't exist. + let result = birdcage.add_exception(Exception::Read("/does/not/exist".into())); + + // Ensure it is appropriately reported that exception was NOT added. + match result { + Err(Error::InvalidPath(path)) => assert_eq!(path, PathBuf::from("/does/not/exist")), + _ => panic!("expected path error"), + } + + // Ensure locking is always successful. + birdcage.lock().unwrap(); +}