diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6d892c9..d621129 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,7 +12,7 @@ You need: ```bash sudo apt install xvfb openbox xdotool xauth x11-utils imagemagick xclip \ - bubblewrap pkg-config libxkbcommon-x11-dev + bubblewrap uidmap pkg-config libxkbcommon-x11-dev ``` ## Build diff --git a/README.md b/README.md index fbd4286..df8ece7 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ Requires Linux. Install the runtime dependencies, then build and install in one ```bash sudo apt install xvfb openbox xdotool xauth x11-utils imagemagick xclip \ - bubblewrap pkg-config libxkbcommon-x11-dev + bubblewrap uidmap pkg-config libxkbcommon-x11-dev ./install.sh ``` @@ -325,6 +325,7 @@ The MCP exposes ~86 tools. To avoid dumping them all into the agent's context, i - **Pre-1.0.** Interfaces and tool schemas can change between versions. - **Single-user trust model.** The control socket is a same-uid Unix socket (mode 0600); there is no cross-user protection by design. Run as a dedicated user for multi-user isolation. - **Mount/network enforcement needs bubblewrap.** Without it, those policies are declared but not enforced (the runtime tells you which). +- **Sandboxed launches drop your supplementary groups where the host allows it.** Groups such as `docker` or `lxd` are root-equivalent. Dropping them needs `uidmap` (`newuidmap`/`newgidmap`), a subordinate id range for your user in `/etc/subuid` and `/etc/subgid` (Ubuntu's `adduser` creates one), util-linux 2.40 or newer, user namespaces that `unshare` may use, and a host without `no_new_privs`. Where any of this is missing, sandboxed apps keep your groups as before; `doctor` reports why under `sandbox_group_drop`. When dropping applies, every supplementary group goes, including groups such as `video`, `audio` or a shared project group, so files a sandboxed app could reach only through such a group become unreadable to it. Ubuntu 23.10 and newer restrict unprivileged user namespaces with AppArmor (`kernel.apparmor_restrict_unprivileged_userns=1`). To allow it, either give `/usr/bin/unshare` an AppArmor profile with `userns,` (like Ubuntu's own `lxc-usernsexec` profile; this lifts the restriction for every program that runs `unshare`), or set the sysctl to 0. Alternatively, remove the root-equivalent groups from your user. - **Live viewer control is not the hard permission boundary** — the permission ceiling is authoritative, while unreadable live-control state fails closed for mutating daemon IPC. ## Docs diff --git a/scripts/runtime_isolation_smoke.py b/scripts/runtime_isolation_smoke.py index a9ef160..32b1c84 100644 --- a/scripts/runtime_isolation_smoke.py +++ b/scripts/runtime_isolation_smoke.py @@ -28,6 +28,9 @@ def cmd(*args): assert v.get("ok",True),v return v +# Hosts that cannot drop supplementary groups keep them, as before. +doctor=json.loads(subprocess.run([BIN,"doctor"],env=ENV,capture_output=True,text=True,timeout=60).stdout) +drops_groups="1" if "drop supplementary groups via" in doctor["runtime"]["sandbox_group_drop"]["detail"] else "0" mcp=Mcp(BIN,ROOT/"mcp.log",env=ENV,headless=True) try: for mounted in (False,True): @@ -45,7 +48,7 @@ def cmd(*args): probe="""import os,sys,json,socket from pathlib import Path runtime=Path(sys.argv[1]); hostpid=sys.argv[2]; own_x_socket='X'+sys.argv[3].split(':',1)[1].split('.')[0] -paths=[runtime/'control.sock',runtime/'session-bus',runtime/'i3.sock',Path('/proc')/hostpid/'root'/str(runtime).lstrip('/')/'control.sock',*map(Path,sys.argv[4:])] +paths=[runtime/'control.sock',runtime/'session-bus',runtime/'i3.sock',Path('/proc')/hostpid/'root'/str(runtime).lstrip('/')/'control.sock',*map(Path,sys.argv[5:])] ibus=Path.home()/'.cache'/'ibus' paths+=[p for p in ibus.glob('dbus-*')] if ibus.is_dir() else [] reachable=[] @@ -56,14 +59,15 @@ def cmd(*args): finally: s.close() foreign_x=[p.name for p in Path('/tmp/.X11-unix').iterdir() if p.name!=own_x_socket] host_bus=any(p.exists() for p in [Path('/run/dbus/system_bus_socket'),Path('/run/user')/str(os.getuid())/'bus']) -print(json.dumps({'reachable':reachable,'pid':os.getpid(),'host_proc':Path('/proc',hostpid).exists(),'foreign_x':foreign_x,'host_bus':host_bus})) -raise SystemExit(42 if reachable or Path('/proc',hostpid).exists() or foreign_x or host_bus else 0) +groups=[g for g in os.getgroups() if g!=os.getgid()] if sys.argv[4]=='1' else [] +print(json.dumps({'reachable':reachable,'pid':os.getpid(),'host_proc':Path('/proc',hostpid).exists(),'foreign_x':foreign_x,'host_bus':host_bus,'groups':groups})) +raise SystemExit(42 if reachable or Path('/proc',hostpid).exists() or foreign_x or host_bus or groups else 0) """ terminal=mcp.tool("workspace_run_in_terminal",{"id":id,"terminal_id":"private","command":["sh","-c","printf PRIVATE_TERMINAL_OK; exec tail -f /dev/null"],"wait_window":True,"timeout_ms":10000}) assert terminal["ok"],terminal # Another sandboxed app must not reach the terminal's tmux server, a # sibling workspace's control socket, or the host input-method bus. - run=cmd("workspace","run","--id",id,"--timeout-ms",10000,"--","python3","-c",probe,status["runtime_dir"],status["daemon_pid"],status["display"], + run=cmd("workspace","run","--id",id,"--timeout-ms",10000,"--","python3","-c",probe,status["runtime_dir"],status["daemon_pid"],status["display"],drops_groups, terminal["terminal"]["tmux_socket_path"],Path(sibling["runtime_dir"])/"control.sock") assert run["exit_code"]==0,run screen=mcp.tool("workspace_terminal_read",{"id":id,"terminal_id":"private"}) diff --git a/src/launch.rs b/src/launch.rs index f8fab19..5c2dc97 100644 --- a/src/launch.rs +++ b/src/launch.rs @@ -586,6 +586,277 @@ pub(crate) fn resolve_command_path(program: &str) -> Option { .find(|candidate| is_executable_file(candidate)) } +/// Unprivileged bubblewrap keeps the caller's supplementary groups, and groups +/// such as docker or lxd are root-equivalent. When the owner has any, a +/// sandboxed launch starts through `unshare` with a user namespace whose maps +/// come from the setuid newuidmap/newgidmap (so setgroups stays allowed), +/// drops every supplementary group and execs bubblewrap as the same uid with +/// no capabilities. Inner root maps to one subordinate id nothing runs as. +/// Best effort: when the groups cannot be dropped on this host, launches +/// behave as before and keep them; `doctor` reports why. +pub(crate) fn sandbox_launcher_prefix() -> Vec { + cached_group_drop().clone().unwrap_or_default() +} + +/// Whether sandboxed launches drop the owner's supplementary groups here, and +/// why not when they cannot. Computed once per process: a daemon started +/// before the host was fixed keeps its result until it restarts. +pub(crate) fn group_drop_status() -> std::result::Result { + cached_group_drop() + .as_ref() + .map(|prefix| !prefix.is_empty()) + .map_err(Clone::clone) +} + +fn cached_group_drop() -> &'static std::result::Result, String> { + static PREFIX: std::sync::OnceLock, String>> = + std::sync::OnceLock::new(); + PREFIX.get_or_init(|| group_drop_prefix().map_err(|error| format!("{error:#}"))) +} + +fn group_drop_prefix() -> Result> { + let (uid, gid) = unsafe { (libc::geteuid(), libc::getegid()) }; + if supplementary_groups()?.iter().all(|group| *group == gid) { + return Ok(Vec::new()); + } + let missing = |what: &str| { + anyhow::anyhow!( + "supplementary groups are kept in sandboxed launches ({what}); \ + install uidmap and give your user a subordinate range, e.g. \ + `sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 $USER`" + ) + }; + for tool in ["newuidmap", "newgidmap"] { + resolve_executable_path(tool).ok_or_else(|| missing(&format!("{tool} not found")))?; + } + let unshare = resolve_executable_path("unshare").ok_or_else(|| missing("unshare not found"))?; + // Repeated --map-users/--map-groups ranges arrived in util-linux 2.40. + let version = Command::new(&unshare) + .arg("--version") + .env_clear() + .output() + .ok() + .and_then(|output| String::from_utf8(output.stdout).ok()) + .and_then(|text| util_linux_version(&text)); + if version.is_none_or(|version| version < (2, 40)) { + return Err(missing("util-linux 2.40 or newer is required for unshare")); + } + let name = user_name(uid); + let sub_uid = subordinate_start(Path::new("/etc/subuid"), uid, name.as_deref()) + .ok_or_else(|| missing("no /etc/subuid range"))?; + let sub_gid = subordinate_start(Path::new("/etc/subgid"), uid, name.as_deref()) + .ok_or_else(|| missing("no /etc/subgid range"))?; + let prefix = vec![ + unshare.display().to_string(), + "--user".to_string(), + format!("--map-users={uid}:{uid}:1"), + format!("--map-users=0:{sub_uid}:1"), + format!("--map-groups={gid}:{gid}:1"), + format!("--map-groups=0:{sub_gid}:1"), + "--setuid".to_string(), + uid.to_string(), + "--setgid".to_string(), + gid.to_string(), + "--".to_string(), + ]; + group_drop_preflight(&prefix)?; + Ok(prefix) +} + +fn util_linux_version(text: &str) -> Option<(u32, u32)> { + let version = text.split_whitespace().last()?; + let mut parts = version.split('.'); + Some((parts.next()?.parse().ok()?, parts.next()?.parse().ok()?)) +} + +/// Runs the real chain once (the wrapper, then bubblewrap inside it), so a +/// host that refuses either (user namespaces restricted by AppArmor, +/// newuidmap denied, no_new_privs) keeps the previous behavior instead of +/// every app exiting right after spawn. +fn group_drop_preflight(prefix: &[String]) -> Result<()> { + let bwrap = resolve_executable_path("bwrap").context("bwrap not found")?; + let truth = resolve_executable_path("true").context("true not found")?; + let output = Command::new(&prefix[0]) + .args(&prefix[1..]) + .arg(bwrap) + .args([ + "--ro-bind", + "/", + "/", + "--dev", + "/dev", + "--unshare-pid", + "--proc", + "/proc", + "--unshare-net", + "--", + ]) + .arg(truth) + .env_clear() + .output() + .context("could not run unshare")?; + if output.status.success() { + return Ok(()); + } + let restricted = fs::read_to_string("/proc/sys/kernel/apparmor_restrict_unprivileged_userns") + .is_ok_and(|value| value.trim() == "1"); + bail!( + "supplementary groups are kept in sandboxed launches: dropping them failed ({}){}", + String::from_utf8_lossy(&output.stderr).trim(), + if restricted { + ". AppArmor restricts unprivileged user namespaces here \ + (kernel.apparmor_restrict_unprivileged_userns=1); see the README section \ + on supplementary groups" + } else { + "" + } + ) +} + +/// A sandboxed launch command and the private descriptor carrying its +/// arguments. Keep `args_fd` alive until the command is spawned. +pub(crate) struct SandboxedCommand { + pub(crate) command: Command, + pub(crate) args_fd: std::os::fd::OwnedFd, +} + +/// Builds a sandboxed launch. The launcher and bubblewrap run from absolute +/// paths with an empty environment; the app's environment and argv reach +/// bubblewrap through `--args` on a close-on-exec memfd that only this child +/// inherits. Launch variables such as LD_PRELOAD or PATH therefore never +/// apply to the launcher, and stay out of the world-readable command line. +pub(crate) fn sandboxed_command( + sandbox_args: &[String], + program: &str, + program_args: &[String], + env: &[(std::ffi::OsString, std::ffi::OsString)], +) -> Result { + use std::os::fd::AsRawFd; + use std::os::unix::process::CommandExt; + let bwrap = resolve_executable_path("bwrap").context("bwrap not found")?; + let args_fd = bwrap_args_memfd(sandbox_args, env)?; + let prefix = sandbox_launcher_prefix(); + let mut command = match prefix.split_first() { + Some((launcher, args)) => { + let mut command = Command::new(launcher); + command.args(args).arg(&bwrap); + command + } + None => Command::new(&bwrap), + }; + let fd = args_fd.as_raw_fd(); + // bubblewrap reads only options from --args; the program stays on the + // command line after `--`. + command + .arg("--args") + .arg(fd.to_string()) + .arg("--") + .arg(program) + .args(program_args) + .env_clear(); + unsafe { + command.pre_exec(move || { + if libc::fcntl(fd, libc::F_SETFD, 0) != 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + Ok(SandboxedCommand { command, args_fd }) +} + +/// The environment (`--setenv`) and the sandbox options, as NUL-separated +/// bubblewrap arguments in a close-on-exec memfd. bubblewrap itself starts +/// with an empty environment, so it passes only these variables on. +fn bwrap_args_memfd( + sandbox_args: &[String], + env: &[(std::ffi::OsString, std::ffi::OsString)], +) -> Result { + use std::os::fd::FromRawFd; + use std::os::unix::ffi::OsStrExt; + // Arguments are NUL-separated: a NUL inside any value would split it + // into extra bubblewrap options. + let mut bytes = Vec::new(); + let mut push = |value: &[u8]| -> Result<()> { + if value.contains(&0) { + bail!("sandbox argument or environment value contains a NUL byte"); + } + bytes.extend_from_slice(value); + bytes.push(0); + Ok(()) + }; + for (name, value) in env { + push(b"--setenv")?; + push(name.as_bytes())?; + push(value.as_bytes())?; + } + for arg in sandbox_args { + push(arg.as_bytes())?; + } + let raw = unsafe { libc::memfd_create(c"bwrap-args".as_ptr(), libc::MFD_CLOEXEC) }; + if raw < 0 { + return Err(std::io::Error::last_os_error()).context("create bwrap argument memfd"); + } + let args_fd = unsafe { std::os::fd::OwnedFd::from_raw_fd(raw) }; + // A memfd on a standard stream would be replaced by the child's stdio. + if raw <= 2 { + bail!("bwrap argument memfd landed on standard stream {raw}"); + } + let mut file = std::fs::File::from(args_fd.try_clone()?); + std::io::Write::write_all(&mut file, &bytes).context("write bwrap arguments")?; + std::io::Seek::rewind(&mut file).context("rewind bwrap arguments")?; + Ok(args_fd) +} + +fn supplementary_groups() -> Result> { + let count = unsafe { libc::getgroups(0, std::ptr::null_mut()) }; + if count < 0 { + return Err(std::io::Error::last_os_error()).context("read supplementary groups"); + } + let mut groups = vec![0 as libc::gid_t; count as usize]; + let count = unsafe { libc::getgroups(count, groups.as_mut_ptr()) }; + if count < 0 { + return Err(std::io::Error::last_os_error()).context("read supplementary groups"); + } + groups.truncate(count as usize); + Ok(groups) +} + +fn user_name(uid: libc::uid_t) -> Option { + let mut buffer = vec![0 as libc::c_char; 16 * 1024]; + let mut entry: libc::passwd = unsafe { std::mem::zeroed() }; + let mut result = std::ptr::null_mut(); + let status = unsafe { + libc::getpwuid_r( + uid, + &mut entry, + buffer.as_mut_ptr(), + buffer.len(), + &mut result, + ) + }; + if status != 0 || result.is_null() { + return None; + } + unsafe { std::ffi::CStr::from_ptr(entry.pw_name) } + .to_str() + .ok() + .map(str::to_string) +} + +/// The first id of the user's range in an /etc/subuid-style file, matched by +/// name or numeric uid. +fn subordinate_start(path: &Path, uid: libc::uid_t, name: Option<&str>) -> Option { + let uid = uid.to_string(); + fs::read_to_string(path).ok()?.lines().find_map(|line| { + let mut fields = line.trim().split(':'); + let owner = fields.next()?; + let start = fields.next()?.parse::().ok()?; + let count = fields.next()?.parse::().ok()?; + (count >= 1 && (owner == uid || Some(owner) == name)).then_some(start) + }) +} + pub(crate) fn resolve_executable_path(program: &str) -> Option { if program.contains('/') { let path = PathBuf::from(program); @@ -603,6 +874,55 @@ fn is_executable_file(path: &Path) -> bool { #[cfg(test)] mod tests { + #[test] + fn launch_environment_reaches_bubblewrap_only_through_private_arguments() { + let env = [ + ("LD_PRELOAD".into(), "/tmp/inject.so".into()), + ("PATH".into(), "/nonexistent".into()), + ]; + let fd = bwrap_args_memfd( + &["--ro-bind".to_string(), "/".to_string(), "/".to_string()], + &env, + ) + .expect("memfd"); + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut std::fs::File::from(fd), &mut bytes).expect("read"); + let args = bytes + .split(|byte| *byte == 0) + .map(|arg| String::from_utf8_lossy(arg).into_owned()) + .collect::>(); + assert_eq!( + &args[..9], + [ + "--setenv", + "LD_PRELOAD", + "/tmp/inject.so", + "--setenv", + "PATH", + "/nonexistent", + "--ro-bind", + "/", + "/", + ] + ); + assert!(bwrap_args_memfd(&["--chdir".into(), "/w\0--bind\0/\0/x".into()], &[]).is_err()); + assert!(bwrap_args_memfd(&[], &[("A".into(), "b\0c".into())]).is_err()); + } + + #[test] + fn subordinate_ranges_match_by_name_or_uid() { + let path = test_runtime("subuid"); + fs::write( + &path, + "other:200000:65536\nalice:100000:0\nalice:300000:65536\n1000:400000:10\n", + ) + .expect("write subuid"); + assert_eq!(subordinate_start(&path, 1001, Some("alice")), Some(300000)); + assert_eq!(subordinate_start(&path, 1000, None), Some(400000)); + assert_eq!(subordinate_start(&path, 1002, Some("bob")), None); + fs::remove_file(path).expect("remove subuid"); + } + use super::*; fn native_status( diff --git a/src/profile.rs b/src/profile.rs index 7db37c1..5434b31 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -872,6 +872,9 @@ fn validate_mount_path(path: &Path, field: &str) -> Result<()> { if path.as_os_str().is_empty() { bail!("profile mount {field} cannot be empty"); } + if std::os::unix::ffi::OsStrExt::as_bytes(path.as_os_str()).contains(&0) { + bail!("profile mount {field} cannot contain a NUL byte"); + } if path .components() .any(|component| matches!(component, Component::ParentDir)) diff --git a/src/workspace.rs b/src/workspace.rs index 6efc040..0ebdc88 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -156,6 +156,9 @@ pub struct RuntimeReport { pub screenshot: Check, pub clipboard: Check, pub policy: PolicyRuntimeCapabilities, + /// Whether sandboxed launches drop the owner's supplementary groups; when + /// they cannot, sandboxed apps keep them as before. + pub sandbox_group_drop: Check, } #[derive(Debug, Clone, Serialize, JsonSchema)] @@ -1151,6 +1154,21 @@ pub fn doctor_report() -> DoctorReport { screenshot: first_available_command(&["ffmpeg", "import", "scrot"]), clipboard: first_available_command(&["xclip", "xsel"]), policy: policy_runtime_capabilities(), + sandbox_group_drop: match crate::launch::group_drop_status() { + Ok(false) => Check { + ok: true, + detail: "no supplementary groups to drop".to_string(), + }, + Ok(true) => Check { + ok: true, + detail: "sandboxed launches drop supplementary groups via newuidmap/newgidmap" + .to_string(), + }, + Err(reason) => Check { + ok: false, + detail: reason, + }, + }, }; let viewer = ViewerRuntimeReport { host_display: host_display_check(), @@ -3855,6 +3873,30 @@ fn write_applied_policy_file( Ok(policy_path) } +/// The environment a sandboxed app gets: the daemon's own environment with +/// the launch rules applied (scrubbed host session variables, workspace +/// variables, the caller's launch variables, the client bus). Bubblewrap +/// receives it through `--setenv` instead of inheriting it. +fn sandboxed_app_environment( + status: &WorkspaceStatus, + launch_env: &[EnvVar], +) -> Vec<(std::ffi::OsString, std::ffi::OsString)> { + let mut rules = Command::new("true"); + crate::launch::configure_launch_environment(&mut rules, status, launch_env, true); + let mut env = env::vars_os().collect::>(); + for (name, value) in rules.get_envs() { + match value { + Some(value) => { + env.insert(name.to_os_string(), value.to_os_string()); + } + None => { + env.remove(name); + } + } + } + env.into_iter().collect() +} + fn create_client_runtime(runtime: &Path) -> Result<()> { create_owned_private_dir(&runtime.join("client"), "workspace client runtime") } @@ -7108,14 +7150,19 @@ fn spawn_app(state: &mut DaemonState, spec: LaunchSpec) -> Result )?; let (mount_isolation, network_isolation) = launch_isolation_labels_from_sandbox(sandbox.as_ref()); + // Held until spawn: the descriptor that carries a sandboxed launch's + // arguments and environment to bubblewrap. + let mut sandbox_args_fd = None; let mut child_command = if let Some(sandbox) = &sandbox { - let mut command = Command::new("bwrap"); - command - .args(&sandbox.args) - .arg("--") - .arg(&spec.command[0]) - .args(&spec.command[1..]); - command + let env = sandboxed_app_environment(&state.status, &spec.env); + let built = crate::launch::sandboxed_command( + &sandbox.args, + &spec.command[0], + &spec.command[1..], + &env, + )?; + sandbox_args_fd = Some(built.args_fd); + built.command } else { let mut command = Command::new(&spec.command[0]); command.args(&spec.command[1..]); @@ -7136,16 +7183,19 @@ fn spawn_app(state: &mut DaemonState, spec: LaunchSpec) -> Result child_command.current_dir(cwd); } } - crate::launch::configure_launch_environment( - &mut child_command, - &state.status, - &spec.env, - sandbox.is_some(), - ); + if sandbox.is_none() { + crate::launch::configure_launch_environment( + &mut child_command, + &state.status, + &spec.env, + false, + ); + } child_command.process_group(0); let child = child_command .spawn() .with_context(|| format!("failed to launch {}", launch_description(&spec.command)))?; + drop(sandbox_args_fd); let pid = child.id(); let stdout_path = rename_app_log(&log_paths.stdout, pid, "stdout")?; let stderr_path = rename_app_log(&log_paths.stderr, pid, "stderr")?; @@ -7414,7 +7464,16 @@ fn tmux_command(socket_path: &Path, sandbox: &TmuxClientSandbox, args: &[&str]) TmuxClientSandbox::Unknown => bail!("terminal client sandbox was not resolved"), TmuxClientSandbox::Host => Command::new(tmux), TmuxClientSandbox::Bubblewrap(sandbox_args) => { - let mut command = Command::new("bwrap"); + let bwrap = resolve_executable_path("bwrap").context("bwrap not found")?; + let prefix = crate::launch::sandbox_launcher_prefix(); + let mut command = match prefix.split_first() { + Some((launcher, launcher_args)) => { + let mut command = Command::new(launcher); + command.args(launcher_args).arg(bwrap); + command + } + None => Command::new(bwrap), + }; command .args(sandbox_args) .arg("--die-with-parent") @@ -10039,7 +10098,12 @@ mod tests { })); let command = tmux_command(&socket, &TmuxClientSandbox::Bubblewrap(args), &["ls"]) .expect("client command"); - assert_eq!(command.get_program(), "bwrap"); + // bwrap, or unshare in front of it where supplementary groups are dropped. + let program = Path::new(command.get_program()); + assert!( + program.ends_with("bwrap") || program.ends_with("unshare"), + "{program:?}" + ); assert!(command.get_args().any(|arg| arg == "--die-with-parent")); assert!(command .get_envs()