From 34d72293e5f7756c851a1d075b3c0dbb0d9b6e71 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Tue, 28 Nov 2023 19:59:52 +0100 Subject: [PATCH 1/2] Ignore lockdown errors for invalid paths This fixes an issue where locking down the Linux sandbox would fail if a path was deleted after adding the exception but before locking down the sandbox. --- CHANGELOG.md | 1 + Cargo.toml | 1 + integration/delete_before_lockdown.rs | 22 ++++++++++++++++++++++ integration/harness.rs | 1 + src/linux/namespaces.rs | 5 ++++- 5 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 integration/delete_before_lockdown.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 827e723..df0aa54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - (macOS) Modifying exceptions for paths affected by existing exceptions - (Linux) Symlink/Canonical path's exceptions overriding each other - (Linux) PID namespace support +- (Linux) Sandbox lockdown failing when deleting file after adding exception ## [v0.5.0] - 2023-10-13 diff --git a/Cargo.toml b/Cargo.toml index 582ea31..100a8ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ harness = false [target.'cfg(target_os = "linux")'.dependencies] seccompiler = "0.3.0" libc = "0.2.132" +log = "0.4.20" [dev-dependencies] clap = { version = "3.2.17", features = ["derive"] } diff --git a/integration/delete_before_lockdown.rs b/integration/delete_before_lockdown.rs new file mode 100644 index 0000000..ffc05df --- /dev/null +++ b/integration/delete_before_lockdown.rs @@ -0,0 +1,22 @@ + +use birdcage::{Birdcage, Exception, Sandbox}; +use tempfile::NamedTempFile; + +use crate::TestSetup; + +pub fn setup() -> TestSetup { + // Create temporary file. + let tempfile = NamedTempFile::new().unwrap(); + + // Setup sandbox exceptions. + let mut sandbox = Birdcage::new(); + sandbox.add_exception(Exception::Read(tempfile.path().into())).unwrap(); + + drop(tempfile); + + TestSetup { sandbox, data: String::new() } +} + +pub fn validate(_data: String) { + // We just want to test sandbox creation worked. +} diff --git a/integration/harness.rs b/integration/harness.rs index 970c8a3..69388bb 100644 --- a/integration/harness.rs +++ b/integration/harness.rs @@ -6,6 +6,7 @@ test_mods! { mod canonicalize; #[cfg(target_os = "linux")] mod consistent_id_mappings; + mod delete_before_lockdown; mod env; mod exec; mod exec_symlinked_dir; diff --git a/src/linux/namespaces.rs b/src/linux/namespaces.rs index f759a6a..5e77323 100644 --- a/src/linux/namespaces.rs +++ b/src/linux/namespaces.rs @@ -55,7 +55,10 @@ pub(crate) fn setup_mount_namespace(exceptions: PathExceptions) -> io::Result<() let dst_c = CString::new(dst.as_os_str().as_bytes()).unwrap(); // Create mount target. - copy_tree(&path, &new_root)?; + if let Err(err) = copy_tree(&path, &new_root) { + log::error!("skipping birdcage exception {path:?}: {err}"); + continue; + } // Bind path with full permissions. bind_mount(&src_c, &dst_c)?; From 80d1a317f2e276caf7728a6135e849bc636f24dc Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Wed, 29 Nov 2023 22:41:05 +0100 Subject: [PATCH 2/2] Make testfile deletion more explicit --- integration/delete_before_lockdown.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/delete_before_lockdown.rs b/integration/delete_before_lockdown.rs index ffc05df..f0ad37b 100644 --- a/integration/delete_before_lockdown.rs +++ b/integration/delete_before_lockdown.rs @@ -12,7 +12,7 @@ pub fn setup() -> TestSetup { let mut sandbox = Birdcage::new(); sandbox.add_exception(Exception::Read(tempfile.path().into())).unwrap(); - drop(tempfile); + tempfile.close().unwrap(); TestSetup { sandbox, data: String::new() } }