diff --git a/docs/backends/seatbelt/seatbelt-backend.md b/docs/backends/seatbelt/seatbelt-backend.md index 718681674..c061f8eb2 100644 --- a/docs/backends/seatbelt/seatbelt-backend.md +++ b/docs/backends/seatbelt/seatbelt-backend.md @@ -171,6 +171,9 @@ The `/dev/*` entries are writable because shell redirections (`>/dev/null`, `, +) -> Result { + build_profile_with_terminals(request, proxy_address, &[]) +} + +/// Bind terminal paths discovered by the runner, never supplied by the workload. +pub(crate) fn build_profile_with_terminals( + request: &ExecutionRequest, + proxy_address: Option<&ProxyAddress>, + terminal_paths: &[PathBuf], ) -> Result { if let Some(override_profile) = request .seatbelt @@ -69,12 +78,18 @@ pub fn build_profile_with_proxy( out.push_str(SYSTEM_READ_ALLOW); write_developer_dir_rule(&mut out); - // Pseudo-terminal access — when the executor binary runs under a pty - // the sandboxed shell inherits that TTY, so it sees a real terminal - // and calls `isatty()` / `tcgetattr()` / `ttyname()` against it. - // Without these rules, those calls fail with EPERM because the - // kernel calls block on the secondary fd. + // Keep the controlling terminal usable without granting other host PTYs. out.push_str(TTY_ALLOW); + for path in terminal_paths { + let path = path + .to_str() + .ok_or_else(|| "Seatbelt: assigned terminal path is not valid UTF-8".to_string())?; + let _ = writeln!( + out, + "(allow file-read* file-write* file-ioctl (literal {}))", + quote_scheme(path) + ); + } // Policy-derived allow rules. let resolved = ResolvedPaths::from_policy(&request.policy)?; @@ -250,18 +265,14 @@ fn push_developer_dir_rule(out: &mut String, dir: &Path) { let _ = writeln!(out, "(allow file-read* (subpath {}))", quote_scheme(path)); } -/// Pseudo-terminal device access required by the inner shell when the -/// runner attaches it to a pty. The secondary fd we hand the child as -/// stdin/stdout/stderr lives at `/dev/ttysNNN`, and the shell calls -/// `isatty()` (→ `tcgetattr` → ioctl) plus `ttyname()` against it. We -/// also need read access to `/dev/tty` because most shells re-open it -/// at startup, and read access to `/dev/fd` for the `/dev/stdout` etc. -/// indirection some tools use. +/// Controlling-terminal access and kernel-scoped access to newly allocated PTYs. const TTY_ALLOW: &str = "\ -;; --- pseudo-terminal access (inherited TTY when run under a pty) --- +;; --- controlling terminal and sandbox-allocated pseudo-terminals --- +(allow file-read* file-write* file-ioctl (literal \"/dev/tty\")) (allow file-read* file-write* file-ioctl - (literal \"/dev/tty\") - (regex #\"^/dev/ttys[0-9]+$\")) + (require-all + (regex #\"^/dev/ttys[0-9]+$\") + (extension \"com.apple.sandbox.pty\"))) (allow file-read* (subpath \"/dev/fd\")) "; @@ -560,8 +571,7 @@ fn write_ui_rules(out: &mut String, request: &ExecutionRequest) { out.push_str(";; --- guiAccess: allow pseudo-TTY for shell sessions ---\n"); out.push_str("(allow pseudo-tty)\n"); out.push_str("(allow file-read* file-write* file-ioctl\n"); - out.push_str(" (regex #\"/dev/ttys[0-9]+\")\n"); - out.push_str(" (regex #\"/dev/ptmx\"))\n"); + out.push_str(" (literal \"/dev/ptmx\"))\n"); // POSIX shared memory and IPC — required by Terminal.app and // other apps that use notification center or shared memory. @@ -606,9 +616,7 @@ fn write_nested_pty_rules(out: &mut String, request: &ExecutionRequest) { } out.push_str(";; --- nestedPty: allow inner process to allocate its own pty ---\n"); out.push_str("(allow pseudo-tty)\n"); - // /dev/ptmx is the primary multiplexer; opening it is what posix_openpt - // does under the hood. The TTY_ALLOW baseline already grants access to - // /dev/ttysNNN (the secondary side). + // TTY_ALLOW restricts secondary-device access to the kernel's PTY extension. out.push_str("(allow file-read* file-write* file-ioctl\n"); out.push_str(" (literal \"/dev/ptmx\"))\n"); } diff --git a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs index 3db2e8b93..e53f46af1 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs @@ -18,6 +18,7 @@ use std::ffi::{CStr, CString}; use std::fmt::Write as FmtWrite; +use std::io::IsTerminal; use std::os::fd::AsFd; use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; @@ -40,7 +41,9 @@ use crate::mxc_pty::{LivePty, PtySize as UnixPtySize}; #[cfg(test)] use crate::seatbelt_common::default_env::DEFAULT_SANDBOX_PATH; use crate::seatbelt_common::default_env::{env_pairs, resolved_env}; +#[cfg(test)] use crate::seatbelt_common::profile_builder::build_profile_with_proxy; +use crate::seatbelt_common::profile_builder::build_profile_with_terminals; #[link(name = "proc")] unsafe extern "C" { @@ -308,15 +311,12 @@ impl SandboxBackend for SeatbeltScriptRunner { crate::mxc_common::proxy_env::redact_proxy_url(&address.to_url()) )); } - let profile = build_profile_with_proxy(request, proxy_address).map_err(error_response)?; - log_generated_profile(&profile, logger); - // Effective posture, not the raw flag: a `guiAccess` the UI policy // suppressed emits no GUI rules, so it must not cost the caller // streaming either. let gui_access = crate::seatbelt_common::seatbelt_policy::gui_access_effective(request); - spawn_exec(&profile, request, gui_access, stdio, logger, proxy_address) + spawn_exec(request, gui_access, stdio, logger, proxy_address) } } @@ -328,7 +328,6 @@ impl SandboxBackend for SeatbeltScriptRunner { /// it gets a caller-owned terminal and leads its own session. `gui_access` /// apps require inherited stdio. fn spawn_exec( - profile: &str, request: &ExecutionRequest, gui_access: bool, stdio: StdioMode, @@ -358,8 +357,8 @@ fn spawn_exec( // non-interactive. let new_group = stdio == StdioMode::Inherit && timeout_from(request).is_some(); let (mut command, pty) = build_sandbox_command( - profile, - &request.script_code, + request, + proxy_address, new_session, new_group, match stdio { @@ -781,20 +780,15 @@ impl Drop for SeatbeltSandboxProcess { /// `sandbox_init` is not formally async-signal-safe but is used in this /// pattern by Chromium and other production macOS sandboxes. fn build_sandbox_command( - profile: &str, - script_code: &str, + request: &ExecutionRequest, + proxy_address: Option<&ProxyAddress>, new_session: bool, new_group: bool, pty_size: Option, logger: &mut Logger, ) -> Result<(Command, Option), ScriptResponse> { - let profile_cstr = CString::new(profile) - .map_err(|e| error_response(format!("seatbelt profile contains embedded NUL byte: {e}")))?; - - let _ = writeln!(logger, "Seatbelt: applying sandbox via sandbox_init"); - let mut command = Command::new(DEFAULT_SHELL); - command.arg("-c").arg(script_code); + command.arg("-c").arg(&request.script_code); // When requested (streaming path), put the child in its own session / // process group via `setsid()` so a caller can tree-kill it with a single @@ -844,6 +838,22 @@ fn build_sandbox_command( }) .transpose()?; + let terminal_paths = match &pty { + Some(pty) => vec![pty.secondary_path().to_path_buf()], + None if !new_session => inherited_terminal_paths().map_err(|error| { + error_response(format!( + "Seatbelt: failed to resolve inherited terminals: {error}" + )) + })?, + None => Vec::new(), + }; + let profile = build_profile_with_terminals(request, proxy_address, &terminal_paths) + .map_err(error_response)?; + log_generated_profile(&profile, logger); + let profile_cstr = CString::new(profile) + .map_err(|e| error_response(format!("seatbelt profile contains embedded NUL byte: {e}")))?; + let _ = writeln!(logger, "Seatbelt: applying sandbox via sandbox_init"); + // SAFETY: The closure runs after fork(), before exec(). We only call // sandbox_init with a pre-allocated CString — no Rust allocations // happen inside the closure. sandbox_init is used in this fork+exec @@ -873,6 +883,20 @@ fn build_sandbox_command( Ok((command, pty)) } +fn inherited_terminal_paths() -> std::io::Result> { + let (stdin, stdout, stderr) = (std::io::stdin(), std::io::stdout(), std::io::stderr()); + let mut paths = Vec::new(); + for fd in [stdin.as_fd(), stdout.as_fd(), stderr.as_fd()] { + if fd.is_terminal() { + let path = nix::unistd::ttyname(fd).map_err(std::io::Error::from)?; + if !paths.contains(&path) { + paths.push(path); + } + } + } + Ok(paths) +} + /// Emit the generated profile to `logger` for `--debug` / `--log-file`. /// /// One `writeln!` per line, unprefixed: the file sink stamps each write, and @@ -1025,11 +1049,167 @@ mod tests { request } + fn terminal_probe( + request: &ExecutionRequest, + code: &str, + args: &[&str], + with_pty: bool, + ) -> std::process::Output { + let mut request = request.clone(); + request.script_code = r#"exec /usr/bin/python3 -I -c "$@""#.to_string(); + let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); + let (mut command, _pty) = build_sandbox_command( + &request, + None, + !with_pty, + false, + with_pty.then_some(PtySize::default()), + &mut logger, + ) + .expect("build probe command"); + command + .arg("terminal-probe") + .arg(code) + .args(args) + .current_dir("/") + .env_clear() + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if !with_pty { + command.stdin(Stdio::null()); + } + let mut child = command.spawn().expect("spawn terminal probe"); + if let Err(error) = wait_with_timeout(&mut child, Some(Duration::from_secs(20))) { + let _ = child.kill(); + let _ = child.wait(); + match error { + WaitError::Timeout => panic!("terminal probe timed out"), + WaitError::Io(error) => panic!("terminal probe wait failed: {error}"), + } + } + child.wait_with_output().expect("collect terminal probe") + } + + fn assert_unrelated_terminal_denied(nested_pty: bool, gui_access: bool, with_pty: bool) { + use std::os::fd::AsRawFd; + + use nix::fcntl::{fcntl, FcntlArg, FdFlag}; + + let terminal = nix::pty::openpty(None, None).expect("allocate disposable host PTY"); + for fd in [&terminal.master, &terminal.slave] { + fcntl(fd.as_raw_fd(), FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC)) + .expect("do not inherit the host PTY"); + } + let path = nix::unistd::ttyname(&terminal.slave).expect("host PTY path"); + for (read, write) in [(true, false), (false, true), (true, true)] { + std::fs::OpenOptions::new() + .read(read) + .write(write) + .open(&path) + .expect("host can open the disposable terminal"); + } + let mut request = base_request(); + request.seatbelt = Some(SeatbeltConfig { + nested_pty, + gui_access, + ..Default::default() + }); + request.policy.ui.disable = !gui_access; + let output = terminal_probe( + &request, + r#" +import errno, fcntl, os, sys, termios +for mode in (os.O_RDONLY, os.O_WRONLY, os.O_RDWR): + try: + fd = os.open(sys.argv[1], mode | os.O_NOCTTY | os.O_NONBLOCK) + except OSError as error: + assert error.errno in (errno.EPERM, errno.EACCES), error + else: + os.close(fd) + raise AssertionError("unrelated terminal opened") +if sys.argv[2] == "allow": + master, slave = os.openpty() + try: + assert os.isatty(slave) + termios.tcgetattr(slave) + fcntl.ioctl(slave, termios.TIOCGETD, bytes(4)) + finally: + os.close(slave) + os.close(master) +else: + try: + master, slave = os.openpty() + except OSError as error: + assert error.errno in (errno.EPERM, errno.EACCES), error + else: + os.close(slave) + os.close(master) + raise AssertionError("nested PTY allocation allowed") +"#, + &[ + path.to_str().expect("UTF-8 terminal path"), + if nested_pty || gui_access { + "allow" + } else { + "deny" + }, + ], + with_pty, + ); + assert!( + output.status.success(), + "terminal isolation probe failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + #[test] + fn nested_pty_cannot_open_unrelated_host_terminals() { + assert_unrelated_terminal_denied(true, false, false); + } + + #[test] + fn nested_pty_disabled_cannot_open_unrelated_host_terminals() { + assert_unrelated_terminal_denied(false, false, false); + } + + #[test] + fn gui_access_cannot_open_unrelated_host_terminals() { + assert_unrelated_terminal_denied(false, true, false); + } + + #[test] + fn caller_owned_pty_cannot_open_unrelated_host_terminals() { + assert_unrelated_terminal_denied(true, false, true); + } + + #[test] + fn preopened_terminal_keeps_stdio_and_controlling_tty() { + let output = terminal_probe( + &base_request(), + r#" +import fcntl, os, termios +assert os.isatty(0) +termios.tcgetattr(0) +fcntl.ioctl(0, termios.TIOCGETD, bytes(4)) +fd = os.open("/dev/tty", os.O_RDWR | os.O_NOCTTY) +termios.tcgetattr(fd) +os.close(fd) +"#, + &[], + true, + ); + assert!( + output.status.success(), + "assigned terminal probe failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + #[test] fn gui_access_rejects_caller_owned_pty() { let mut logger = Logger::new(crate::mxc_common::logger::Mode::Buffer); let error = spawn_exec( - "(version 1)\n(allow default)", &base_request(), true, StdioMode::Pty(PtySize::default()), diff --git a/src/mxc-sdk/src/core/mxc_pty/mod.rs b/src/mxc-sdk/src/core/mxc_pty/mod.rs index 29d7e536f..bf38cf31c 100644 --- a/src/mxc-sdk/src/core/mxc_pty/mod.rs +++ b/src/mxc-sdk/src/core/mxc_pty/mod.rs @@ -110,6 +110,8 @@ pub struct PtySize { #[cfg(any(target_os = "linux", target_os = "macos"))] pub struct LivePty { primary: std::fs::File, + #[cfg(target_os = "macos")] + secondary_path: std::path::PathBuf, access: Mutex, size: Mutex, eof: PtyEof, @@ -357,6 +359,8 @@ impl LivePty { ws_ypixel: size.pixel_height, }); let (master, slave) = open_pty(winsize.as_ref())?; + #[cfg(target_os = "macos")] + let secondary_path = nix::unistd::ttyname(&slave).map_err(std::io::Error::from)?; let eof = if forward_eof { PtyEof::Forwarded(terminal_discipline(&slave)?) } else { @@ -393,6 +397,8 @@ impl LivePty { let writer = primary.try_clone()?; Ok(Self { primary, + #[cfg(target_os = "macos")] + secondary_path, access: Mutex::new(PtyAccess { writer: Some(writer), reader_claimed: false, @@ -403,6 +409,11 @@ impl LivePty { }) } + #[cfg(target_os = "macos")] + pub(crate) fn secondary_path(&self) -> &std::path::Path { + &self.secondary_path + } + /// Clone the merged terminal output reader. pub fn try_clone_reader(&self) -> std::io::Result> { self.try_clone_reader_with_canceller() diff --git a/tests/scripts/run_seatbelt_all_tests.sh b/tests/scripts/run_seatbelt_all_tests.sh index 955cf169b..5019e0799 100755 --- a/tests/scripts/run_seatbelt_all_tests.sh +++ b/tests/scripts/run_seatbelt_all_tests.sh @@ -79,6 +79,7 @@ run_test "Seatbelt Environment" "$SCRIPT_DIR/run_seatbelt_environment_test.sh" run_test "Seatbelt UI" "$SCRIPT_DIR/run_seatbelt_ui_test.sh" run_test "Seatbelt guiAccess" "$SCRIPT_DIR/run_seatbelt_gui_access_test.sh" run_test "Seatbelt Options" "$SCRIPT_DIR/run_seatbelt_options_test.sh" +run_test "Seatbelt Terminal Isolation" "$SCRIPT_DIR/run_seatbelt_tty_test.sh" run_test "Seatbelt Profile Output" "$SCRIPT_DIR/run_seatbelt_profile_test.sh" run_test "Seatbelt Examples" "$SCRIPT_DIR/run_seatbelt_examples_test.sh" diff --git a/tests/scripts/run_seatbelt_tty_test.sh b/tests/scripts/run_seatbelt_tty_test.sh new file mode 100755 index 000000000..ab99ec794 --- /dev/null +++ b/tests/scripts/run_seatbelt_tty_test.sh @@ -0,0 +1,139 @@ +#!/bin/bash +# Only disposable PTYs are used; existing host terminal sessions are untouched. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/seatbelt_common.sh +. "$SCRIPT_DIR/lib/seatbelt_common.sh" + +[ "$(id -u)" -ne 0 ] || fail "run the terminal-isolation test as an unprivileged user" +require_python3_probe + +OUT=$(/usr/bin/python3 - "$MXC_EXEC_MAC" "$DEVDIR" <<'PY' +import base64 +import contextlib +import json +import os +import re +import shlex +import subprocess +import sys + +binary, developer_dir = sys.argv[1:] + +probe = r""" +import errno, fcntl, os, sys, termios + +unrelated_path, access, nested, assigned_path = sys.argv[1:] +for mode in (os.O_RDONLY, os.O_WRONLY, os.O_RDWR): + try: + fd = os.open(unrelated_path, mode | os.O_NOCTTY | os.O_NONBLOCK) + except OSError as error: + if access != "deny" or error.errno not in (errno.EPERM, errno.EACCES): + raise + else: + os.close(fd) + if access == "deny": + raise AssertionError("unrelated host terminal opened") + +if assigned_path: + assert os.isatty(0), "assigned stdin is not a terminal" + termios.tcgetattr(0) + fcntl.ioctl(0, termios.TIOCGETD, bytes(4)) + fd = os.open(assigned_path, os.O_RDWR | os.O_NOCTTY | os.O_NONBLOCK) + try: + fcntl.ioctl(fd, termios.TIOCGETD, bytes(4)) + finally: + os.close(fd) +else: + assert not os.isatty(0), "unexpected inherited terminal" + +try: + master, slave = os.openpty() +except OSError as error: + if nested != "deny" or error.errno not in (errno.EPERM, errno.EACCES): + raise +else: + try: + if nested == "deny": + raise AssertionError("nested PTY allocation was not blocked") + termios.tcgetattr(slave) + fcntl.ioctl(slave, termios.TIOCGETD, bytes(4)) + finally: + os.close(slave) + os.close(master) + +print("TTY_PROBE_OK") +""" + + +def allocate_terminal(cleanup): + master, slave = os.openpty() + cleanup.callback(os.close, master) + cleanup.callback(os.close, slave) + return slave, os.ttyname(slave) + + +with contextlib.ExitStack() as cleanup: + _, unrelated_path = allocate_terminal(cleanup) + assigned_slave, assigned_path = allocate_terminal(cleanup) + if not re.fullmatch(r"/dev/ttys[0-9]+", unrelated_path): + raise AssertionError("the disposable terminal does not match the PTY rule") + if os.stat(unrelated_path).st_uid != os.geteuid(): + raise AssertionError("the unrelated terminal must belong to the test user") + for mode in (os.O_RDONLY, os.O_WRONLY, os.O_RDWR): + fd = os.open(unrelated_path, mode | os.O_NOCTTY | os.O_NONBLOCK) + os.close(fd) + + cases = [ + ("explicit filesystem grant", None, "allow", "allow"), + ("default policy", None, "deny", "allow"), + ("nestedPty=false", {"nestedPty": False}, "deny", "deny"), + ("guiAccess", {"guiAccess": True, "nestedPty": False}, "deny", "allow"), + ] + for inherited in (False, True): + for label, options, access, nested in cases: + label += " / " + ("inherited terminal" if inherited else "no terminal") + command = [ + "/usr/bin/python3", "-I", "-c", probe, + unrelated_path, access, nested, assigned_path if inherited else "", + ] + request = { + "version": "1.0.0", + "containment": "seatbelt", + "process": { + "commandLine": "exec " + " ".join(shlex.quote(arg) for arg in command), + "timeout": 20000, + }, + "filesystem": { + "readonlyPaths": [developer_dir], + "readwritePaths": [unrelated_path] if access == "allow" else [], + }, + } + if options is not None: + request["seatbelt"] = options + if options and options.get("guiAccess"): + request["ui"] = {"disable": False} + encoded = base64.b64encode(json.dumps(request).encode()).decode() + result = subprocess.run( + [binary, "--config-base64", encoded], + stdin=assigned_slave if inherited else subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + close_fds=True, + start_new_session=True, + timeout=30, + ) + if result.returncode != 0 or "TTY_PROBE_OK" not in result.stdout.splitlines(): + raise AssertionError( + f"{label} failed (exit {result.returncode})\n" + f"{result.stdout}\n{result.stderr}" + ) + print(f"Verified: {label}", flush=True) +PY +) || fail "terminal-isolation probe failed" "$OUT" + +printf '%s\n' "$OUT" +pass "unrelated same-user PTYs are denied while assigned and permitted nested PTYs work" +summary "Seatbelt terminal isolation"