diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index ee53e7d437e0..97f1f5232f61 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -5,6 +5,14 @@ use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; use std::future::Future; use std::io; +#[cfg(target_os = "linux")] +use std::net::Ipv4Addr; +#[cfg(target_os = "linux")] +use std::net::TcpListener; +#[cfg(target_os = "linux")] +use std::net::TcpStream; +#[cfg(target_os = "linux")] +use std::os::fd::AsRawFd; use std::path::Path; use std::path::PathBuf; use std::process::ExitStatus; @@ -427,6 +435,34 @@ fn unix_sock_body() { recvd ); + let sent = libc::sendto( + fds[0], + msg.as_ptr() as *const libc::c_void, + msg.len(), + 0, + std::ptr::null(), + 0, + ); + assert!( + sent >= 0, + "sendto(NULL, 0) failed: {}", + io::Error::last_os_error() + ); + let recvd = libc::recvfrom( + fds[1], + buf.as_mut_ptr() as *mut libc::c_void, + buf.len(), + 0, + std::ptr::null_mut(), + std::ptr::null_mut(), + ); + assert!( + recvd >= 0, + "recvfrom() after sendto(NULL, 0) failed: {}", + io::Error::last_os_error() + ); + assert_eq!(&buf[..(recvd as usize)], &msg[..]); + // Also exercise AF_UNIX stream socketpair quickly to ensure AF_UNIX in general works. let mut sfds = [0i32; 2]; let sr = libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, sfds.as_mut_ptr()); @@ -460,23 +496,113 @@ fn unix_sock_body() { #[tokio::test] async fn allow_unix_socketpair_recvfrom() { - run_code_under_sandbox( + let result = run_code_under_sandbox( "allow_unix_socketpair_recvfrom", &SandboxPolicy::new_read_only_policy(), || async { unix_sock_body() }, ) .await .expect("should be able to reexec"); + assert_sandbox_reexec_succeeded(result); } const IN_SANDBOX_ENV_VAR: &str = "IN_SANDBOX"; +#[cfg(target_os = "linux")] +const INHERITED_CONNECTED_SOCKET_FD_ENV_VAR: &str = "INHERITED_CONNECTED_SOCKET_FD"; + +#[tokio::test] +#[cfg(target_os = "linux")] +async fn inherited_connected_tcp_socket_cannot_send_after_sandbox_exec() { + let mut sandbox_env = HashMap::new(); + let mut connected_socket_guards: Option<(TcpStream, TcpStream)> = None; + + if std::env::var(IN_SANDBOX_ENV_VAR).is_err() { + sandbox_env = match linux_sandbox_test_env().await { + Some(env) => env, + None => return, + }; + + let listener = + TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("bind local TCP listener"); + let stream = + TcpStream::connect(listener.local_addr().expect("listener address")).expect("connect"); + let inherited_fd = stream.as_raw_fd(); + clear_cloexec(inherited_fd); + sandbox_env.insert( + INHERITED_CONNECTED_SOCKET_FD_ENV_VAR.to_string(), + inherited_fd.to_string(), + ); + let accepted_stream = listener.accept().expect("accept connection").0; + connected_socket_guards = Some((stream, accepted_stream)); + } + + let result = run_code_under_sandbox_with_env( + "inherited_connected_tcp_socket_cannot_send_after_sandbox_exec", + &SandboxPolicy::new_read_only_policy(), + sandbox_env, + || async { inherited_connected_tcp_socket_send_body() }, + ) + .await + .expect("should be able to reexec"); + drop(connected_socket_guards); + assert_sandbox_reexec_succeeded(result); +} +#[cfg(target_os = "linux")] #[expect(clippy::expect_used)] +fn inherited_connected_tcp_socket_send_body() { + let fd = std::env::var(INHERITED_CONNECTED_SOCKET_FD_ENV_VAR) + .expect("inherited fd env var should be set") + .parse::() + .expect("inherited fd should parse"); + let msg = b"should_not_escape"; + let sent = unsafe { + libc::sendto( + fd, + msg.as_ptr() as *const libc::c_void, + msg.len(), + 0, + std::ptr::null(), + 0, + ) + }; + assert!( + sent < 0, + "sendto(NULL, 0) on inherited connected TCP fd unexpectedly wrote {sent} bytes" + ); +} + +#[cfg(target_os = "linux")] +fn clear_cloexec(fd: libc::c_int) { + let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) }; + assert!(flags >= 0, "F_GETFD failed: {}", io::Error::last_os_error()); + let result = unsafe { libc::fcntl(fd, libc::F_SETFD, flags & !libc::FD_CLOEXEC) }; + assert!( + result >= 0, + "F_SETFD failed: {}", + io::Error::last_os_error() + ); +} + pub async fn run_code_under_sandbox( test_selector: &str, policy: &SandboxPolicy, child_body: F, ) -> io::Result> +where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, +{ + run_code_under_sandbox_with_env(test_selector, policy, HashMap::new(), child_body).await +} + +#[expect(clippy::expect_used)] +pub async fn run_code_under_sandbox_with_env( + test_selector: &str, + policy: &SandboxPolicy, + mut env: HashMap, + child_body: F, +) -> io::Result> where F: FnOnce() -> Fut + Send + 'static, Fut: Future + Send + 'static, @@ -495,13 +621,14 @@ where // Your existing launcher: let command_cwd = std::env::current_dir().expect("should be able to get current dir"); let sandbox_cwd = command_cwd.clone(); + env.insert(IN_SANDBOX_ENV_VAR.to_string(), "1".to_string()); let mut child = spawn_command_under_sandbox( cmds, command_cwd, policy, sandbox_cwd.as_path(), stdio_policy, - HashMap::from([("IN_SANDBOX".into(), "1".into())]), + env, ) .await?; @@ -513,3 +640,9 @@ where Ok(None) } } + +fn assert_sandbox_reexec_succeeded(status: Option) { + if let Some(status) = status { + assert!(status.success(), "sandboxed child exited with {status:?}"); + } +} diff --git a/codex-rs/linux-sandbox/src/fd_cleanup.rs b/codex-rs/linux-sandbox/src/fd_cleanup.rs new file mode 100644 index 000000000000..10b590a676ed --- /dev/null +++ b/codex-rs/linux-sandbox/src/fd_cleanup.rs @@ -0,0 +1,108 @@ +//! File descriptor hygiene before entering the sandboxed command. + +use std::io::ErrorKind; + +const ESCALATE_SOCKET_ENV_VAR: &str = "CODEX_ESCALATE_SOCKET"; + +/// Close helper-inherited descriptors unless they are standard input/output/error, +/// already close-on-exec, or known helper IPC. +/// +/// The sandboxed command can still create allowed local IPC after exec, but it +/// must not inherit an already-connected network socket from the launcher. +pub(crate) fn close_inherited_exec_fds() { + let preserved_fd = inherited_fd_to_preserve(); + let fds = match non_stdio_fds_from_proc() { + Ok(fds) => fds, + Err(err) if err.kind() == ErrorKind::NotFound => { + mark_inherited_exec_fds_cloexec(preserved_fd); + return; + } + Err(err) => panic!("failed to enumerate inherited file descriptors: {err}"), + }; + for fd in fds { + if Some(fd) == preserved_fd { + continue; + } + close_fd_if_inheritable(fd); + } +} + +fn inherited_fd_to_preserve() -> Option { + std::env::var(ESCALATE_SOCKET_ENV_VAR) + .ok() + .and_then(|fd| fd.parse::().ok()) + .filter(|fd| *fd > libc::STDERR_FILENO) +} + +fn mark_inherited_exec_fds_cloexec(preserved_fd: Option) { + let start = (libc::STDERR_FILENO + 1) as libc::c_uint; + let Some(preserved_fd) = preserved_fd + .and_then(|fd| u32::try_from(fd).ok()) + .filter(|fd| *fd >= start) + else { + mark_fd_range_cloexec(start, u32::MAX); + return; + }; + + if preserved_fd > start { + mark_fd_range_cloexec(start, preserved_fd - 1); + } + if preserved_fd < u32::MAX { + mark_fd_range_cloexec(preserved_fd + 1, u32::MAX); + } +} + +fn mark_fd_range_cloexec(first: libc::c_uint, last: libc::c_uint) { + let result = unsafe { + libc::syscall( + libc::SYS_close_range, + first, + last, + libc::CLOSE_RANGE_CLOEXEC, + ) + }; + if result != 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to mark inherited file descriptors close-on-exec: {err}"); + } +} + +fn non_stdio_fds_from_proc() -> std::io::Result> { + let mut fds = Vec::new(); + for entry in std::fs::read_dir("/proc/self/fd")? { + let entry = entry?; + let file_name = entry.file_name(); + let Some(name) = file_name.to_str() else { + continue; + }; + let Ok(fd) = name.parse::() else { + continue; + }; + if fd > libc::STDERR_FILENO { + fds.push(fd); + } + } + Ok(fds) +} + +fn close_fd_if_inheritable(fd: libc::c_int) { + let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) }; + if flags == -1 { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EBADF) { + panic!("failed to inspect inherited file descriptor {fd}: {err}"); + } + return; + } + if flags & libc::FD_CLOEXEC != 0 { + return; + } + + let result = unsafe { libc::close(fd) }; + if result != 0 { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() != Some(libc::EBADF) { + panic!("failed to close inherited file descriptor {fd}: {err}"); + } + } +} diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index 5794982530e6..0bb65683f63d 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -190,7 +190,6 @@ fn install_network_seccomp_filter_on_current_thread( deny_syscall(&mut rules, libc::SYS_getpeername); deny_syscall(&mut rules, libc::SYS_getsockname); deny_syscall(&mut rules, libc::SYS_shutdown); - deny_syscall(&mut rules, libc::SYS_sendto); deny_syscall(&mut rules, libc::SYS_sendmmsg); // NOTE: allowing recvfrom allows some tools like: `cargo clippy` // to run with their socketpair + child processes for sub-proc @@ -209,8 +208,27 @@ fn install_network_seccomp_filter_on_current_thread( libc::AF_UNIX as u64, )?])?; + // Allow send()/self-pipe wakeups on already-connected sockets, but + // continue denying sendto() when it names a destination address. + let sendto_dest_addr_rule = SeccompRule::new(vec![SeccompCondition::new( + 4, // fifth argument (dest_addr) + SeccompCmpArgLen::Qword, + SeccompCmpOp::Ne, + 0, + )?])?; + let sendto_addrlen_rule = SeccompRule::new(vec![SeccompCondition::new( + 5, // sixth argument (addrlen) + SeccompCmpArgLen::Dword, + SeccompCmpOp::Ne, + 0, + )?])?; + rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]); rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); + rules.insert( + libc::SYS_sendto, + vec![sendto_dest_addr_rule, sendto_addrlen_rule], + ); } NetworkSeccompMode::ProxyRouted => { // In proxy-routed mode we allow IP sockets in the isolated diff --git a/codex-rs/linux-sandbox/src/lib.rs b/codex-rs/linux-sandbox/src/lib.rs index 900287c99dc4..0c2d0c31ffa9 100644 --- a/codex-rs/linux-sandbox/src/lib.rs +++ b/codex-rs/linux-sandbox/src/lib.rs @@ -6,6 +6,8 @@ #[cfg(target_os = "linux")] mod bwrap; #[cfg(target_os = "linux")] +mod fd_cleanup; +#[cfg(target_os = "linux")] mod landlock; #[cfg(target_os = "linux")] mod launcher; diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 8f39837de1b9..a9197afb9789 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; use crate::bwrap::create_bwrap_command_args; +use crate::fd_cleanup::close_inherited_exec_fds; use crate::landlock::apply_sandbox_policy_to_current_thread; use crate::launcher::exec_bwrap; use crate::launcher::preferred_bwrap_supports_argv0; @@ -729,6 +730,8 @@ fn exec_or_panic(command: Vec) -> ! { let mut c_args_ptrs: Vec<*const libc::c_char> = c_args.iter().map(|arg| arg.as_ptr()).collect(); c_args_ptrs.push(std::ptr::null()); + close_inherited_exec_fds(); + unsafe { libc::execvp(c_command.as_ptr(), c_args_ptrs.as_ptr()); }