From 3d6e9e7828ee46200f565e9e78a24d0470746916 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 11 Nov 2026 17:12:53 +0530 Subject: [PATCH 1/2] 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 { From 46c7657e21747016a682d8ad0e194ae9d2bf4ca0 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 11 Nov 2026 17:26:58 +0530 Subject: [PATCH 2/2] Start every bwrap jail under a seccomp deny list The bubblewrap backend installed no seccomp filter, so keyctl, bpf, perf, io_uring, mount, setns, file handles and similar calls stayed reachable from a jail. A deny list compiled with seccompiler now goes to bwrap as two --add-seccomp-fd filters on pipes mapped into the child: EPERM for the deny list, ENOSYS for clone3 so the C library falls back to a clone whose flags the filter can check. ptrace stays allowed on Linux 4.8 and later; unshare and clone are denied only with a CLONE_NEW* flag. A bwrap without --add-seccomp-fd, or an architecture other than x86_64 and aarch64, leaves the backend unavailable rather than starting jails unfiltered. --- Cargo.lock | 11 + crates/tinybox-jail/Cargo.toml | 6 + crates/tinybox-jail/src/bwrap/mod.rs | 100 ++++++-- crates/tinybox-jail/src/bwrap/mod_tests.rs | 15 ++ crates/tinybox-jail/src/bwrap/seccomp.rs | 241 ++++++++++++++++++ .../tinybox-jail/src/bwrap/seccomp_tests.rs | 70 +++++ crates/tinybox-jail/tests/bwrap_isolation.rs | 76 ++++++ .../tests/fixtures/fake-bwrap-noseccomp.sh | 36 +++ .../tests/fixtures/fake-bwrap-old.sh | 5 + .../tests/fixtures/fake-bwrap-probe.sh | 5 + .../tinybox-jail/tests/fixtures/fake-bwrap.sh | 5 + 11 files changed, 555 insertions(+), 15 deletions(-) create mode 100644 crates/tinybox-jail/src/bwrap/seccomp.rs create mode 100644 crates/tinybox-jail/src/bwrap/seccomp_tests.rs create mode 100755 crates/tinybox-jail/tests/fixtures/fake-bwrap-noseccomp.sh diff --git a/Cargo.lock b/Cargo.lock index 3dcd31f7..7c66c052 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1551,6 +1551,15 @@ dependencies = [ "syn 3.0.3", ] +[[package]] +name = "seccompiler" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4ae55de56877481d112a559bbc12667635fdaf5e005712fd4e2b2fa50ffc884" +dependencies = [ + "libc", +] + [[package]] name = "serde" version = "1.0.229" @@ -1919,9 +1928,11 @@ version = "0.2.6" dependencies = [ "command-fds", "landlock", + "libc", "log", "nix 0.31.3", "rustix", + "seccompiler", "serde", "serde_json", "tempfile", diff --git a/crates/tinybox-jail/Cargo.toml b/crates/tinybox-jail/Cargo.toml index fcf4c2af..b483caa9 100644 --- a/crates/tinybox-jail/Cargo.toml +++ b/crates/tinybox-jail/Cargo.toml @@ -40,6 +40,12 @@ 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"] } +# Compiles the seccomp deny list bwrap installs on every jail (`bwrap/seccomp.rs`); +# its only dependency is libc. Default features off: no JSON frontend. +seccompiler = { version = "0.5", default-features = false } +# The per-architecture syscall numbers that deny list names. Already in the +# lockfile through several dependencies. +libc = "0.2" [target.'cfg(windows)'.dependencies] # AppContainer profile APIs, ACL editing, STARTUPINFOEXW process spawn, and the diff --git a/crates/tinybox-jail/src/bwrap/mod.rs b/crates/tinybox-jail/src/bwrap/mod.rs index 4b5e8544..5e9fc1ec 100644 --- a/crates/tinybox-jail/src/bwrap/mod.rs +++ b/crates/tinybox-jail/src/bwrap/mod.rs @@ -36,8 +36,8 @@ //! [`JailBackend::isolation`] is the **minimum over every configuration the //! backend accepts**. It is reported as `Process`, the closest defined level //! below `Kernel`, but note what it means here: pid, mount, user and ipc -//! namespaces and a private filesystem view, with the host *network* shared and -//! no seccomp. That is more than the plain `Process` doc describes and less than +//! namespaces, a private filesystem view and the seccomp deny list, with the +//! host *network* shared. That is more than the plain `Process` doc describes and less than //! `Kernel`; do not read it as "same process table". `Process` with `--disable-userns` support (bubblewrap //! 0.8+), `None` without it. [`JailBackend::isolation_for`] reaches //! [`IsolationLevel::Kernel`] only for a jail that denies network (a new net @@ -45,15 +45,23 @@ //! loopback services and abstract sockets are reachable. Hosts must ask //! [`JailBackend::is_suitable_for_untrusted_jail`]; the per-backend //! `is_suitable_for_untrusted_code` is false by construction. `Kernel` here -//! means separate pid, mount, user, ipc and uts (and net) namespaces: it does -//! not imply seccomp or resource limits. +//! means separate pid, mount, user, ipc and uts (and net) namespaces plus that +//! deny list: it does not imply resource limits. +//! +//! # Seccomp +//! +//! Every jail starts under a seccomp deny list (`seccomp.rs`): kernel keyrings, +//! other processes' memory, `bpf`/`perf`/`userfaultfd`/`io_uring`, every mount +//! call, `setns` and namespace-creating `clone`/`unshare`, file handles, +//! modules, `kexec`, reboot, host clock, swap and syslog, terminal injection +//! ioctls, and the x86 legacy calls. Denied calls fail with `EPERM` (`clone3` +//! with `ENOSYS`, so the C library falls back to `clone`). `ptrace` stays +//! allowed on Linux 4.8 and later, confined to the jail's pid namespace. A +//! `bwrap` without `--add-seccomp-fd`, or an architecture other than `x86_64` +//! and `aarch64`, leaves the backend unavailable rather than unfiltered. //! //! # Known gaps (not closed in this backend) //! -//! - No seccomp filter: `keyctl` (the user's kernel keyrings), `ptrace`, -//! `bpf`, `io_uring` and similar syscalls remain reachable. A filter needs a -//! file descriptor handed to `bwrap --seccomp`; the launcher could carry one, -//! 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, **only where `bwrap` lacks @@ -98,6 +106,7 @@ use crate::jail::{Jail, JailBackend}; mod args; mod launcher; +mod seccomp; /// Backend name reported by [`BubblewrapBackend`]. pub const BWRAP_BACKEND_NAME: &str = "bubblewrap"; @@ -116,6 +125,8 @@ struct Verified { /// `bwrap` lists `--bind-fd` and the kernel has `openat2`: grants are /// bound from descriptors rather than re-resolved by path. bind_fds: bool, + /// The compiled seccomp filters every jail starts under (see `seccomp`). + seccomp: Vec>, } /// Bubblewrap (`bwrap`) namespace jail backend. @@ -180,6 +191,15 @@ impl BubblewrapBackend { self.verified.as_ref().is_some_and(|v| v.bind_fds) } + /// Whether jails start under the seccomp deny list. True whenever the + /// backend is available: a `bwrap` that cannot load it is not used. + #[must_use] + pub fn installs_seccomp(&self) -> bool { + self.verified + .as_ref() + .is_some_and(|v| !v.seccomp.is_empty()) + } + /// Test constructor that skips the probe, for fake launchers. #[cfg(test)] fn unverified(program: impl Into, disable_userns: bool) -> Self { @@ -188,6 +208,7 @@ impl BubblewrapBackend { program: program.into(), disable_userns, bind_fds: false, + seccomp: Vec::new(), }), } } @@ -203,23 +224,55 @@ fn is_trusted_binary(path: &Path) -> bool { /// Runs throwaway sandboxes to learn whether `program` really isolates, and /// whether it supports `--disable-userns`. +/// +/// A `bwrap` that cannot install the seccomp filters (no `--add-seccomp-fd`, +/// or no filter for this architecture) is not used: a jail is refused rather +/// than started unfiltered. fn verify(program: &Path) -> Option { + let seccomp = match seccomp_filters(program) { + Ok(filters) => filters, + Err(error) => { + log::debug!("[cwd_jail:bwrap] unavailable: {error}"); + return None; + } + }; for disable_userns in [true, false] { if probe(program, disable_userns) { return Some(Verified { program: program.to_path_buf(), disable_userns, bind_fds: supports_fd_binds(program), + seccomp, }); } } 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) +/// The filters for this kernel and architecture, if `program` can load them. +fn seccomp_filters(program: &Path) -> io::Result>> { + if !help_lists(program, "--add-seccomp-fd") { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + format!( + "{} cannot load seccomp filters (no --add-seccomp-fd); jails are refused", + program.display() + ), + )); + } + let release = std::fs::read_to_string("/proc/sys/kernel/osrelease")?; + let kernel = seccomp::kernel_version(&release).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("unreadable kernel release {release:?}"), + ) + })?; + seccomp::filters(kernel) +} + +/// Whether `program --help` names `flag` as an option. +fn help_lists(program: &Path, flag: &str) -> bool { + Command::new(program) .arg("--help") .env_clear() .stdin(Stdio::null()) @@ -228,8 +281,14 @@ fn supports_fd_binds(program: &Path) -> bool { .is_ok_and(|out| { String::from_utf8_lossy(&out.stdout) .split_whitespace() - .any(|word| word == "--bind-fd") - }); + .any(|word| word == flag) + }) +} + +/// 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 = help_lists(program, "--bind-fd"); let openat2 = nix::fcntl::openat2( nix::fcntl::AT_FDCWD, "/", @@ -337,7 +396,7 @@ 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 args::JailArgs { mut argv, fds } = args::jail_args_with_fds( + let args::JailArgs { mut argv, mut fds } = args::jail_args_with_fds( jail, cmd.get_current_dir(), args::Options { @@ -345,6 +404,17 @@ fn prepare_command(verified: &Verified, jail: &Jail, cmd: &Command) -> io::Resul bind_fds: verified.bind_fds, }, )?; + // Each filter on its own pipe, numbered after the grants. A filter is a + // few kilobytes, well inside a pipe's buffer, so writing it before bwrap + // reads cannot block. + for filter in &verified.seccomp { + let (reader, mut writer) = std::io::pipe()?; + io::Write::write_all(&mut writer, filter)?; + drop(writer); + let number = args::FIRST_GRANT_FD + i32::try_from(fds.len()).map_err(io::Error::other)?; + argv.extend(["--add-seccomp-fd".into(), number.to_string().into()]); + fds.push((reader.into(), number)); + } argv.push("--clearenv".into()); for (key, value) in cmd.get_envs() { if let Some(value) = value { diff --git a/crates/tinybox-jail/src/bwrap/mod_tests.rs b/crates/tinybox-jail/src/bwrap/mod_tests.rs index 4643623f..722b14ed 100644 --- a/crates/tinybox-jail/src/bwrap/mod_tests.rs +++ b/crates/tinybox-jail/src/bwrap/mod_tests.rs @@ -16,6 +16,7 @@ fn verified(program: &str) -> Verified { program: program.into(), disable_userns: true, bind_fds: false, + seccomp: Vec::new(), } } @@ -193,6 +194,20 @@ fn a_launcher_that_passes_the_probe_is_available_with_process_isolation_minimum( ); } +#[test] +fn a_bwrap_that_cannot_load_seccomp_filters_is_not_used() { + // It passes the isolation probe, but a jail would start unfiltered. + let backend = BubblewrapBackend::with_program(fixture("fake-bwrap-noseccomp.sh")); + assert!(!backend.is_available()); + assert!(!backend.installs_seccomp()); +} + +#[test] +fn a_launcher_that_passes_the_probe_installs_the_seccomp_filters() { + let backend = BubblewrapBackend::with_program(fixture("fake-bwrap-probe.sh")); + assert!(backend.installs_seccomp()); +} + #[test] fn a_bwrap_without_disable_userns_is_available_but_claims_no_isolation() { let backend = BubblewrapBackend::with_program(fixture("fake-bwrap-old.sh")); diff --git a/crates/tinybox-jail/src/bwrap/seccomp.rs b/crates/tinybox-jail/src/bwrap/seccomp.rs new file mode 100644 index 00000000..8a2a55e7 --- /dev/null +++ b/crates/tinybox-jail/src/bwrap/seccomp.rs @@ -0,0 +1,241 @@ +//! The seccomp filters a bubblewrap jail starts under. +//! +//! A deny list, not an allow list: jailed programs are arbitrary (MCP servers on +//! Node, Python or Go, shell commands), so the filter removes kernel attack +//! surface and escape primitives no ordinary tool needs rather than guessing +//! what every runtime calls. A denied call fails with `EPERM`; `clone3` fails +//! with `ENOSYS` instead, because its flags live in memory a filter cannot read +//! and the C library only falls back to `clone`, whose flags it can, on +//! `ENOSYS`. Seccomp takes one action per filter, so that is a second filter. +//! +//! `bwrap --add-seccomp-fd` installs them on the child after its own mount and +//! namespace setup. A program built for another architecture (an i386 binary on +//! `x86_64`) is killed by the architecture check every filter starts with. +//! +//! Left reachable on purpose: `ptrace` on Linux 4.8 and later, where seccomp is +//! re-checked after a tracer changes a call and the jail's pid namespace limits +//! it to the jail's own processes (on older kernels it is denied); `unshare` +//! and `clone` without a `CLONE_NEW*` flag; `personality`; and socket families, +//! which the network namespace already confines. + +use std::collections::BTreeMap; +use std::io; + +use seccompiler::{ + BpfProgram, SeccompAction, SeccompCmpArgLen, SeccompCmpOp, SeccompCondition, SeccompFilter, + SeccompRule, TargetArch, +}; + +/// The namespace-creating `clone`/`unshare` flags. `CLONE_NEWTIME` is +/// `unshare`-only: in `clone` its bit is part of the exit signal. +const CLONE_NEW: [u64; 7] = [ + 0x0002_0000, // CLONE_NEWNS + 0x0200_0000, // CLONE_NEWCGROUP + 0x0400_0000, // CLONE_NEWUTS + 0x0800_0000, // CLONE_NEWIPC + 0x1000_0000, // CLONE_NEWUSER + 0x2000_0000, // CLONE_NEWPID + 0x4000_0000, // CLONE_NEWNET +]; +const CLONE_NEWTIME: u64 = 0x80; + +/// `ioctl` requests that type into the controlling terminal. +const TIOCSTI: u64 = 0x5412; +const TIOCLINUX: u64 = 0x541C; + +/// The x32 ABI's syscall numbers on `x86_64`: same numbers with this bit set, +/// under the same architecture value, so each denial is repeated with it. +#[cfg(target_arch = "x86_64")] +const X32_SYSCALL_BIT: i64 = 0x4000_0000; + +/// Syscalls denied whatever their arguments. +fn always_denied(kernel: (u32, u32)) -> Vec { + let mut denied = vec![ + // The user's kernel keyrings hold credentials. + libc::SYS_keyctl, + libc::SYS_add_key, + libc::SYS_request_key, + // Other processes' memory. + libc::SYS_process_vm_readv, + libc::SYS_process_vm_writev, + // Kernel programs, perf and fault handling: privilege-escalation surface. + libc::SYS_bpf, + libc::SYS_perf_event_open, + libc::SYS_userfaultfd, + libc::SYS_io_uring_setup, + libc::SYS_io_uring_enter, + libc::SYS_io_uring_register, + // The jail's view is fixed by bwrap; nothing inside remounts it. + libc::SYS_mount, + libc::SYS_umount2, + libc::SYS_pivot_root, + libc::SYS_move_mount, + libc::SYS_open_tree, + libc::SYS_fsopen, + libc::SYS_fsconfig, + libc::SYS_fsmount, + libc::SYS_fspick, + libc::SYS_mount_setattr, + libc::SYS_setns, + // File handles open any file on a mount, granted or not. + libc::SYS_open_by_handle_at, + libc::SYS_name_to_handle_at, + // Host-wide or privileged state. + libc::SYS_init_module, + libc::SYS_finit_module, + libc::SYS_delete_module, + libc::SYS_kexec_load, + libc::SYS_kexec_file_load, + libc::SYS_reboot, + libc::SYS_swapon, + libc::SYS_swapoff, + libc::SYS_acct, + libc::SYS_settimeofday, + libc::SYS_clock_settime, + libc::SYS_clock_adjtime, + libc::SYS_adjtimex, + libc::SYS_syslog, + libc::SYS_quotactl, + libc::SYS_vhangup, + ]; + #[cfg(target_arch = "x86_64")] + denied.extend([ + libc::SYS_iopl, + libc::SYS_ioperm, + libc::SYS_modify_ldt, + libc::SYS_uselib, + libc::SYS__sysctl, + ]); + // Before 4.8 a tracer could rewrite a call after seccomp had allowed it. + if kernel < (4, 8) { + denied.push(libc::SYS_ptrace); + } + denied +} + +/// One rule per flag: the call matches when any of them is set in argument 0. +fn any_flag(flags: &[u64]) -> io::Result> { + flags + .iter() + .map(|&flag| { + condition(0, SeccompCmpOp::MaskedEq(flag), flag) + .and_then(|condition| rule(vec![condition])) + }) + .collect() +} + +fn condition(arg: u8, op: SeccompCmpOp, value: u64) -> io::Result { + SeccompCondition::new(arg, SeccompCmpArgLen::Dword, op, value).map_err(|error| invalid(&error)) +} + +fn rule(conditions: Vec) -> io::Result { + SeccompRule::new(conditions).map_err(|error| invalid(&error)) +} + +fn invalid(error: &seccompiler::BackendError) -> io::Error { + io::Error::new(io::ErrorKind::InvalidInput, error.to_string()) +} + +/// The seccomp target for an architecture name (`std::env::consts::ARCH`), +/// if a filter is defined for it. +fn target_arch(name: &str) -> io::Result { + match name { + "x86_64" => Ok(TargetArch::x86_64), + "aarch64" => Ok(TargetArch::aarch64), + other => Err(io::Error::new( + io::ErrorKind::Unsupported, + format!("no seccomp filter is defined for {other}; the jail is refused"), + )), + } +} + +/// `major.minor` of a `uname -r` style release (`6.8.0-45-generic`). +pub(super) fn kernel_version(release: &str) -> Option<(u32, u32)> { + let mut parts = release + .split(|c: char| !c.is_ascii_digit()) + .filter(|part| !part.is_empty()); + Some((parts.next()?.parse().ok()?, parts.next()?.parse().ok()?)) +} + +/// Every rule keyed by syscall, with the x32 twin of each on `x86_64`. +fn with_x32(rules: BTreeMap>) -> BTreeMap> { + #[cfg(target_arch = "x86_64")] + { + let mut rules = rules; + let twins: Vec<(i64, Vec)> = rules + .iter() + .filter(|(nr, _)| **nr & X32_SYSCALL_BIT == 0) + .map(|(nr, chain)| (nr | X32_SYSCALL_BIT, chain.clone())) + .collect(); + rules.extend(twins); + rules + } + #[cfg(not(target_arch = "x86_64"))] + rules +} + +/// The two filters, compiled for this process's architecture and serialized +/// as `bwrap` reads them: the `EPERM` deny list, then `clone3` → `ENOSYS`. +/// +/// # Errors +/// `Unsupported` on an architecture without a filter; `InvalidInput` if the +/// filter does not compile. +pub(super) fn filters(kernel: (u32, u32)) -> io::Result>> { + let arch = target_arch(std::env::consts::ARCH)?; + let mut denied: BTreeMap> = always_denied(kernel) + .into_iter() + .map(|nr| (nr, Vec::new())) + .collect(); + denied.insert(libc::SYS_clone, any_flag(&CLONE_NEW)?); + let mut unshare = CLONE_NEW.to_vec(); + unshare.push(CLONE_NEWTIME); + denied.insert(libc::SYS_unshare, any_flag(&unshare)?); + denied.insert( + libc::SYS_ioctl, + [TIOCSTI, TIOCLINUX] + .into_iter() + .map(|request| condition(1, SeccompCmpOp::Eq, request).and_then(|c| rule(vec![c]))) + .collect::>()?, + ); + let eperm = errno(libc::EPERM)?; + let enosys = errno(libc::ENOSYS)?; + [ + (with_x32(denied), eperm), + ( + with_x32(BTreeMap::from([(libc::SYS_clone3, Vec::new())])), + enosys, + ), + ] + .into_iter() + .map(|(rules, action)| { + let filter = SeccompFilter::new(rules, SeccompAction::Allow, action, arch) + .map_err(|error| invalid(&error))?; + BpfProgram::try_from(filter) + .map(|program| serialize(&program)) + .map_err(|error| invalid(&error)) + }) + .collect() +} + +fn errno(value: i32) -> io::Result { + u32::try_from(value) + .map(SeccompAction::Errno) + .map_err(io::Error::other) +} + +/// `struct sock_filter` as the kernel lays it out: `u16 code, u8 jt, u8 jf, +/// u32 k`, native endian. +fn serialize(program: &BpfProgram) -> Vec { + let mut bytes = Vec::with_capacity(program.len() * 8); + for instruction in program { + bytes.extend(instruction.code.to_ne_bytes()); + bytes.push(instruction.jt); + bytes.push(instruction.jf); + bytes.extend(instruction.k.to_ne_bytes()); + } + bytes +} + +#[cfg(test)] +#[path = "seccomp_tests.rs"] +mod tests; diff --git a/crates/tinybox-jail/src/bwrap/seccomp_tests.rs b/crates/tinybox-jail/src/bwrap/seccomp_tests.rs new file mode 100644 index 00000000..03e8fb98 --- /dev/null +++ b/crates/tinybox-jail/src/bwrap/seccomp_tests.rs @@ -0,0 +1,70 @@ +//! The seccomp deny list: what it holds and that it compiles per architecture. + +use super::*; + +#[test] +fn a_kernel_release_reads_as_major_and_minor() { + assert_eq!(kernel_version("6.8.0-45-generic\n"), Some((6, 8))); + assert_eq!(kernel_version("4.4.0"), Some((4, 4))); + assert_eq!( + kernel_version("5.15.167.4-microsoft-standard-WSL2"), + Some((5, 15)) + ); + assert_eq!(kernel_version("garbage"), None); +} + +#[test] +fn ptrace_is_denied_only_before_linux_4_8() { + assert!(always_denied((4, 4)).contains(&libc::SYS_ptrace)); + assert!(!always_denied((4, 8)).contains(&libc::SYS_ptrace)); + assert!(!always_denied((6, 8)).contains(&libc::SYS_ptrace)); +} + +#[test] +fn the_escape_and_attack_surface_calls_are_denied() { + let denied = always_denied((6, 8)); + for nr in [ + libc::SYS_keyctl, + libc::SYS_bpf, + libc::SYS_io_uring_setup, + libc::SYS_mount, + libc::SYS_setns, + libc::SYS_open_by_handle_at, + libc::SYS_process_vm_readv, + ] { + assert!(denied.contains(&nr), "syscall {nr} is not denied"); + } +} + +#[test] +fn an_architecture_without_a_filter_is_refused() { + let kind = target_arch("riscv64").err().map(|error| error.kind()); + assert_eq!(kind, Some(io::ErrorKind::Unsupported)); + assert!(target_arch("x86_64").is_ok() && target_arch("aarch64").is_ok()); +} + +#[test] +fn both_filters_compile_to_whole_bpf_instructions() -> io::Result<()> { + let filters = filters((6, 8))?; + + assert_eq!( + filters.len(), + 2, + "the EPERM list and the clone3 ENOSYS filter" + ); + for filter in &filters { + assert!( + !filter.is_empty() && filter.len() % 8 == 0, + "{}", + filter.len() + ); + } + Ok(()) +} + +#[test] +fn namespace_flags_are_matched_one_rule_each() -> io::Result<()> { + // Any one flag must match on its own: rules in a chain are alternatives. + assert_eq!(any_flag(&CLONE_NEW)?.len(), CLONE_NEW.len()); + Ok(()) +} diff --git a/crates/tinybox-jail/tests/bwrap_isolation.rs b/crates/tinybox-jail/tests/bwrap_isolation.rs index 600f084d..550373fd 100644 --- a/crates/tinybox-jail/tests/bwrap_isolation.rs +++ b/crates/tinybox-jail/tests/bwrap_isolation.rs @@ -528,3 +528,79 @@ fn nested_user_namespaces_are_blocked_on_a_hardened_backend() -> io::Result<()> done("disable-userns", start); Ok(()) } + +/// Runs `script` with the jail's `python3`, or `None` when the host has none. +fn python( + backend: &tinybox_jail::BubblewrapBackend, + jail: &tinybox_jail::Jail, + script: &str, +) -> io::Result> { + if !Path::new("/usr/bin/python3").exists() { + eprintln!("skipped: no /usr/bin/python3 to issue raw syscalls"); + return Ok(None); + } + let (_, out) = sh(backend, jail, &format!("/usr/bin/python3 -c '{script}'"))?; + Ok(Some(out)) +} + +/// Raw syscalls from inside the jail: keyctl (denied), getpid (allowed) and a +/// ptrace attach to the jail's own child (allowed on Linux 4.8+). +const SYSCALL_PROBE: &str = r#" +import ctypes, errno, os, platform, time +libc = ctypes.CDLL(None, use_errno=True) +keyctl, getpid = {"x86_64": (250, 39), "aarch64": (219, 172)}[platform.machine()] +r = libc.syscall(keyctl, 0, -3, 0) +print("keyctl", r, errno.errorcode.get(ctypes.get_errno(), "none")) +print("getpid", libc.syscall(getpid) == os.getpid()) +pid = os.fork() +if pid == 0: + time.sleep(5) + os._exit(0) +r = libc.ptrace(16, pid, 0, 0) +print("ptrace", r, errno.errorcode.get(ctypes.get_errno(), "none") if r else "ok") +os.kill(pid, 9) +os.waitpid(pid, 0) +"#; + +#[test] +fn a_denied_syscall_fails_with_eperm_and_allowed_ones_still_work() -> io::Result<()> { + let Some(backend) = backend()? else { + return Ok(()); + }; + let start = Instant::now(); + assert!(backend.installs_seccomp()); + let root = tempfile::tempdir()?; + let Some(out) = python(&backend, &jail(root.path()), SYSCALL_PROBE)? else { + return Ok(()); + }; + + assert!( + out.contains("keyctl -1 EPERM"), + "keyctl was not denied: {out}" + ); + assert!( + out.contains("getpid True"), + "an allowed syscall failed: {out}" + ); + done("seccomp", start); + Ok(()) +} + +#[test] +fn a_jailed_process_can_trace_its_own_child() -> io::Result<()> { + let Some(backend) = backend()? else { + return Ok(()); + }; + let start = Instant::now(); + let root = tempfile::tempdir()?; + let Some(out) = python(&backend, &jail(root.path()), SYSCALL_PROBE)? else { + return Ok(()); + }; + + assert!( + out.contains("ptrace 0 ok"), + "ptrace of a jail child failed: {out}" + ); + done("seccomp-ptrace", start); + Ok(()) +} diff --git a/crates/tinybox-jail/tests/fixtures/fake-bwrap-noseccomp.sh b/crates/tinybox-jail/tests/fixtures/fake-bwrap-noseccomp.sh new file mode 100755 index 00000000..5b123bf1 --- /dev/null +++ b/crates/tinybox-jail/tests/fixtures/fake-bwrap-noseccomp.sh @@ -0,0 +1,36 @@ +#!/bin/bash +# Stand-in for bwrap in unit tests: reads the NUL-separated argument list from +# `--args 9` like the real thing, applies --setenv, and runs the command +# unconfined. Behaviour depends on the file name: +# fake-bwrap.sh passes nothing (the isolation probe must reject it) +# fake-bwrap-probe.sh also answers the isolation probe +# fake-bwrap-old.sh like -probe, but rejects --disable-userns +# fake-bwrap-noseccomp.sh like -probe, but its --help lacks --add-seccomp-fd +name=$(basename "$0") +if [ "$1" = "--help" ]; then + [ "$name" = "fake-bwrap-noseccomp.sh" ] || echo " --add-seccomp-fd FD Load and use seccomp rules from FD (repeatable)" + exit 0 +fi +[ "$1" = "--args" ] && [ "$3" = "--" ] || { echo "fake bwrap: expected --args N --" >&2; exit 2; } +shift 3 +mode=opts +key= +while IFS= read -r -d '' line <&9; do + case $mode in + opts) + case $line in + --setenv) mode=setenv_k ;; + --disable-userns) + if [ "$name" = "fake-bwrap-old.sh" ]; then + echo "bwrap: Unknown option --disable-userns" >&2 + exit 1 + fi ;; + esac ;; + setenv_k) key=$line; mode=setenv_v ;; + setenv_v) export "$key=$line"; mode=opts ;; + esac +done +if [ "$name" != "fake-bwrap.sh" ] && [ "$1" = "/bin/sh" ]; then + case $3 in *"echo isolated"*) echo isolated; exit 0 ;; esac +fi +exec "$@" diff --git a/crates/tinybox-jail/tests/fixtures/fake-bwrap-old.sh b/crates/tinybox-jail/tests/fixtures/fake-bwrap-old.sh index b64f5b00..5b123bf1 100755 --- a/crates/tinybox-jail/tests/fixtures/fake-bwrap-old.sh +++ b/crates/tinybox-jail/tests/fixtures/fake-bwrap-old.sh @@ -5,7 +5,12 @@ # fake-bwrap.sh passes nothing (the isolation probe must reject it) # fake-bwrap-probe.sh also answers the isolation probe # fake-bwrap-old.sh like -probe, but rejects --disable-userns +# fake-bwrap-noseccomp.sh like -probe, but its --help lacks --add-seccomp-fd name=$(basename "$0") +if [ "$1" = "--help" ]; then + [ "$name" = "fake-bwrap-noseccomp.sh" ] || echo " --add-seccomp-fd FD Load and use seccomp rules from FD (repeatable)" + exit 0 +fi [ "$1" = "--args" ] && [ "$3" = "--" ] || { echo "fake bwrap: expected --args N --" >&2; exit 2; } shift 3 mode=opts diff --git a/crates/tinybox-jail/tests/fixtures/fake-bwrap-probe.sh b/crates/tinybox-jail/tests/fixtures/fake-bwrap-probe.sh index b64f5b00..5b123bf1 100755 --- a/crates/tinybox-jail/tests/fixtures/fake-bwrap-probe.sh +++ b/crates/tinybox-jail/tests/fixtures/fake-bwrap-probe.sh @@ -5,7 +5,12 @@ # fake-bwrap.sh passes nothing (the isolation probe must reject it) # fake-bwrap-probe.sh also answers the isolation probe # fake-bwrap-old.sh like -probe, but rejects --disable-userns +# fake-bwrap-noseccomp.sh like -probe, but its --help lacks --add-seccomp-fd name=$(basename "$0") +if [ "$1" = "--help" ]; then + [ "$name" = "fake-bwrap-noseccomp.sh" ] || echo " --add-seccomp-fd FD Load and use seccomp rules from FD (repeatable)" + exit 0 +fi [ "$1" = "--args" ] && [ "$3" = "--" ] || { echo "fake bwrap: expected --args N --" >&2; exit 2; } shift 3 mode=opts diff --git a/crates/tinybox-jail/tests/fixtures/fake-bwrap.sh b/crates/tinybox-jail/tests/fixtures/fake-bwrap.sh index b64f5b00..5b123bf1 100755 --- a/crates/tinybox-jail/tests/fixtures/fake-bwrap.sh +++ b/crates/tinybox-jail/tests/fixtures/fake-bwrap.sh @@ -5,7 +5,12 @@ # fake-bwrap.sh passes nothing (the isolation probe must reject it) # fake-bwrap-probe.sh also answers the isolation probe # fake-bwrap-old.sh like -probe, but rejects --disable-userns +# fake-bwrap-noseccomp.sh like -probe, but its --help lacks --add-seccomp-fd name=$(basename "$0") +if [ "$1" = "--help" ]; then + [ "$name" = "fake-bwrap-noseccomp.sh" ] || echo " --add-seccomp-fd FD Load and use seccomp rules from FD (repeatable)" + exit 0 +fi [ "$1" = "--args" ] && [ "$3" = "--" ] || { echo "fake bwrap: expected --args N --" >&2; exit 2; } shift 3 mode=opts