From 97658c91d85a15879aaede08a02288a3d62515dd Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 11 Nov 2026 18:40:59 +0530 Subject: [PATCH] Refuse a missing deny_read inside a 2.1 jail's mounts, and document the mask gap The 2.1 native resource jail masked a missing deny_read path with --ro-bind /dev/null, which makes bwrap create an empty file at that path in the caller's tree; and a path created later was not covered anyway. As the bwrap jail does since #107, a missing denial inside a mount is now refused and one outside every mount is skipped. The docs said missing denials are ignored, or retained to cover later creation, which bubblewrap does neither. They also did not say that a mask lasts only as long as the path it covers: a host that replaces a denied path inside a grant (an atomic rename) exposes the new file to a running jail, shown on bubblewrap 0.9.0 and Linux 7.0. That is now a documented known gap with what a host should do instead. Part of #104 --- crates/tinybox-bus/src/jail.rs | 8 ++- crates/tinybox-jail/src/bwrap/mod.rs | 15 ++++-- crates/tinybox-jail/src/jail.rs | 4 +- crates/tinybox-linux/src/sandbox/args.rs | 35 ++++++++++++- crates/tinybox-linux/src/sandbox/mod_tests.rs | 49 ++++++++++++++++++- 5 files changed, 103 insertions(+), 8 deletions(-) diff --git a/crates/tinybox-bus/src/jail.rs b/crates/tinybox-bus/src/jail.rs index c4d686a8..18114400 100644 --- a/crates/tinybox-bus/src/jail.rs +++ b/crates/tinybox-bus/src/jail.rs @@ -29,8 +29,12 @@ pub struct NativeJailConfig { pub require_process_isolation: bool, /// Absolute paths the child may neither read nor write, even where the /// root or a grant would otherwise reach them (credential stores, for a - /// provider that reads allow-by-default). Paths that do not exist are - /// ignored. Added in contract 1.20; empty by default. + /// provider that reads allow-by-default). A path that does not exist is + /// ignored outside every grant, and refused inside one, by bubblewrap + /// backends (a later-created path would not be covered). 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 dab0b501..9a867171 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 3b9aa4dc..cba15980 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, /// Optional resource values to apply where the backend supports them. /// diff --git a/crates/tinybox-linux/src/sandbox/args.rs b/crates/tinybox-linux/src/sandbox/args.rs index b0af54bf..ff23f564 100644 --- a/crates/tinybox-linux/src/sandbox/args.rs +++ b/crates/tinybox-linux/src/sandbox/args.rs @@ -543,6 +543,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(()) } @@ -642,7 +669,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 843f2465..5533dc64 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; @@ -611,6 +611,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();