Support FD mounts with older system Bubblewrap versions (#39404)

## Why

System Bubblewrap installations that lack `--ro-bind-fd` cannot directly create
the descriptor-backed read-only mounts used by the Linux sandbox.

## What changed

- Detect `--ro-bind-fd` support when probing system Bubblewrap.
- On older versions, translate descriptor-backed mounts to `/proc/self/fd`
  read-only binds and pass their descriptors and destinations to the trusted
  inner sandbox stage for verification.
- Reject malformed, duplicate, mismatched, or symlink-substituted mounts, and
  close inherited descriptors before running sandboxed code.

## Testing

Added unit coverage for capability detection, legacy argument translation,
invalid mount rejection, inode verification, and descriptor closure.

GitOrigin-RevId: dfc0a457b572cdd9aed093fb5abf57360e9724a2
This commit is contained in:
Adam Perry @ OpenAI
2026-08-19 03:57:32 +00:00
committed by copyberry
parent 956f590ad5
commit 6cc2ba8a95
5 changed files with 448 additions and 0 deletions

View File

@@ -0,0 +1,91 @@
//! Authenticate descriptor-backed mounts before sandboxed code can inherit them.
use std::collections::HashSet;
use std::fs;
use std::fs::File;
use std::io;
use std::os::fd::AsRawFd;
use std::os::fd::FromRawFd;
use std::os::unix::fs::MetadataExt;
use std::path::Path;
pub(crate) fn verify_fd_mounts(mounts: &[String]) -> io::Result<()> {
let mut claimed_descriptors = HashSet::with_capacity(mounts.len());
for mount in mounts {
let (descriptor, destination) = mount.split_once(':').ok_or_else(|| {
io::Error::new(
io::ErrorKind::InvalidInput,
format!("descriptor-backed mount must contain FD:DEST: {mount}"),
)
})?;
let descriptor = descriptor.parse::<libc::c_int>().map_err(|_| {
io::Error::new(
io::ErrorKind::InvalidInput,
format!("descriptor-backed mount has an invalid descriptor: {mount}"),
)
})?;
if descriptor <= libc::STDERR_FILENO {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("descriptor-backed mount cannot use a standard descriptor: {descriptor}"),
));
}
if !claimed_descriptors.insert(descriptor) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("descriptor-backed mount reuses descriptor {descriptor}"),
));
}
// The launcher transfers each distinct descriptor exactly once across
// bubblewrap, so first confirm it is live before assuming ownership.
let flags = unsafe { libc::fcntl(descriptor, libc::F_GETFD) };
if flags < 0 {
return Err(io::Error::last_os_error());
}
// SAFETY: the launcher transferred this live descriptor to this stage,
// and the set above prevents claiming the same descriptor twice.
let file = unsafe { File::from_raw_fd(descriptor) };
let result =
unsafe { libc::fcntl(file.as_raw_fd(), libc::F_SETFD, flags | libc::FD_CLOEXEC) };
if result < 0 {
return Err(io::Error::last_os_error());
}
let destination = Path::new(destination);
if !destination.is_absolute() {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"descriptor-backed mount destination must be absolute: {}",
destination.display()
),
));
}
let descriptor_metadata = file.metadata()?;
let destination_metadata = fs::symlink_metadata(destination)?;
if (descriptor_metadata.dev(), descriptor_metadata.ino())
!= (destination_metadata.dev(), destination_metadata.ino())
{
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!(
"descriptor-backed mount does not match its destination: {}",
destination.display()
),
));
}
// Closing immediately prevents a writable host directory descriptor
// from reaching bridge workers or the sandboxed command.
drop(file);
}
Ok(())
}
#[cfg(test)]
#[path = "fd_mount_tests.rs"]
mod tests;

View File

@@ -0,0 +1,108 @@
use super::verify_fd_mounts;
use pretty_assertions::assert_eq;
use std::fs::File;
use std::os::fd::IntoRawFd;
/// A matching mount authenticates its original inode and closes the inherited descriptor.
#[test]
fn matching_mount_closes_inherited_descriptor() {
let root = tempfile::tempdir().expect("temporary directory should be created");
let descriptor = File::open(root.path())
.expect("directory descriptor should open")
.into_raw_fd();
let marker = format!("{descriptor}:{}", root.path().display());
verify_fd_mounts(&[marker]).expect("matching mount should verify");
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
assert_eq!(
std::io::Error::last_os_error().raw_os_error(),
Some(libc::EBADF)
);
}
/// A swapped destination fails closed without leaking the original descriptor.
#[test]
fn mismatched_mount_closes_inherited_descriptor() {
let source = tempfile::tempdir().expect("source directory should be created");
let destination = tempfile::tempdir().expect("destination directory should be created");
let descriptor = File::open(source.path())
.expect("source directory descriptor should open")
.into_raw_fd();
let marker = format!("{descriptor}:{}", destination.path().display());
let error = verify_fd_mounts(&[marker]).expect_err("different inodes must be rejected");
assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
assert_eq!(
std::io::Error::last_os_error().raw_os_error(),
Some(libc::EBADF)
);
}
/// A symlink to the original inode is not itself the authenticated mount.
#[test]
fn symlinked_mount_destination_closes_inherited_descriptor() {
let root = tempfile::tempdir().expect("temporary directory should be created");
let source = root.path().join("source");
let destination = root.path().join("destination");
std::fs::create_dir(&source).expect("source directory should be created");
std::os::unix::fs::symlink(&source, &destination)
.expect("mount destination symlink should be created");
let descriptor = File::open(&source)
.expect("source directory descriptor should open")
.into_raw_fd();
let marker = format!("{descriptor}:{}", destination.display());
let error = verify_fd_mounts(&[marker]).expect_err("symlinked mounts must be rejected");
assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
assert_eq!(
std::io::Error::last_os_error().raw_os_error(),
Some(libc::EBADF)
);
}
/// Malformed mount markers cannot claim standard streams or relative destinations.
#[test]
fn malformed_mount_markers_are_rejected() {
for marker in [
"missing-separator",
"invalid:/tmp",
"0:/tmp",
"1:/tmp",
"2:/tmp",
] {
let error = verify_fd_mounts(&[marker.to_string()])
.expect_err("malformed mount marker must be rejected");
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
}
let root = tempfile::tempdir().expect("temporary directory should be created");
let descriptor = File::open(root.path())
.expect("directory descriptor should open")
.into_raw_fd();
let error = verify_fd_mounts(&[format!("{descriptor}:relative")])
.expect_err("relative mount destinations must be rejected");
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
}
/// A transferred descriptor can be consumed only once even if a marker is repeated.
#[test]
fn duplicate_mount_descriptors_are_rejected() {
let root = tempfile::tempdir().expect("temporary directory should be created");
let descriptor = File::open(root.path())
.expect("directory descriptor should open")
.into_raw_fd();
let marker = format!("{descriptor}:{}", root.path().display());
let error = verify_fd_mounts(&[marker.clone(), marker])
.expect_err("a mount descriptor must not be consumed twice");
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
}

View File

@@ -25,12 +25,14 @@ enum BubblewrapLauncher {
struct SystemBwrapLauncher {
program: AbsolutePathBuf,
supports_argv0: bool,
supports_ro_bind_fd: bool,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
struct SystemBwrapCapabilities {
supports_argv0: bool,
supports_perms: bool,
supports_ro_bind_fd: bool,
}
pub(crate) fn exec_bwrap(mut argv: Vec<String>, preserved_files: Vec<File>) -> ! {
@@ -38,6 +40,10 @@ pub(crate) fn exec_bwrap(mut argv: Vec<String>, preserved_files: Vec<File>) -> !
match preferred_bwrap_launcher() {
BubblewrapLauncher::System(launcher) => {
if !launcher.supports_ro_bind_fd {
translate_legacy_bwrap_fd_mounts(&mut argv)
.unwrap_or_else(|error| panic!("invalid legacy bubblewrap fd mount: {error}"));
}
exec_system_bwrap(&launcher.program, argv, preserved_files)
}
BubblewrapLauncher::Bundled(launcher) => launcher.exec(argv, preserved_files),
@@ -50,6 +56,73 @@ pub(crate) fn exec_bwrap(mut argv: Vec<String>, preserved_files: Vec<File>) -> !
}
}
fn translate_legacy_bwrap_fd_mounts(argv: &mut Vec<String>) -> Result<(), String> {
let command_separator = argv
.iter()
.position(|argument| argument == "--")
.ok_or_else(|| "bubblewrap argv is missing the command separator '--'".to_string())?;
let mut verification_args = Vec::new();
let mut verified_fds = Vec::new();
let mut argument_index = 0;
while argument_index < command_separator {
if argv[argument_index] != "--ro-bind-fd" {
argument_index += 1;
continue;
}
let fd_argument = argv
.get(argument_index + 1)
.filter(|_| argument_index + 1 < command_separator)
.ok_or_else(|| "--ro-bind-fd is missing its file descriptor".to_string())?;
let fd = fd_argument
.parse::<libc::c_int>()
.map_err(|_| format!("invalid --ro-bind-fd file descriptor: {fd_argument}"))?;
if fd <= libc::STDERR_FILENO {
return Err(format!(
"--ro-bind-fd file descriptor must not use standard descriptors: {fd}"
));
}
if verified_fds.contains(&fd) {
return Err(format!("duplicate --ro-bind-fd file descriptor: {fd}"));
}
let destination = argv
.get(argument_index + 2)
.filter(|_| argument_index + 2 < command_separator)
.ok_or_else(|| "--ro-bind-fd is missing its mount destination".to_string())?;
if !Path::new(destination).is_absolute() {
return Err(format!(
"--ro-bind-fd mount destination must be absolute: {destination}"
));
}
verification_args.push("--verify-fd-mount".to_string());
verification_args.push(format!("{fd}:{destination}"));
verified_fds.push(fd);
argv[argument_index] = "--ro-bind".to_string();
argv[argument_index + 1] = format!("/proc/self/fd/{fd}");
argument_index += 3;
}
if verification_args.is_empty() {
return Ok(());
}
let inner_command = command_separator + 1;
if argv.get(inner_command).is_none() {
return Err("bubblewrap argv is missing the inner command after '--'".to_string());
}
if !argv[inner_command + 1..]
.iter()
.take_while(|argument| argument.as_str() != "--")
.any(|argument| argument == "--apply-seccomp-then-exec")
{
return Err("descriptor-backed mounts require the trusted inner sandbox stage".to_string());
}
argv.splice(inner_command + 1..inner_command + 1, verification_args);
Ok(())
}
fn preferred_bwrap_launcher() -> BubblewrapLauncher {
static LAUNCHER: OnceLock<BubblewrapLauncher> = OnceLock::new();
LAUNCHER
@@ -83,6 +156,7 @@ fn system_bwrap_launcher_for_path_with_probe(
let Some(SystemBwrapCapabilities {
supports_argv0,
supports_perms: true,
supports_ro_bind_fd,
}) = system_bwrap_capabilities(system_bwrap_path)
else {
return None;
@@ -97,6 +171,7 @@ fn system_bwrap_launcher_for_path_with_probe(
Some(SystemBwrapLauncher {
program: system_bwrap_path,
supports_argv0,
supports_ro_bind_fd,
})
}
@@ -125,6 +200,7 @@ fn system_bwrap_capabilities(system_bwrap_path: &Path) -> Option<SystemBwrapCapa
Some(SystemBwrapCapabilities {
supports_argv0: stdout.contains("--argv0") || stderr.contains("--argv0"),
supports_perms: stdout.contains("--perms") || stderr.contains("--perms"),
supports_ro_bind_fd: stdout.contains("--ro-bind-fd") || stderr.contains("--ro-bind-fd"),
})
}
@@ -160,6 +236,7 @@ fn exec_system_bwrap(
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use std::os::unix::fs::PermissionsExt;
use tempfile::NamedTempFile;
#[test]
@@ -173,11 +250,13 @@ mod tests {
Some(SystemBwrapCapabilities {
supports_argv0: true,
supports_perms: true,
supports_ro_bind_fd: true,
})
}),
Some(SystemBwrapLauncher {
program: expected,
supports_argv0: true,
supports_ro_bind_fd: true,
})
);
}
@@ -192,11 +271,13 @@ mod tests {
Some(SystemBwrapCapabilities {
supports_argv0: false,
supports_perms: true,
supports_ro_bind_fd: false,
})
}),
Some(SystemBwrapLauncher {
program: AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"),
supports_argv0: false,
supports_ro_bind_fd: false,
})
);
}
@@ -210,12 +291,166 @@ mod tests {
Some(SystemBwrapCapabilities {
supports_argv0: false,
supports_perms: false,
supports_ro_bind_fd: false,
})
}),
None
);
}
#[test]
fn detects_fd_backed_read_only_mount_support_in_system_bwrap_help() {
let temp_dir = tempfile::tempdir().expect("temp directory");
let fake_bwrap_path = temp_dir.path().join("bwrap");
std::fs::write(
&fake_bwrap_path,
"#!/bin/sh\nprintf '%s\\n' '--as-pid-1' '--perms' '--argv0' '--ro-bind-fd'\n",
)
.expect("write fake bubblewrap");
std::fs::set_permissions(&fake_bwrap_path, std::fs::Permissions::from_mode(0o755))
.expect("make fake bubblewrap executable");
assert_eq!(
system_bwrap_capabilities(&fake_bwrap_path),
Some(SystemBwrapCapabilities {
supports_argv0: true,
supports_perms: true,
supports_ro_bind_fd: true,
})
);
}
#[test]
fn translates_fd_mounts_for_legacy_system_bubblewrap() {
let mut argv = vec![
"bwrap".to_string(),
"--ro-bind-fd".to_string(),
"7".to_string(),
"/tmp/socket-root".to_string(),
"--".to_string(),
"/usr/bin/codex-linux-sandbox".to_string(),
"--apply-seccomp-then-exec".to_string(),
"--".to_string(),
"echo".to_string(),
"--ro-bind-fd".to_string(),
];
translate_legacy_bwrap_fd_mounts(&mut argv).expect("fd mount should translate");
assert_eq!(
argv,
vec![
"bwrap".to_string(),
"--ro-bind".to_string(),
"/proc/self/fd/7".to_string(),
"/tmp/socket-root".to_string(),
"--".to_string(),
"/usr/bin/codex-linux-sandbox".to_string(),
"--verify-fd-mount".to_string(),
"7:/tmp/socket-root".to_string(),
"--apply-seccomp-then-exec".to_string(),
"--".to_string(),
"echo".to_string(),
"--ro-bind-fd".to_string(),
]
);
}
#[test]
fn translates_multiple_fd_mounts_for_legacy_system_bubblewrap() {
let mut argv = vec![
"bwrap".to_string(),
"--ro-bind-fd".to_string(),
"7".to_string(),
"/tmp/first".to_string(),
"--ro-bind-fd".to_string(),
"9".to_string(),
"/tmp/second:with-colon".to_string(),
"--".to_string(),
"codex-linux-sandbox".to_string(),
"--apply-seccomp-then-exec".to_string(),
"--".to_string(),
"true".to_string(),
];
translate_legacy_bwrap_fd_mounts(&mut argv).expect("fd mounts should translate");
assert_eq!(
argv,
vec![
"bwrap".to_string(),
"--ro-bind".to_string(),
"/proc/self/fd/7".to_string(),
"/tmp/first".to_string(),
"--ro-bind".to_string(),
"/proc/self/fd/9".to_string(),
"/tmp/second:with-colon".to_string(),
"--".to_string(),
"codex-linux-sandbox".to_string(),
"--verify-fd-mount".to_string(),
"7:/tmp/first".to_string(),
"--verify-fd-mount".to_string(),
"9:/tmp/second:with-colon".to_string(),
"--apply-seccomp-then-exec".to_string(),
"--".to_string(),
"true".to_string(),
]
);
}
#[test]
fn rejects_malformed_legacy_bubblewrap_fd_mounts() {
for (fd, destination) in [
("invalid", "/tmp/socket-root"),
("2", "/tmp/socket-root"),
("7", "relative/socket-root"),
] {
let mut argv = vec![
"bwrap".to_string(),
"--ro-bind-fd".to_string(),
fd.to_string(),
destination.to_string(),
"--".to_string(),
"codex-linux-sandbox".to_string(),
];
assert!(translate_legacy_bwrap_fd_mounts(&mut argv).is_err());
}
}
#[test]
fn rejects_fd_mounts_without_the_trusted_inner_sandbox_stage() {
let mut argv = vec![
"bwrap".to_string(),
"--ro-bind-fd".to_string(),
"7".to_string(),
"/tmp/socket-root".to_string(),
"--".to_string(),
"/bin/true".to_string(),
"--".to_string(),
"--apply-seccomp-then-exec".to_string(),
];
assert!(translate_legacy_bwrap_fd_mounts(&mut argv).is_err());
}
#[test]
fn rejects_duplicate_legacy_bubblewrap_fd_mounts() {
let mut argv = vec![
"bwrap".to_string(),
"--ro-bind-fd".to_string(),
"7".to_string(),
"/tmp/first".to_string(),
"--ro-bind-fd".to_string(),
"7".to_string(),
"/tmp/second".to_string(),
"--".to_string(),
"codex-linux-sandbox".to_string(),
];
assert!(translate_legacy_bwrap_fd_mounts(&mut argv).is_err());
}
#[test]
fn ignores_system_bwrap_when_system_bwrap_is_missing() {
assert_eq!(

View File

@@ -12,6 +12,8 @@ mod bwrap;
#[cfg(target_os = "linux")]
mod exec_util;
#[cfg(target_os = "linux")]
mod fd_mount;
#[cfg(target_os = "linux")]
mod landlock;
#[cfg(target_os = "linux")]
mod launcher;

View File

@@ -131,6 +131,10 @@ pub struct LandlockCommand {
#[arg(long = "proxy-route-spec", hide = true)]
pub proxy_route_spec: Option<String>,
/// Inherited fallback mounts that must be authenticated before sandboxed code runs.
#[arg(long = "verify-fd-mount", hide = true)]
pub verify_fd_mounts: Vec<String>,
/// When set, skip mounting a fresh `/proc` even though PID isolation is
/// still enabled. This is primarily intended for restrictive container
/// environments that deny `--proc /proc`.
@@ -158,6 +162,7 @@ pub fn run_main() -> ! {
apply_seccomp_then_exec,
allow_network_for_proxy,
proxy_route_spec,
verify_fd_mounts,
no_proc,
command,
} = LandlockCommand::parse();
@@ -165,6 +170,9 @@ pub fn run_main() -> ! {
if command.is_empty() {
panic!("No command specified to execute.");
}
if !apply_seccomp_then_exec && !verify_fd_mounts.is_empty() {
panic!("--verify-fd-mount is only supported in the inner sandbox stage");
}
ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_legacy_landlock);
let EffectivePermissions {
permission_profile,
@@ -181,6 +189,10 @@ pub fn run_main() -> ! {
// Inner stage: apply seccomp/no_new_privs after bubblewrap has already
// established the filesystem view.
if apply_seccomp_then_exec {
if let Err(err) = crate::fd_mount::verify_fd_mounts(&verify_fd_mounts) {
panic!("failed to verify descriptor-backed bubblewrap mount: {err}");
}
let mut capability_header = [LINUX_CAPABILITY_VERSION_3, 0];
let mut capability_sets = [[0_u32; 3]; 2];
// SAFETY: capability ABI version 3 uses a [version, pid] header and