Fix AF_UNIX, sockpair, recvfrom in linux sandbox (#2309)
When using codex-tui on a linux system I was unable to run `cargo clippy` inside of codex due to: ``` [pid 3548377] socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, <unfinished ...> [pid 3548370] close(8 <unfinished ...> [pid 3548377] <... socketpair resumed>0x7ffb97f4ed60) = -1 EPERM (Operation not permitted) ``` And ``` 3611300 <... recvfrom resumed>0x708b8b5cffe0, 8, 0, NULL, NULL) = -1 EPERM (Operation not permitted) ``` This PR: * Fixes a bug that disallowed AF_UNIX to allow it on `socket()` * Adds recvfrom() to the syscall allow list, this should be fine since we disable opening new sockets. But we should validate there is not a open socket inheritance issue. * Allow socketpair to be called for AF_UNIX * Adds tests for AF_UNIX components * All of which allows running `cargo clippy` within the sandbox on linux, and possibly other tooling using a fork server model + AF_UNIX comms.
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -751,6 +751,7 @@ dependencies = [
|
|||||||
"codex-common",
|
"codex-common",
|
||||||
"codex-core",
|
"codex-core",
|
||||||
"codex-ollama",
|
"codex-ollama",
|
||||||
|
"libc",
|
||||||
"owo-colors",
|
"owo-colors",
|
||||||
"predicates",
|
"predicates",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
|||||||
@@ -41,5 +41,6 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
|
|||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
assert_cmd = "2"
|
assert_cmd = "2"
|
||||||
|
libc = "0.2"
|
||||||
predicates = "3"
|
predicates = "3"
|
||||||
tempfile = "3.13.0"
|
tempfile = "3.13.0"
|
||||||
|
|||||||
@@ -4,7 +4,10 @@
|
|||||||
use codex_core::protocol::SandboxPolicy;
|
use codex_core::protocol::SandboxPolicy;
|
||||||
use codex_core::spawn::StdioPolicy;
|
use codex_core::spawn::StdioPolicy;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
use std::future::Future;
|
||||||
|
use std::io;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
use std::process::ExitStatus;
|
||||||
use tokio::process::Child;
|
use tokio::process::Child;
|
||||||
|
|
||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
@@ -90,3 +93,128 @@ if __name__ == '__main__':
|
|||||||
let status = child.wait().await.expect("should wait for child process");
|
let status = child.wait().await.expect("should wait for child process");
|
||||||
assert!(status.success(), "python exited with {status:?}");
|
assert!(status.success(), "python exited with {status:?}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn unix_sock_body() {
|
||||||
|
unsafe {
|
||||||
|
let mut fds = [0i32; 2];
|
||||||
|
let r = libc::socketpair(libc::AF_UNIX, libc::SOCK_DGRAM, 0, fds.as_mut_ptr());
|
||||||
|
assert_eq!(
|
||||||
|
r,
|
||||||
|
0,
|
||||||
|
"socketpair(AF_UNIX, SOCK_DGRAM) failed: {}",
|
||||||
|
io::Error::last_os_error()
|
||||||
|
);
|
||||||
|
|
||||||
|
let msg = b"hello_unix";
|
||||||
|
// write() from one end (generic write is allowed)
|
||||||
|
let sent = libc::write(fds[0], msg.as_ptr() as *const libc::c_void, msg.len());
|
||||||
|
assert!(sent >= 0, "write() failed: {}", io::Error::last_os_error());
|
||||||
|
|
||||||
|
// recvfrom() on the other end. We don’t need the address for socketpair,
|
||||||
|
// so we pass null pointers for src address.
|
||||||
|
let mut buf = [0u8; 64];
|
||||||
|
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() failed: {}",
|
||||||
|
io::Error::last_os_error()
|
||||||
|
);
|
||||||
|
|
||||||
|
let recvd_slice = &buf[..(recvd as usize)];
|
||||||
|
assert_eq!(
|
||||||
|
recvd_slice,
|
||||||
|
&msg[..],
|
||||||
|
"payload mismatch: sent {} bytes, got {} bytes",
|
||||||
|
msg.len(),
|
||||||
|
recvd
|
||||||
|
);
|
||||||
|
|
||||||
|
// 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());
|
||||||
|
assert_eq!(
|
||||||
|
sr,
|
||||||
|
0,
|
||||||
|
"socketpair(AF_UNIX, SOCK_STREAM) failed: {}",
|
||||||
|
io::Error::last_os_error()
|
||||||
|
);
|
||||||
|
let snt2 = libc::write(sfds[0], msg.as_ptr() as *const libc::c_void, msg.len());
|
||||||
|
assert!(
|
||||||
|
snt2 >= 0,
|
||||||
|
"write(stream) failed: {}",
|
||||||
|
io::Error::last_os_error()
|
||||||
|
);
|
||||||
|
let mut b2 = [0u8; 64];
|
||||||
|
let rcv2 = libc::recv(sfds[1], b2.as_mut_ptr() as *mut libc::c_void, b2.len(), 0);
|
||||||
|
assert!(
|
||||||
|
rcv2 >= 0,
|
||||||
|
"recv(stream) failed: {}",
|
||||||
|
io::Error::last_os_error()
|
||||||
|
);
|
||||||
|
|
||||||
|
// Clean up
|
||||||
|
let _ = libc::close(sfds[0]);
|
||||||
|
let _ = libc::close(sfds[1]);
|
||||||
|
let _ = libc::close(fds[0]);
|
||||||
|
let _ = libc::close(fds[1]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn allow_unix_socketpair_recvfrom() {
|
||||||
|
run_code_under_sandbox(
|
||||||
|
"allow_unix_socketpair_recvfrom",
|
||||||
|
&SandboxPolicy::ReadOnly,
|
||||||
|
|| async { unix_sock_body() },
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect("should be able to reexec");
|
||||||
|
}
|
||||||
|
|
||||||
|
const IN_SANDBOX_ENV_VAR: &str = "IN_SANDBOX";
|
||||||
|
|
||||||
|
pub async fn run_code_under_sandbox<F, Fut>(
|
||||||
|
test_selector: &str,
|
||||||
|
policy: &SandboxPolicy,
|
||||||
|
child_body: F,
|
||||||
|
) -> io::Result<Option<ExitStatus>>
|
||||||
|
where
|
||||||
|
F: FnOnce() -> Fut + Send + 'static,
|
||||||
|
Fut: Future<Output = ()> + Send + 'static,
|
||||||
|
{
|
||||||
|
if std::env::var(IN_SANDBOX_ENV_VAR).is_err() {
|
||||||
|
let exe = std::env::current_exe()?;
|
||||||
|
let mut cmds = vec![exe.to_string_lossy().into_owned(), "--exact".into()];
|
||||||
|
let mut stdio_policy = StdioPolicy::RedirectForShellTool;
|
||||||
|
// Allow for us to pass forward --nocapture / use the right stdio policy.
|
||||||
|
if std::env::args().any(|a| a == "--nocapture") {
|
||||||
|
cmds.push("--nocapture".into());
|
||||||
|
stdio_policy = StdioPolicy::Inherit;
|
||||||
|
}
|
||||||
|
cmds.push(test_selector.into());
|
||||||
|
|
||||||
|
// Your existing launcher:
|
||||||
|
let mut child = spawn_command_under_sandbox(
|
||||||
|
cmds,
|
||||||
|
policy,
|
||||||
|
std::env::current_dir().expect("should be able to get current dir"),
|
||||||
|
stdio_policy,
|
||||||
|
HashMap::from([("IN_SANDBOX".into(), "1".into())]),
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
let status = child.wait().await?;
|
||||||
|
Ok(Some(status))
|
||||||
|
} else {
|
||||||
|
// Child branch: run the provided body.
|
||||||
|
child_body().await;
|
||||||
|
Ok(None)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -104,7 +104,9 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(),
|
|||||||
deny_syscall(libc::SYS_sendto);
|
deny_syscall(libc::SYS_sendto);
|
||||||
deny_syscall(libc::SYS_sendmsg);
|
deny_syscall(libc::SYS_sendmsg);
|
||||||
deny_syscall(libc::SYS_sendmmsg);
|
deny_syscall(libc::SYS_sendmmsg);
|
||||||
deny_syscall(libc::SYS_recvfrom);
|
// NOTE: allowing recvfrom allows some tools like: `cargo clippy` to run
|
||||||
|
// with their socketpair + child processes for sub-proc management
|
||||||
|
// deny_syscall(libc::SYS_recvfrom);
|
||||||
deny_syscall(libc::SYS_recvmsg);
|
deny_syscall(libc::SYS_recvmsg);
|
||||||
deny_syscall(libc::SYS_recvmmsg);
|
deny_syscall(libc::SYS_recvmmsg);
|
||||||
deny_syscall(libc::SYS_getsockopt);
|
deny_syscall(libc::SYS_getsockopt);
|
||||||
@@ -115,12 +117,12 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(),
|
|||||||
let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new(
|
let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new(
|
||||||
0, // first argument (domain)
|
0, // first argument (domain)
|
||||||
SeccompCmpArgLen::Dword,
|
SeccompCmpArgLen::Dword,
|
||||||
SeccompCmpOp::Eq,
|
SeccompCmpOp::Ne,
|
||||||
libc::AF_UNIX as u64,
|
libc::AF_UNIX as u64,
|
||||||
)?])?;
|
)?])?;
|
||||||
|
|
||||||
rules.insert(libc::SYS_socket, vec![unix_only_rule]);
|
rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]);
|
||||||
rules.insert(libc::SYS_socketpair, vec![]); // always deny (Unix can use socketpair but fine, keep open?)
|
rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); // always deny (Unix can use socketpair but fine, keep open?)
|
||||||
|
|
||||||
let filter = SeccompFilter::new(
|
let filter = SeccompFilter::new(
|
||||||
rules,
|
rules,
|
||||||
|
|||||||
Reference in New Issue
Block a user