From 9c1981b07119291c5976a41cd92dc35e30d39823 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 15:03:16 -0700 Subject: [PATCH 01/16] sandbox: drop the control_socket opt-out knob Whether a sandbox shows up in sandlock ps is the operator's concern, not a policy setting, and no tool exposes the knob anyway: it had no CLI flag, no SDK field, and serde skipped it, so only the Rust builder could reach it and nothing in-tree ever did. Worse, setting it false skipped setup_runtime_dir, which is also where the UID-wide name collision check lives, so the knob quietly disabled name uniqueness. The one real reason to run without a socket, a nested sandbox whose outer policy blocks /dev/shm, is already handled by the best-effort fallback that warns and continues. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 2 +- crates/sandlock-core/src/sandbox.rs | 88 +++++++------------ crates/sandlock-core/src/sandbox/builder.rs | 19 ---- .../tests/integration/test_control.rs | 30 +------ docs/sandbox-reference.md | 1 - 5 files changed, 33 insertions(+), 107 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 96309ecd..6cb78c77 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -985,7 +985,7 @@ fn validate_cli_name(name: &str) -> Result<(), String> { /// Query the control socket for the virtual→real port map, returning a /// compact display string or `"-"` when the socket is missing (e.g. -/// `--no-supervisor` or `control_socket = false`). +/// `--no-supervisor`). fn query_ports(name: &str) -> String { use sandlock_core::control::send_control_request; match send_control_request(name, "ports", serde_json::Value::Object(Default::default())) { diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 8d06d60c..63bcd09b 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -362,12 +362,6 @@ impl BindPorts { } } -/// Serde default for `control_socket` — deserialized configs that don't -/// mention the field still get introspection enabled. -fn default_control_socket() -> bool { - true -} - /// Sandbox configuration. #[derive(Serialize, Deserialize)] pub struct Sandbox { @@ -530,12 +524,6 @@ pub struct Sandbox { /// allows one `SECCOMP_FILTER_FLAG_NEW_LISTENER` per task. pub no_supervisor: bool, - /// Enable the per-sandbox control socket for introspection (`sandlock ps`, - /// `sandlock inspect`, etc.). Defaults to `true`. Set to `false` to skip - /// the runtime dir, pid file, and control-socket tokio task entirely. - #[serde(skip, default = "default_control_socket")] - pub control_socket: bool, - // User-namespace identity (run-as uid/gid) pub user: Option, @@ -653,7 +641,6 @@ impl Clone for Sandbox { num_cpus: self.num_cpus, port_remap: self.port_remap, no_supervisor: self.no_supervisor, - control_socket: self.control_socket, user: self.user, policy_fn: self.policy_fn.clone(), name: self.name.clone(), @@ -1989,8 +1976,7 @@ impl Sandbox { // Even for --no-supervisor sandboxes, write a pid file so sandlock ps // can discover and list them. The control socket is only created when - // a supervisor exists (inside the if-let below). Honour the - // control_socket opt-out knob. + // a supervisor exists (inside the if-let below). // // Use setup_runtime_dir_no_socket to get the liveness check — a // no-supervisor sandbox with the same name as a live sandbox must @@ -2003,7 +1989,7 @@ impl Sandbox { // notif-fd write is the child's setup-complete signal, so returning // before it races killpg against setpgid and can deadlock Drop's // waitpid against a child parked on the ready pipe. - if no_supervisor && self.control_socket { + if no_supervisor { let sandbox_name = self.rt().name.clone(); let supervisor_pid = std::process::id() as i32; match crate::control::setup_runtime_dir_no_socket( @@ -2059,50 +2045,38 @@ impl Sandbox { // Best-effort: in nested sandboxes /dev/shm may be restricted by // the outer sandlock's landlock policy. Warn and continue without // a control socket rather than failing the sandbox. - // - // Honour the control_socket opt-out knob: when false, skip the - // entire runtime dir + socket setup. let sandbox_name = self.rt().name.clone(); let supervisor_pid = std::process::id() as i32; - let control_listener: Option; - if self.control_socket { - match crate::control::setup_runtime_dir( - &sandbox_name, - pid, - supervisor_pid, - self.mode.as_deref(), - ) { - Ok((listener, control_dir)) => { - self.rt_mut().control_dir = Some(control_dir); - control_listener = Some(listener); - } - Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - // Name collision with a live sandbox — hard-fail. - // A second sandbox with the same name would be - // invisible to ps and its Drop would remove the - // first one's runtime dir. - return Err(SandboxRuntimeError::Child(format!( - "sandbox '{}' is already running: {}", - sandbox_name, e - )) - .into()); - } - Err(e) => { - // Best-effort: in nested sandboxes /dev/shm may be - // restricted by the outer sandlock's landlock policy. - // Warn and continue without a control socket rather - // than failing the sandbox. - eprintln!( - "sandlock: control socket setup failed for '{}': {} \ - (introspection unavailable for this sandbox)", - sandbox_name, e - ); - control_listener = None; - } + let control_listener = match crate::control::setup_runtime_dir( + &sandbox_name, + pid, + supervisor_pid, + self.mode.as_deref(), + ) { + Ok((listener, control_dir)) => { + self.rt_mut().control_dir = Some(control_dir); + Some(listener) } - } else { - control_listener = None; - } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + // Name collision with a live sandbox — hard-fail. + // A second sandbox with the same name would be + // invisible to ps and its Drop would remove the + // first one's runtime dir. + return Err(SandboxRuntimeError::Child(format!( + "sandbox '{}' is already running: {}", + sandbox_name, e + )) + .into()); + } + Err(e) => { + eprintln!( + "sandlock: control socket setup failed for '{}': {} \ + (introspection unavailable for this sandbox)", + sandbox_name, e + ); + None + } + }; if self.time_start.is_some() || self.random_seed.is_some() { let time_offset = self.time_start.map(|t| crate::time::calculate_time_offset(t)); diff --git a/crates/sandlock-core/src/sandbox/builder.rs b/crates/sandlock-core/src/sandbox/builder.rs index 6eaa2112..5400e49c 100644 --- a/crates/sandlock-core/src/sandbox/builder.rs +++ b/crates/sandlock-core/src/sandbox/builder.rs @@ -202,13 +202,6 @@ pub struct SandboxBuilder { #[cfg_attr(feature = "cli", clap(skip))] pub no_supervisor: bool, - /// Enable the per-sandbox control socket for introspection. Defaults to - /// `true`. When `false`, no runtime dir, pid file, or control-socket task - /// is created — `sandlock ps` and `sandlock inspect` will not see this - /// sandbox. - #[cfg_attr(feature = "cli", clap(skip))] - pub control_socket: bool, - #[cfg_attr(feature = "cli", arg(long = "user", value_name = "UID:GID"))] pub user: Option, @@ -301,7 +294,6 @@ impl Default for SandboxBuilder { num_cpus: None, port_remap: false, no_supervisor: false, - control_socket: true, user: None, protection_policy: ProtectionPolicy::default(), policy_fn: None, @@ -365,7 +357,6 @@ impl Clone for SandboxBuilder { num_cpus: self.num_cpus, port_remap: self.port_remap, no_supervisor: self.no_supervisor, - control_socket: self.control_socket, user: self.user, protection_policy: self.protection_policy.clone(), policy_fn: self.policy_fn.clone(), @@ -740,15 +731,6 @@ impl SandboxBuilder { self } - /// Enable or disable the per-sandbox control socket. Defaults to `true`. - /// When `false`, no runtime dir, pid file, or control-socket task is - /// created — `sandlock ps` and `sandlock inspect` will not see this - /// sandbox. - pub fn control_socket(mut self, v: bool) -> Self { - self.control_socket = v; - self - } - pub fn policy_fn( mut self, f: impl Fn(crate::policy_fn::SyscallEvent, &mut crate::policy_fn::PolicyContext) -> crate::policy_fn::Verdict + Send + Sync + 'static, @@ -1056,7 +1038,6 @@ impl SandboxBuilder { num_cpus: self.num_cpus, port_remap: self.port_remap, no_supervisor: self.no_supervisor, - control_socket: self.control_socket, user: self.user, policy_fn: self.policy_fn, name: self.name, diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index f32b2275..e9b1c382 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -367,7 +367,7 @@ fn test_control_sandbox_to_json() { } // ============================================================ -// Name collision, --no-supervisor, control_socket=false, ports +// Name collision, --no-supervisor, ports // ============================================================ #[test] @@ -499,34 +499,6 @@ fn test_control_no_supervisor() { let _ = child.wait(); } -#[test] -fn test_control_socket_disabled() { - // control_socket = false is a builder field, not a CLI flag. - // The sandbox runs without binding the control socket — the pid file - // is still written (via setup_runtime_dir_no_socket) so ps still sees - // it, but config/ports/kill via the socket fail gracefully. - let sb = sandlock_core::Sandbox::builder() - .fs_read("/usr") - .fs_read("/bin") - .control_socket(false) - .build() - .unwrap(); - assert!( - !sb.control_socket, - "control_socket should be false" - ); - - // Also test the default (true). - let sb2 = sandlock_core::Sandbox::builder() - .fs_read("/usr") - .build() - .unwrap(); - assert!( - sb2.control_socket, - "control_socket should default to true" - ); -} - #[test] fn test_control_ports_verb() { let name = format!("test-ctrl-ports-{}", std::process::id()); diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index 5f2e2058..c189b4a8 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -413,7 +413,6 @@ and have no TOML counterpart. | `policy_fn` | `Callable \| None`| `None` | Per-event dynamic policy callback. See the project README's "Dynamic Policy" section. | | `init_fn` | `Callable \| None`| `None` | Callback invoked once in the template process prior to COW fork. | | `work_fn` | `Callable \| None`| `None` | Callback invoked in each COW clone; receives `clone_id` as its argument. | -| `control_socket` | `bool` | `True` | Enable the per-sandbox control socket for introspection (`sandlock ps`, `sandlock inspect`). When `False`, no runtime dir, pid file, or control-socket task is created: the sandbox is invisible to `sandlock ps` / `sandlock inspect`. `no_supervisor` sandboxes only create a control socket when `control_socket=True`. | ## Advanced From 846ee2665b74b2e480736c782047ccf43cbc303c Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 15:52:08 -0700 Subject: [PATCH 02/16] control: address sandboxes by abstract socket name The /dev/shm runtime directory needed a writable path from any outer policy, left stale directories for ps to prune, and made name uniqueness a side effect of a directory existing. An abstract Unix socket named sandlock// has none of those: bind fails on a taken name, the name dies with the process, and /proc/net/unix lists it. Names must now avoid whitespace and control characters because that listing is whitespace-delimited. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 3 + crates/sandlock-core/src/control.rs | 352 +++++++--------------------- crates/sandlock-core/src/sandbox.rs | 9 +- 3 files changed, 89 insertions(+), 275 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 6cb78c77..16cf3bd2 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -974,6 +974,9 @@ fn validate_cli_name(name: &str) -> Result<(), String> { if name.contains('\0') { return Err("sandbox name must not contain NUL bytes".into()); } + if name.bytes().any(|b| b <= 0x20 || b == 0x7f) { + return Err("sandbox name must not contain whitespace or control characters".into()); + } if name.contains('/') { return Err("sandbox name must not contain '/'".into()); } diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 5f991ad6..6776209a 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -1,189 +1,66 @@ -//! Per-sandbox Unix control socket for introspection. +//! Per-sandbox control socket for introspection. //! -//! Every sandbox (CLI, Python SDK, embedded) gets a runtime directory under -//! `/dev/shm/sandlock-$UID//` containing: +//! Every sandbox (CLI, Python SDK, embedded) binds one abstract Unix +//! stream socket named `\0sandlock//` from the supervisor +//! process before the child is released. Abstract names live in the +//! kernel, not the filesystem: bind on a taken name fails, so the name is +//! the UID-wide sandbox mutex; the name vanishes with the process, so +//! nothing is ever stale; `/proc/net/unix` lists them, so `sandlock ps` +//! needs no registry on disk; and a nested sandlock needs no writable +//! directory from the outer policy, only permission to create a socket. //! -//! * `pid` — two-line pid file (`child_pid\nsupervisor_pid\n`); lets -//! `sandlock ps` list and prune dead sandboxes without opening the -//! socket. The child PID is used for `/proc` introspection (UPTIME, -//! CMD); the supervisor PID owns the control socket and is used for -//! liveness checks. -//! * `control.sock` — Unix stream socket bound by the supervisor before the -//! child is forked. Serves the introspection wire protocol. +//! Abstract names carry no permission bits, so the server checks +//! SO_PEERCRED and closes any connection from another uid. //! //! ## Wire protocol //! -//! 4-byte big-endian length prefix, then UTF-8 JSON. One client at a time per -//! socket. +//! 4-byte big-endian length prefix, then UTF-8 JSON. One request per +//! connection. //! //! Request: //! ```json -//! {"v": 1, "verb": "config", "args": {}} +//! {"v": 1, "verb": "info", "args": {}} //! ``` //! //! Response: //! ```json -//! {"v": 1, "ok": true, "data": { ...effective Sandbox policy... }} +//! {"v": 1, "ok": true, "data": {"child_pid": 1234, "supervisor_pid": 1233, "mode": null}} //! ``` //! or //! ```json //! {"v": 1, "ok": false, "err": "..."} //! ``` +//! +//! Verbs: `info` (pids and mode), `config` (effective policy as +//! `ProfileInput`), `ports` (virtual to real port map). -use std::os::unix::net::UnixListener; -use std::path::{Path, PathBuf}; +use std::os::linux::net::SocketAddrExt; +use std::os::unix::net::{SocketAddr, UnixListener}; +use std::path::PathBuf; use std::sync::Arc; use crate::sandbox::Sandbox; use crate::seccomp::ctx::SupervisorCtx; // ============================================================ -// Public API — runtime dir helpers (used by core + CLI) +// Socket address // ============================================================ -/// Return the per-user runtime directory root. -pub(crate) fn runtime_dir_uid(uid: u32) -> PathBuf { - PathBuf::from(format!("/dev/shm/sandlock-{}", uid)) +/// Bytes after the leading NUL of the abstract name. Public so tests can +/// address a socket of another uid. +pub fn socket_name(uid: u32, name: &str) -> Vec { + format!("sandlock/{uid}/{name}").into_bytes() } -/// Return the per-sandbox runtime directory for a given name. -pub fn sandbox_dir(name: &str) -> PathBuf { +fn socket_addr(name: &str) -> std::io::Result { let uid = unsafe { libc::getuid() }; - runtime_dir_uid(uid).join(name) -} - -/// Return the pid file path inside a sandbox runtime dir. -pub fn pid_path(dir: &Path) -> PathBuf { - dir.join("pid") -} - -/// Return the control socket path inside a sandbox runtime dir. -pub fn sock_path(dir: &Path) -> PathBuf { - dir.join("control.sock") -} - -/// Read a sandbox's operating-mode marker (e.g. "learn") from its runtime -/// dir. `None` for ordinary runs, which write no mode file. -pub fn sandbox_mode(name: &str) -> Option { - let s = std::fs::read_to_string(sandbox_dir(name).join("mode")).ok()?; - let s = s.trim(); - if s.is_empty() { None } else { Some(s.to_string()) } -} - -/// Read the supervisor PID from a runtime dir's pid file. -/// Returns `None` if the file is missing, unparseable, or does not -/// contain two lines (child_pid\nsupervisor_pid\n). -fn read_supervisor_pid(dir: &Path) -> Option { - let content = std::fs::read_to_string(pid_path(dir)).ok()?; - // Line 2 is the supervisor PID. - content.lines().nth(1)?.trim().parse().ok() -} - -// ============================================================ -// Runtime dir lifecycle — called from sandbox-core -// ============================================================ - -/// Create the per-sandbox runtime directory and write the pid file — shared -/// by the supervisor and no_supervisor paths. Returns the dir path. -/// -/// # Name collision -/// -/// If a runtime directory already exists for `name` and its supervisor is -/// still alive, this returns `ErrorKind::AlreadyExists`. Stale dirs (dead -/// supervisor) are removed and recreated. -/// -/// # no_supervisor callers -/// -/// The `no_supervisor` path in `do_spawn` calls this directly (without the -/// socket) instead of duplicating a bare `remove_dir_all` + `create_dir_all` -/// that had no liveness check and would wipe a live sandbox's pid file on a -/// name collision. -pub(crate) fn setup_runtime_dir( - name: &str, - child_pid: i32, - supervisor_pid: i32, - mode: Option<&str>, -) -> Result<(UnixListener, PathBuf), std::io::Error> { - let dir = setup_runtime_dir_no_socket(name, child_pid, supervisor_pid, mode)?; - - // Bind control socket. - let sp = sock_path(&dir); - let listener = UnixListener::bind(&sp)?; - - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&sp, std::fs::Permissions::from_mode(0o600))?; - } - - Ok((listener, dir)) -} - -/// Create the per-sandbox runtime directory and write the pid file, without -/// binding a control socket. Used by the `no_supervisor` path (no socket -/// exists) and as the common prefix of `setup_runtime_dir` for the supervisor -/// path. -pub(crate) fn setup_runtime_dir_no_socket( - name: &str, - child_pid: i32, - supervisor_pid: i32, - mode: Option<&str>, -) -> Result { - let dir = sandbox_dir(name); - - // Check for name collision: if the dir exists and the sandbox is still - // alive, refuse to overwrite it. - if dir.exists() { - if let Some(pid) = read_supervisor_pid(&dir) { - if unsafe { libc::kill(pid, 0) } == 0 { - return Err(std::io::Error::new( - std::io::ErrorKind::AlreadyExists, - format!("sandbox '{}' is already running (PID {})", name, pid), - )); - } - } - // Dead or unparseable — safe to remove. - std::fs::remove_dir_all(&dir)?; - } - std::fs::create_dir_all(&dir)?; - - // Restrict to owner. - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700))?; - } - - // Write pid file atomically via temp + rename so list_live_sandboxes - // never sees a partially-written or empty pid file. - // Operating-mode marker for the `sandlock ps` STATUS column. Written - // before the pid file so a listing never sees the sandbox without it. - if let Some(m) = mode { - std::fs::write(dir.join("mode"), m)?; - } - - let pid_path = pid_path(&dir); - let tmp_path = dir.join(".pid.tmp"); - std::fs::write(&tmp_path, format!("{}\n{}\n", child_pid, supervisor_pid))?; - std::fs::rename(&tmp_path, &pid_path)?; - - Ok(dir) + SocketAddr::from_abstract_name(socket_name(uid, name)) } -/// Remove the per-sandbox runtime directory. Best-effort: failures are logged -/// but never propagated (called from Drop paths). -pub fn cleanup_runtime_dir(dir: &Path) { - let pid_file = pid_path(dir); - if pid_file.exists() { - let _ = std::fs::remove_file(&pid_file); - } - let sp = sock_path(dir); - if sp.exists() { - let _ = std::fs::remove_file(&sp); - } - if dir.exists() { - let _ = std::fs::remove_dir(dir); - } +/// Bind the sandbox's control socket. `AddrInUse` means a live sandbox of +/// this uid already owns the name. +pub(crate) fn bind_control_socket(name: &str) -> std::io::Result { + UnixListener::bind_addr(&socket_addr(name)?) } // ============================================================ @@ -470,107 +347,36 @@ async fn write_response( } // ============================================================ -// Pruning — called by sandlock ps to clean up stale dirs +// Discovery // ============================================================ -/// Walk `/dev/shm/sandlock-$UID/` and return entries for every live sandbox. -/// Dead sandboxes (supervisor process is gone) are pruned. -/// -/// Returns `(name, child_pid)` pairs for live sandboxes. The child PID is -/// used by `sandlock ps` for `/proc//stat` and `/proc//cmdline`. -/// -/// Directories younger than 2 seconds are never pruned, even if the pid -/// file is missing or unparseable — this avoids a race with `setup_runtime_dir` -/// which creates the dir before writing the pid file. -pub fn list_live_sandboxes() -> Result, std::io::Error> { - let uid = unsafe { libc::getuid() }; - let root = runtime_dir_uid(uid); - if !root.exists() { - return Ok(Vec::new()); - } - - let mut live = Vec::new(); - let entries = match std::fs::read_dir(&root) { - Ok(e) => e, - Err(_) => return Ok(Vec::new()), - }; - - let now = std::time::SystemTime::now(); - - for entry in entries { - let entry = match entry { - Ok(e) => e, - Err(_) => continue, - }; - let dir = entry.path(); - if !dir.is_dir() { - continue; - } - - // Parse the pid file. Format: child_pid\nsupervisor_pid\n - let pid_file = pid_path(&dir); - let pid_str = match std::fs::read_to_string(&pid_file) { - Ok(s) => s, - Err(_) => { - // No pid file — could be a dir being set up concurrently. - // Don't prune if the dir was modified less than 2 seconds ago. - if !dir_is_recent(&dir, &now) { - let _ = std::fs::remove_dir_all(&dir); - } - continue; +/// Names of every listening control socket belonging to `uid`, parsed +/// from `/proc/net/unix` text. Columns: Num RefCount Protocol Flags Type +/// St Inode Path; Flags 00010000 is __SO_ACCEPTCON, a listening socket. +pub(crate) fn parse_proc_net_unix(text: &str, uid: u32) -> Vec { + let prefix = format!("@sandlock/{uid}/"); + let mut names: Vec = text + .lines() + .skip(1) + .filter_map(|line| { + let mut fields = line.split_whitespace(); + let flags = fields.nth(3)?; + let path = fields.nth(3)?; + if flags != "00010000" { + return None; } - }; - - let mut lines = pid_str.lines(); - let child_pid: i32 = match lines.next().and_then(|l| l.trim().parse().ok()) { - Some(p) => p, - None => { - if !dir_is_recent(&dir, &now) { - let _ = std::fs::remove_dir_all(&dir); - } - continue; - } - }; - let supervisor_pid: i32 = match lines.next().and_then(|l| l.trim().parse().ok()) { - Some(p) => p, - None => { - if !dir_is_recent(&dir, &now) { - let _ = std::fs::remove_dir_all(&dir); - } - continue; - } - }; - - // Liveness check: use supervisor PID since the supervisor owns - // the control socket. If the supervisor is dead, the sandbox is - // effectively dead even if the child still runs. - if unsafe { libc::kill(supervisor_pid, 0) } == 0 { - let name = match dir.file_name().and_then(|n| n.to_str()) { - Some(n) => n.to_string(), - None => continue, - }; - live.push((name, child_pid)); - } else { - // Dead: prune. - let _ = std::fs::remove_dir_all(&dir); - } - } - - // Sort by name for deterministic output. - live.sort_by(|a, b| a.0.cmp(&b.0)); - Ok(live) + path.strip_prefix(&prefix).map(str::to_string) + }) + .collect(); + names.sort(); + names.dedup(); + names } -/// Return true if `dir` was modified less than 2 seconds ago. -fn dir_is_recent(dir: &Path, now: &std::time::SystemTime) -> bool { - if let Ok(meta) = std::fs::metadata(dir) { - if let Ok(mtime) = meta.modified() { - if let Ok(elapsed) = now.duration_since(mtime) { - return elapsed.as_secs() < 2; - } - } - } - false +/// Names of the caller's live sandboxes, sorted. +pub fn list_sandboxes() -> std::io::Result> { + let text = std::fs::read_to_string("/proc/net/unix")?; + Ok(parse_proc_net_unix(&text, unsafe { libc::getuid() })) } // ============================================================ @@ -645,33 +451,33 @@ mod tests { use super::*; #[test] - fn test_runtime_dir_paths() { - let dir = sandbox_dir("test-sandbox"); - assert!(dir.to_string_lossy().contains("test-sandbox")); - assert!(dir.to_string_lossy().contains("sandlock-")); + fn longest_name_fits_sun_path() { + let name = "x".repeat(64); + // Leading NUL plus the name must fit the kernel's 108-byte sun_path. + assert!(socket_name(u32::MAX, &name).len() + 1 <= 108); } #[test] - fn test_runtime_dir_mode_file_roundtrip() { - // Unique name: sandbox names are uid-wide, never reuse a fixed one. - let name = format!("test-mode-{}", std::process::id()); - let pid = std::process::id() as i32; - - let dir = setup_runtime_dir_no_socket(&name, pid, pid, Some("learn")).unwrap(); - assert_eq!(sandbox_mode(&name).as_deref(), Some("learn")); - cleanup_runtime_dir(&dir); - - let dir = setup_runtime_dir_no_socket(&name, pid, pid, None).unwrap(); - assert_eq!(sandbox_mode(&name), None); - cleanup_runtime_dir(&dir); + fn parses_listening_sockets_for_uid_only() { + let text = "Num RefCount Protocol Flags Type St Inode Path\n\ + 0000000000000000: 00000002 00000000 00010000 0001 01 11628860 @sandlock/1000/alpha\n\ + 0000000000000000: 00000003 00000000 00000000 0001 03 11628861 @sandlock/1000/alpha\n\ + 0000000000000000: 00000002 00000000 00010000 0001 01 11628862 @sandlock/1001/other\n\ + 0000000000000000: 00000002 00000000 00010000 0001 01 11628863 /run/user/1000/bus\n\ + 0000000000000000: 00000002 00000000 00010000 0001 01 11628864 @sandlock/1000/beta\n"; + assert_eq!(parse_proc_net_unix(text, 1000), vec!["alpha", "beta"]); + assert_eq!(parse_proc_net_unix(text, 1001), vec!["other"]); } #[test] - fn test_list_live_sandboxes_empty() { - // When no sandboxes are running, returns empty. - let result = list_live_sandboxes().unwrap(); - // May or may not be empty depending on test environment; just ensure - // it doesn't error. - assert!(result.iter().all(|(_, pid)| *pid > 0)); + fn bind_is_the_name_mutex_and_listing_follows_the_listener() { + // Unique name: sandbox names are uid-wide, never reuse a fixed one. + let name = format!("test-ctrl-unit-{}", std::process::id()); + let listener = bind_control_socket(&name).unwrap(); + assert!(list_sandboxes().unwrap().contains(&name)); + let err = bind_control_socket(&name).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::AddrInUse); + drop(listener); + assert!(!list_sandboxes().unwrap().contains(&name)); } } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 63bcd09b..5f8fb7a0 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2565,8 +2565,13 @@ fn sandbox_validate_name(name: String) -> Result Date: Wed, 2 Sep 2026 16:00:36 -0700 Subject: [PATCH 03/16] sandbox: bind the control socket in every mode and serve info Every sandbox now binds its abstract control socket at the same point in do_spawn, whether or not a seccomp-notify supervisor exists, so ps sees --no-supervisor and nested sandboxes without a pid file. The new info verb carries what the pid file and mode marker held. The peer uid check turns from a warning into a refusal because an abstract name has no directory mode to rely on. Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 151 +++++++++++++-------------- crates/sandlock-core/src/sandbox.rs | 152 ++++++++-------------------- 2 files changed, 116 insertions(+), 187 deletions(-) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 6776209a..3b6355af 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -36,7 +36,6 @@ use std::os::linux::net::SocketAddrExt; use std::os::unix::net::{SocketAddr, UnixListener}; -use std::path::PathBuf; use std::sync::Arc; use crate::sandbox::Sandbox; @@ -64,82 +63,73 @@ pub(crate) fn bind_control_socket(name: &str) -> std::io::Result { } // ============================================================ -// Control loop — spawned as a dedicated tokio task +// Control loop, spawned as a dedicated tokio task // ============================================================ -/// Spawn the control-loop task. Returns immediately after spawning; the task -/// runs until the listener is closed or the supervisor shuts down. -/// -/// Takes ownership of `sandbox` (moved into the task) so the config snapshot -/// lives for the lifetime of the control loop. The sandbox clone has -/// `init_fn = None` (FnOnce can't be cloned), so the value is `Send`. +/// What the `info` verb reports: everything `sandlock ps` and `kill` need. +#[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] +pub struct SandboxInfo { + pub child_pid: i32, + pub supervisor_pid: i32, + pub mode: Option, +} + +/// Spawn the control-loop task. `ctx` is `None` for sandboxes without a +/// seccomp-notify supervisor (`--no-supervisor`, nested); those still +/// answer `info` and the static `config`, and report no ports. pub(crate) fn spawn_control_loop( listener: UnixListener, - ctx: Arc, + ctx: Option>, sandbox: Sandbox, - dir: PathBuf, + info: SandboxInfo, ) -> tokio::task::JoinHandle<()> { - // Use a Mutex to satisfy Sync (Sandbox is not Sync due to the type-level - // presence of Box, even though our clone has init_fn=None). - // The control loop only reads, so a Mutex is fine. + // Mutex only to satisfy Sync: Sandbox carries a Box slot + // even though this clone's is None. let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); tokio::spawn(async move { - control_loop(listener, ctx, sandbox, dir).await; + control_loop(listener, ctx, sandbox, info).await; }) } -/// Accept connections on the control socket and serve one request per -/// connection (single-client-at-a-time, no concurrency). +fn peer_uid(stream: &tokio::net::UnixStream) -> Option { + use std::os::unix::io::AsRawFd; + let mut cred: libc::ucred = unsafe { std::mem::zeroed() }; + let mut len = std::mem::size_of::() as libc::socklen_t; + let rc = unsafe { + libc::getsockopt( + stream.as_raw_fd(), + libc::SOL_SOCKET, + libc::SO_PEERCRED, + &mut cred as *mut _ as *mut libc::c_void, + &mut len, + ) + }; + (rc == 0).then_some(cred.uid) +} + async fn control_loop( listener: UnixListener, - ctx: Arc, + ctx: Option>, sandbox: Arc>, - _dir: PathBuf, + info: SandboxInfo, ) { - // Convert std listener to tokio. listener.set_nonblocking(true).ok(); let listener = match tokio::net::UnixListener::from_std(listener) { Ok(l) => l, Err(_) => return, }; + let my_uid = unsafe { libc::getuid() }; loop { let (stream, _addr) = match listener.accept().await { Ok(pair) => pair, Err(_) => return, }; - - // Optional: audit peer credentials (same-UID trust boundary). - // SO_PEERCRED is cheap and surfaces unexpected mismatches. - #[cfg(unix)] - { - use std::os::unix::io::AsRawFd; - let raw = stream.as_raw_fd(); - let mut cred: libc::ucred = unsafe { std::mem::zeroed() }; - let mut len = std::mem::size_of::() as libc::socklen_t; - if unsafe { - libc::getsockopt( - raw, - libc::SOL_SOCKET, - libc::SO_PEERCRED, - &mut cred as *mut _ as *mut libc::c_void, - &mut len, - ) - } == 0 - { - let my_uid = unsafe { libc::getuid() }; - if cred.uid != my_uid { - eprintln!( - "sandlock: control socket: peer uid {} != my uid {} — \ - unexpected; dir 0700 should prevent this", - cred.uid, my_uid - ); - } - } + // Abstract names have no permission bits, so this is the only gate. + if peer_uid(&stream) != Some(my_uid) { + continue; } - - // Serve one request; close after. - serve_one(stream, &ctx, &sandbox).await; + serve_one(stream, ctx.as_ref(), &sandbox, &info).await; } } @@ -168,8 +158,9 @@ pub struct ControlResponse { async fn serve_one( stream: tokio::net::UnixStream, - ctx: &Arc, + ctx: Option<&Arc>, sandbox: &Arc>, + info: &SandboxInfo, ) { use tokio::io::AsyncReadExt; @@ -214,6 +205,7 @@ async fn serve_one( } match req.verb.as_str() { + "info" => handle_info(&mut stream, info).await, "config" => handle_config(&mut stream, ctx, sandbox).await, "ports" => handle_ports(&mut stream, ctx).await, _ => { @@ -228,15 +220,27 @@ async fn serve_one( } } +async fn handle_info(stream: &mut tokio::net::UnixStream, info: &SandboxInfo) { + let resp = match serde_json::to_value(info) { + Ok(data) => ControlResponse { v: 1, ok: true, data: Some(data), err: None }, + Err(e) => ControlResponse { + v: 1, + ok: false, + data: None, + err: Some(format!("serialize error: {}", e)), + }, + }; + let _ = write_response(stream, &resp).await; +} + async fn handle_config( stream: &mut tokio::net::UnixStream, - ctx: &Arc, + ctx: Option<&Arc>, sandbox: &Arc>, ) { - // Collect dynamic policy_fn denies. - let dynamic_denied: Vec = { - let pfn = ctx.policy_fn.lock().await; - pfn.denied.denied_paths() + let dynamic_denied: Vec = match ctx { + Some(ctx) => ctx.policy_fn.lock().await.denied.denied_paths(), + None => Vec::new(), }; // Build the effective profile. @@ -270,15 +274,11 @@ async fn handle_config( async fn handle_ports( stream: &mut tokio::net::UnixStream, - ctx: &Arc, + ctx: Option<&Arc>, ) { - // Read the current virtual→real port map from the supervisor's - // NetworkState. This is the live mapping at request-time — more - // accurate than a static registry that only refreshes on bind and - // goes stale on SIGKILL. - let ports: std::collections::HashMap = { - let ns = ctx.network.lock().await; - ns.port_map.virtual_to_real.clone() + let ports: std::collections::HashMap = match ctx { + Some(ctx) => ctx.network.lock().await.port_map.virtual_to_real.clone(), + None => Default::default(), }; let data = match serde_json::to_value(&ports) { @@ -393,23 +393,14 @@ pub fn send_control_request( use std::io::{Read, Write}; use std::os::unix::net::UnixStream; - let dir = sandbox_dir(name); - - // Check supervisor liveness before attempting connect. If the - // supervisor is dead the socket is stale and connect() would fail - // with a confusing "No such file" — give a clearer message. - if let Some(pid) = read_supervisor_pid(&dir) { - if unsafe { libc::kill(pid, 0) } != 0 { - return Err(format!( - "sandbox '{}' supervisor (PID {}) is not running", - name, pid - )); + let addr = socket_addr(name).map_err(|e| format!("socket address for '{}': {}", name, e))?; + let mut stream = match UnixStream::connect_addr(&addr) { + Ok(s) => s, + Err(e) if e.kind() == std::io::ErrorKind::ConnectionRefused => { + return Err(format!("no sandbox named '{}'", name)); } - } - - let sp = sock_path(&dir); - let mut stream = UnixStream::connect(&sp) - .map_err(|e| format!("connect to {:?}: {}", sp, e))?; + Err(e) => return Err(format!("connect to sandbox '{}': {}", name, e)), + }; // Set a 2-second timeout on reads so a wedged supervisor does not // block the CLI forever. diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 5f8fb7a0..469ba1c5 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -274,7 +274,6 @@ struct Runtime { throttle_handle: Option>, loadavg_handle: Option>, control_handle: Option>, - control_dir: Option, _stdout_read: Option, _stderr_read: Option, // Drains of the capture pipes above, each holding either the task still @@ -923,11 +922,6 @@ impl Sandbox { if let Some(h) = rt.loadavg_handle.take() { h.abort(); } if let Some(h) = rt.control_handle.take() { h.abort(); } - // Clean up the per-sandbox runtime dir on normal exit. - if let Some(ref dir) = rt.control_dir { - crate::control::cleanup_runtime_dir(dir); - } - // A transactional-pipeline stage leaves the branch in the shared COW // state for the next stage / the coordinator's single commit — don't // take it out (that would strip the upper from later stages) and don't @@ -1526,7 +1520,6 @@ impl Sandbox { shared_cow: None, tty_foreground_taken: false, control_handle: None, - control_dir: None, })); clones.push(clone_sb); } @@ -1608,7 +1601,6 @@ impl Sandbox { throttle_handle: None, loadavg_handle: None, control_handle: None, - control_dir: None, _stdout_read: None, _stderr_read: None, stdout_drain: None, @@ -1974,51 +1966,40 @@ impl Sandbox { let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; - // Even for --no-supervisor sandboxes, write a pid file so sandlock ps - // can discover and list them. The control socket is only created when - // a supervisor exists (inside the if-let below). - // - // Use setup_runtime_dir_no_socket to get the liveness check — a - // no-supervisor sandbox with the same name as a live sandbox must - // refuse to start rather than unconditionally remove_dir_all the - // live one's pid file. - // // This must stay after the notif-fd read above. Any error return // from do_spawn relies on Drop's killpg to reap the child, which // only works once the child has setpgid'd into its own group; the // notif-fd write is the child's setup-complete signal, so returning // before it races killpg against setpgid and can deadlock Drop's // waitpid against a child parked on the ready pipe. - if no_supervisor { - let sandbox_name = self.rt().name.clone(); - let supervisor_pid = std::process::id() as i32; - match crate::control::setup_runtime_dir_no_socket( - &sandbox_name, - pid, - supervisor_pid, - self.mode.as_deref(), - ) { - Ok(dir) => { - self.rt_mut().control_dir = Some(dir); - } - Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - // Name collision with a live sandbox — hard-fail, same as - // the supervisor path. Continuing would leave a second - // sandbox running invisible to ps. - return Err(SandboxRuntimeError::Child(format!( - "sandbox '{}' is already running: {}", - sandbox_name, e - )) - .into()); - } - Err(e) => { - eprintln!( - "sandlock: runtime dir setup failed for '{}': {}", - sandbox_name, e - ); - } + let sandbox_name = self.rt().name.clone(); + let control_info = crate::control::SandboxInfo { + child_pid: pid, + supervisor_pid: std::process::id() as i32, + mode: self.mode.clone(), + }; + // std creates the listener with SOCK_CLOEXEC, and the child has + // already forked, so it never holds this fd. + let mut control_listener = match crate::control::bind_control_socket(&sandbox_name) { + Ok(l) => Some(l), + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { + return Err(SandboxRuntimeError::Child(format!( + "sandbox '{}' is already running", + sandbox_name + )) + .into()); } - } + Err(e) => { + // A nested sandlock whose outer policy denies AF_UNIX lands + // here; the sandbox still runs, it is just not introspectable. + eprintln!( + "sandlock: control socket setup failed for '{}': {} \ + (introspection unavailable for this sandbox)", + sandbox_name, e + ); + None + } + }; let is_nested_mode = notif_fd_num == 0; @@ -2038,46 +2019,6 @@ impl Sandbox { }; if let Some(notif_fd) = notif_fd { - // Set up the per-sandbox runtime dir and control socket. Must - // happen before the notif supervisor is spawned so the socket - // exists when the child is released. - // - // Best-effort: in nested sandboxes /dev/shm may be restricted by - // the outer sandlock's landlock policy. Warn and continue without - // a control socket rather than failing the sandbox. - let sandbox_name = self.rt().name.clone(); - let supervisor_pid = std::process::id() as i32; - let control_listener = match crate::control::setup_runtime_dir( - &sandbox_name, - pid, - supervisor_pid, - self.mode.as_deref(), - ) { - Ok((listener, control_dir)) => { - self.rt_mut().control_dir = Some(control_dir); - Some(listener) - } - Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - // Name collision with a live sandbox — hard-fail. - // A second sandbox with the same name would be - // invisible to ps and its Drop would remove the - // first one's runtime dir. - return Err(SandboxRuntimeError::Child(format!( - "sandbox '{}' is already running: {}", - sandbox_name, e - )) - .into()); - } - Err(e) => { - eprintln!( - "sandlock: control socket setup failed for '{}': {} \ - (introspection unavailable for this sandbox)", - sandbox_name, e - ); - None - } - }; - if self.time_start.is_some() || self.random_seed.is_some() { let time_offset = self.time_start.map(|t| crate::time::calculate_time_offset(t)); if let Err(e) = crate::vdso::patch(pid, time_offset, self.random_seed.is_some()) { @@ -2271,11 +2212,7 @@ impl Sandbox { let handlers = std::mem::take(&mut self.rt_mut().handlers); let (startup_tx, startup_rx) = tokio::sync::oneshot::channel(); - // Clone ctx for the control loop before moving the original into - // the notif supervisor. Only set up if the control dir was - // successfully created above. let control_ctx = Arc::clone(&ctx); - let control_dir_opt = self.rt().control_dir.clone(); self.rt_mut().notif_handle = Some(tokio::spawn( notif::supervisor(notif_fd, ctx, handlers, startup_tx), @@ -2296,20 +2233,15 @@ impl Sandbox { } } - // Spawn the control-socket loop as a dedicated tokio task. // Independent of the seccomp-notify loop so accept() never adds // latency to syscall notification processing. - if let (Some(listener), Some(dir_path)) = - (control_listener, control_dir_opt) - { - self.rt_mut().control_handle = Some( - crate::control::spawn_control_loop( - listener, - control_ctx, - sandbox_snapshot, - dir_path, - ) - ); + if let Some(listener) = control_listener.take() { + self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( + listener, + Some(control_ctx), + sandbox_snapshot, + control_info.clone(), + )); } let la_resource = Arc::clone(&res_state); @@ -2325,6 +2257,17 @@ impl Sandbox { })); } + // No notify supervisor (--no-supervisor or nested): still answer ps, + // inspect, and kill, with the static policy and no ports. + if let Some(listener) = control_listener.take() { + self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( + listener, + None, + self.clone(), + control_info, + )); + } + if let Some(cpu_pct) = self.max_cpu { if cpu_pct < 100 { let child_pid = pid; @@ -2483,11 +2426,6 @@ impl Drop for Sandbox { if let Some(ParkedDrain::Running(h)) = slot { h.abort(); } } - // Clean up the per-sandbox runtime dir on abnormal exit / Drop. - if let Some(ref dir) = rt.control_dir { - crate::control::cleanup_runtime_dir(dir); - } - let is_error = matches!( rt.state, RuntimeState::Stopped(ref s) if !matches!(s, crate::result::ExitStatus::Code(0)) From 0fc61763972d7ca038255efd7b19557dd639ee15 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 16:07:52 -0700 Subject: [PATCH 04/16] cli: discover sandboxes through /proc/net/unix ps lists the caller's abstract control sockets and asks each for info; a socket that does not answer is still shown, as unresponsive, because its existence proves the process is alive. kill gets both pids from the same verb and signals them as before, with nothing left on disk to clean up afterwards. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 106 +++++++++++----------------- crates/sandlock-core/src/control.rs | 23 +++++- 2 files changed, 61 insertions(+), 68 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 16cf3bd2..f6e4aaba 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -275,31 +275,41 @@ async fn main() -> Result<()> { } Command::Ps => { - match sandlock_core::control::list_live_sandboxes() { - Ok(sandboxes) if sandboxes.is_empty() => { - println!("No running sandboxes."); - } - Ok(sandboxes) => { - println!( - "{:<32} {:>8} {:>12} {:<10} {:<24} {}", - "NAME", "PID", "UPTIME", "STATUS", "PORTS", "CMD" - ); - for (name, pid) in &sandboxes { - let uptime = proc_uptime(*pid).unwrap_or_else(|| "?".to_string()); - let cmd = proc_cmdline(*pid).unwrap_or_else(|| "?".to_string()); - let ports = query_ports(name); - let status = sandlock_core::control::sandbox_mode(name) - .unwrap_or_else(|| "running".to_string()); - println!( - "{:<32} {:>8} {:>12} {:<10} {:<24} {}", - name, pid, uptime, status, ports, cmd - ); - } - } + let names = match sandlock_core::control::list_sandboxes() { + Ok(n) => n, Err(e) => { eprintln!("sandlock: failed to list sandboxes: {}", e); std::process::exit(1); } + }; + if names.is_empty() { + println!("No running sandboxes."); + } else { + println!( + "{:<32} {:>8} {:>12} {:<10} {:<24} {}", + "NAME", "PID", "UPTIME", "STATUS", "PORTS", "CMD" + ); + for name in &names { + match sandlock_core::control::sandbox_info(name) { + Ok(info) => { + let pid = info.child_pid; + let uptime = proc_uptime(pid).unwrap_or_else(|| "?".to_string()); + let cmd = proc_cmdline(pid).unwrap_or_else(|| "?".to_string()); + let ports = query_ports(name); + let status = info.mode.unwrap_or_else(|| "running".to_string()); + println!( + "{:<32} {:>8} {:>12} {:<10} {:<24} {}", + name, pid, uptime, status, ports, cmd + ); + } + // The socket exists, so the process is alive; it + // just is not answering. + Err(_) => println!( + "{:<32} ? ? unresponsive ? ?", + name + ), + } + } } } @@ -345,56 +355,20 @@ async fn main() -> Result<()> { eprintln!("sandlock: {e}"); std::process::exit(1); } - // Read both PIDs from the per-sandbox pid file (no socket - // round-trip). Format: child_pid\nsupervisor_pid\n - let dir = sandlock_core::control::sandbox_dir(&name); - let pid_file = sandlock_core::control::pid_path(&dir); - let pid_str = match std::fs::read_to_string(&pid_file) { - Ok(s) => s, - Err(_) => { - eprintln!("sandlock: no sandbox named '{}'", name); - std::process::exit(1); - } - }; - let mut lines = pid_str.lines(); - let child_pid: i32 = match lines.next().and_then(|l| l.trim().parse().ok()) { - Some(p) => p, - None => { - eprintln!("sandlock: invalid pid file for '{}'", name); - std::process::exit(1); - } - }; - let supervisor_pid: i32 = match lines.next().and_then(|l| l.trim().parse().ok()) { - Some(p) => p, - None => { - eprintln!("sandlock: invalid pid file for '{}'", name); + let info = match sandlock_core::control::sandbox_info(&name) { + Ok(i) => i, + Err(e) => { + eprintln!("sandlock: {}", e); std::process::exit(1); } }; - - // Check supervisor liveness (the process that owns the socket). - if unsafe { libc::kill(supervisor_pid, 0) } != 0 { - eprintln!( - "sandlock: sandbox '{}' (supervisor PID {}) is not running", - name, supervisor_pid - ); - std::process::exit(1); - } - - // killpg on child_pid kills the entire process group (child + - // descendants). Also signal the supervisor directly in case - // it's in a different process group. - unsafe { libc::killpg(child_pid, libc::SIGKILL) }; - unsafe { libc::kill(supervisor_pid, libc::SIGKILL) }; - - // SIGKILL bypasses Drop, so the supervisor never runs its own - // cleanup. Remove the runtime dir here so it doesn't linger - // until the next `sandlock ps` prunes it. - sandlock_core::control::cleanup_runtime_dir(&dir); - + // killpg takes the child's whole process group; the supervisor + // may sit in a different group, so signal it directly too. + unsafe { libc::killpg(info.child_pid, libc::SIGKILL) }; + unsafe { libc::kill(info.supervisor_pid, libc::SIGKILL) }; println!( "Killed sandbox '{}' (child PID {}, supervisor PID {})", - name, child_pid, supervisor_pid + name, info.child_pid, info.supervisor_pid ); } diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 3b6355af..48231f22 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -383,6 +383,15 @@ pub fn list_sandboxes() -> std::io::Result> { // Client helpers — used by sandlock-cli to talk to the socket // ============================================================ +fn unresponsive(name: &str, e: std::io::Error) -> String { + match e.kind() { + std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut => { + format!("sandbox '{}' is unresponsive", name) + } + _ => format!("read from sandbox '{}': {}", name, e), + } +} + /// Send a request to a sandbox's control socket and return the JSON response /// body (the `data` field, or error). pub fn send_control_request( @@ -425,18 +434,28 @@ pub fn send_control_request( // Read response. let mut len_buf = [0u8; 4]; - stream.read_exact(&mut len_buf).map_err(|e| format!("read len: {}", e))?; + stream.read_exact(&mut len_buf).map_err(|e| unresponsive(name, e))?; let resp_len = u32::from_be_bytes(len_buf) as usize; if resp_len > 65536 { return Err("response too large".to_string()); } let mut resp_body = vec![0u8; resp_len]; - stream.read_exact(&mut resp_body).map_err(|e| format!("read body: {}", e))?; + stream.read_exact(&mut resp_body).map_err(|e| unresponsive(name, e))?; serde_json::from_slice(&resp_body) .map_err(|e| format!("parse response: {}", e)) } +/// Ask a sandbox for its pids and mode. +pub fn sandbox_info(name: &str) -> Result { + let resp = send_control_request(name, "info", serde_json::Value::Object(Default::default()))?; + if !resp.ok { + return Err(resp.err.unwrap_or_else(|| "info failed".into())); + } + let data = resp.data.ok_or_else(|| "empty info response".to_string())?; + serde_json::from_value(data).map_err(|e| format!("parse info response: {}", e)) +} + #[cfg(test)] mod tests { use super::*; From ba3d1af190deab3bfb42f787edac77b8a264dea2 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 16:17:24 -0700 Subject: [PATCH 05/16] tests: cover the abstract control socket lifecycle The prune and pid-file tests have no subject left. In their place: a SIGKILLed supervisor's name disappears and is reusable at once, a stopped supervisor is still listed as unresponsive, a --no-supervisor sandbox answers info, config, and an empty ports map, and another uid gets no response (root only). Signed-off-by: Cong Wang --- .../tests/integration/test_control.rs | 266 +++++++++--------- 1 file changed, 133 insertions(+), 133 deletions(-) diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index e9b1c382..b84e28e9 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -4,7 +4,7 @@ //! sandbox via the CLI binary and querying its `config` verb, verifying that //! the effective policy returned matches the sandbox's configured policy. -use std::ffi::CString; +use std::os::linux::net::SocketAddrExt; use std::process::Command; use std::time::Duration; @@ -177,73 +177,6 @@ fn test_control_unknown_verb() { assert!(result.is_err(), "should error for nonexistent sandbox"); } -#[test] -fn test_control_prunes_stale_dirs_via_cli() { - let name = format!("test-ctrl-prune-{}", std::process::id()); - let mut child = start_sleep_sandbox(&name); - - match wait_for_sandbox(&name) { - Ok(()) => { - let dir = sandlock_core::control::sandbox_dir(&name); - assert!(dir.exists(), "runtime dir should exist: {:?}", dir); - - // Read the child PID from the pid file (first line only; - // the file now has format child_pid\nsupervisor_pid\n). - let pid_file = sandlock_core::control::pid_path(&dir); - let child_pid: i32 = std::fs::read_to_string(&pid_file) - .unwrap() - .lines() - .next() - .and_then(|l| l.trim().parse().ok()) - .expect("first line of pid file should be child PID"); - - // Kill the supervisor process (SIGKILL — no Drop cleanup). - child.kill().expect("kill supervisor"); - child.wait().expect("wait supervisor"); - - // Also kill the sandboxed child (sleep), otherwise kill(pid,0) - // still sees it as alive. - unsafe { libc::kill(child_pid, libc::SIGKILL) }; - - // Wait a moment for the child to die. - std::thread::sleep(std::time::Duration::from_millis(500)); - - // The stale dir may or may not still exist (depends on whether - // the supervisor's Drop ran before SIGKILL was delivered). - // Either way, list_live_sandboxes should not list this sandbox - // and the dir should be gone after pruning. - let sandboxes = sandlock_core::control::list_live_sandboxes().unwrap(); - assert!( - !sandboxes.iter().any(|(n, _)| n == &name), - "sandbox should not be listed after kill (pruned): {:?}", - sandboxes - ); - - // The stale dir should be gone after pruning. - assert!(!dir.exists(), "stale dir should be pruned: {:?}", dir); - } - Err(e) => { - let stderr_output = child_stderr(&mut child); - let _ = child.kill(); - panic!("{}; child stderr: {}", e, stderr_output); - } - } -} - -#[test] -fn test_control_runtime_dir_paths() { - let dir = sandlock_core::control::sandbox_dir("test-xyz"); - let s = dir.to_string_lossy(); - assert!(s.contains("sandlock-"), "dir should contain sandlock-: {}", s); - assert!(s.contains("test-xyz"), "dir should contain name: {}", s); - - let pid_file = sandlock_core::control::pid_path(&dir); - assert_eq!(pid_file.file_name().unwrap(), "pid"); - - let sock = sandlock_core::control::sock_path(&dir); - assert_eq!(sock.file_name().unwrap(), "control.sock"); -} - #[test] fn test_control_sandbox_to_profile() { let sb = sandlock_core::Sandbox::builder() @@ -281,11 +214,6 @@ fn test_control_mode_stays_out_of_profile() { assert!(!toml_str.contains("mode"), "mode leaked into profile: {toml_str}"); } -#[test] -fn test_control_sandbox_mode_absent_for_plain_runs() { - assert_eq!(sandlock_core::control::sandbox_mode("no-such-sandbox-mode"), None); -} - #[test] fn test_control_sandbox_to_profile_dedups_net_rules() { // "*" expands to tcp://* + udp://* at parse time, so the explicit @@ -410,9 +338,8 @@ fn test_control_name_collision() { #[test] fn test_control_name_collision_no_supervisor() { - // The no_supervisor path shares setup_runtime_dir_no_socket and must - // hard-fail on a live-name collision just like the supervisor path; - // continuing would leave a second sandbox running invisible to ps. + // The no_supervisor path binds the same abstract name and must fail on + // a live collision just like the supervisor path. let name = format!("test-ctrl-collision-nosup-{}", std::process::id()); let mut first = start_sleep_sandbox(&name); @@ -487,6 +414,18 @@ fn test_control_no_supervisor() { "ps should have PORTS column: {}", stdout ); + + let info = sandlock_core::control::sandbox_info(&name).expect("info"); + assert!(info.child_pid > 0 && info.supervisor_pid > 0); + assert_eq!(info.mode, None); + + let inspect = sandlock_bin().args(["inspect", &name]).output().expect("inspect"); + assert!(inspect.status.success(), "inspect should answer without a supervisor"); + + let ports = sandlock_core::control::send_control_request( + &name, "ports", serde_json::Value::Object(Default::default()), + ).expect("ports"); + assert_eq!(ports.data, Some(serde_json::json!({}))); } Err(e) => { let stderr_output = child_stderr(&mut child); @@ -573,9 +512,8 @@ fn test_control_ps_ports_column() { #[tokio::test] async fn test_control_invalid_names() { - // Names that would escape /dev/shm/sandlock-$UID must be rejected - // at spawn time (sandbox_resolve_name → sandbox_validate_name). - for bad in &["/", "..", ".", "a/b", "../etc"] { + // Names that could not be listed back from /proc/net/unix, or that look like paths, are rejected at spawn time. + for bad in &["/", "..", ".", "a/b", "../etc", "a b", "tab\tname", "nl\nname"] { let result = sandlock_core::Sandbox::builder() .fs_read("/usr") .fs_read("/bin") @@ -592,16 +530,125 @@ async fn test_control_invalid_names() { "sandbox name {:?} should be rejected", bad ); } +} + +// ============================================================ +// Lifecycle: the name lives and dies with the supervisor +// ============================================================ + +/// Poll `list_sandboxes` until `name` is absent, or give up after 3s. +fn wait_for_gone(name: &str) -> bool { + for _ in 0..30 { + let names = sandlock_core::control::list_sandboxes().unwrap(); + if !names.iter().any(|n| n == name) { + return true; + } + std::thread::sleep(Duration::from_millis(100)); + } + false +} + +#[test] +fn test_control_killed_supervisor_vanishes_and_name_is_reusable() { + let name = format!("test-ctrl-vanish-{}", std::process::id()); + let mut first = start_sleep_sandbox(&name); + if let Err(e) = wait_for_sandbox(&name) { + let stderr_output = child_stderr(&mut first); + let _ = first.kill(); + panic!("{}; child stderr: {}", e, stderr_output); + } + let child_pid = sandlock_core::control::sandbox_info(&name).expect("info").child_pid; + + // SIGKILL skips Drop entirely: nothing runs any cleanup. + first.kill().expect("kill supervisor"); + first.wait().expect("wait supervisor"); + unsafe { libc::kill(child_pid, libc::SIGKILL) }; + + assert!(wait_for_gone(&name), "name should disappear with the supervisor"); + + let mut second = start_sleep_sandbox(&name); + match wait_for_sandbox(&name) { + Ok(()) => {} + Err(e) => { + let stderr_output = child_stderr(&mut second); + let _ = second.kill(); + panic!("name should be reusable immediately: {}; stderr: {}", e, stderr_output); + } + } + let _ = second.kill(); + let _ = second.wait(); +} + +#[test] +fn test_control_stopped_supervisor_is_listed_as_unresponsive() { + let name = format!("test-ctrl-stopped-{}", std::process::id()); + let mut child = start_sleep_sandbox(&name); + if let Err(e) = wait_for_sandbox(&name) { + let stderr_output = child_stderr(&mut child); + let _ = child.kill(); + panic!("{}; child stderr: {}", e, stderr_output); + } - // Test that sandbox_dir alone does join freely — the validation layer - // is what prevents bad names from reaching filesystem ops. - let dir = sandlock_core::control::sandbox_dir(".."); - let dir_str = dir.to_string_lossy(); + unsafe { libc::kill(child.id() as i32, libc::SIGSTOP) }; + let out = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + unsafe { libc::kill(child.id() as i32, libc::SIGCONT) }; + + let stdout = String::from_utf8_lossy(&out.stdout); + let line = stdout.lines().find(|l| l.contains(&name)); assert!( - dir_str.ends_with(".."), - "sandbox_dir('..') must append the name as-is (caller must validate): {}", - dir_str + line.is_some_and(|l| l.contains("unresponsive")), + "a stopped supervisor should still be listed, as unresponsive: {}", + stdout ); + + let _ = child.kill(); + let _ = child.wait(); +} + +#[test] +fn test_control_refuses_other_uid() { + if unsafe { libc::geteuid() } != 0 { + eprintln!("skip: needs root to switch uid"); + return; + } + let name = format!("test-ctrl-peer-{}", std::process::id()); + let mut child = start_sleep_sandbox(&name); + if let Err(e) = wait_for_sandbox(&name) { + let stderr_output = child_stderr(&mut child); + let _ = child.kill(); + panic!("{}; child stderr: {}", e, stderr_output); + } + + // The socket belongs to uid 0; connect as nobody from a forked child and + // expect the server to close without answering. + let addr = std::os::unix::net::SocketAddr::from_abstract_name( + sandlock_core::control::socket_name(0, &name), + ).unwrap(); + let pid = unsafe { libc::fork() }; + if pid == 0 { + use std::io::{Read, Write}; + unsafe { libc::setresuid(65534, 65534, 65534) }; + let code = match std::os::unix::net::UnixStream::connect_addr(&addr) { + Ok(mut s) => { + let body = br#"{"v":1,"verb":"info","args":{}}"#; + let _ = s.write_all(&(body.len() as u32).to_be_bytes()); + let _ = s.write_all(body); + let _ = s.set_read_timeout(Some(Duration::from_secs(2))); + let mut buf = [0u8; 4]; + match s.read(&mut buf) { + Ok(0) => 0, + _ => 1, + } + } + Err(_) => 0, + }; + unsafe { libc::_exit(code) }; + } + let mut status = 0; + unsafe { libc::waitpid(pid, &mut status, 0) }; + let _ = child.kill(); + let _ = child.wait(); + assert_eq!(libc::WEXITSTATUS(status), 0, "another uid must get no response"); } // ============================================================ @@ -665,50 +712,3 @@ fn test_control_cli_kill_nonexistent() { stderr ); } - -// ============================================================ -// pid file format — two lines required (old single-line shim removed) -// ============================================================ - -#[test] -fn test_control_single_line_pid_file_is_pruned() { - let dir = sandlock_core::control::sandbox_dir("test-single-line-pid"); - std::fs::create_dir_all(&dir).expect("create test dir"); - - // Write a pid file with only one line — the format that never shipped. - let pid_path = sandlock_core::control::pid_path(&dir); - std::fs::write(&pid_path, "12345\n").expect("write single-line pid file"); - - // Set the dir mtime to >2s ago so the recency check allows pruning. - // list_live_sandboxes won't prune dirs modified less than 2s ago - // (concurrent setup protection). - let old_time = libc::timespec { - tv_sec: 1000, // Unix epoch + 1000s — ancient - tv_nsec: 0, - }; - let times = [old_time, old_time]; - let dir_cstr = CString::new(dir.to_str().unwrap()).expect("valid C string"); - let rc = unsafe { - libc::utimensat( - libc::AT_FDCWD, - dir_cstr.as_ptr(), - times.as_ptr(), - 0, - ) - }; - assert_eq!(rc, 0, "utimensat failed on {:?}", dir); - - // list_live_sandboxes must prune this dir (supervisor_pid parse fails - // and the mtime is old). - let sandboxes = sandlock_core::control::list_live_sandboxes() - .expect("list_live_sandboxes"); - assert!( - !sandboxes.iter().any(|(n, _)| n == "test-single-line-pid"), - "single-line pid dir should not be listed, got: {:?}", - sandboxes - ); - assert!( - !dir.exists(), - "single-line pid dir should be pruned" - ); -} From 407e0699fb3680b4242bf35edec0fb1370cd486f Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 16:24:46 -0700 Subject: [PATCH 06/16] tests: reap the stopped supervisor before asserting A failed assertion must not leak a live sandbox, so the supervisor is killed and reaped before the assert instead of after it. Signed-off-by: Cong Wang --- crates/sandlock-core/tests/integration/test_control.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index b84e28e9..3c2f61ec 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -416,7 +416,7 @@ fn test_control_no_supervisor() { ); let info = sandlock_core::control::sandbox_info(&name).expect("info"); - assert!(info.child_pid > 0 && info.supervisor_pid > 0); + assert!(info.child_pid > 0 && info.supervisor_pid > 0, "info should report real pids: {:?}", info); assert_eq!(info.mode, None); let inspect = sandlock_bin().args(["inspect", &name]).output().expect("inspect"); @@ -595,14 +595,15 @@ fn test_control_stopped_supervisor_is_listed_as_unresponsive() { let stdout = String::from_utf8_lossy(&out.stdout); let line = stdout.lines().find(|l| l.contains(&name)); + + let _ = child.kill(); + let _ = child.wait(); + assert!( line.is_some_and(|l| l.contains("unresponsive")), "a stopped supervisor should still be listed, as unresponsive: {}", stdout ); - - let _ = child.kill(); - let _ = child.wait(); } #[test] From 15fa93d273237376467264011b502d1fc32ae389 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 16:47:20 -0700 Subject: [PATCH 07/16] sandbox: release the control socket name before wait returns wait() only aborted the control task, and the listener that holds the abstract name was dropped whenever the runtime got to cancelling it, so a caller that ran the same name twice in a row could hit the collision check. Awaiting the aborted task makes the release synchronous, which is what the runtime directory removal used to be. Signed-off-by: Cong Wang --- crates/sandlock-core/src/sandbox.rs | 7 ++++++- .../tests/integration/test_control.rs | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 469ba1c5..b7029567 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -920,7 +920,12 @@ impl Sandbox { rt.policy_fn_worker = None; if let Some(h) = rt.throttle_handle.take() { h.abort(); } if let Some(h) = rt.loadavg_handle.take() { h.abort(); } - if let Some(h) = rt.control_handle.take() { h.abort(); } + // Awaiting the aborted task drops its listener, so the name is free + // for reuse the moment wait() returns. + if let Some(h) = rt.control_handle.take() { + h.abort(); + let _ = h.await; + } // A transactional-pipeline stage leaves the branch in the shared COW // state for the next stage / the coordinator's single commit — don't diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 3c2f61ec..49203a22 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -652,6 +652,27 @@ fn test_control_refuses_other_uid() { assert_eq!(libc::WEXITSTATUS(status), 0, "another uid must get no response"); } +/// wait() must release the name before it returns: a caller that runs the +/// same name twice in a row must not hit the collision check. +#[tokio::test] +async fn test_control_name_is_free_when_wait_returns() { + let name = format!("test-ctrl-reuse-{}", std::process::id()); + for _ in 0..2 { + let result = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/proc") + .build() + .unwrap() + .with_name(&name) + .run(&["true"]) + .await; + assert!(result.is_ok(), "second run with the same name must succeed: {:?}", result.err()); + } +} + // ============================================================ // CLI kill / config input validation // ============================================================ From 19cf69d21d61b5c4e3a079ce9768fd26224e5576 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 16:47:30 -0700 Subject: [PATCH 08/16] control: tidy leftovers from the socket registry change A doc comment still described the /dev/shm runtime dir, and two lines this branch introduced tripped clippy (int_plus_one in a unit test, print_literal in the ps header). Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 5 +---- crates/sandlock-core/src/control.rs | 2 +- crates/sandlock-core/src/sandbox.rs | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index f6e4aaba..ddafd1ff 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -285,10 +285,7 @@ async fn main() -> Result<()> { if names.is_empty() { println!("No running sandboxes."); } else { - println!( - "{:<32} {:>8} {:>12} {:<10} {:<24} {}", - "NAME", "PID", "UPTIME", "STATUS", "PORTS", "CMD" - ); + println!("NAME PID UPTIME STATUS PORTS CMD"); for name in &names { match sandlock_core::control::sandbox_info(name) { Ok(info) => { diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 48231f22..22c58089 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -464,7 +464,7 @@ mod tests { fn longest_name_fits_sun_path() { let name = "x".repeat(64); // Leading NUL plus the name must fit the kernel's 108-byte sun_path. - assert!(socket_name(u32::MAX, &name).len() + 1 <= 108); + assert!(socket_name(u32::MAX, &name).len() < 108); } #[test] diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index b7029567..a749ad1b 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2486,9 +2486,9 @@ fn sandbox_resolve_name(name: Option<&str>) -> Result-` suffix that makes an internally generated sandbox name -/// unique across processes and within one. The runtime dir under -/// /dev/shm/sandlock-$UID/ is claimed per name and a live collision is a hard -/// error, so no internal caller may use a fixed name. +/// unique across processes and within one. The control socket name is +/// claimed per sandbox name and a live collision is a hard error, so no +/// internal caller may use a fixed name. pub(crate) fn unique_instance_id() -> String { format!( "{}-{}", From ed7b4bacb58aa4730000c7471690d81f089324d0 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 17:15:25 -0700 Subject: [PATCH 09/16] control: authenticate the listener and bound each request Abstract names have no owner, so another uid could bind sandlock// and hand ps and kill pids of its choosing; the client now checks SO_PEERCRED on the listener as the server already does on the peer. A request is bounded to five seconds so one stalled connection cannot wedge introspection, which kill now depends on. The uid each side expects is a parameter so the refusal paths are tested without a second uid, replacing a root-only test that had never run. Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 125 ++++++++++++++++-- .../tests/integration/test_control.rs | 55 +------- 2 files changed, 117 insertions(+), 63 deletions(-) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 22c58089..74090480 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -45,9 +45,8 @@ use crate::seccomp::ctx::SupervisorCtx; // Socket address // ============================================================ -/// Bytes after the leading NUL of the abstract name. Public so tests can -/// address a socket of another uid. -pub fn socket_name(uid: u32, name: &str) -> Vec { +/// Bytes after the leading NUL of the abstract name. +pub(crate) fn socket_name(uid: u32, name: &str) -> Vec { format!("sandlock/{uid}/{name}").into_bytes() } @@ -87,17 +86,16 @@ pub(crate) fn spawn_control_loop( // even though this clone's is None. let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); tokio::spawn(async move { - control_loop(listener, ctx, sandbox, info).await; + control_loop(listener, ctx, sandbox, info, unsafe { libc::getuid() }).await; }) } -fn peer_uid(stream: &tokio::net::UnixStream) -> Option { - use std::os::unix::io::AsRawFd; +fn peer_uid(fd: std::os::unix::io::RawFd) -> Option { let mut cred: libc::ucred = unsafe { std::mem::zeroed() }; let mut len = std::mem::size_of::() as libc::socklen_t; let rc = unsafe { libc::getsockopt( - stream.as_raw_fd(), + fd, libc::SOL_SOCKET, libc::SO_PEERCRED, &mut cred as *mut _ as *mut libc::c_void, @@ -107,18 +105,23 @@ fn peer_uid(stream: &tokio::net::UnixStream) -> Option { (rc == 0).then_some(cred.uid) } +/// Accept one connection at a time and serve one request per connection. +/// `my_uid` is a parameter so a test can prove the refusal path without a +/// second uid. The timeout keeps one stalled client from wedging +/// introspection, which kill now depends on. async fn control_loop( listener: UnixListener, ctx: Option>, sandbox: Arc>, info: SandboxInfo, + my_uid: u32, ) { + use std::os::unix::io::AsRawFd; listener.set_nonblocking(true).ok(); let listener = match tokio::net::UnixListener::from_std(listener) { Ok(l) => l, Err(_) => return, }; - let my_uid = unsafe { libc::getuid() }; loop { let (stream, _addr) = match listener.accept().await { @@ -126,10 +129,14 @@ async fn control_loop( Err(_) => return, }; // Abstract names have no permission bits, so this is the only gate. - if peer_uid(&stream) != Some(my_uid) { + if peer_uid(stream.as_raw_fd()) != Some(my_uid) { continue; } - serve_one(stream, ctx.as_ref(), &sandbox, &info).await; + let _ = tokio::time::timeout( + std::time::Duration::from_secs(5), + serve_one(stream, ctx.as_ref(), &sandbox, &info), + ) + .await; } } @@ -392,14 +399,26 @@ fn unresponsive(name: &str, e: std::io::Error) -> String { } } -/// Send a request to a sandbox's control socket and return the JSON response -/// body (the `data` field, or error). +/// Send a request to a sandbox's control socket and return the response. pub fn send_control_request( name: &str, verb: &str, args: serde_json::Value, +) -> Result { + send_control_request_as(name, verb, args, unsafe { libc::getuid() }) +} + +/// `my_uid` is a parameter so a test can prove the refusal without a +/// second uid. SO_PEERCRED on a connected stream reports the listener's +/// credentials, so a name squatted by another user is rejected here. +fn send_control_request_as( + name: &str, + verb: &str, + args: serde_json::Value, + my_uid: u32, ) -> Result { use std::io::{Read, Write}; + use std::os::unix::io::AsRawFd; use std::os::unix::net::UnixStream; let addr = socket_addr(name).map_err(|e| format!("socket address for '{}': {}", name, e))?; @@ -410,6 +429,9 @@ pub fn send_control_request( } Err(e) => return Err(format!("connect to sandbox '{}': {}", name, e)), }; + if peer_uid(stream.as_raw_fd()) != Some(my_uid) { + return Err(format!("socket for '{}' is owned by another user", name)); + } // Set a 2-second timeout on reads so a wedged supervisor does not // block the CLI forever. @@ -490,4 +512,83 @@ mod tests { drop(listener); assert!(!list_sandboxes().unwrap().contains(&name)); } + + use std::io::{Read, Write}; + use std::os::unix::net::UnixStream; + + fn test_sandbox() -> Sandbox { + Sandbox::builder().fs_read("/usr").build().unwrap() + } + + fn info() -> SandboxInfo { + SandboxInfo { child_pid: 4242, supervisor_pid: 4241, mode: None } + } + + /// Bind a listener for `name`, run the control loop on it with + /// `expected_uid`, and return the task handle. + fn serve(name: &str, expected_uid: u32) -> tokio::task::JoinHandle<()> { + let listener = bind_control_socket(name).unwrap(); + let sandbox = Arc::new(tokio::sync::Mutex::new(test_sandbox())); + tokio::spawn(control_loop(listener, None, sandbox, info(), expected_uid)) + } + + /// Connect as ourselves, send an info request, and return what the + /// server sent back (empty on a silent close). + fn raw_info_request(name: &str) -> Vec { + let mut s = UnixStream::connect_addr(&socket_addr(name).unwrap()).unwrap(); + let body = br#"{"v":1,"verb":"info","args":{}}"#; + s.write_all(&(body.len() as u32).to_be_bytes()).unwrap(); + s.write_all(body).unwrap(); + s.set_read_timeout(Some(std::time::Duration::from_secs(2))).unwrap(); + let mut out = Vec::new(); + let _ = s.read_to_end(&mut out); + out + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn server_answers_its_own_uid() { + let name = format!("test-ctrl-own-{}", std::process::id()); + let task = serve(&name, unsafe { libc::getuid() }); + let out = tokio::task::spawn_blocking(move || raw_info_request(&name)).await.unwrap(); + task.abort(); + assert!(out.len() > 4, "expected a response, got {} bytes", out.len()); + let resp: ControlResponse = serde_json::from_slice(&out[4..]).unwrap(); + assert!(resp.ok); + let got: SandboxInfo = serde_json::from_value(resp.data.unwrap()).unwrap(); + assert_eq!((got.child_pid, got.supervisor_pid), (4242, 4241)); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn server_closes_on_another_uid_without_answering() { + let name = format!("test-ctrl-foreign-{}", std::process::id()); + let task = serve(&name, unsafe { libc::getuid() }.wrapping_add(1)); + let out = tokio::task::spawn_blocking(move || raw_info_request(&name)).await.unwrap(); + task.abort(); + assert!(out.is_empty(), "another uid must get no bytes, got {:?}", out); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn client_refuses_a_listener_owned_by_another_uid() { + let name = format!("test-ctrl-squat-{}", std::process::id()); + let task = serve(&name, unsafe { libc::getuid() }); + let expect = unsafe { libc::getuid() }.wrapping_add(1); + let n = name.clone(); + let err = tokio::task::spawn_blocking(move || { + send_control_request_as(&n, "info", serde_json::Value::Object(Default::default()), expect) + .unwrap_err() + }) + .await + .unwrap(); + task.abort(); + assert!(err.contains("owned by another user"), "got: {err}"); + } + + #[test] + fn listing_survives_a_foreign_non_utf8_name() { + let addr = SocketAddr::from_abstract_name(b"sandlock-probe-\xff\xfe").unwrap(); + let _foreign = UnixListener::bind_addr(&addr).unwrap(); + let name = format!("test-ctrl-utf8-{}", std::process::id()); + let _ours = bind_control_socket(&name).unwrap(); + assert!(list_sandboxes().unwrap().contains(&name)); + } } diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 49203a22..ef8e96de 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -1,10 +1,9 @@ -//! Integration tests for the per-sandbox control socket (RFC #68). +//! Integration tests for the per-sandbox control socket. //! -//! These tests exercise the control-socket wire protocol by starting a real -//! sandbox via the CLI binary and querying its `config` verb, verifying that -//! the effective policy returned matches the sandbox's configured policy. +//! Each test starts a real sandbox through the CLI binary and drives the +//! abstract control socket the way `sandlock ps`, `inspect`, `ports`, and +//! `kill` do: discovery through /proc/net/unix, then info/config/ports. -use std::os::linux::net::SocketAddrExt; use std::process::Command; use std::time::Duration; @@ -606,52 +605,6 @@ fn test_control_stopped_supervisor_is_listed_as_unresponsive() { ); } -#[test] -fn test_control_refuses_other_uid() { - if unsafe { libc::geteuid() } != 0 { - eprintln!("skip: needs root to switch uid"); - return; - } - let name = format!("test-ctrl-peer-{}", std::process::id()); - let mut child = start_sleep_sandbox(&name); - if let Err(e) = wait_for_sandbox(&name) { - let stderr_output = child_stderr(&mut child); - let _ = child.kill(); - panic!("{}; child stderr: {}", e, stderr_output); - } - - // The socket belongs to uid 0; connect as nobody from a forked child and - // expect the server to close without answering. - let addr = std::os::unix::net::SocketAddr::from_abstract_name( - sandlock_core::control::socket_name(0, &name), - ).unwrap(); - let pid = unsafe { libc::fork() }; - if pid == 0 { - use std::io::{Read, Write}; - unsafe { libc::setresuid(65534, 65534, 65534) }; - let code = match std::os::unix::net::UnixStream::connect_addr(&addr) { - Ok(mut s) => { - let body = br#"{"v":1,"verb":"info","args":{}}"#; - let _ = s.write_all(&(body.len() as u32).to_be_bytes()); - let _ = s.write_all(body); - let _ = s.set_read_timeout(Some(Duration::from_secs(2))); - let mut buf = [0u8; 4]; - match s.read(&mut buf) { - Ok(0) => 0, - _ => 1, - } - } - Err(_) => 0, - }; - unsafe { libc::_exit(code) }; - } - let mut status = 0; - unsafe { libc::waitpid(pid, &mut status, 0) }; - let _ = child.kill(); - let _ = child.wait(); - assert_eq!(libc::WEXITSTATUS(status), 0, "another uid must get no response"); -} - /// wait() must release the name before it returns: a caller that runs the /// same name twice in a row must not hit the collision check. #[tokio::test] From d30417d9f1c506ecdbce32e2db2648f29a0393cc Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 17:15:31 -0700 Subject: [PATCH 10/16] docs: note the control socket's scope coupling and refresh ps samples The Landlock abstract-socket scope is now what keeps the confined child off its supervisor's control socket, which is worth saying where the mask is built. The README ps and kill samples predate the STATUS and PORTS columns and the two-pid kill message. Signed-off-by: Cong Wang --- README.md | 8 ++++---- crates/sandlock-core/src/landlock.rs | 2 ++ crates/sandlock-core/src/sandbox.rs | 2 ++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 624ab045..cedfef8c 100644 --- a/README.md +++ b/README.md @@ -771,9 +771,9 @@ stop them: ``` $ sandlock ps -NAME PID UPTIME CMD -api.local 12345 5m python3 server.py -web.local 12346 3m python3 server.py +NAME PID UPTIME STATUS PORTS CMD +api.local 12345 5m running 8080→41235 python3 server.py +web.local 12346 3m running 8080→41236 python3 server.py $ sandlock inspect api.local --toml | head -10 [config] @@ -783,7 +783,7 @@ http_inject_ca = [] ... $ sandlock kill web.local -Killed sandbox 'web.local' (PID 12346) +Killed sandbox 'web.local' (child PID 12346, supervisor PID 12340) ``` This enables external reverse proxies (nginx, envoy) to route traffic diff --git a/crates/sandlock-core/src/landlock.rs b/crates/sandlock-core/src/landlock.rs index 4026a392..490e32b9 100644 --- a/crates/sandlock-core/src/landlock.rs +++ b/crates/sandlock-core/src/landlock.rs @@ -246,6 +246,8 @@ pub(crate) fn compute_scope_mask(abi: u32, pol: &ProtectionPolicy) -> u64 { ); let mut mask: u64 = 0; + // This scope is also what keeps the confined child, which shares the + // supervisor's uid, off the supervisor's abstract control socket. if ProtectionStatus::resolve(Protection::AbstractUnixSocketScope, abi, pol) == ProtectionStatus::Active { diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index a749ad1b..0706807e 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2422,6 +2422,8 @@ impl Drop for Sandbox { rt.policy_fn_worker = None; if let Some(h) = rt.throttle_handle.take() { h.abort(); } if let Some(h) = rt.loadavg_handle.take() { h.abort(); } + // Drop cannot await; the name is released when the runtime drops + // the aborted task. wait() is the synchronous path. if let Some(h) = rt.control_handle.take() { h.abort(); } // Nobody is left to collect these; aborting closes the read ends. From 83a62799f0a74e6339b26f65381189c505113358 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 17:15:41 -0700 Subject: [PATCH 11/16] control: tolerate foreign abstract names when listing /proc/net/unix renders abstract names as raw bytes, so any local process could bind a non-UTF-8 name and make read_to_string fail for every user's sandlock ps. Ours are ASCII by validation, so a lossy decode only affects names that never match the prefix. Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 74090480..e738f3fe 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -382,7 +382,9 @@ pub(crate) fn parse_proc_net_unix(text: &str, uid: u32) -> Vec { /// Names of the caller's live sandboxes, sorted. pub fn list_sandboxes() -> std::io::Result> { - let text = std::fs::read_to_string("/proc/net/unix")?; + // Any process can bind an abstract name that is not UTF-8; ours are + // ASCII, so a mangled foreign name just fails the prefix match. + let text = String::from_utf8_lossy(&std::fs::read("/proc/net/unix")?).into_owned(); Ok(parse_proc_net_unix(&text, unsafe { libc::getuid() })) } From e84a35a3e839004e0cfd3c64dab0fde922327d51 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 17:26:59 -0700 Subject: [PATCH 12/16] sandbox: close inherited control sockets first thing in the child An abstract name stays bound while any fd refers to it, and a forked child keeps every inherited fd until it execs, or forever in the case of a COW clone. A sandbox created but not yet started therefore pinned the control socket name of every sibling in the process, which showed up as a spurious "already running" when a sibling's name was reused right after wait(). The child now scans /proc/self/fd and closes any socket whose abstract name is a sandlock control socket before it does anything else. That also means a confined child never holds a handle to another supervisor's control socket, even briefly. Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 30 ++++++++++++++++ crates/sandlock-core/src/sandbox.rs | 2 ++ .../tests/integration/test_control.rs | 34 +++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index e738f3fe..55f46c37 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -61,6 +61,36 @@ pub(crate) fn bind_control_socket(name: &str) -> std::io::Result { UnixListener::bind_addr(&socket_addr(name)?) } +/// First thing in a forked child. An abstract name stays bound while any +/// fd refers to it, and a child keeps its inherited fds until it execs (a +/// COW clone never does), so a parked child would pin every sibling's name. +pub(crate) fn close_inherited_control_sockets() { + let Ok(dir) = std::fs::read_dir("/proc/self/fd") else { return }; + for entry in dir.flatten() { + let fd = entry.file_name().to_str().and_then(|s| s.parse::().ok()); + if let Some(fd) = fd.filter(|&fd| is_control_socket(fd)) { + unsafe { libc::close(fd) }; + } + } +} + +fn is_control_socket(fd: i32) -> bool { + let mut addr: libc::sockaddr_un = unsafe { std::mem::zeroed() }; + let mut len = std::mem::size_of::() as libc::socklen_t; + let rc = unsafe { + libc::getsockname(fd, &mut addr as *mut _ as *mut libc::sockaddr, &mut len) + }; + if rc != 0 || addr.sun_family != libc::AF_UNIX as libc::sa_family_t { + return false; + } + let path_len = (len as usize).saturating_sub(std::mem::offset_of!(libc::sockaddr_un, sun_path)); + let path: Vec = addr.sun_path[..path_len.min(addr.sun_path.len())] + .iter() + .map(|&c| c as u8) + .collect(); + path.first() == Some(&0) && path[1..].starts_with(b"sandlock/") +} + // ============================================================ // Control loop, spawned as a dedicated tokio task // ============================================================ diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 0706807e..c49761cb 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1429,6 +1429,7 @@ impl Sandbox { } if pid == 0 { + crate::control::close_inherited_control_sockets(); drop(ctrl_parent); unsafe { libc::setpgid(0, 0) }; unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) }; @@ -1875,6 +1876,7 @@ impl Sandbox { if pid == 0 { // ===== CHILD PROCESS ===== + crate::control::close_inherited_control_sockets(); let io_overrides = self.rt().io_overrides; if let Some((stdin_fd, stdout_fd, stderr_fd)) = io_overrides { if let Some(fd) = stdin_fd { unsafe { libc::dup2(fd, 0) }; } diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index ef8e96de..38102931 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -626,6 +626,40 @@ async fn test_control_name_is_free_when_wait_returns() { } } +/// A child forked while another sandbox's listener exists inherits that fd, +/// and an abstract name stays bound while any fd refers to it. The child +/// must drop those copies before it parks, or a created-but-not-started +/// sandbox pins every other name in the process. +#[tokio::test] +async fn test_control_parked_child_does_not_pin_other_names() { + let policy = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/proc") + .build() + .unwrap(); + let pid = std::process::id(); + + let mut first = policy.clone().with_name(format!("test-ctrl-pinned-{pid}")); + first.create(&["true"]).await.unwrap(); + let mut parked = policy.clone().with_name(format!("test-ctrl-parker-{pid}")); + parked.create(&["true"]).await.unwrap(); + + first.start().unwrap(); + first.wait().await.unwrap(); + + let again = policy + .clone() + .with_name(format!("test-ctrl-pinned-{pid}")) + .run(&["true"]) + .await; + parked.start().unwrap(); + let _ = parked.wait().await; + assert!(again.is_ok(), "a parked sibling must not pin the name: {:?}", again.err()); +} + // ============================================================ // CLI kill / config input validation // ============================================================ From 60821744c01d3324e53ad887a9ec800470e27427 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 18:58:57 -0700 Subject: [PATCH 13/16] control: learn both pids from the kernel kill and ps used the info verb to find the child and supervisor pids, so a supervisor that never answered, stopped or deadlocked, could only be found by grepping ss for its abstract socket. listen() stamps the caller's pid into a socket and SO_PEERCRED hands that stamp to whoever connects, so the pids need no cooperation if the right process calls listen(). The supervisor now binds a second abstract socket, sandlock///pgrp, before it forks; the child inherits it, calls listen() right after setpgid(), and closes it, while the supervisor keeps the fd and drains the never-spoken-on connections. kill connects to both sockets, reads the peer pids, and does killpg on the child and kill on the supervisor without sending a request. Both pids are live by construction: the sockets die with the supervisor, and a dead child stays a zombie holding its pid until the supervisor reaps it, at which point wait() closes the sockets. The pids leave the wire protocol, info carries only the mode, and binding before the fork means a name collision fails with no child to reap. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 51 ++-- crates/sandlock-core/src/context.rs | 7 + crates/sandlock-core/src/control.rs | 277 +++++++++++++----- crates/sandlock-core/src/sandbox.rs | 74 +++-- .../tests/integration/test_control.rs | 45 ++- 5 files changed, 322 insertions(+), 132 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index ddafd1ff..1885549b 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -287,25 +287,28 @@ async fn main() -> Result<()> { } else { println!("NAME PID UPTIME STATUS PORTS CMD"); for name in &names { - match sandlock_core::control::sandbox_info(name) { - Ok(info) => { - let pid = info.child_pid; - let uptime = proc_uptime(pid).unwrap_or_else(|| "?".to_string()); - let cmd = proc_cmdline(pid).unwrap_or_else(|| "?".to_string()); - let ports = query_ports(name); - let status = info.mode.unwrap_or_else(|| "running".to_string()); - println!( - "{:<32} {:>8} {:>12} {:<10} {:<24} {}", - name, pid, uptime, status, ports, cmd - ); - } - // The socket exists, so the process is alive; it - // just is not answering. - Err(_) => println!( - "{:<32} ? ? unresponsive ? ?", - name + // The pids need no answer from the supervisor; the rest + // does, and a supervisor that does not answer is still + // alive because its socket is. + let (status, ports) = match sandlock_core::control::sandbox_info(name) { + Ok(info) => ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), ), - } + Err(_) => ("unresponsive".to_string(), "?".to_string()), + }; + let (pid, uptime, cmd) = match sandlock_core::control::sandbox_pids(name) { + Ok(pids) => ( + pids.child.to_string(), + proc_uptime(pids.child).unwrap_or_else(|| "?".to_string()), + proc_cmdline(pids.child).unwrap_or_else(|| "?".to_string()), + ), + Err(_) => ("?".to_string(), "?".to_string(), "?".to_string()), + }; + println!( + "{:<32} {:>8} {:>12} {:<10} {:<24} {}", + name, pid, uptime, status, ports, cmd + ); } } } @@ -352,8 +355,10 @@ async fn main() -> Result<()> { eprintln!("sandlock: {e}"); std::process::exit(1); } - let info = match sandlock_core::control::sandbox_info(&name) { - Ok(i) => i, + // Both pids come from the kernel, so this works on a supervisor + // that is stopped or wedged. + let pids = match sandlock_core::control::sandbox_pids(&name) { + Ok(p) => p, Err(e) => { eprintln!("sandlock: {}", e); std::process::exit(1); @@ -361,11 +366,11 @@ async fn main() -> Result<()> { }; // killpg takes the child's whole process group; the supervisor // may sit in a different group, so signal it directly too. - unsafe { libc::killpg(info.child_pid, libc::SIGKILL) }; - unsafe { libc::kill(info.supervisor_pid, libc::SIGKILL) }; + unsafe { libc::killpg(pids.child, libc::SIGKILL) }; + unsafe { libc::kill(pids.supervisor, libc::SIGKILL) }; println!( "Killed sandbox '{}' (child PID {}, supervisor PID {})", - name, info.child_pid, info.supervisor_pid + name, pids.child, pids.supervisor ); } diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 1ee25133..ea7a3d0b 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -237,6 +237,9 @@ pub(crate) struct ChildSpawnArgs<'a> { /// parent death in the child without assuming PID 1 is always init /// (incorrect in containers where the entrypoint runs as PID 1). pub parent_pid: libc::pid_t, + /// The sandbox's pgrp socket, listened on right after setpgid() so + /// `sandlock kill` learns the group leader from SO_PEERCRED. + pub pgrp_socket: Option, /// Make the child the terminal's foreground process group before exec. /// Only interactive (fully inherited) stdio wants this; a captured or /// piped run taking the foreground demotes the embedding process to a @@ -288,6 +291,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { sandbox_name, extra_syscalls, parent_pid, + pgrp_socket, foreground, } = args; // Helper: abort child on error. Includes the OS error automatically. @@ -305,6 +309,9 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { if unsafe { libc::setpgid(0, 0) } != 0 { fail!("setpgid"); } + if let Some(fd) = pgrp_socket { + crate::control::publish_pgrp(fd); + } // 1b. Interactive runs only: if stdin is a terminal, become the // foreground process group so interactive shells can read from the diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 55f46c37..e12f05bc 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -1,16 +1,25 @@ -//! Per-sandbox control socket for introspection. +//! Per-sandbox control sockets for introspection and kill. //! -//! Every sandbox (CLI, Python SDK, embedded) binds one abstract Unix -//! stream socket named `\0sandlock//` from the supervisor -//! process before the child is released. Abstract names live in the -//! kernel, not the filesystem: bind on a taken name fails, so the name is -//! the UID-wide sandbox mutex; the name vanishes with the process, so -//! nothing is ever stale; `/proc/net/unix` lists them, so `sandlock ps` -//! needs no registry on disk; and a nested sandlock needs no writable -//! directory from the outer policy, only permission to create a socket. +//! Every sandbox (CLI, Python SDK, embedded) binds two abstract Unix +//! stream sockets before it forks. `\0sandlock//` is the +//! control endpoint; the supervisor calls listen() on it. The child +//! inherits `\0sandlock///pgrp` and calls listen() on that one +//! right after setpgid(), then closes it; the supervisor keeps the fd. +//! Abstract names live in the kernel, not the filesystem: bind on a taken +//! name fails, so the first name is the UID-wide sandbox mutex; both names +//! vanish with the supervisor, so nothing is ever stale; `/proc/net/unix` +//! lists them, so `sandlock ps` needs no registry on disk; and a nested +//! sandlock needs no writable directory from the outer policy, only +//! permission to create a socket. //! -//! Abstract names carry no permission bits, so the server checks -//! SO_PEERCRED and closes any connection from another uid. +//! listen() stamps the caller's pid into the socket and SO_PEERCRED hands +//! that stamp to whoever connects, so a client learns the supervisor's pid +//! from the first socket and the child's, which is its process group, from +//! the second, without the supervisor answering anything. That is what +//! `sandlock kill` uses, so it works on a supervisor that is stopped or +//! wedged. Abstract names carry no permission bits, so both sides check +//! the SO_PEERCRED uid: the server closes any connection from another uid +//! and the client refuses a listener owned by one. //! //! ## Wire protocol //! @@ -24,18 +33,19 @@ //! //! Response: //! ```json -//! {"v": 1, "ok": true, "data": {"child_pid": 1234, "supervisor_pid": 1233, "mode": null}} +//! {"v": 1, "ok": true, "data": {"mode": null}} //! ``` //! or //! ```json //! {"v": 1, "ok": false, "err": "..."} //! ``` //! -//! Verbs: `info` (pids and mode), `config` (effective policy as +//! Verbs: `info` (mode), `config` (effective policy as //! `ProfileInput`), `ports` (virtual to real port map). use std::os::linux::net::SocketAddrExt; -use std::os::unix::net::{SocketAddr, UnixListener}; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; +use std::os::unix::net::{SocketAddr, UnixListener, UnixStream}; use std::sync::Arc; use crate::sandbox::Sandbox; @@ -50,25 +60,78 @@ pub(crate) fn socket_name(uid: u32, name: &str) -> Vec { format!("sandlock/{uid}/{name}").into_bytes() } +/// Sandbox names reject `/`, so the suffix cannot collide with a name. +fn pgrp_socket_name(uid: u32, name: &str) -> Vec { + format!("sandlock/{uid}/{name}/pgrp").into_bytes() +} + fn socket_addr(name: &str) -> std::io::Result { let uid = unsafe { libc::getuid() }; SocketAddr::from_abstract_name(socket_name(uid, name)) } -/// Bind the sandbox's control socket. `AddrInUse` means a live sandbox of -/// this uid already owns the name. -pub(crate) fn bind_control_socket(name: &str) -> std::io::Result { - UnixListener::bind_addr(&socket_addr(name)?) +fn pgrp_socket_addr(name: &str) -> std::io::Result { + let uid = unsafe { libc::getuid() }; + SocketAddr::from_abstract_name(pgrp_socket_name(uid, name)) +} + +/// Both sockets of one sandbox, bound before it forks. `control` already +/// listens, from the supervisor. `pgrp` is bound only: the child calls +/// listen() on it after setpgid(), so its peer pid is the group leader. +#[derive(Debug)] +pub(crate) struct ControlSockets { + pub control: UnixListener, + pub pgrp: OwnedFd, +} + +/// `AddrInUse` means a live sandbox of this uid already owns the name. +pub(crate) fn bind_control_sockets(name: &str) -> std::io::Result { + let control = UnixListener::bind_addr(&socket_addr(name)?)?; + let pgrp = bind_only(&pgrp_socket_addr(name)?)?; + Ok(ControlSockets { control, pgrp }) +} + +/// std has no bind-without-listen, and listen() must be the child's call. +fn bind_only(addr: &SocketAddr) -> std::io::Result { + let fd = unsafe { libc::socket(libc::AF_UNIX, libc::SOCK_STREAM | libc::SOCK_CLOEXEC, 0) }; + if fd < 0 { + return Err(std::io::Error::last_os_error()); + } + let fd = unsafe { OwnedFd::from_raw_fd(fd) }; + let name = addr.as_abstract_name().expect("abstract address"); + let mut sun: libc::sockaddr_un = unsafe { std::mem::zeroed() }; + sun.sun_family = libc::AF_UNIX as libc::sa_family_t; + for (dst, &src) in sun.sun_path[1..].iter_mut().zip(name) { + *dst = src as libc::c_char; + } + let len = std::mem::offset_of!(libc::sockaddr_un, sun_path) + 1 + name.len(); + let rc = unsafe { + libc::bind(fd.as_raw_fd(), &sun as *const _ as *const libc::sockaddr, len as libc::socklen_t) + }; + if rc != 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(fd) +} + +/// In the child, after setpgid(). listen() records this pid as the +/// socket's peer credential; the supervisor keeps the socket alive. +pub(crate) fn publish_pgrp(fd: RawFd) { + unsafe { + libc::listen(fd, libc::SOMAXCONN); + libc::close(fd); + } } /// First thing in a forked child. An abstract name stays bound while any /// fd refers to it, and a child keeps its inherited fds until it execs (a /// COW clone never does), so a parked child would pin every sibling's name. -pub(crate) fn close_inherited_control_sockets() { +/// `keep` is the child's own pgrp socket, which it still has to listen on. +pub(crate) fn close_inherited_control_sockets(keep: Option) { let Ok(dir) = std::fs::read_dir("/proc/self/fd") else { return }; for entry in dir.flatten() { let fd = entry.file_name().to_str().and_then(|s| s.parse::().ok()); - if let Some(fd) = fd.filter(|&fd| is_control_socket(fd)) { + if let Some(fd) = fd.filter(|&fd| Some(fd) != keep && is_control_socket(fd)) { unsafe { libc::close(fd) }; } } @@ -95,11 +158,9 @@ fn is_control_socket(fd: i32) -> bool { // Control loop, spawned as a dedicated tokio task // ============================================================ -/// What the `info` verb reports: everything `sandlock ps` and `kill` need. +/// What the `info` verb reports. Pids are not here: the sockets carry them. #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct SandboxInfo { - pub child_pid: i32, - pub supervisor_pid: i32, pub mode: Option, } @@ -107,7 +168,7 @@ pub struct SandboxInfo { /// seccomp-notify supervisor (`--no-supervisor`, nested); those still /// answer `info` and the static `config`, and report no ports. pub(crate) fn spawn_control_loop( - listener: UnixListener, + sockets: ControlSockets, ctx: Option>, sandbox: Sandbox, info: SandboxInfo, @@ -115,12 +176,13 @@ pub(crate) fn spawn_control_loop( // Mutex only to satisfy Sync: Sandbox carries a Box slot // even though this clone's is None. let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); + let pgrp = UnixListener::from(sockets.pgrp); tokio::spawn(async move { - control_loop(listener, ctx, sandbox, info, unsafe { libc::getuid() }).await; + control_loop(sockets.control, Some(pgrp), ctx, sandbox, info, unsafe { libc::getuid() }).await; }) } -fn peer_uid(fd: std::os::unix::io::RawFd) -> Option { +fn peer_cred(fd: RawFd) -> Option { let mut cred: libc::ucred = unsafe { std::mem::zeroed() }; let mut len = std::mem::size_of::() as libc::socklen_t; let rc = unsafe { @@ -132,34 +194,53 @@ fn peer_uid(fd: std::os::unix::io::RawFd) -> Option { &mut len, ) }; - (rc == 0).then_some(cred.uid) + (rc == 0).then_some(cred) +} + +fn into_tokio(listener: UnixListener) -> Option { + listener.set_nonblocking(true).ok()?; + tokio::net::UnixListener::from_std(listener).ok() } /// Accept one connection at a time and serve one request per connection. /// `my_uid` is a parameter so a test can prove the refusal path without a /// second uid. The timeout keeps one stalled client from wedging -/// introspection, which kill now depends on. +/// introspection. Clients only connect to the pgrp socket for its peer +/// credential and never speak, so those connections are accepted and +/// dropped to keep its backlog empty; a child that never called listen() +/// makes accept() fail with EINVAL, after which the socket is left alone. async fn control_loop( listener: UnixListener, + pgrp: Option, ctx: Option>, sandbox: Arc>, info: SandboxInfo, my_uid: u32, ) { - use std::os::unix::io::AsRawFd; - listener.set_nonblocking(true).ok(); - let listener = match tokio::net::UnixListener::from_std(listener) { - Ok(l) => l, - Err(_) => return, - }; + let Some(listener) = into_tokio(listener) else { return }; + let mut pgrp = pgrp.and_then(into_tokio); loop { - let (stream, _addr) = match listener.accept().await { - Ok(pair) => pair, - Err(_) => return, + let drain = async { + match &pgrp { + Some(l) => l.accept().await, + None => std::future::pending().await, + } + }; + let stream = tokio::select! { + accepted = listener.accept() => match accepted { + Ok((stream, _)) => stream, + Err(_) => return, + }, + drained = drain => { + if drained.is_err() { + pgrp = None; + } + continue; + } }; // Abstract names have no permission bits, so this is the only gate. - if peer_uid(stream.as_raw_fd()) != Some(my_uid) { + if peer_cred(stream.as_raw_fd()).map(|c| c.uid) != Some(my_uid) { continue; } let _ = tokio::time::timeout( @@ -402,7 +483,8 @@ pub(crate) fn parse_proc_net_unix(text: &str, uid: u32) -> Vec { if flags != "00010000" { return None; } - path.strip_prefix(&prefix).map(str::to_string) + let name = path.strip_prefix(&prefix)?; + (!name.contains('/')).then(|| name.to_string()) }) .collect(); names.sort(); @@ -431,6 +513,63 @@ fn unresponsive(name: &str, e: std::io::Error) -> String { } } +/// Connect to one of a sandbox's sockets and return the stream with the +/// listener's credentials. `my_uid` is a parameter so a test can prove the +/// refusal without a second uid. SO_PEERCRED on a connected stream reports +/// the process that called listen(), so a name squatted by another user is +/// rejected here, and the pid is that process as seen from this pid +/// namespace. +fn connect_as(addr: &SocketAddr, my_uid: u32) -> Result<(UnixStream, libc::ucred), std::io::Error> { + let stream = UnixStream::connect_addr(addr)?; + match peer_cred(stream.as_raw_fd()) { + Some(cred) if cred.uid == my_uid => Ok((stream, cred)), + _ => Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "owned by another user", + )), + } +} + +fn connect_control(name: &str, my_uid: u32) -> Result<(UnixStream, libc::ucred), String> { + let addr = socket_addr(name).map_err(|e| format!("socket address for '{}': {}", name, e))?; + connect_as(&addr, my_uid).map_err(|e| match e.kind() { + std::io::ErrorKind::ConnectionRefused => format!("no sandbox named '{}'", name), + std::io::ErrorKind::PermissionDenied => { + format!("socket for '{}' is owned by another user", name) + } + _ => format!("connect to sandbox '{}': {}", name, e), + }) +} + +/// The two pids `kill` needs, both stamped by the kernel at listen() time. +#[derive(Debug, Clone, Copy)] +pub struct SandboxPids { + /// The child, which leads its own process group. + pub child: i32, + pub supervisor: i32, +} + +/// Needs no cooperation from the supervisor, so it works on one that is +/// stopped or wedged. +pub fn sandbox_pids(name: &str) -> Result { + sandbox_pids_as(name, unsafe { libc::getuid() }) +} + +fn sandbox_pids_as(name: &str, my_uid: u32) -> Result { + let (_, supervisor) = connect_control(name, my_uid)?; + let addr = pgrp_socket_addr(name).map_err(|e| format!("socket address for '{}': {}", name, e))?; + // The name exists, so the supervisor is up; the child has not reached + // listen() yet if this is refused. + let (_, child) = connect_as(&addr, my_uid).map_err(|e| match e.kind() { + std::io::ErrorKind::ConnectionRefused => format!("sandbox '{}' is still starting", name), + std::io::ErrorKind::PermissionDenied => { + format!("socket for '{}' is owned by another user", name) + } + _ => format!("connect to sandbox '{}': {}", name, e), + })?; + Ok(SandboxPids { child: child.pid, supervisor: supervisor.pid }) +} + /// Send a request to a sandbox's control socket and return the response. pub fn send_control_request( name: &str, @@ -440,9 +579,6 @@ pub fn send_control_request( send_control_request_as(name, verb, args, unsafe { libc::getuid() }) } -/// `my_uid` is a parameter so a test can prove the refusal without a -/// second uid. SO_PEERCRED on a connected stream reports the listener's -/// credentials, so a name squatted by another user is rejected here. fn send_control_request_as( name: &str, verb: &str, @@ -450,20 +586,8 @@ fn send_control_request_as( my_uid: u32, ) -> Result { use std::io::{Read, Write}; - use std::os::unix::io::AsRawFd; - use std::os::unix::net::UnixStream; - let addr = socket_addr(name).map_err(|e| format!("socket address for '{}': {}", name, e))?; - let mut stream = match UnixStream::connect_addr(&addr) { - Ok(s) => s, - Err(e) if e.kind() == std::io::ErrorKind::ConnectionRefused => { - return Err(format!("no sandbox named '{}'", name)); - } - Err(e) => return Err(format!("connect to sandbox '{}': {}", name, e)), - }; - if peer_uid(stream.as_raw_fd()) != Some(my_uid) { - return Err(format!("socket for '{}' is owned by another user", name)); - } + let (mut stream, _) = connect_control(name, my_uid)?; // Set a 2-second timeout on reads so a wedged supervisor does not // block the CLI forever. @@ -500,7 +624,7 @@ fn send_control_request_as( .map_err(|e| format!("parse response: {}", e)) } -/// Ask a sandbox for its pids and mode. +/// Ask a sandbox for its mode. pub fn sandbox_info(name: &str) -> Result { let resp = send_control_request(name, "info", serde_json::Value::Object(Default::default()))?; if !resp.ok { @@ -518,7 +642,7 @@ mod tests { fn longest_name_fits_sun_path() { let name = "x".repeat(64); // Leading NUL plus the name must fit the kernel's 108-byte sun_path. - assert!(socket_name(u32::MAX, &name).len() < 108); + assert!(pgrp_socket_name(u32::MAX, &name).len() < 108); } #[test] @@ -528,7 +652,8 @@ mod tests { 0000000000000000: 00000003 00000000 00000000 0001 03 11628861 @sandlock/1000/alpha\n\ 0000000000000000: 00000002 00000000 00010000 0001 01 11628862 @sandlock/1001/other\n\ 0000000000000000: 00000002 00000000 00010000 0001 01 11628863 /run/user/1000/bus\n\ - 0000000000000000: 00000002 00000000 00010000 0001 01 11628864 @sandlock/1000/beta\n"; + 0000000000000000: 00000002 00000000 00010000 0001 01 11628864 @sandlock/1000/beta\n\ + 0000000000000000: 00000002 00000000 00010000 0001 01 11628865 @sandlock/1000/beta/pgrp\n"; assert_eq!(parse_proc_net_unix(text, 1000), vec!["alpha", "beta"]); assert_eq!(parse_proc_net_unix(text, 1001), vec!["other"]); } @@ -537,14 +662,34 @@ mod tests { fn bind_is_the_name_mutex_and_listing_follows_the_listener() { // Unique name: sandbox names are uid-wide, never reuse a fixed one. let name = format!("test-ctrl-unit-{}", std::process::id()); - let listener = bind_control_socket(&name).unwrap(); + let sockets = bind_control_sockets(&name).unwrap(); assert!(list_sandboxes().unwrap().contains(&name)); - let err = bind_control_socket(&name).unwrap_err(); + let err = bind_control_sockets(&name).unwrap_err(); assert_eq!(err.kind(), std::io::ErrorKind::AddrInUse); - drop(listener); + drop(sockets); assert!(!list_sandboxes().unwrap().contains(&name)); } + /// The pids come from the kernel's record of who called listen(), not + /// from anything the sandbox says; nobody serves these sockets here. + #[test] + fn client_learns_both_pids_from_the_kernel() { + let name = format!("test-ctrl-pids-{}", std::process::id()); + let sockets = bind_control_sockets(&name).unwrap(); + let me = std::process::id() as i32; + + let err = sandbox_pids(&name).unwrap_err(); + assert!(err.contains("still starting"), "before listen: {err}"); + + assert_eq!(unsafe { libc::listen(sockets.pgrp.as_raw_fd(), 1) }, 0); + let pids = sandbox_pids(&name).unwrap(); + assert_eq!((pids.child, pids.supervisor), (me, me)); + + let expect = unsafe { libc::getuid() }.wrapping_add(1); + let err = sandbox_pids_as(&name, expect).unwrap_err(); + assert!(err.contains("owned by another user"), "got: {err}"); + } + use std::io::{Read, Write}; use std::os::unix::net::UnixStream; @@ -553,15 +698,15 @@ mod tests { } fn info() -> SandboxInfo { - SandboxInfo { child_pid: 4242, supervisor_pid: 4241, mode: None } + SandboxInfo { mode: Some("test".into()) } } /// Bind a listener for `name`, run the control loop on it with /// `expected_uid`, and return the task handle. fn serve(name: &str, expected_uid: u32) -> tokio::task::JoinHandle<()> { - let listener = bind_control_socket(name).unwrap(); + let listener = bind_control_sockets(name).unwrap().control; let sandbox = Arc::new(tokio::sync::Mutex::new(test_sandbox())); - tokio::spawn(control_loop(listener, None, sandbox, info(), expected_uid)) + tokio::spawn(control_loop(listener, None, None, sandbox, info(), expected_uid)) } /// Connect as ourselves, send an info request, and return what the @@ -587,7 +732,7 @@ mod tests { let resp: ControlResponse = serde_json::from_slice(&out[4..]).unwrap(); assert!(resp.ok); let got: SandboxInfo = serde_json::from_value(resp.data.unwrap()).unwrap(); - assert_eq!((got.child_pid, got.supervisor_pid), (4242, 4241)); + assert_eq!(got.mode.as_deref(), Some("test")); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -620,7 +765,7 @@ mod tests { let addr = SocketAddr::from_abstract_name(b"sandlock-probe-\xff\xfe").unwrap(); let _foreign = UnixListener::bind_addr(&addr).unwrap(); let name = format!("test-ctrl-utf8-{}", std::process::id()); - let _ours = bind_control_socket(&name).unwrap(); + let _ours = bind_control_sockets(&name).unwrap(); assert!(list_sandboxes().unwrap().contains(&name)); } } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index c49761cb..fee2c4ea 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1429,7 +1429,7 @@ impl Sandbox { } if pid == 0 { - crate::control::close_inherited_control_sockets(); + crate::control::close_inherited_control_sockets(None); drop(ctrl_parent); unsafe { libc::setpgid(0, 0) }; unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) }; @@ -1869,6 +1869,31 @@ impl Sandbox { let foreground = stdio.all_inherit(); let tty_foreground_taken = foreground && unsafe { libc::isatty(0) } == 1; + // Bound before the fork so a name collision fails with no child to + // reap. Both fds are CLOEXEC; the child closes its copies itself. + let sandbox_name = self.rt().name.clone(); + let mut control_sockets = match crate::control::bind_control_sockets(&sandbox_name) { + Ok(s) => Some(s), + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { + return Err(SandboxRuntimeError::Child(format!( + "sandbox '{}' is already running", + sandbox_name + )) + .into()); + } + Err(e) => { + // A nested sandlock whose outer policy denies AF_UNIX lands + // here; the sandbox still runs, it is just not introspectable. + eprintln!( + "sandlock: control socket setup failed for '{}': {} \ + (introspection unavailable for this sandbox)", + sandbox_name, e + ); + None + } + }; + let pgrp_socket = control_sockets.as_ref().map(|s| s.pgrp.as_raw_fd()); + let pid = unsafe { libc::fork() }; if pid < 0 { return Err(SandboxRuntimeError::Fork(std::io::Error::last_os_error()).into()); @@ -1876,7 +1901,7 @@ impl Sandbox { if pid == 0 { // ===== CHILD PROCESS ===== - crate::control::close_inherited_control_sockets(); + crate::control::close_inherited_control_sockets(pgrp_socket); let io_overrides = self.rt().io_overrides; if let Some((stdin_fd, stdout_fd, stderr_fd)) = io_overrides { if let Some(fd) = stdin_fd { unsafe { libc::dup2(fd, 0) }; } @@ -1932,7 +1957,6 @@ impl Sandbox { .map(|h| h.0 as u32) .collect(); - let sandbox_name = self.rt().name.clone(); // In-process entrypoint (OCI PID-1) names the process from cmd[0]; // otherwise execve the command. let entry = match self.in_child_main { @@ -1948,6 +1972,7 @@ impl Sandbox { sandbox_name: Some(sandbox_name.as_str()), extra_syscalls: &extra_syscalls, parent_pid, + pgrp_socket, foreground, }); } @@ -1973,40 +1998,7 @@ impl Sandbox { let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; - // This must stay after the notif-fd read above. Any error return - // from do_spawn relies on Drop's killpg to reap the child, which - // only works once the child has setpgid'd into its own group; the - // notif-fd write is the child's setup-complete signal, so returning - // before it races killpg against setpgid and can deadlock Drop's - // waitpid against a child parked on the ready pipe. - let sandbox_name = self.rt().name.clone(); - let control_info = crate::control::SandboxInfo { - child_pid: pid, - supervisor_pid: std::process::id() as i32, - mode: self.mode.clone(), - }; - // std creates the listener with SOCK_CLOEXEC, and the child has - // already forked, so it never holds this fd. - let mut control_listener = match crate::control::bind_control_socket(&sandbox_name) { - Ok(l) => Some(l), - Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { - return Err(SandboxRuntimeError::Child(format!( - "sandbox '{}' is already running", - sandbox_name - )) - .into()); - } - Err(e) => { - // A nested sandlock whose outer policy denies AF_UNIX lands - // here; the sandbox still runs, it is just not introspectable. - eprintln!( - "sandlock: control socket setup failed for '{}': {} \ - (introspection unavailable for this sandbox)", - sandbox_name, e - ); - None - } - }; + let control_info = crate::control::SandboxInfo { mode: self.mode.clone() }; let is_nested_mode = notif_fd_num == 0; @@ -2242,9 +2234,9 @@ impl Sandbox { // Independent of the seccomp-notify loop so accept() never adds // latency to syscall notification processing. - if let Some(listener) = control_listener.take() { + if let Some(sockets) = control_sockets.take() { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( - listener, + sockets, Some(control_ctx), sandbox_snapshot, control_info.clone(), @@ -2266,9 +2258,9 @@ impl Sandbox { // No notify supervisor (--no-supervisor or nested): still answer ps, // inspect, and kill, with the static policy and no ports. - if let Some(listener) = control_listener.take() { + if let Some(sockets) = control_sockets.take() { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( - listener, + sockets, None, self.clone(), control_info, diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 38102931..6c4d9698 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -414,8 +414,10 @@ fn test_control_no_supervisor() { stdout ); + let pids = sandlock_core::control::sandbox_pids(&name).expect("pids"); + assert_eq!(pids.supervisor, child.id() as i32, "supervisor pid: {:?}", pids); + assert_eq!(unsafe { libc::getpgid(pids.child) }, pids.child, "child leads its group: {:?}", pids); let info = sandlock_core::control::sandbox_info(&name).expect("info"); - assert!(info.child_pid > 0 && info.supervisor_pid > 0, "info should report real pids: {:?}", info); assert_eq!(info.mode, None); let inspect = sandlock_bin().args(["inspect", &name]).output().expect("inspect"); @@ -556,7 +558,7 @@ fn test_control_killed_supervisor_vanishes_and_name_is_reusable() { let _ = first.kill(); panic!("{}; child stderr: {}", e, stderr_output); } - let child_pid = sandlock_core::control::sandbox_info(&name).expect("info").child_pid; + let child_pid = sandlock_core::control::sandbox_pids(&name).expect("pids").child; // SIGKILL skips Drop entirely: nothing runs any cleanup. first.kill().expect("kill supervisor"); @@ -605,6 +607,45 @@ fn test_control_stopped_supervisor_is_listed_as_unresponsive() { ); } +/// Poll until `pid` is gone, or give up after 3s. +fn wait_for_pid_gone(pid: i32) -> bool { + for _ in 0..30 { + if unsafe { libc::kill(pid, 0) } != 0 { + return true; + } + std::thread::sleep(Duration::from_millis(100)); + } + false +} + +/// A stopped supervisor answers nothing, and kill must not need it to. +#[test] +fn test_control_kill_terminates_a_stopped_supervisor() { + let name = format!("test-ctrl-killstop-{}", std::process::id()); + let mut child = start_sleep_sandbox(&name); + if let Err(e) = wait_for_sandbox(&name) { + let stderr_output = child_stderr(&mut child); + let _ = child.kill(); + panic!("{}; child stderr: {}", e, stderr_output); + } + let pids = sandlock_core::control::sandbox_pids(&name).expect("pids"); + assert_eq!(pids.supervisor, child.id() as i32); + + unsafe { libc::kill(child.id() as i32, libc::SIGSTOP) }; + let out = sandlock_bin().args(["kill", &name]).output().expect("sandlock kill"); + let supervisor_gone = child.wait(); + unsafe { libc::kill(pids.child, libc::SIGCONT) }; + + assert!( + out.status.success(), + "kill should succeed against a stopped supervisor: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(supervisor_gone.is_ok_and(|s| !s.success()), "supervisor should be SIGKILLed"); + assert!(wait_for_pid_gone(pids.child), "child {} should be dead", pids.child); + assert!(wait_for_gone(&name), "name should be free after kill"); +} + /// wait() must release the name before it returns: a caller that runs the /// same name twice in a row must not hit the collision check. #[tokio::test] From 010e67d91da287c050aa50e64d8c2d0f76f497b5 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 19:16:40 -0700 Subject: [PATCH 14/16] control: retire the runtime dir from comments Several comments still explained unique sandbox names by the per-UID runtime directory they used to claim, and the mode field's doc said the marker was written there at spawn time. The registry is gone; names are claimed by binding an abstract socket and the mode is served over it, so the comments now say that. Signed-off-by: Cong Wang --- crates/sandlock-core/src/pipeline.rs | 6 +++--- crates/sandlock-core/src/sandbox.rs | 4 ++-- crates/sandlock-core/src/transaction.rs | 2 +- crates/sandlock-core/tests/integration/test_control.rs | 5 +++-- crates/sandlock-core/tests/integration/test_determinism.rs | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/sandlock-core/src/pipeline.rs b/crates/sandlock-core/src/pipeline.rs index 2230d611..102b4f58 100644 --- a/crates/sandlock-core/src/pipeline.rs +++ b/crates/sandlock-core/src/pipeline.rs @@ -47,7 +47,7 @@ impl Stage { /// Run this single stage and return the result. pub async fn run(self, timeout: Option) -> Result { let cmd_refs: Vec<&str> = self.args.iter().map(|s| s.as_str()).collect(); - // Names claim a per-UID runtime dir and a live collision is a hard + // Names claim a per-UID socket name and a live collision is a hard // error, so every internally assigned name carries a unique id. let mut sb = self.sandbox.with_name( format!("stage-{}", crate::sandbox::unique_instance_id())); @@ -184,7 +184,7 @@ async fn run_pipeline(stages: Vec) -> Result { let (cap_stderr_r, cap_stderr_w) = make_pipe().map_err(SandboxRuntimeError::Io)?; // Spawn each stage. Stage names share one unique run id so concurrent - // pipelines in the same UID never collide on their runtime dirs. + // pipelines in the same UID never collide on their socket names. let run = crate::sandbox::unique_instance_id(); let mut sandboxes: Vec = Vec::with_capacity(n); @@ -370,7 +370,7 @@ async fn run_gather( // Spawn producers: each writes stdout to its pipe. Source and consumer // names share one unique run id so concurrent gathers in the same UID - // never collide on their runtime dirs. + // never collide on their socket names. let run = crate::sandbox::unique_instance_id(); let mut sandboxes: Vec = Vec::with_capacity(n + 1); for (i, ns) in sources.into_iter().enumerate() { diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index fee2c4ea..73fe9ba1 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -535,8 +535,8 @@ pub struct Sandbox { #[serde(skip)] pub name: Option, - /// Operating-mode marker (e.g. "learn") written to the runtime dir at - /// spawn time and shown as STATUS by `sandlock ps`, so an operator sees + /// Operating-mode marker (e.g. "learn") served by the control socket + /// and shown as STATUS by `sandlock ps`, so an operator sees /// why a sandbox exists (learn's read-everything observation run would /// otherwise be indistinguishable from a dangerously permissive one). /// Instance metadata like `name`, not policy — never serialized. diff --git a/crates/sandlock-core/src/transaction.rs b/crates/sandlock-core/src/transaction.rs index b8c0009d..85175be0 100644 --- a/crates/sandlock-core/src/transaction.rs +++ b/crates/sandlock-core/src/transaction.rs @@ -693,7 +693,7 @@ async fn drive_txn_stages( let tee_fd: Option = if tee_stderr { open_stderr_tee() } else { None }; // Stage names share one unique run id so concurrent transactions in the - // same UID never collide on their runtime dirs. + // same UID never collide on their socket names. let run = crate::sandbox::unique_instance_id(); for (i, stage) in stages.into_iter().enumerate() { let at = |source: SandlockError| TxnError::Stage { index: i, source }; diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 6c4d9698..76bc7921 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -1,8 +1,9 @@ //! Integration tests for the per-sandbox control socket. //! //! Each test starts a real sandbox through the CLI binary and drives the -//! abstract control socket the way `sandlock ps`, `inspect`, `ports`, and -//! `kill` do: discovery through /proc/net/unix, then info/config/ports. +//! abstract control sockets the way `sandlock ps`, `inspect`, `ports`, and +//! `kill` do: discovery through /proc/net/unix, pids from SO_PEERCRED, +//! then info/config/ports. use std::process::Command; use std::time::Duration; diff --git a/crates/sandlock-core/tests/integration/test_determinism.rs b/crates/sandlock-core/tests/integration/test_determinism.rs index 2bac634e..b69138f0 100644 --- a/crates/sandlock-core/tests/integration/test_determinism.rs +++ b/crates/sandlock-core/tests/integration/test_determinism.rs @@ -211,7 +211,7 @@ async fn test_hostname_virtualization() { .unwrap(); // Unique per process so concurrent test binaries never collide on the - // per-name runtime dir. + // per-name control socket. let name = format!("mybox-{}", std::process::id()); // Verify uname() returns the virtual hostname. From c26c51fcdec3b80206797bb8a3729ae0aa9d1840 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 3 Sep 2026 22:31:23 -0700 Subject: [PATCH 15/16] sandbox: publish the pgrp socket before the child's dup2 loops The extra fd mappings dup2 onto fixed low targets (3 upward for gather, 3..5 for checkpoint restore) while the pgrp socket takes the lowest free fd in the supervisor, so a target could be the socket's own number. The child then listened on the caller's fd, silently failed with ENOTSOCK, and closed the mapping it had just installed: sandlock kill never learned the group leader and the workload lost a descriptor. Publish right after the socket sweep, before anything renumbers fds. setpgid() moves with it, since killpg() needs the group to exist before the socket accepts, so confine_child no longer creates the group itself. The new test pins the pgrp socket onto an extra fd target in a fresh single-threaded process and checks both the pid lookup and the mapped fd survive. Signed-off-by: Cong Wang --- crates/sandlock-core/src/context.rs | 14 +- crates/sandlock-core/src/sandbox.rs | 13 +- .../tests/integration/test_control.rs | 132 ++++++++++++++++++ 3 files changed, 145 insertions(+), 14 deletions(-) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index ea7a3d0b..23d2e492 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -237,9 +237,6 @@ pub(crate) struct ChildSpawnArgs<'a> { /// parent death in the child without assuming PID 1 is always init /// (incorrect in containers where the entrypoint runs as PID 1). pub parent_pid: libc::pid_t, - /// The sandbox's pgrp socket, listened on right after setpgid() so - /// `sandlock kill` learns the group leader from SO_PEERCRED. - pub pgrp_socket: Option, /// Make the child the terminal's foreground process group before exec. /// Only interactive (fully inherited) stdio wants this; a captured or /// piped run taking the foreground demotes the embedding process to a @@ -291,7 +288,6 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { sandbox_name, extra_syscalls, parent_pid, - pgrp_socket, foreground, } = args; // Helper: abort child on error. Includes the OS error automatically. @@ -305,15 +301,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { use std::io::Write; - // 1. New process group - if unsafe { libc::setpgid(0, 0) } != 0 { - fail!("setpgid"); - } - if let Some(fd) = pgrp_socket { - crate::control::publish_pgrp(fd); - } - - // 1b. Interactive runs only: if stdin is a terminal, become the + // 1. Interactive runs only: if stdin is a terminal, become the // foreground process group so interactive shells can read from the // TTY. Captured/piped runs must not: the embedding process keeps // the terminal (issue #164). diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 73fe9ba1..f19984ed 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1902,6 +1902,18 @@ impl Sandbox { if pid == 0 { // ===== CHILD PROCESS ===== crate::control::close_inherited_control_sockets(pgrp_socket); + // killpg() needs the group to exist before anyone can connect, + // and the dup2 loops below have fixed targets that can be this + // socket's own fd number, so both come first. + if unsafe { libc::setpgid(0, 0) } != 0 { + use std::io::Write; + let err = std::io::Error::last_os_error(); + let _ = writeln!(std::io::stderr(), "sandlock child: setpgid: {err}"); + unsafe { libc::_exit(127) }; + } + if let Some(fd) = pgrp_socket { + crate::control::publish_pgrp(fd); + } let io_overrides = self.rt().io_overrides; if let Some((stdin_fd, stdout_fd, stderr_fd)) = io_overrides { if let Some(fd) = stdin_fd { unsafe { libc::dup2(fd, 0) }; } @@ -1972,7 +1984,6 @@ impl Sandbox { sandbox_name: Some(sandbox_name.as_str()), extra_syscalls: &extra_syscalls, parent_pid, - pgrp_socket, foreground, }); } diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 76bc7921..8590fd10 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -702,6 +702,138 @@ async fn test_control_parked_child_does_not_pin_other_names() { assert!(again.is_ok(), "a parked sibling must not pin the name: {:?}", again.err()); } +// ============================================================ +// pgrp socket vs extra fd targets +// ============================================================ + +fn open_devnull() -> std::os::fd::OwnedFd { + use std::os::fd::FromRawFd; + let fd = unsafe { libc::open(c"/dev/null".as_ptr(), libc::O_RDONLY | libc::O_CLOEXEC) }; + assert!(fd >= 0, "open /dev/null: {}", std::io::Error::last_os_error()); + unsafe { std::os::fd::OwnedFd::from_raw_fd(fd) } +} + +/// Fill every hole in the fd table so later allocations are consecutive +/// from the returned number. `keep` holds the fillers open. +fn make_fd_table_contiguous(keep: &mut Vec) -> i32 { + use std::os::fd::AsRawFd; + let top = (0..4096).rev().find(|&fd| unsafe { libc::fcntl(fd, libc::F_GETFD) } >= 0).unwrap(); + loop { + let filler = open_devnull(); + let fd = filler.as_raw_fd(); + keep.push(filler); + if fd > top { + return fd + 1; + } + } +} + +/// The fd number in this process bound to `name`'s pgrp socket. +fn pgrp_fd_of(name: &str) -> Option { + let want = format!("\0sandlock/{}/{}/pgrp", unsafe { libc::getuid() }, name); + (0..4096).find(|&fd| { + let mut addr: libc::sockaddr_un = unsafe { std::mem::zeroed() }; + let mut len = std::mem::size_of::() as libc::socklen_t; + let rc = unsafe { libc::getsockname(fd, &mut addr as *mut _ as *mut libc::sockaddr, &mut len) }; + if rc != 0 { + return false; + } + let path_len = (len as usize).saturating_sub(std::mem::offset_of!(libc::sockaddr_un, sun_path)); + let path: Vec = addr.sun_path[..path_len].iter().map(|&c| c as u8).collect(); + path == want.as_bytes() + }) +} + +const FD_LAYOUT_ENV: &str = "SANDLOCK_TEST_FD_LAYOUT_CHILD"; +const FD_LAYOUT_TEST: &str = "test_control::test_control_pgrp_published_before_extra_fd_dup2"; + +/// The child dup2s extra fds onto fixed low targets, and the pgrp socket +/// takes the lowest free fd in the supervisor, so a target can be the pgrp +/// socket's own number. Publishing after the dup2 would listen on the +/// caller's fd and then close it. +#[test] +fn test_control_pgrp_published_before_extra_fd_dup2() { + // Fd numbers are only predictable while no other thread allocates, + // so the body runs alone in a fresh process. + if std::env::var_os(FD_LAYOUT_ENV).is_none() { + let status = Command::new(std::env::current_exe().unwrap()) + .args(["--exact", FD_LAYOUT_TEST, "--test-threads=1"]) + .env(FD_LAYOUT_ENV, "1") + .status() + .unwrap(); + assert!(status.success(), "fd layout body failed in the child process"); + return; + } + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(pgrp_published_before_extra_fd_dup2()); +} + +async fn pgrp_published_before_extra_fd_dup2() { + use std::io::{Read, Write}; + use std::os::fd::AsRawFd; + + let policy = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/proc") + .build() + .unwrap(); + let pid = std::process::id(); + let mut fillers = Vec::new(); + + // Learn how many fds create() allocates before the pgrp socket. + let probe_name = format!("test-ctrl-pgrp-probe-{pid}"); + let (probe_out_r, probe_out_w) = std::io::pipe().unwrap(); + let base = make_fd_table_contiguous(&mut fillers); + let mut probe = policy.clone().with_name(&probe_name); + probe + .create_with_gather_io(&["true"], None, Some(probe_out_w.as_raw_fd()), None, Vec::new()) + .await + .unwrap(); + let offset = pgrp_fd_of(&probe_name).expect("probe pgrp socket") - base; + probe.start().unwrap(); + probe.wait().await.unwrap(); + drop(probe); + drop((probe_out_r, probe_out_w)); + for _ in 0..16 { + tokio::task::yield_now().await; + } + + // Now aim an extra fd at exactly that number. + let name = format!("test-ctrl-pgrp-dup2-{pid}"); + let (data_r, mut data_w) = std::io::pipe().unwrap(); + data_w.write_all(b"ping\n").unwrap(); + drop(data_w); + let (mut out_r, out_w) = std::io::pipe().unwrap(); + let target = make_fd_table_contiguous(&mut fillers) + offset; + let mut sb = policy.with_name(&name); + sb.create_with_gather_io( + &["cat", &format!("/proc/self/fd/{target}")], + None, + Some(out_w.as_raw_fd()), + None, + vec![(target, data_r.as_raw_fd())], + ) + .await + .unwrap(); + assert_eq!(pgrp_fd_of(&name), Some(target), "fd layout assumption broke"); + + let pids = sandlock_core::control::sandbox_pids(&name).expect("pgrp socket listens before start"); + assert_eq!(Some(pids.child), sb.pid()); + + sb.start().unwrap(); + let result = sb.wait().await.unwrap(); + drop(out_w); + let mut out = String::new(); + out_r.read_to_string(&mut out).unwrap(); + assert_eq!(out, "ping\n", "extra fd must survive: {result:?}"); +} + // ============================================================ // CLI kill / config input validation // ============================================================ From 30d029b02b3dc4a71df73ce9bc6ade251412a6d3 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 3 Sep 2026 22:50:32 -0700 Subject: [PATCH 16/16] control: track live control fds instead of scanning /proc in the child The forked child used to find inherited control sockets by walking /proc/self/fd and matching abstract names, and gave up silently when it could not read it. The exec path had CLOEXEC and close_range behind it, but the COW template never execs and never reaches close_range, so a supervisor without a readable /proc handed every sibling sandbox's listener, and any in-flight inspect connection, to user code. Now every control fd is a ControlFd that registers its number on a process-wide list and unregisters as it closes, and both fork sites fork under that list's lock, so the child closes exactly the live fds with no /proc and no fallback. Accepted connections are ControlFds too, served through AsyncFd rather than handed to tokio's socket types, so they are covered by the same sweep. Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 265 ++++++++++++++++++++++------ crates/sandlock-core/src/sandbox.rs | 8 +- 2 files changed, 214 insertions(+), 59 deletions(-) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index e12f05bc..b5c629b9 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -44,9 +44,13 @@ //! `ProfileInput`), `ports` (virtual to real port map). use std::os::linux::net::SocketAddrExt; -use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; +use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::os::unix::net::{SocketAddr, UnixListener, UnixStream}; -use std::sync::Arc; +use std::pin::Pin; +use std::sync::{Arc, Mutex, PoisonError}; +use std::task::{Context, Poll}; + +use tokio::io::unix::AsyncFd; use crate::sandbox::Sandbox; use crate::seccomp::ctx::SupervisorCtx; @@ -75,19 +79,113 @@ fn pgrp_socket_addr(name: &str) -> std::io::Result { SocketAddr::from_abstract_name(pgrp_socket_name(uid, name)) } +// ============================================================ +// Live control fds +// ============================================================ + +/// Every control fd this process holds, so a forked child can close them +/// without reading /proc. Bind, close, and fork() all take the lock, so a +/// child never sees an fd that is half registered. +static LIVE: Mutex> = Mutex::new(Vec::new()); + +fn live() -> std::sync::MutexGuard<'static, Vec> { + LIVE.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// A socket fd that stays on the live list until it closes. +#[derive(Debug)] +pub(crate) struct ControlFd(RawFd); + +impl ControlFd { + fn register(fd: OwnedFd) -> Self { + let fd = fd.into_raw_fd(); + live().push(fd); + ControlFd(fd) + } + + fn set_nonblocking(&self) -> std::io::Result<()> { + let flags = unsafe { libc::fcntl(self.0, libc::F_GETFL) }; + if flags < 0 || unsafe { libc::fcntl(self.0, libc::F_SETFL, flags | libc::O_NONBLOCK) } < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + } + + fn accept(&self) -> std::io::Result { + let fd = unsafe { + libc::accept4( + self.0, + std::ptr::null_mut(), + std::ptr::null_mut(), + libc::SOCK_CLOEXEC | libc::SOCK_NONBLOCK, + ) + }; + if fd < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(ControlFd::register(unsafe { OwnedFd::from_raw_fd(fd) })) + } + + fn read(&self, buf: &mut [u8]) -> std::io::Result { + let n = unsafe { libc::read(self.0, buf.as_mut_ptr() as *mut libc::c_void, buf.len()) }; + if n < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(n as usize) + } + + fn write(&self, buf: &[u8]) -> std::io::Result { + let n = unsafe { libc::write(self.0, buf.as_ptr() as *const libc::c_void, buf.len()) }; + if n < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(n as usize) + } +} + +impl AsRawFd for ControlFd { + fn as_raw_fd(&self) -> RawFd { + self.0 + } +} + +impl Drop for ControlFd { + fn drop(&mut self) { + let mut live = live(); + live.retain(|&fd| fd != self.0); + unsafe { libc::close(self.0) }; + } +} + +/// fork() with the live list locked. The child closes every control fd but +/// `keep` before anything else runs, so the sandbox never holds one; `keep` +/// is the child's own pgrp socket, which it still has to listen on. +pub(crate) fn fork_without_control_fds(keep: Option) -> libc::pid_t { + let live = live(); + let pid = unsafe { libc::fork() }; + if pid == 0 { + for &fd in live.iter() { + if Some(fd) != keep { + unsafe { libc::close(fd) }; + } + } + } + pid +} + /// Both sockets of one sandbox, bound before it forks. `control` already /// listens, from the supervisor. `pgrp` is bound only: the child calls /// listen() on it after setpgid(), so its peer pid is the group leader. #[derive(Debug)] pub(crate) struct ControlSockets { - pub control: UnixListener, - pub pgrp: OwnedFd, + pub control: ControlFd, + pub pgrp: ControlFd, } /// `AddrInUse` means a live sandbox of this uid already owns the name. pub(crate) fn bind_control_sockets(name: &str) -> std::io::Result { - let control = UnixListener::bind_addr(&socket_addr(name)?)?; - let pgrp = bind_only(&pgrp_socket_addr(name)?)?; + let control = ControlFd::register(UnixListener::bind_addr(&socket_addr(name)?)?.into()); + let pgrp = ControlFd::register(bind_only(&pgrp_socket_addr(name)?)?); Ok(ControlSockets { control, pgrp }) } @@ -123,37 +221,6 @@ pub(crate) fn publish_pgrp(fd: RawFd) { } } -/// First thing in a forked child. An abstract name stays bound while any -/// fd refers to it, and a child keeps its inherited fds until it execs (a -/// COW clone never does), so a parked child would pin every sibling's name. -/// `keep` is the child's own pgrp socket, which it still has to listen on. -pub(crate) fn close_inherited_control_sockets(keep: Option) { - let Ok(dir) = std::fs::read_dir("/proc/self/fd") else { return }; - for entry in dir.flatten() { - let fd = entry.file_name().to_str().and_then(|s| s.parse::().ok()); - if let Some(fd) = fd.filter(|&fd| Some(fd) != keep && is_control_socket(fd)) { - unsafe { libc::close(fd) }; - } - } -} - -fn is_control_socket(fd: i32) -> bool { - let mut addr: libc::sockaddr_un = unsafe { std::mem::zeroed() }; - let mut len = std::mem::size_of::() as libc::socklen_t; - let rc = unsafe { - libc::getsockname(fd, &mut addr as *mut _ as *mut libc::sockaddr, &mut len) - }; - if rc != 0 || addr.sun_family != libc::AF_UNIX as libc::sa_family_t { - return false; - } - let path_len = (len as usize).saturating_sub(std::mem::offset_of!(libc::sockaddr_un, sun_path)); - let path: Vec = addr.sun_path[..path_len.min(addr.sun_path.len())] - .iter() - .map(|&c| c as u8) - .collect(); - path.first() == Some(&0) && path[1..].starts_with(b"sandlock/") -} - // ============================================================ // Control loop, spawned as a dedicated tokio task // ============================================================ @@ -176,9 +243,9 @@ pub(crate) fn spawn_control_loop( // Mutex only to satisfy Sync: Sandbox carries a Box slot // even though this clone's is None. let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); - let pgrp = UnixListener::from(sockets.pgrp); tokio::spawn(async move { - control_loop(sockets.control, Some(pgrp), ctx, sandbox, info, unsafe { libc::getuid() }).await; + let ControlSockets { control, pgrp } = sockets; + control_loop(control, Some(pgrp), ctx, sandbox, info, unsafe { libc::getuid() }).await; }) } @@ -197,9 +264,64 @@ fn peer_cred(fd: RawFd) -> Option { (rc == 0).then_some(cred) } -fn into_tokio(listener: UnixListener) -> Option { - listener.set_nonblocking(true).ok()?; - tokio::net::UnixListener::from_std(listener).ok() +fn into_async(fd: ControlFd) -> Option> { + fd.set_nonblocking().ok()?; + AsyncFd::new(fd).ok() +} + +async fn accept(listener: &AsyncFd) -> std::io::Result { + loop { + let mut guard = listener.readable().await?; + match guard.try_io(|inner| inner.get_ref().accept()) { + Ok(result) => return result, + Err(_would_block) => continue, + } + } +} + +/// One accepted connection, driven through the reactor while its fd stays +/// on the live list. +struct ControlStream(AsyncFd); + +impl tokio::io::AsyncRead for ControlStream { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> Poll> { + loop { + let mut guard = std::task::ready!(self.0.poll_read_ready(cx))?; + let unfilled = buf.initialize_unfilled(); + match guard.try_io(|inner| inner.get_ref().read(unfilled)) { + Ok(Ok(n)) => { + buf.advance(n); + return Poll::Ready(Ok(())); + } + Ok(Err(e)) => return Poll::Ready(Err(e)), + Err(_would_block) => continue, + } + } + } +} + +impl tokio::io::AsyncWrite for ControlStream { + fn poll_write(self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &[u8]) -> Poll> { + loop { + let mut guard = std::task::ready!(self.0.poll_write_ready(cx))?; + match guard.try_io(|inner| inner.get_ref().write(buf)) { + Ok(result) => return Poll::Ready(result), + Err(_would_block) => continue, + } + } + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } } /// Accept one connection at a time and serve one request per connection. @@ -210,26 +332,26 @@ fn into_tokio(listener: UnixListener) -> Option { /// dropped to keep its backlog empty; a child that never called listen() /// makes accept() fail with EINVAL, after which the socket is left alone. async fn control_loop( - listener: UnixListener, - pgrp: Option, + listener: ControlFd, + pgrp: Option, ctx: Option>, sandbox: Arc>, info: SandboxInfo, my_uid: u32, ) { - let Some(listener) = into_tokio(listener) else { return }; - let mut pgrp = pgrp.and_then(into_tokio); + let Some(listener) = into_async(listener) else { return }; + let mut pgrp = pgrp.and_then(into_async); loop { let drain = async { match &pgrp { - Some(l) => l.accept().await, + Some(l) => accept(l).await, None => std::future::pending().await, } }; let stream = tokio::select! { - accepted = listener.accept() => match accepted { - Ok((stream, _)) => stream, + accepted = accept(&listener) => match accepted { + Ok(stream) => stream, Err(_) => return, }, drained = drain => { @@ -243,6 +365,7 @@ async fn control_loop( if peer_cred(stream.as_raw_fd()).map(|c| c.uid) != Some(my_uid) { continue; } + let Ok(stream) = AsyncFd::new(stream).map(ControlStream) else { continue }; let _ = tokio::time::timeout( std::time::Duration::from_secs(5), serve_one(stream, ctx.as_ref(), &sandbox, &info), @@ -275,7 +398,7 @@ pub struct ControlResponse { } async fn serve_one( - stream: tokio::net::UnixStream, + stream: ControlStream, ctx: Option<&Arc>, sandbox: &Arc>, info: &SandboxInfo, @@ -338,7 +461,7 @@ async fn serve_one( } } -async fn handle_info(stream: &mut tokio::net::UnixStream, info: &SandboxInfo) { +async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo) { let resp = match serde_json::to_value(info) { Ok(data) => ControlResponse { v: 1, ok: true, data: Some(data), err: None }, Err(e) => ControlResponse { @@ -352,7 +475,7 @@ async fn handle_info(stream: &mut tokio::net::UnixStream, info: &SandboxInfo) { } async fn handle_config( - stream: &mut tokio::net::UnixStream, + stream: &mut ControlStream, ctx: Option<&Arc>, sandbox: &Arc>, ) { @@ -391,7 +514,7 @@ async fn handle_config( } async fn handle_ports( - stream: &mut tokio::net::UnixStream, + stream: &mut ControlStream, ctx: Option<&Arc>, ) { let ports: std::collections::HashMap = match ctx { @@ -425,7 +548,7 @@ async fn handle_ports( /// Write a length-prefixed JSON response. Rejects bodies over 64 KB /// (mirrors the client-side cap in `send_control_request`). async fn write_response( - stream: &mut tokio::net::UnixStream, + stream: &mut ControlStream, resp: &ControlResponse, ) -> std::io::Result<()> { use tokio::io::AsyncWriteExt; @@ -670,6 +793,40 @@ mod tests { assert!(!list_sandboxes().unwrap().contains(&name)); } + /// The live list is exactly what a forked child closes, so it has to + /// follow every bind and every drop. + #[test] + fn live_list_follows_bind_and_drop() { + let name = format!("test-ctrl-live-{}", std::process::id()); + let sockets = bind_control_sockets(&name).unwrap(); + let (control, pgrp) = (sockets.control.as_raw_fd(), sockets.pgrp.as_raw_fd()); + assert!(live().contains(&control) && live().contains(&pgrp)); + drop(sockets); + assert!(!live().contains(&control) && !live().contains(&pgrp)); + } + + /// A forked child keeps only the pgrp socket it was told to, with no + /// help from /proc. + #[test] + fn forked_child_keeps_only_its_pgrp_socket() { + let pid = std::process::id(); + let mine = bind_control_sockets(&format!("test-ctrl-fork-mine-{pid}")).unwrap(); + let sibling = bind_control_sockets(&format!("test-ctrl-fork-sibling-{pid}")).unwrap(); + let keep = mine.pgrp.as_raw_fd(); + let closed = [mine.control.as_raw_fd(), sibling.control.as_raw_fd(), sibling.pgrp.as_raw_fd()]; + + let child = fork_without_control_fds(Some(keep)); + assert!(child >= 0, "fork: {}", std::io::Error::last_os_error()); + if child == 0 { + let is_open = |fd: RawFd| unsafe { libc::fcntl(fd, libc::F_GETFD) } >= 0; + let ok = is_open(keep) && closed.iter().all(|&fd| !is_open(fd)); + unsafe { libc::_exit(if ok { 0 } else { 1 }) }; + } + let mut status = 0; + assert_eq!(unsafe { libc::waitpid(child, &mut status, 0) }, child); + assert!(libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0, "child status {status:#x}"); + } + /// The pids come from the kernel's record of who called listen(), not /// from anything the sandbox says; nobody serves these sockets here. #[test] diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index f19984ed..1e5a7c48 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1422,14 +1422,13 @@ impl Sandbox { } } - let pid = unsafe { libc::fork() }; + let pid = crate::control::fork_without_control_fds(None); if pid < 0 { unsafe { libc::close(ctrl_child_fd) }; return Err(SandboxRuntimeError::Fork(std::io::Error::last_os_error()).into()); } if pid == 0 { - crate::control::close_inherited_control_sockets(None); drop(ctrl_parent); unsafe { libc::setpgid(0, 0) }; unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) }; @@ -1870,7 +1869,7 @@ impl Sandbox { let tty_foreground_taken = foreground && unsafe { libc::isatty(0) } == 1; // Bound before the fork so a name collision fails with no child to - // reap. Both fds are CLOEXEC; the child closes its copies itself. + // reap. The child sheds its copies as it forks. let sandbox_name = self.rt().name.clone(); let mut control_sockets = match crate::control::bind_control_sockets(&sandbox_name) { Ok(s) => Some(s), @@ -1894,14 +1893,13 @@ impl Sandbox { }; let pgrp_socket = control_sockets.as_ref().map(|s| s.pgrp.as_raw_fd()); - let pid = unsafe { libc::fork() }; + let pid = crate::control::fork_without_control_fds(pgrp_socket); if pid < 0 { return Err(SandboxRuntimeError::Fork(std::io::Error::last_os_error()).into()); } if pid == 0 { // ===== CHILD PROCESS ===== - crate::control::close_inherited_control_sockets(pgrp_socket); // killpg() needs the group to exist before anyone can connect, // and the dup2 loops below have fixed targets that can be this // socket's own fd number, so both come first.