diff --git a/crates/tinybox-bus/src/jail.rs b/crates/tinybox-bus/src/jail.rs index 73b3b134..deeaed1e 100644 --- a/crates/tinybox-bus/src/jail.rs +++ b/crates/tinybox-bus/src/jail.rs @@ -32,8 +32,10 @@ pub struct NativeJailConfig { /// provider that reads allow-by-default). A path that does not exist yet is /// kept and resolved again just before spawn, except that the bubblewrap /// backend refuses a missing denial inside one of the jail's mounts: it - /// would have to create the path to mask it. Added in contract 1.20; empty - /// by default. + /// would have to create the path to mask it. A denial inside a grant + /// covers the path as it exists when the jail starts; a host that replaces + /// it during the run exposes the new file (see the bubblewrap backend's + /// known gaps). Added in contract 1.20; empty by default. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub deny_read: Vec, /// Optional hard resource limits; omitted for compatibility with older requests. diff --git a/crates/tinybox-jail/src/bwrap/mod.rs b/crates/tinybox-jail/src/bwrap/mod.rs index 67e51ad7..9abfd7a9 100644 --- a/crates/tinybox-jail/src/bwrap/mod.rs +++ b/crates/tinybox-jail/src/bwrap/mod.rs @@ -64,9 +64,18 @@ //! `--bind-fd`/`--ro-bind-fd` (bubblewrap 0.10+); that is follow-up work. //! The window is only open while another sandbox with write access to the //! same tree is running. -//! - `deny_read` paths must exist when the jail starts: a path created later -//! under a mounted grant is **not** protected. One nested more than a level -//! inside a writable grant is refused. +//! - `deny_read` paths must exist when the jail starts. A missing one inside a +//! mount is refused, since a path created later would not be covered; one +//! nested more than a level inside a writable grant is refused too. +//! - **A mask lasts only as long as the path it covers.** A denial is a mount +//! over that path's directory entry, and the kernel drops a mount from other +//! mount namespaces when its entry is unlinked or renamed over. So if a +//! host process replaces a denied path inside a grant while the jail runs +//! (an atomic rename, as credential helpers and editors write), the jail +//! sees the new file. Shown on bubblewrap 0.9.0 and Linux 7.0 for a file +//! and a directory. bwrap cannot prevent it. A host should deny a credential +//! store outside every grant (it is then never mounted at all), or deny its +//! parent directory, rather than mask a file it rewrites. //! - Landlock is not layered inside: the kernel denies `mount` and //! `pivot_root` to a Landlock-restricted thread, which would break `bwrap`'s //! own setup. diff --git a/crates/tinybox-jail/src/jail.rs b/crates/tinybox-jail/src/jail.rs index d71ffa11..1be1ec62 100644 --- a/crates/tinybox-jail/src/jail.rs +++ b/crates/tinybox-jail/src/jail.rs @@ -32,7 +32,9 @@ pub struct Jail { /// [`carves_denials`](JailBackend::carves_denials) subtracts them from /// grants; allow-list backends deny paths outside grants by default. The /// native module rejects overlaps that an allow-list backend cannot enforce. - /// Missing paths are retained so backend rules can cover later creation. + /// Missing paths are retained here; backends decide: Landlock and Seatbelt + /// rules cover later creation, while bubblewrap refuses a missing path + /// inside a mount. pub deny_read: Vec, /// File or directory names the child may neither read nor write wherever /// they appear. Only a backend that diff --git a/crates/tinybox-linux/src/sandbox/args.rs b/crates/tinybox-linux/src/sandbox/args.rs index b2dcfdb9..b8a3e13f 100644 --- a/crates/tinybox-linux/src/sandbox/args.rs +++ b/crates/tinybox-linux/src/sandbox/args.rs @@ -562,6 +562,33 @@ fn validate_native_jail_command(request: &NativeJailCommand) -> std::io::Result< )); } + refuse_missing_denials_inside_mounts(request) +} + +/// A `deny_read` path must exist when it lies inside one of the jail's mounts: +/// masking a missing one (`--ro-bind /dev/null `) makes bwrap create it +/// on the host, an empty file in the caller's tree, and a path created after +/// the jail starts would not be covered anyway. One outside every mount is +/// never visible in the sandbox, so it is skipped when the argv is built. +fn refuse_missing_denials_inside_mounts(request: &NativeJailCommand) -> std::io::Result<()> { + let mounts: Vec<&Path> = std::iter::once(&request.root) + .chain(&request.read_only) + .chain(&request.read_write) + .map(PathBuf::as_path) + .collect(); + for denied in &request.deny_read { + if denied.symlink_metadata().is_err() + && mounts.iter().any(|mount| denied.starts_with(mount)) + { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "deny_read path {} must exist before starting the jail", + denied.display() + ), + )); + } + } Ok(()) } @@ -664,7 +691,13 @@ fn native_bwrap_argv( destination.display().to_string(), ]); } - for denied in &request.deny_read { + // A missing denial reaching here lies outside every mount (see + // `refuse_missing_denials_inside_mounts`): nothing to mask. + for denied in request + .deny_read + .iter() + .filter(|denied| denied.symlink_metadata().is_ok()) + { if denied.is_dir() { argv.extend(["--tmpfs".into(), denied.display().to_string()]); } else { diff --git a/crates/tinybox-linux/src/sandbox/mod_tests.rs b/crates/tinybox-linux/src/sandbox/mod_tests.rs index f36ad5b1..acd3482e 100644 --- a/crates/tinybox-linux/src/sandbox/mod_tests.rs +++ b/crates/tinybox-linux/src/sandbox/mod_tests.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; #[cfg(target_os = "linux")] use std::os::unix::fs::PermissionsExt as _; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; use async_trait::async_trait; @@ -705,6 +705,53 @@ fn native_jail_command_refuses_invalid_resource_and_isolation_requests() Ok(()) } +#[test] +#[cfg(target_os = "linux")] +fn a_missing_denial_inside_a_mount_is_refused_and_one_outside_is_skipped() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let cwd = root.path().join("work"); + std::fs::create_dir(&cwd)?; + let elsewhere = tempfile::tempdir()?; + let inside = root.path().join("credentials.json"); + let outside = elsewhere.path().join("not-mounted"); + let request = |deny_read: Vec| crate::NativeJailCommand { + root: root.path().to_path_buf(), + cwd: cwd.clone(), + read_only: Vec::new(), + read_write: Vec::new(), + deny_read, + allow_net: false, + allow_subprocess: true, + cpu_millis: Some(1000), + memory_bytes: None, + pids_max: None, + argv: vec!["/bin/true".into()], + env: BTreeMap::new(), + }; + let systemd_run = crate::trusted_systemd_run_program()?; + let build = |deny_read| { + crate::native_jail_argv_with_programs( + &request(deny_read), + &systemd_run, + Path::new("/bin/true"), + ) + }; + + // Masking it would make bwrap create an empty file in the caller's tree. + let kind = build(vec![inside.clone()]).err().map(|error| error.kind()); + assert_eq!(kind, Some(std::io::ErrorKind::NotFound)); + assert!(!inside.exists(), "nothing may be created on the host"); + + // Outside every mount the sandbox never shows it: nothing to mask. + let argv = build(vec![outside.clone()])?; + assert!( + !argv.iter().any(|part| Path::new(part) == outside), + "{argv:?}" + ); + Ok(()) +} + #[tokio::test] async fn without_limits_there_is_no_scope_at_all() -> Result<()> { let (sandbox, host) = sandbox();