diff --git a/crates/tinybox-linux/src/lib.rs b/crates/tinybox-linux/src/lib.rs index 74ffe242..18f605a4 100644 --- a/crates/tinybox-linux/src/lib.rs +++ b/crates/tinybox-linux/src/lib.rs @@ -47,6 +47,6 @@ mod sandbox; pub use probe::{NamespaceProbe, UsernsStatus}; pub use sandbox::{ - NAME, NamespaceSandbox, NativeJailCommand, WORKSPACE_MOUNT, native_jail_argv, + NAME, NamespaceSandbox, NativeJailCommand, NativeJailLaunch, WORKSPACE_MOUNT, native_jail_argv, native_jail_argv_with_programs, trusted_systemd_run_program, }; diff --git a/crates/tinybox-linux/src/sandbox/args.rs b/crates/tinybox-linux/src/sandbox/args.rs index 57641092..f38db3f0 100644 --- a/crates/tinybox-linux/src/sandbox/args.rs +++ b/crates/tinybox-linux/src/sandbox/args.rs @@ -342,7 +342,7 @@ pub struct NativeJailCommand { /// # Errors /// Returns an error when a request is invalid, unsupported, or systemd's /// command-argument expansion behavior cannot be verified. -pub fn native_jail_argv(request: &NativeJailCommand) -> std::io::Result> { +pub fn native_jail_argv(request: &NativeJailCommand) -> std::io::Result { native_jail_argv_with_programs( request, &trusted_systemd_run_program()?, @@ -360,13 +360,50 @@ pub fn native_jail_argv_with_programs( request: &NativeJailCommand, systemd_run: &Path, bwrap: &Path, -) -> std::io::Result> { +) -> std::io::Result { validate_native_jail_command(request)?; let systemd_run = validate_trusted_program(systemd_run)?; let bwrap = validate_trusted_program(bwrap)?; let supports_expand_environment = systemd_run_supports_safe_arguments(&systemd_run)?; + let env_file = if request.env.is_empty() { + None + } else { + let avoid: Vec<&Path> = std::iter::once(request.root.as_path()) + .chain(request.read_only.iter().map(PathBuf::as_path)) + .chain(request.read_write.iter().map(PathBuf::as_path)) + .collect(); + Some(super::env_file::EnvArgsFile::create(&request.env, &avoid)?) + }; let argv = native_bwrap_argv(request, supports_expand_environment, &systemd_run, &bwrap); - Ok(native_launcher_argv(wrap_scope_command(argv))) + Ok(NativeJailLaunch { + argv: native_launcher_argv( + env_file.as_ref().map(super::env_file::EnvArgsFile::path), + wrap_scope_command(argv), + ), + env_file, + }) +} + +/// A native jail's launcher command, and the private file carrying its +/// environment (see `env_file`). +/// +/// Keep it alive until the launcher has started: the launcher deletes the file +/// once it has it open, and dropping this removes it if the launcher never ran. +#[derive(Debug)] +pub struct NativeJailLaunch { + /// The launcher's program and arguments. Holds no environment value. + pub argv: Vec, + env_file: Option, +} + +impl NativeJailLaunch { + /// Where the target's environment waits for the launcher, if it has one. + #[must_use] + pub fn env_file(&self) -> Option<&Path> { + self.env_file + .as_ref() + .map(super::env_file::EnvArgsFile::path) + } } /// Sample cgroup counters from inside the transient scope before it is stopped. @@ -673,27 +710,33 @@ fn native_bwrap_argv( request.cwd.display().to_string(), "--clearenv".into(), ]); - for (key, value) in &request.env { - argv.extend(["--setenv".into(), key.clone(), value.clone()]); + // The target environment is read from the private file the launcher opened + // as descriptor 9, never from this command line (see `env_file`). + if !request.env.is_empty() { + argv.extend(["--args".into(), super::env_file::ENV_ARGS_FD.into()]); } argv.push("--".into()); argv.extend(request.argv.iter().cloned()); // A scope launcher inherits the host process's descriptors. The trusted // shim closes every non-stdio descriptor before opening systemd's bus or // starting bubblewrap, preventing a pre-opened manager/cgroup FD from - // bypassing the mount and network namespaces. Target values remain argv - // data and the shim receives only a fixed, sanitized launcher environment. + // bypassing the mount and network namespaces. The target environment is + // not argv data: it travels in the private file on descriptor 9, and the + // shim receives only a fixed, sanitized launcher environment. argv } -fn native_launcher_argv(argv: Vec) -> Vec { +/// The launcher shim, given the private environment file (or `-` when the +/// target has no environment) and then the scope command. +fn native_launcher_argv(env_file: Option<&Path>, argv: Vec) -> Vec { let mut launcher = vec![ "/bin/bash".to_owned(), "--noprofile".to_owned(), "--norc".to_owned(), "-c".to_owned(), - "[ -d /proc/self/fd ] || exit 125; shopt -s nullglob; fdpaths=(/proc/self/fd/*); ((${#fdpaths[@]} > 0)) || exit 125; for fdpath in \"${fdpaths[@]}\"; do fd=${fdpath##*/}; case $fd in 0|1|2|*[!0-9]*) continue;; esac; eval \"exec ${fd}>&-\" || exit 125; done; unit=; for arg in \"$@\"; do case $arg in --unit=*) unit=${arg#--unit=};; esac; done; [ -n \"$unit\" ] || exit 125; \"$@\"; status=$?; if ! /usr/bin/systemctl --user stop \"$unit\"; then /usr/bin/systemctl --user is-active --quiet \"$unit\" && exit 125; fi; exit \"$status\"".to_owned(), + "[ -d /proc/self/fd ] || exit 125; shopt -s nullglob; envfile=$1; shift; keep=; if [ \"$envfile\" != - ]; then exec 9<\"$envfile\" || exit 125; command -p rm -f -- \"$envfile\" || exit 125; command -p rmdir -- \"${envfile%/*}\" 2>/dev/null; keep=9; fi; fdpaths=(/proc/self/fd/*); ((${#fdpaths[@]} > 0)) || exit 125; for fdpath in \"${fdpaths[@]}\"; do fd=${fdpath##*/}; case $fd in 0|1|2|*[!0-9]*) continue;; esac; [ \"$fd\" = \"$keep\" ] && continue; eval \"exec ${fd}>&-\" || exit 125; done; unit=; for arg in \"$@\"; do case $arg in --unit=*) unit=${arg#--unit=};; esac; done; [ -n \"$unit\" ] || exit 125; \"$@\"; status=$?; if ! /usr/bin/systemctl --user stop \"$unit\"; then /usr/bin/systemctl --user is-active --quiet \"$unit\" && exit 125; fi; exit \"$status\"".to_owned(), "tinybox-native-launcher".to_owned(), + env_file.map_or_else(|| "-".to_owned(), |path| path.display().to_string()), ]; launcher.extend(argv); launcher diff --git a/crates/tinybox-linux/src/sandbox/env_file.rs b/crates/tinybox-linux/src/sandbox/env_file.rs new file mode 100644 index 00000000..e4b0306d --- /dev/null +++ b/crates/tinybox-linux/src/sandbox/env_file.rs @@ -0,0 +1,133 @@ +//! Keeping a jailed command's environment out of every command line. +//! +//! `/proc//cmdline` is readable by every process of the same user, so a +//! `--setenv TOKEN value` on the `bwrap`, `systemd-run` or launcher command +//! line hands the value to all of them. The `--setenv` pairs are written, +//! NUL-separated, to a private file (mode 0600 in a fresh 0700 directory) that +//! the launcher opens as descriptor 9 and deletes before anything else runs; +//! `bwrap --args 9` reads them from there. Setting the variables on the +//! launcher's own environment instead would expose them to the host-side +//! programs on the way (the shell, `systemd-run`), and let a value such as +//! `LD_PRELOAD` run code there. + +use std::collections::BTreeMap; +use std::fmt::Write as _; +use std::io::{self, Read as _}; +use std::path::{Path, PathBuf}; + +/// The descriptor the launcher opens the file on, and `bwrap --args` reads. +pub(super) const ENV_ARGS_FD: &str = "9"; + +/// A private file holding a jailed command's `--setenv` arguments. +/// +/// Removed when dropped, in case the launcher never ran to remove it. +#[derive(Debug)] +pub(super) struct EnvArgsFile { + path: PathBuf, +} + +impl EnvArgsFile { + /// Writes `env` as `--setenv KEY VALUE` triples into a new private file in + /// `$XDG_RUNTIME_DIR` (else the temp directory), refusing a directory that + /// lies under any of `avoid`, since the jailed program could then read it. + /// + /// # Errors + /// `InvalidInput` when a key or value holds a NUL; `PermissionDenied` when + /// every candidate directory is under `avoid`; otherwise the I/O error. + pub(super) fn create(env: &BTreeMap, avoid: &[&Path]) -> io::Result { + let mut contents = Vec::new(); + for (key, value) in env { + if key.contains('\0') || value.contains('\0') { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "an environment entry holds a NUL", + )); + } + for part in ["--setenv", key.as_str(), value.as_str()] { + contents.extend_from_slice(part.as_bytes()); + contents.push(0); + } + } + let base = private_base(avoid)?; + let dir = base.join(format!("tinybox-env-{}", random_hex()?)); + let path = dir.join("env"); + if let Err(error) = write_private(&dir, &path, &contents) { + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + return Err(error); + } + Ok(Self { path }) + } + + /// Where the file is; the launcher's first argument. + #[must_use] + pub(super) fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for EnvArgsFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + if let Some(dir) = self.path.parent() { + let _ = std::fs::remove_dir(dir); + } + } +} + +/// Creates `dir` (0700) and writes `contents` into a new `path` (0600) in it. +#[cfg(unix)] +fn write_private(dir: &Path, path: &Path, contents: &[u8]) -> io::Result<()> { + use std::io::Write as _; + use std::os::unix::fs::{DirBuilderExt as _, OpenOptionsExt as _}; + + std::fs::DirBuilder::new().mode(0o700).create(dir)?; + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(path)? + .write_all(contents) +} + +/// The native jail runs only on Linux; elsewhere there is nothing to launch. +#[cfg(not(unix))] +fn write_private(_dir: &Path, _path: &Path, _contents: &[u8]) -> io::Result<()> { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "private environment files need a Unix host", + )) +} + +/// `$XDG_RUNTIME_DIR` when set and outside `avoid`, else the temp directory, +/// canonicalized before the check so a link into a grant is caught. +fn private_base(avoid: &[&Path]) -> io::Result { + std::env::var_os("XDG_RUNTIME_DIR") + .map(PathBuf::from) + .filter(|dir| dir.is_absolute()) + .into_iter() + .chain(std::iter::once(std::env::temp_dir())) + .filter_map(|dir| dir.canonicalize().ok()) + .find(|dir| dir.is_dir() && !avoid.iter().any(|grant| dir.starts_with(grant))) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::PermissionDenied, + "no private directory outside the jail's grants for its environment", + ) + }) +} + +/// 24 hex characters from the kernel's random source. +fn random_hex() -> io::Result { + let mut random = [0_u8; 12]; + std::fs::File::open("/dev/urandom")?.read_exact(&mut random)?; + let mut out = String::new(); + for byte in random { + let _ = write!(out, "{byte:02x}"); + } + Ok(out) +} + +#[cfg(all(test, unix))] +#[path = "env_file_tests.rs"] +mod tests; diff --git a/crates/tinybox-linux/src/sandbox/env_file_tests.rs b/crates/tinybox-linux/src/sandbox/env_file_tests.rs new file mode 100644 index 00000000..a7e212f1 --- /dev/null +++ b/crates/tinybox-linux/src/sandbox/env_file_tests.rs @@ -0,0 +1,65 @@ +//! The private environment file: contents, permissions and cleanup. + +use super::*; +use std::os::unix::fs::PermissionsExt as _; + +fn env(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(key, value)| ((*key).to_owned(), (*value).to_owned())) + .collect() +} + +#[test] +fn the_file_holds_nul_separated_setenv_triples() -> io::Result<()> { + let file = EnvArgsFile::create(&env(&[("A", "1"), ("TOKEN", "s3cret value")]), &[])?; + + let contents = std::fs::read(file.path())?; + + assert_eq!( + contents, + b"--setenv\0A\x001\0--setenv\0TOKEN\0s3cret value\0".to_vec() + ); + Ok(()) +} + +#[test] +fn the_file_and_its_directory_are_private_and_removed_on_drop() -> io::Result<()> { + let file = EnvArgsFile::create(&env(&[("K", "v")]), &[])?; + let path = file.path().to_path_buf(); + let dir = path + .parent() + .map(Path::to_path_buf) + .ok_or(io::ErrorKind::NotFound)?; + + assert_eq!( + std::fs::metadata(&path)?.permissions().mode() & 0o777, + 0o600 + ); + assert_eq!(std::fs::metadata(&dir)?.permissions().mode() & 0o777, 0o700); + + drop(file); + assert!(!path.exists()); + assert!(!dir.exists()); + Ok(()) +} + +#[test] +fn a_nul_in_an_entry_is_refused() { + for pairs in [[("K\0", "v")], [("K", "v\0")]] { + let kind = EnvArgsFile::create(&env(&pairs), &[]) + .err() + .map(|e| e.kind()); + assert_eq!(kind, Some(io::ErrorKind::InvalidInput)); + } +} + +#[test] +fn a_directory_inside_a_grant_is_never_used() { + // Every candidate (the runtime dir and the temp dir) lies under `/`. + let root = Path::new("/"); + let kind = EnvArgsFile::create(&env(&[("K", "v")]), &[root]) + .err() + .map(|e| e.kind()); + assert_eq!(kind, Some(io::ErrorKind::PermissionDenied)); +} diff --git a/crates/tinybox-linux/src/sandbox/mod.rs b/crates/tinybox-linux/src/sandbox/mod.rs index ab951f1a..0e355984 100644 --- a/crates/tinybox-linux/src/sandbox/mod.rs +++ b/crates/tinybox-linux/src/sandbox/mod.rs @@ -10,10 +10,11 @@ use tinybox_core::{ }; mod args; +mod env_file; pub use args::{ - NativeJailCommand, WORKSPACE_MOUNT, native_jail_argv, native_jail_argv_with_programs, - trusted_systemd_run_program, + NativeJailCommand, NativeJailLaunch, WORKSPACE_MOUNT, native_jail_argv, + native_jail_argv_with_programs, trusted_systemd_run_program, }; /// The name this sandbox registers under. diff --git a/crates/tinybox-linux/src/sandbox/mod_tests.rs b/crates/tinybox-linux/src/sandbox/mod_tests.rs index 843f2465..fd7bdf58 100644 --- a/crates/tinybox-linux/src/sandbox/mod_tests.rs +++ b/crates/tinybox-linux/src/sandbox/mod_tests.rs @@ -452,7 +452,7 @@ fn native_jail_command_keeps_limits_and_target_environment_after_namespace_setup let grant = root.path().join("read-only"); std::fs::create_dir(&grant)?; let systemd_run = crate::trusted_systemd_run_program()?; - let argv = crate::native_jail_argv_with_programs( + let launch = crate::native_jail_argv_with_programs( &crate::NativeJailCommand { root: root.path().to_path_buf(), cwd, @@ -470,6 +470,7 @@ fn native_jail_command_keeps_limits_and_target_environment_after_namespace_setup &systemd_run, Path::new("/bin/true"), )?; + let argv = launch.argv.clone(); assert_eq!(argv.first().map(String::as_str), Some("/bin/bash")); assert!(argv.contains(&systemd_run.display().to_string())); assert!(systemd_run.is_absolute()); @@ -493,15 +494,21 @@ fn native_jail_command_keeps_limits_and_target_environment_after_namespace_setup ); assert!(has(&argv, "--ro-bind")); assert!(has(&argv, grant.to_str().ok_or("grant path is not UTF-8")?)); - assert!(has(&argv, "BASH_ENV")); - let systemd_end = argv + // The target environment is on no command line, the launcher's included: + // `/proc//cmdline` is readable by every process of the user. + assert!(!argv.iter().any(|part| part.contains("BASH_ENV"))); + assert!(!argv.iter().any(|part| part.contains("/tmp/target-init"))); + assert!(!has(&argv, "--setenv")); + let args_at = argv .iter() - .position(|part| part == "--") - .ok_or("missing systemd delimiter")?; - assert!( - !argv[..systemd_end] - .iter() - .any(|part| part.contains("BASH_ENV")) + .position(|part| part == "--args") + .ok_or("bwrap reads the environment from a descriptor")?; + assert_eq!(argv.get(args_at + 1).map(String::as_str), Some("9")); + let file = launch.env_file().ok_or("an environment file")?; + assert!(has(&argv, file.to_str().ok_or("file path is not UTF-8")?)); + assert_eq!( + std::fs::read(file)?, + b"--setenv\0BASH_ENV\0/tmp/target-init\0".to_vec() ); Ok(()) } diff --git a/crates/tinybox-module/src/tinybus_module/native_jail.rs b/crates/tinybox-module/src/tinybus_module/native_jail.rs index 2d7c0799..66bd7830 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail.rs @@ -263,6 +263,7 @@ fn unavailable() -> Error { struct NativeResourceBackend { expected: Mutex>>, requested: Mutex>, + launch: Mutex>, } impl NativeResourceBackend { @@ -270,9 +271,21 @@ impl NativeResourceBackend { Self { expected: Mutex::new(None), requested: Mutex::new(None), + launch: Mutex::new(None), } } + /// Holds the launch, and with it the private environment file, until this + /// execution's backend is dropped. + fn keep(&self, launch: tinybox_linux::NativeJailLaunch) -> Result<()> { + let mut held = self.launch.lock().map_err(|_| unavailable())?; + if held.is_some() { + return Err(unavailable()); + } + *held = Some(launch); + Ok(()) + } + fn bind(&self, argv: &[String]) -> Result<()> { let mut expected = self.expected.lock().map_err(|_| unavailable())?; if expected.is_some() { @@ -423,10 +436,14 @@ fn configure_resource_launcher( .map(Path::to_path_buf) .ok_or_else(unavailable)?; let systemd_run = tinybox_linux::trusted_systemd_run_program().map_err(|_| unavailable())?; - let argv = tinybox_linux::native_jail_argv_with_programs(native, &systemd_run, &bwrap) + let launch = tinybox_linux::native_jail_argv_with_programs(native, &systemd_run, &bwrap) .map_err(|_| unavailable())?; - backend.bind(&argv)?; - command.argv = argv; + backend.bind(&launch.argv)?; + command.argv.clone_from(&launch.argv); + // Held for the execution: the launcher opens and deletes the private file + // carrying the target's environment, and dropping this removes it if the + // launcher never ran. + backend.keep(launch)?; command.env = launcher_environment(); command.cwd = None; Ok(()) diff --git a/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs b/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs index 1f24d975..44424844 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs @@ -1677,3 +1677,145 @@ async fn a_refused_fork_in_a_live_scope_is_reported_as_a_pids_limit_hit() assert!(!journal.memory_oom_hit(), "nothing was killed for memory"); Ok(()) } + +/// Every host process command line that holds `needle`, NULs shown as spaces. +#[cfg(target_os = "linux")] +fn command_lines_containing(needle: &str) -> std::io::Result> { + let mut found = Vec::new(); + for entry in std::fs::read_dir("/proc")? { + let path = entry?.path(); + if !path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.bytes().all(|byte| byte.is_ascii_digit())) + { + continue; + } + if let Ok(cmdline) = std::fs::read(path.join("cmdline")) + && cmdline + .windows(needle.len()) + .any(|window| window == needle.as_bytes()) + { + found.push(String::from_utf8_lossy(&cmdline).replace('\0', " ")); + } + } + Ok(found) +} + +/// The scan the live test relies on does find a value on a command line. +#[cfg(target_os = "linux")] +#[test] +fn the_command_line_scan_finds_a_running_process_argument() +-> std::result::Result<(), Box> { + let marker = format!("scan-marker-{}", std::process::id()); + let mut child = std::process::Command::new("/bin/sh") + .args(["-c", "sleep 30", &marker]) + .spawn()?; + let found = command_lines_containing(&marker); + child.kill()?; + child.wait()?; + assert!(found?.iter().any(|line| line.contains(&marker))); + Ok(()) +} + +/// Live: while a 2.1 jailed command runs, its environment values are on no +/// process's command line on the host, the launcher, `systemd-run` and `bwrap` +/// included, yet the command itself sees them. +#[cfg(target_os = "linux")] +#[test] +fn a_resource_jail_keeps_the_target_environment_off_every_command_line() +-> std::result::Result<(), Box> { + use std::{ + io::Write, + process::{Command, Stdio}, + }; + + if !native_resource_runtime_available() { + eprintln!("skipped: rootless systemd scope and bubblewrap namespaces are unavailable"); + return Ok(()); + } + let root = tempfile::tempdir()?; + let workspace = root.path().join("workspace"); + std::fs::create_dir(&workspace)?; + let secret = format!( + "jail-secret-{}", + root.path() + .file_name() + .and_then(|name| name.to_str()) + .ok_or("temp dir name")? + ); + let mut jail = config(); + jail.allow_subprocess = true; + let request = tinybox_bus::NativeJailRequest { + jail, + resources: Some(tinybox_bus::NativeJailResourceRequest { + cpu_millis: Some(1000), + memory_bytes: None, + pids_max: Some(64), + disk_bytes: None, + }), + }; + let mut command = tinybox_core::ExecRequest::new([ + "/bin/sh", + "-c", + "printf 'READY\\n'; read -r _; printf '%s' \"$JAIL_SECRET\"", + ]); + command.cwd = Some(workspace.clone()); + command.env.insert("JAIL_SECRET".into(), secret.clone()); + let backend = resource_backend(); + let jail = prepare_with_resources(&workspace, &request, &mut command, backend.as_ref())?; + assert!( + !command.argv.iter().any(|part| part.contains(&secret)), + "the launcher's own argv carries the value" + ); + let mut child_command = Command::new(command.argv.first().ok_or("missing launcher")?); + child_command + .args(command.argv.iter().skip(1)) + .env_clear() + .envs(&command.env) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut child = tinybox_jail::spawn_captured_with(backend.as_ref(), &jail, child_command)?; + let mut ready = String::new(); + std::io::BufRead::read_line( + &mut std::io::BufReader::new(child.stdout.as_mut().ok_or("missing stdout pipe")?), + &mut ready, + )?; + if ready != "READY\n" { + let output = child.wait_with_output()?; + return Err(format!( + "the jailed command did not start: status {:?}, stderr {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ) + .into()); + } + + // Every process on the host while the jail runs: launcher, systemd-run, + // the scope wrapper, bwrap and the target. + let leaked = command_lines_containing(&secret)?; + + child + .stdin + .as_mut() + .ok_or("missing stdin pipe")? + .write_all(b"go\n")?; + let output = child.wait_with_output()?; + assert!( + leaked.is_empty(), + "environment value on a command line: {leaked:?}" + ); + assert!( + output.status.success(), + "jailed command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + // The command still received the value, from the private file. + assert!( + String::from_utf8_lossy(&output.stdout).ends_with(&secret), + "stdout: {}", + String::from_utf8_lossy(&output.stdout) + ); + Ok(()) +}