From 3d6e9e7828ee46200f565e9e78a24d0470746916 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 11 Nov 2026 17:12:53 +0530 Subject: [PATCH] Bind bwrap grants from descriptors, closing the check-then-mount gap A nested grant's symlink check ran before bwrap re-resolved the grant's path, so a program with write access to the parent could swap in a link between them. Where bwrap lists --bind-fd and the kernel has openat2, each grant is now opened once as an O_PATH descriptor, a nested one beneath its parent with RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH, and bound with --bind-fd. Descriptors are mapped only in the child after fork (command-fds), never inheritable in the host. Elsewhere grants stay bound by path, as before, and BubblewrapBackend::binds_by_descriptor reports which is in effect. --- Cargo.lock | 12 ++ crates/tinybox-jail/Cargo.toml | 7 + crates/tinybox-jail/src/bwrap/args.rs | 175 +++++++++++++++--- crates/tinybox-jail/src/bwrap/args_tests.rs | 108 +++++++++++ crates/tinybox-jail/src/bwrap/launcher.rs | 17 +- .../tinybox-jail/src/bwrap/launcher_tests.rs | 54 +++++- crates/tinybox-jail/src/bwrap/mod.rs | 84 +++++++-- crates/tinybox-jail/src/bwrap/mod_tests.rs | 1 + crates/tinybox-jail/tests/bwrap_isolation.rs | 21 +++ 9 files changed, 434 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9d8bd48..3dcd31f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -285,6 +285,16 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "command-fds" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b60b5124979fccd9addd89d8b97a1d6eebb4950694520c75ddd722535ea443f" +dependencies = [ + "nix 0.31.3", + "thiserror", +] + [[package]] name = "command-group" version = "5.0.1" @@ -1907,8 +1917,10 @@ dependencies = [ name = "tinybox-jail" version = "0.2.6" dependencies = [ + "command-fds", "landlock", "log", + "nix 0.31.3", "rustix", "serde", "serde_json", diff --git a/crates/tinybox-jail/Cargo.toml b/crates/tinybox-jail/Cargo.toml index 2be21df6..fcf4c2af 100644 --- a/crates/tinybox-jail/Cargo.toml +++ b/crates/tinybox-jail/Cargo.toml @@ -33,6 +33,13 @@ landlock = { version = "0.4", optional = true } # `NamespaceProbe` locates `bwrap`; reused so there is one definition of how the # Linux backends find it. tinybox-linux.workspace = true +# Hands the bwrap launcher the grant descriptors it binds (`--bind-fd`), mapped +# only in the child after fork; std cannot without `unsafe`, which the +# workspace forbids. Pinned: it runs in the jail's spawn path. +command-fds = "=0.3.3" +# `openat2(RESOLVE_NO_SYMLINKS)` for those descriptors, safely. Already in the +# lockfile, and the version command-fds builds on. +nix = { version = "0.31", default-features = false, features = ["fs"] } [target.'cfg(windows)'.dependencies] # AppContainer profile APIs, ACL editing, STARTUPINFOEXW process spawn, and the diff --git a/crates/tinybox-jail/src/bwrap/args.rs b/crates/tinybox-jail/src/bwrap/args.rs index 1444d4fe..b1557825 100644 --- a/crates/tinybox-jail/src/bwrap/args.rs +++ b/crates/tinybox-jail/src/bwrap/args.rs @@ -14,6 +14,7 @@ use std::ffi::OsString; use std::io; +use std::os::fd::{OwnedFd, RawFd}; use std::path::{Path, PathBuf}; use crate::jail::Jail; @@ -44,12 +45,27 @@ pub(super) const ETC_ENTRIES: &[&str] = &[ ]; /// Switches that depend on the `bwrap` build or the host, decided by the probe. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Default)] pub(super) struct Options { /// Pass `--disable-userns` (bubblewrap 0.8+): the child cannot create /// nested user namespaces, which closes the usual escape route to a /// capability-bearing namespace. pub disable_userns: bool, + /// Bind the jail's grants from descriptors (`--bind-fd`/`--ro-bind-fd`) + /// opened here, instead of by path (see [`jail_args_with_fds`]). + pub bind_fds: bool, +} + +/// The first descriptor a grant is handed to `bwrap` on; the launcher keeps +/// this one and the ones after it open. +pub(super) const FIRST_GRANT_FD: RawFd = 10; + +/// A `bwrap` argument list, plus the descriptors it binds grants from, each +/// with the number the child must see it on. +#[derive(Debug, Default)] +pub(super) struct JailArgs { + pub argv: Vec, + pub fds: Vec<(OwnedFd, RawFd)>, } #[derive(Debug)] @@ -65,6 +81,8 @@ enum Kind { struct Mount { path: PathBuf, kind: Kind, + /// One of the jail's own grants, not a baseline system path. + grant: bool, } impl Mount { @@ -126,10 +144,12 @@ fn baseline_mounts() -> Vec { Ok(target) => Mount { path, kind: Kind::Symlink(target), + grant: false, }, Err(_) => Mount { path, kind: Kind::Bind { writable: false }, + grant: false, }, }); } @@ -137,6 +157,7 @@ fn baseline_mounts() -> Vec { mounts.push(Mount { path: PathBuf::from(entry), kind: Kind::Bind { writable: false }, + grant: false, }); } @@ -166,28 +187,36 @@ pub(super) fn jail_args( cwd: Option<&Path>, options: Options, ) -> io::Result> { - for path in std::iter::once(&jail.root) - .chain(&jail.read_write) - .chain(&jail.read_only) - .chain(&jail.deny_read) - .map(PathBuf::as_path) - .chain(cwd) - { - require_clean(path)?; - } - // A grant inside a writable one must not reach its target through a - // symlink the jailed program could have planted. - let writable: Vec<&Path> = std::iter::once(&jail.root) - .chain(&jail.read_write) - .map(PathBuf::as_path) - .collect(); - let nested: Vec<&Path> = jail - .read_only - .iter() - .chain(&jail.read_write) - .map(PathBuf::as_path) - .collect(); - crate::jail::reject_symlinked_grants(&writable, &nested)?; + let paths_only = Options { + bind_fds: false, + ..options + }; + jail_args_with_fds(jail, cwd, paths_only).map(|args| args.argv) +} + +/// [`jail_args`], with the grants bound from descriptors when +/// `options.bind_fds` is set. +/// +/// A path check followed by `bwrap --bind PATH` leaves a window in which a +/// program with write access to a grant's parent can swap a directory for a +/// symlink. Here each grant is opened once, as an `O_PATH` descriptor, and +/// `bwrap` binds that descriptor, so what was checked is what is mounted. A +/// grant inside an already-opened grant is opened relative to it with +/// `RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH`, so no symlink on that stretch is +/// followed (another sandbox may be able to write there even when this one +/// cannot); any other grant is opened by its declared path, whose ancestors +/// the host controls. A missing read-only grant is skipped, as `--ro-bind-try` +/// skips it. +/// +/// # Errors +/// As [`jail_args`], plus the error from opening a grant (`ELOOP` when a +/// symlink was planted inside a writable grant). +pub(super) fn jail_args_with_fds( + jail: &Jail, + cwd: Option<&Path>, + options: Options, +) -> io::Result { + check_paths(jail, cwd)?; let mut mounts = baseline_mounts(); for (path, writable) in std::iter::once((&jail.root, true)) .chain(jail.read_write.iter().map(|p| (p, true))) @@ -211,6 +240,7 @@ pub(super) fn jail_args( mounts.push(Mount { path: path.clone(), kind: Kind::Bind { writable }, + grant: true, }); } @@ -240,6 +270,7 @@ pub(super) fn jail_args( argv.extend([os(flag), os(path)]); } + let mut fds = GrantFds::default(); for mount in &mounts { match &mount.kind { Kind::Symlink(target) => argv.extend([ @@ -247,6 +278,20 @@ pub(super) fn jail_args( target.clone().into_os_string(), mount.path.clone().into_os_string(), ]), + Kind::Bind { writable } if options.bind_fds && mount.grant => { + let Some(fd) = fds.open(&mount.path, *writable)? else { + continue; + }; + argv.extend([ + os(if *writable { + "--bind-fd" + } else { + "--ro-bind-fd" + }), + fd.to_string().into(), + mount.path.clone().into_os_string(), + ]); + } Kind::Bind { writable } => argv.extend([ os(if *writable { "--bind" } else { "--ro-bind-try" }), mount.path.clone().into_os_string(), @@ -261,7 +306,89 @@ pub(super) fn jail_args( os("--chdir"), cwd.unwrap_or(&jail.root).as_os_str().to_os_string(), ]); - Ok(argv) + Ok(JailArgs { + argv, + fds: fds.into_mappings(), + }) +} + +/// Every path is absolute and clean, and no grant inside a writable one +/// reaches its target through a symlink the jailed program could have planted. +fn check_paths(jail: &Jail, cwd: Option<&Path>) -> io::Result<()> { + for path in std::iter::once(&jail.root) + .chain(&jail.read_write) + .chain(&jail.read_only) + .chain(&jail.deny_read) + .map(PathBuf::as_path) + .chain(cwd) + { + require_clean(path)?; + } + let writable: Vec<&Path> = std::iter::once(&jail.root) + .chain(&jail.read_write) + .map(PathBuf::as_path) + .collect(); + let nested: Vec<&Path> = jail + .read_only + .iter() + .chain(&jail.read_write) + .map(PathBuf::as_path) + .collect(); + crate::jail::reject_symlinked_grants(&writable, &nested) +} + +/// The grant descriptors opened so far, shallowest first, numbered from +/// [`FIRST_GRANT_FD`]. +#[derive(Default)] +struct GrantFds { + opened: Vec<(PathBuf, OwnedFd)>, +} + +impl GrantFds { + /// Opens `path` and returns the number the child will see it on, or + /// `None` for a missing read-only grant. + fn open(&mut self, path: &Path, writable: bool) -> io::Result> { + use nix::fcntl::{OFlag, OpenHow, ResolveFlag}; + let flags = OFlag::O_PATH | OFlag::O_CLOEXEC; + // The deepest grant already opened that contains this one. + let parent = self + .opened + .iter() + .filter(|(dir, _)| path.starts_with(dir) && path != dir) + .max_by_key(|(dir, _)| dir.components().count()); + let opened = match parent { + Some((dir, fd)) => nix::fcntl::openat2( + fd, + path.strip_prefix(dir).map_err(io::Error::other)?, + OpenHow::new() + .flags(flags) + .resolve(ResolveFlag::RESOLVE_NO_SYMLINKS | ResolveFlag::RESOLVE_BENEATH), + ), + None => nix::fcntl::openat2(nix::fcntl::AT_FDCWD, path, OpenHow::new().flags(flags)), + }; + let fd = match opened { + Ok(fd) => fd, + Err(nix::errno::Errno::ENOENT) if !writable => return Ok(None), + Err(errno) => { + return Err(io::Error::new( + io::Error::from(errno).kind(), + format!("cannot open jail grant {}: {errno}", path.display()), + )); + } + }; + let number = + FIRST_GRANT_FD + RawFd::try_from(self.opened.len()).map_err(io::Error::other)?; + self.opened.push((path.to_path_buf(), fd)); + Ok(Some(number)) + } + + fn into_mappings(self) -> Vec<(OwnedFd, RawFd)> { + self.opened + .into_iter() + .zip(FIRST_GRANT_FD..) + .map(|((_, fd), number)| (fd, number)) + .collect() + } } /// `deny_read` plus, for each entry under a baseline symlink, the same path diff --git a/crates/tinybox-jail/src/bwrap/args_tests.rs b/crates/tinybox-jail/src/bwrap/args_tests.rs index c52adde5..52f832ce 100644 --- a/crates/tinybox-jail/src/bwrap/args_tests.rs +++ b/crates/tinybox-jail/src/bwrap/args_tests.rs @@ -4,6 +4,7 @@ use super::*; const OPTS: Options = Options { disable_userns: true, + bind_fds: false, }; fn build(jail: &Jail) -> io::Result> { @@ -55,6 +56,7 @@ fn disable_userns_is_omitted_when_the_probe_found_no_support() -> io::Result<()> None, Options { disable_userns: false, + bind_fds: false, }, )?; assert!(!argv.iter().any(|a| a == "--disable-userns")); @@ -348,3 +350,109 @@ fn a_denial_through_a_baseline_symlink_also_covers_the_link_target() -> io::Resu ); Ok(()) } + +const FD_OPTS: Options = Options { + disable_userns: true, + bind_fds: true, +}; + +fn strings(argv: &[OsString]) -> Vec { + argv.iter() + .map(|a| a.to_string_lossy().into_owned()) + .collect() +} + +#[test] +fn grants_are_bound_from_descriptors_and_baseline_paths_by_path() -> io::Result<()> { + let root = tempfile::tempdir()?; + let nested = root.path().join("vendor"); + std::fs::create_dir(&nested)?; + let j = jail(root.path()).add_read_only(&nested); + + let built = jail_args_with_fds(&j, None, FD_OPTS)?; + let argv = strings(&built.argv); + + let numbers: Vec = built.fds.iter().map(|(_, number)| *number).collect(); + assert_eq!(numbers, vec![FIRST_GRANT_FD, FIRST_GRANT_FD + 1]); + let root_at = argv + .windows(3) + .position(|w| w[0] == "--bind-fd" && w[1] == "10" && Path::new(&w[2]) == root.path()); + let nested_at = argv + .windows(3) + .position(|w| w[0] == "--ro-bind-fd" && w[1] == "11" && Path::new(&w[2]) == nested); + assert!(root_at.is_some() && nested_at.is_some(), "{argv:?}"); + assert!( + root_at < nested_at, + "the nested grant must land on top: {argv:?}" + ); + // Neither grant is also bound by path, which bwrap would re-resolve. + assert_eq!(triple(&argv, "--bind", root.path()), None); + assert_eq!(triple(&argv, "--ro-bind-try", &nested), None); + // Baseline system paths are still bound by path. + assert!(argv.iter().any(|a| a == "--ro-bind-try"), "{argv:?}"); + Ok(()) +} + +#[test] +fn a_symlink_swapped_in_after_the_check_is_not_followed() -> io::Result<()> { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + let nested = root.path().join("vendor"); + let mut fds = GrantFds::default(); + assert_eq!(fds.open(root.path(), true)?, Some(FIRST_GRANT_FD)); + + // What `reject_symlinked_grants` checked was a directory; by the time the + // grant is opened another program has replaced it with a link out. + std::os::unix::fs::symlink(outside.path(), &nested)?; + let error = fds + .open(&nested, false) + .err() + .ok_or_else(|| io::Error::other("a symlinked nested grant was opened"))?; + + assert!(error.to_string().contains("vendor"), "{error}"); + assert!( + error.to_string().contains("ELOOP") || error.to_string().contains("symbolic links"), + "{error}" + ); + Ok(()) +} + +#[test] +fn a_symlink_above_a_grant_is_left_to_the_host() -> io::Result<()> { + // A top-level grant spelled through a host symlink (`/var/run` style) + // still opens: only the stretch inside another grant is held to no links. + let real = tempfile::tempdir()?; + let links = tempfile::tempdir()?; + let link = links.path().join("work"); + std::os::unix::fs::symlink(real.path(), &link)?; + + let mut fds = GrantFds::default(); + assert_eq!(fds.open(&link, true)?, Some(FIRST_GRANT_FD)); + Ok(()) +} + +#[test] +fn a_missing_read_only_grant_is_skipped_as_ro_bind_try_skips_it() -> io::Result<()> { + let root = tempfile::tempdir()?; + let missing = root.path().join("not-there"); + let j = jail(root.path()).add_read_only(&missing); + + let args = jail_args_with_fds(&j, None, FD_OPTS)?; + + assert_eq!(args.fds.len(), 1, "only the root is opened"); + assert!( + !strings(&args.argv).iter().any(|a| Path::new(a) == missing), + "a missing grant reaches bwrap" + ); + Ok(()) +} + +#[test] +fn without_descriptor_support_grants_are_bound_by_path() -> io::Result<()> { + let root = tempfile::tempdir()?; + let args = jail_args_with_fds(&jail(root.path()), None, OPTS)?; + + assert!(args.fds.is_empty()); + assert!(triple(&strings(&args.argv), "--bind", root.path()).is_some()); + Ok(()) +} diff --git a/crates/tinybox-jail/src/bwrap/launcher.rs b/crates/tinybox-jail/src/bwrap/launcher.rs index ca5a4680..5b359ded 100644 --- a/crates/tinybox-jail/src/bwrap/launcher.rs +++ b/crates/tinybox-jail/src/bwrap/launcher.rs @@ -27,13 +27,15 @@ use std::path::{Path, PathBuf}; use std::process::Command; /// The script: open and delete the args file, close every other inherited -/// descriptor above 2, then `exec bwrap --args 9 -- COMMAND...`. `$0` is -/// `bwrap`, `$1` the args file, the rest the command and its arguments (bwrap +/// descriptor above 2 except the `$2` grant descriptors from 10 up (see +/// [`super::args::FIRST_GRANT_FD`]), then `exec bwrap --args 9 -- COMMAND...`. +/// `$0` is `bwrap`, `$1` the args file, the rest the command and its arguments (bwrap /// reads options from the file but the command itself must be on its argv; a /// command line is visible to every local user anyway, in the sandbox as on /// any host). const SCRIPT: &str = r#"f=$1 -shift +k=$2 +shift 2 exec 9<"$f" || exit 126 command -p rm -f -- "$f" command -p rmdir -- "${f%/*}" 2>/dev/null @@ -42,7 +44,9 @@ for fd in /proc/self/fd/*; do n=${fd##*/} case $n in 0|1|2|9) ;; - *) eval "exec $n>&-" || exit 126 ;; + *) if [ "$n" -ge 10 ] && [ "$n" -lt $((10 + k)) ]; then :; else + eval "exec $n>&-" || exit 126 + fi ;; esac done exec "$0" --args 9 -- "$@" @@ -155,11 +159,13 @@ impl ArgsFile { } /// The launcher command: an empty environment, the script, `bwrap`, the file, -/// then the command to run inside the sandbox and its arguments. +/// how many grant descriptors to keep, then the command to run inside the +/// sandbox and its arguments. pub(super) fn command( shell: &str, bwrap: &Path, args: &ArgsFile, + grant_fds: usize, program: &std::ffi::OsStr, program_args: &[OsString], ) -> Command { @@ -170,6 +176,7 @@ pub(super) fn command( .arg(SCRIPT) .arg(bwrap) .arg(args.path()) + .arg(grant_fds.to_string()) .arg(program) .args(program_args); launcher diff --git a/crates/tinybox-jail/src/bwrap/launcher_tests.rs b/crates/tinybox-jail/src/bwrap/launcher_tests.rs index 06374151..3869d8bd 100644 --- a/crates/tinybox-jail/src/bwrap/launcher_tests.rs +++ b/crates/tinybox-jail/src/bwrap/launcher_tests.rs @@ -26,7 +26,7 @@ fn run(shell: &str, body: &str, items: &[&str]) -> io::Result { let bwrap = stand_in(dir.path(), body)?; let argv: Vec = items.iter().map(OsString::from).collect(); let file = ArgsFile::create(&argv, &tmp())?; - let out = command(shell, &bwrap, &file, "prog".as_ref(), &["a".into()]).output()?; + let out = command(shell, &bwrap, &file, 0, "prog".as_ref(), &["a".into()]).output()?; file.discard(); Ok(String::from_utf8_lossy(&out.stdout).into_owned()) } @@ -81,7 +81,7 @@ fn the_args_file_is_gone_once_the_launcher_has_started_bwrap() -> io::Result<()> let file = ArgsFile::create(&["x".into()], &tmp())?; let path = file.path().to_path_buf(); assert!( - command(shell(), &bwrap, &file, "prog".as_ref(), &[]) + command(shell(), &bwrap, &file, 0, "prog".as_ref(), &[]) .status()? .success() ); @@ -94,7 +94,7 @@ fn the_launcher_runs_with_an_empty_environment() -> io::Result<()> { let dir = tempfile::tempdir()?; let bwrap = stand_in(dir.path(), "env")?; let file = ArgsFile::create(&[], &tmp())?; - let out = command(shell(), &bwrap, &file, "prog".as_ref(), &[]).output()?; + let out = command(shell(), &bwrap, &file, 0, "prog".as_ref(), &[]).output()?; file.discard(); let text = String::from_utf8_lossy(&out.stdout); assert!( @@ -137,7 +137,7 @@ fn a_launcher_that_cannot_open_the_args_file_fails_closed_without_running_bwrap( dir: dir.path().join("gone"), file: dir.path().join("gone").join("args"), }; - let status = command(shell(), &bwrap, &missing, "prog".as_ref(), &[]).status()?; + let status = command(shell(), &bwrap, &missing, 0, "prog".as_ref(), &[]).status()?; assert_eq!(status.code(), Some(126)); assert!(!marker.exists(), "bwrap must not run without its arguments"); Ok(()) @@ -173,3 +173,49 @@ fn the_args_directory_prefers_xdg_runtime_dir_but_never_one_inside_a_grant() -> assert_eq!(kind, Some(io::ErrorKind::PermissionDenied)); Ok(()) } + +#[test] +fn the_grant_descriptors_reach_bwrap_and_nothing_else_does() -> io::Result<()> { + let shell = shell(); + if !shell.ends_with("bash") { + eprintln!("skipped: no bash"); + return Ok(()); + } + let dir = tempfile::tempdir()?; + let granted = tempfile::tempdir()?; + let leaked = tempfile::NamedTempFile::new()?; + // An inheritable descriptor the launcher must still close. + let stray = std::fs::File::open(leaked.path())?; + rustix::io::fcntl_setfd(&stray, rustix::io::FdFlags::empty())?; + let bwrap = stand_in(dir.path(), "ls -l /proc/self/fd")?; + let file = ArgsFile::create(&[], &tmp())?; + let mut launcher = command(shell, &bwrap, &file, 2, "prog".as_ref(), &[]); + let mappings = vec![ + command_fds::FdMapping { + parent_fd: std::fs::File::open(granted.path())?.into(), + child_fd: 10, + }, + command_fds::FdMapping { + parent_fd: std::fs::File::open(granted.path())?.into(), + child_fd: 11, + }, + ]; + command_fds::CommandFdExt::fd_mappings(&mut launcher, mappings).map_err(io::Error::other)?; + + let out = launcher.output()?; + file.discard(); + drop(stray); + let text = String::from_utf8_lossy(&out.stdout); + let granted = granted.path().display().to_string(); + for fd in ["10 ->", "11 ->"] { + assert!( + text.lines().any(|l| l.contains(fd) && l.contains(&granted)), + "{fd} {text}" + ); + } + assert!( + !text.contains(&leaked.path().display().to_string()), + "{text}" + ); + Ok(()) +} diff --git a/crates/tinybox-jail/src/bwrap/mod.rs b/crates/tinybox-jail/src/bwrap/mod.rs index dab0b501..4b5e8544 100644 --- a/crates/tinybox-jail/src/bwrap/mod.rs +++ b/crates/tinybox-jail/src/bwrap/mod.rs @@ -56,14 +56,16 @@ //! but a vetted BPF program is not part of this change. //! - No CPU, memory or pid limits (`tinybox-linux` offers them through a //! `systemd-run` scope; not wired here). -//! - Check-then-use gap on nested grants: the symlink check on a grant inside a -//! writable directory (`reject_symlinked_grants`) runs before `bwrap` binds -//! the path, so a program that is already running in another session could -//! swap a directory for a symlink in between. Closing it needs `O_PATH` -//! descriptors opened with `openat2(RESOLVE_NO_SYMLINKS)` and -//! `--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. +//! - Check-then-use gap on nested grants, **only where `bwrap` lacks +//! `--bind-fd` or the kernel lacks `openat2` (before Linux 5.6)**. Then the +//! symlink check on a grant inside a writable directory +//! (`reject_symlinked_grants`) runs before `bwrap` binds the path, so a +//! program already running in another session could swap a directory for a +//! symlink in between. Otherwise each grant is opened once as an `O_PATH` +//! descriptor, a nested one beneath its parent's with +//! `RESOLVE_NO_SYMLINKS | RESOLVE_BENEATH`, and bound with +//! `--bind-fd`/`--ro-bind-fd`, so what was checked is what is mounted. +//! [`BubblewrapBackend::binds_by_descriptor`] reports which is in effect. //! - `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. @@ -111,6 +113,9 @@ static DEFAULT_PROBE: OnceLock> = OnceLock::new(); struct Verified { program: PathBuf, disable_userns: bool, + /// `bwrap` lists `--bind-fd` and the kernel has `openat2`: grants are + /// bound from descriptors rather than re-resolved by path. + bind_fds: bool, } /// Bubblewrap (`bwrap`) namespace jail backend. @@ -166,6 +171,15 @@ impl BubblewrapBackend { self.verified.as_ref().map(|v| v.program.as_path()) } + /// Whether grants are bound from descriptors opened at spawn + /// (`--bind-fd`, bubblewrap 0.9+ on Linux 5.6+), which closes the + /// check-then-use gap on nested grants. When false they are bound by path + /// and the gap stays open (see the module docs). + #[must_use] + pub fn binds_by_descriptor(&self) -> bool { + self.verified.as_ref().is_some_and(|v| v.bind_fds) + } + /// Test constructor that skips the probe, for fake launchers. #[cfg(test)] fn unverified(program: impl Into, disable_userns: bool) -> Self { @@ -173,6 +187,7 @@ impl BubblewrapBackend { verified: Some(Verified { program: program.into(), disable_userns, + bind_fds: false, }), } } @@ -194,12 +209,40 @@ fn verify(program: &Path) -> Option { return Some(Verified { program: program.to_path_buf(), disable_userns, + bind_fds: supports_fd_binds(program), }); } } None } +/// Whether `program --help` lists `--bind-fd` (bubblewrap 0.9 does) and +/// `openat2` works on this kernel (Linux 5.6+). +fn supports_fd_binds(program: &Path) -> bool { + let listed = Command::new(program) + .arg("--help") + .env_clear() + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .output() + .is_ok_and(|out| { + String::from_utf8_lossy(&out.stdout) + .split_whitespace() + .any(|word| word == "--bind-fd") + }); + let openat2 = nix::fcntl::openat2( + nix::fcntl::AT_FDCWD, + "/", + nix::fcntl::OpenHow::new().flags(nix::fcntl::OFlag::O_PATH | nix::fcntl::OFlag::O_CLOEXEC), + ) + .is_ok(); + log::debug!( + "[cwd_jail:bwrap] {} --bind-fd listed={listed} openat2={openat2}", + program.display() + ); + listed && openat2 +} + fn probe(program: &Path, disable_userns: bool) -> bool { // A fresh, private, randomly named directory: never a predictable path // another user could pre-create or symlink. @@ -241,7 +284,11 @@ fn run_probe(program: &Path, disable_userns: bool, root: &Path, host_only: &Path return false; }; let jail = Jail::new(root, "bwrap-probe").deny_net(); - let Ok(argv) = args::jail_args(&jail, None, args::Options { disable_userns }) else { + let options = args::Options { + disable_userns, + ..args::Options::default() + }; + let Ok(argv) = args::jail_args(&jail, None, options) else { return false; }; let check: Vec = [ @@ -263,7 +310,7 @@ fn run_probe(program: &Path, disable_userns: bool, root: &Path, host_only: &Path let shell = launcher::pick_shell(|path| Path::new(path).is_file()); // Positive evidence is required: a launcher that exits 0 without running // the check (`true`) must not pass. - let out = launcher::command(shell, program, &file, "/bin/sh".as_ref(), &check) + let out = launcher::command(shell, program, &file, 0, "/bin/sh".as_ref(), &check) .stdin(Stdio::null()) .stderr(Stdio::null()) .output(); @@ -290,11 +337,12 @@ struct Prepared { /// The launcher command carries no environment and its own argv holds only /// paths; the caller's variables and arguments live in the private args file. fn prepare_command(verified: &Verified, jail: &Jail, cmd: &Command) -> io::Result { - let mut argv = args::jail_args( + let args::JailArgs { mut argv, fds } = args::jail_args_with_fds( jail, cmd.get_current_dir(), args::Options { disable_userns: verified.disable_userns, + bind_fds: verified.bind_fds, }, )?; argv.push("--clearenv".into()); @@ -313,13 +361,25 @@ fn prepare_command(verified: &Verified, jail: &Jail, cmd: &Command) -> io::Resul let file = launcher::ArgsFile::create(&argv, &base)?; let shell = launcher::pick_shell(|path| Path::new(path).is_file()); let program_args: Vec = cmd.get_args().map(OsString::from).collect(); - let launcher = launcher::command( + let mut launcher = launcher::command( shell, &verified.program, &file, + fds.len(), cmd.get_program(), &program_args, ); + // Mapped onto 10, 11, … in the child only, after fork: never inheritable + // in this process, where another thread could spawn concurrently. + let mappings = fds + .into_iter() + .map(|(parent_fd, child_fd)| command_fds::FdMapping { + parent_fd, + child_fd, + }) + .collect(); + command_fds::CommandFdExt::fd_mappings(&mut launcher, mappings) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; Ok(Prepared { launcher, args: file, diff --git a/crates/tinybox-jail/src/bwrap/mod_tests.rs b/crates/tinybox-jail/src/bwrap/mod_tests.rs index e1b04c27..4643623f 100644 --- a/crates/tinybox-jail/src/bwrap/mod_tests.rs +++ b/crates/tinybox-jail/src/bwrap/mod_tests.rs @@ -15,6 +15,7 @@ fn verified(program: &str) -> Verified { Verified { program: program.into(), disable_userns: true, + bind_fds: false, } } diff --git a/crates/tinybox-jail/tests/bwrap_isolation.rs b/crates/tinybox-jail/tests/bwrap_isolation.rs index 80c75a73..600f084d 100644 --- a/crates/tinybox-jail/tests/bwrap_isolation.rs +++ b/crates/tinybox-jail/tests/bwrap_isolation.rs @@ -109,6 +109,27 @@ fn a_read_only_grant_nested_in_the_root_stays_read_only() -> io::Result<()> { Ok(()) } +#[test] +fn grants_are_bound_from_descriptors_whenever_bwrap_supports_it() -> io::Result<()> { + let Some(backend) = backend()? else { + return Ok(()); + }; + let start = Instant::now(); + let program = backend + .program() + .ok_or_else(|| io::Error::other("no program"))?; + let help = Command::new(program).arg("--help").output()?; + let listed = String::from_utf8_lossy(&help.stdout) + .split_whitespace() + .any(|word| word == "--bind-fd"); + // openat2 is present on every kernel CI runs (5.6+), so the listing alone + // decides; the nested-grant tests above then ran through descriptors. + assert_eq!(backend.binds_by_descriptor(), listed); + eprintln!("[bwrap-real] binds_by_descriptor={listed}"); + done("bind-fd", start); + Ok(()) +} + #[test] fn a_read_only_grant_nested_in_a_read_write_grant_stays_read_only() -> io::Result<()> { let Some(backend) = backend()? else {