Mask daemon socket paths exposed through ancestor bind mounts (#47079)

## Why

The Linux sandbox rejected bind mounts of ancestors of the app-server socket directory, even when the exposed socket paths could be masked. This prevented sandbox startup with otherwise usable temporary-directory mount layouts.

## What changed

- Collect the socket directory and its exposed ancestor-mount aliases, then mask each path after filesystem binds, including read-only subpaths and the synthetic-mount registry.
- Continue rejecting direct directory or socket aliases and nested mounts that cannot preserve socket isolation.
- Reject non-UTF-8 mask destinations to avoid masking a different path through lossy conversion.

## Testing

Extend mount-analysis tests for ancestor aliases, private temporary directories, and WSLg masks. Extend socket-isolation coverage to verify that ancestor and registry aliases hide daemon sockets while unrelated sockets remain reachable. Add a regression test for non-UTF-8 mask paths.

GitOrigin-RevId: 49a9a764fbe1fddacd99c3a78099fbffff53b944
This commit is contained in:
Nick Steele
2026-09-21 18:04:14 +00:00
committed by copyberry
parent 62ea6d41e8
commit 7cec3e110c
5 changed files with 206 additions and 176 deletions
+67 -104
View File
@@ -423,7 +423,7 @@ fn create_filesystem_args(
options: BwrapOptions,
) -> Result<BwrapArgs> {
let daemon_directory = codex_uds::prepare_shared_daemon_socket_directory()?;
crate::daemon_mounts::reject_daemon_mount_aliases(
let daemon_directories = crate::daemon_mounts::daemon_socket_mask_paths(
&daemon_directory,
options
.mask_wslg_distro
@@ -571,14 +571,14 @@ fn create_filesystem_args(
args.push("--ro-bind".to_string());
args.push(path_to_string(&mount_root));
args.push(path_to_string(&mount_root));
append_daemon_socket_mask(&mut args, &mount_root, &daemon_directory)?;
append_daemon_socket_masks(&mut args, &mount_root, &daemon_directories)?;
}
}
args
};
if args[0] == "--ro-bind" {
append_daemon_socket_mask(&mut args, Path::new("/"), &daemon_directory)?;
append_daemon_socket_masks(&mut args, Path::new("/"), &daemon_directories)?;
}
let mut bwrap_args = BwrapArgs {
args,
@@ -637,7 +637,7 @@ fn create_filesystem_args(
bwrap_args.args.push("--bind".to_string());
bwrap_args.args.push(path_to_string(mount_root));
bwrap_args.args.push(path_to_string(mount_root));
append_daemon_socket_mask(&mut bwrap_args.args, mount_root, &daemon_directory)?;
append_daemon_socket_masks(&mut bwrap_args.args, mount_root, &daemon_directories)?;
let mut read_only_subpaths: Vec<PathBuf> = writable_root
.read_only_subpaths
@@ -664,7 +664,12 @@ fn create_filesystem_args(
);
read_only_subpaths.sort_by_key(|path| path_depth(path));
for subpath in read_only_subpaths {
append_read_only_subpath_args(&mut bwrap_args, &subpath, &allowed_write_paths)?;
append_read_only_subpath_args(
&mut bwrap_args,
&subpath,
&allowed_write_paths,
&daemon_directories,
)?;
}
// Protect the registry only where a writable bind exposes it. Apply
// this before deny masks so a denied parent stays hidden, rather than
@@ -682,6 +687,7 @@ fn create_filesystem_args(
bwrap_args.args.push("--ro-bind".to_string());
bwrap_args.args.push(path_to_string(registry_mount));
bwrap_args.args.push(path_to_string(registry_mount));
append_daemon_socket_masks(&mut bwrap_args.args, registry_mount, &daemon_directories)?;
}
let mut nested_unreadable_roots: Vec<PathBuf> = unreadable_roots
.iter()
@@ -777,27 +783,36 @@ fn append_metadata_path_masks_for_writable_root(
// Mask immediately after every bind that exposes the directory, before policy
// deny masks can hide its parent. Use the mount destination for read-root aliases.
fn append_daemon_socket_mask(
fn append_daemon_socket_masks(
args: &mut Vec<String>,
mount_root: &Path,
daemon_directory: &Path,
daemon_directories: &BTreeSet<PathBuf>,
) -> Result<()> {
let source = fs::canonicalize(mount_root)?;
if source.starts_with(daemon_directory) {
return Err(CodexErr::Fatal(
"app-server socket directory cannot be a sandbox mount root".to_string(),
));
}
if let Ok(relative) = daemon_directory.strip_prefix(source) {
let destination = path_to_string(&mount_root.join(relative));
args.extend([
"--perms".to_string(),
"000".to_string(),
"--tmpfs".to_string(),
destination.clone(),
"--remount-ro".to_string(),
destination,
]);
for daemon_directory in daemon_directories {
if source.starts_with(daemon_directory) {
return Err(CodexErr::Fatal(
"app-server socket directory cannot be a sandbox mount root".to_string(),
));
}
if let Ok(relative) = daemon_directory.strip_prefix(&source) {
let destination = mount_root.join(relative);
// A lossy conversion could mask a different path and expose the socket.
let destination = destination
.to_str()
.ok_or_else(|| {
CodexErr::Fatal("app-server socket mask path must be valid UTF-8".to_string())
})?
.to_string();
args.extend([
"--perms".to_string(),
"000".to_string(),
"--tmpfs".to_string(),
destination.clone(),
"--remount-ro".to_string(),
destination,
]);
}
}
Ok(())
}
@@ -1130,6 +1145,7 @@ fn append_read_only_subpath_args(
bwrap_args: &mut BwrapArgs,
subpath: &Path,
allowed_write_paths: &[PathBuf],
daemon_directories: &BTreeSet<PathBuf>,
) -> Result<()> {
if let Some(symlink) = first_writable_symlink_component_in_path(subpath, allowed_write_paths) {
/*
@@ -1175,11 +1191,7 @@ fn append_read_only_subpath_args(
bwrap_args.args.push("--ro-bind".to_string());
bwrap_args.args.push(path_to_string(subpath));
bwrap_args.args.push(path_to_string(subpath));
append_daemon_socket_mask(
&mut bwrap_args.args,
subpath,
&codex_uds::shared_daemon_socket_directory()?,
)?;
append_daemon_socket_masks(&mut bwrap_args.args, subpath, daemon_directories)?;
}
Ok(())
}
@@ -2175,84 +2187,35 @@ mod tests {
PathBuf::from("/dev/.codex"),
]
);
let dev_mount = args
.args
.windows(2)
.position(|args| args == ["--dev", "/dev"])
.expect("/dev mount");
let root_bind = args
.args
.windows(3)
.position(|args| args == ["--bind", "/", "/"])
.expect("root bind");
let dev_bind = args
.args
.windows(3)
.position(|args| args == ["--bind", "/dev", "/dev"])
.expect("/dev bind");
assert!(dev_mount < root_bind && root_bind < dev_bind);
}
#[test]
fn non_utf8_daemon_socket_mask_path_is_rejected() {
let daemon_directory =
path_to_string(&codex_uds::shared_daemon_socket_directory().unwrap());
assert_eq!(
args.args,
vec![
// Start from a read-only view of the full filesystem.
"--ro-bind".to_string(),
"/".to_string(),
"/".to_string(),
// Recreate a writable /dev inside the sandbox.
"--dev".to_string(),
"/dev".to_string(),
"--perms".to_string(),
"000".to_string(),
"--tmpfs".to_string(),
daemon_directory.clone(),
"--remount-ro".to_string(),
daemon_directory.clone(),
// Make the writable root itself writable again.
"--bind".to_string(),
"/".to_string(),
"/".to_string(),
"--perms".to_string(),
"000".to_string(),
"--tmpfs".to_string(),
daemon_directory.clone(),
"--remount-ro".to_string(),
daemon_directory,
// Mask the default metadata path names under the writable root.
// Because the root is `/` in this test, these carveout paths
// appear directly below `/`.
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/.git".to_string(),
"--remount-ro".to_string(),
"/.git".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/.agents".to_string(),
"--remount-ro".to_string(),
"/.agents".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/.codex".to_string(),
"--remount-ro".to_string(),
"/.codex".to_string(),
"--ro-bind".to_string(),
path_to_string(&synthetic_mount_registry_root()),
path_to_string(&synthetic_mount_registry_root()),
// Rebind /dev after the root bind so device nodes remain
// writable/usable inside the writable root.
"--bind".to_string(),
"/dev".to_string(),
"/dev".to_string(),
// Then mask the metadata names that would otherwise be
// creatable below the writable /dev bind.
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/dev/.git".to_string(),
"--remount-ro".to_string(),
"/dev/.git".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/dev/.agents".to_string(),
"--remount-ro".to_string(),
"/dev/.agents".to_string(),
"--perms".to_string(),
"555".to_string(),
"--tmpfs".to_string(),
"/dev/.codex".to_string(),
"--remount-ro".to_string(),
"/dev/.codex".to_string(),
]
PathBuf::from(OsString::from_vec(b"/tmp/codex-daemon-\xff".to_vec()));
assert!(
append_daemon_socket_masks(
&mut Vec::new(),
Path::new("/"),
&BTreeSet::from([daemon_directory]),
)
.is_err()
);
}
+29 -19
View File
@@ -1,4 +1,4 @@
//! Reject host mount aliases that would bypass the privileged socket directory mask.
//! Find host mount aliases that need the privileged socket directory mask.
//! Mount roots describe filesystem identity; canonical paths alone miss bind mounts.
use rustix::fs::AtFlags;
@@ -13,10 +13,10 @@ use std::os::unix::fs::MetadataExt;
use std::path::Path;
use std::path::PathBuf;
pub(crate) fn reject_daemon_mount_aliases(
pub(crate) fn daemon_socket_mask_paths(
directory: &Path,
masked_root: Option<&Path>,
) -> io::Result<()> {
) -> io::Result<BTreeSet<PathBuf>> {
let directory_file = fs::File::open(directory)?;
let device = directory_file.metadata()?.dev();
let mount_id = fs::read_to_string(format!("/proc/self/fdinfo/{}", directory_file.as_raw_fd()))
@@ -51,7 +51,7 @@ fn check_mounts(
mount_id: Option<&str>,
mountinfo: &[u8],
masked_root: Option<&Path>,
) -> io::Result<()> {
) -> io::Result<BTreeSet<PathBuf>> {
let invalid = || io::Error::other("cannot establish app-server socket mount isolation");
let mut mounts = Vec::new();
for line in mountinfo
@@ -125,33 +125,37 @@ fn check_mounts(
}
(locations.into_iter().next().ok_or_else(invalid)?, None)
};
let mut mask_paths = BTreeSet::from([directory.to_path_buf()]);
for (id, _, _, root, destination) in &mounts {
// Nested mounts can introduce another filesystem (or an individual socket) under the mask.
let nested = destination != directory && destination.starts_with(directory);
let alias = if let Some(root) = root {
if let Ok(relative) = location.strip_prefix(root) {
Some(destination.join(relative))
Some((destination.join(relative), !relative.as_os_str().is_empty()))
} else if root.starts_with(&location) {
Some(destination.clone())
Some((destination.clone(), false))
} else {
None
}
} else {
None
};
let exposed_alias = alias.filter(|(path, _)| {
// An ancestor's path beneath this mount is hidden by it. Keep
// checking other mounts, including aliases mounted beneath it.
let hidden = containing_mount.is_some_and(|(mount_id, containing_mount)| {
*id != mount_id.as_bytes()
&& containing_mount.starts_with(destination)
&& path.starts_with(containing_mount)
});
!path.starts_with(directory)
&& !masked_root.is_some_and(|root| path.starts_with(root))
&& !hidden
});
if nested
|| alias.is_some_and(|path| {
// An ancestor's path beneath this mount is hidden by it. Keep
// checking other mounts, including aliases mounted beneath it.
let hidden = containing_mount.is_some_and(|(mount_id, containing_mount)| {
*id != mount_id.as_bytes()
&& containing_mount.starts_with(destination)
&& path.starts_with(containing_mount)
});
!path.starts_with(directory)
&& !masked_root.is_some_and(|root| path.starts_with(root))
&& !hidden
})
|| exposed_alias
.as_ref()
.is_some_and(|(_, can_mask)| !can_mask)
{
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
@@ -161,8 +165,14 @@ fn check_mounts(
),
));
}
if let Some((path, _)) = exposed_alias {
// An alias of an ancestor exposes the whole directory, so apply the
// same mask there. Direct directory/socket aliases and nested mounts
// remain unsupported because a later bind could reopen the mask.
mask_paths.insert(path);
}
}
Ok(())
Ok(mask_paths)
}
fn mount_path(encoded: &[u8]) -> io::Result<PathBuf> {
@@ -8,21 +8,25 @@ fn check_mounts(
device: &str,
mount_id: Option<&str>,
mountinfo: &[u8],
) -> io::Result<()> {
) -> io::Result<BTreeSet<PathBuf>> {
super::check_mounts(
directory, device, mount_id, mountinfo, /*masked_root*/ None,
)
}
#[test_case("/tmp", "/host-tmp", false; "ancestor alias")]
#[test_case("/tmp", "/host-tmp", true; "ancestor alias")]
#[test_case("/tmp/codex-daemon-1000", "/alias", false; "directory alias")]
#[test_case("/tmp/codex-daemon-1000/rpc.sock", "/alias.sock", false; "socket alias")]
#[test_case("/", "/host", false; "root alias")]
#[test_case("/", "/host", true; "root alias")]
#[test_case("/workspace", "/project", true; "unrelated project bind")]
#[test_case("/tmp", "/tmp", true; "same location")]
#[test_case("/tmp", "/host\\040tmp", false; "escaped alias")]
#[test_case("/tmp", "/host\\040tmp", true; "escaped alias")]
#[test_case("/other", "/tmp/codex-daemon-1000/nested", false; "nested mount")]
fn rejects_only_mounts_that_compromise_the_directory(root: &str, destination: &str, allowed: bool) {
fn accepts_only_mounts_that_can_keep_the_directory_masked(
root: &str,
destination: &str,
allowed: bool,
) {
let mounts =
format!("1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 {root} {destination} rw - ext4 disk rw\n");
let visible_mount = if destination == "/tmp" { "2" } else { "1" };
@@ -64,6 +68,7 @@ fn validates_namespace_mounts_by_device_and_destination(
mount_id,
mounts.as_bytes()
)
.map(|_| ())
.map_err(|error| error.kind()),
expected,
"mount_id: {mount_id:?}"
@@ -87,16 +92,21 @@ fn unrelated_namespace_mount_does_not_hide_a_socket_alias() {
}
#[test]
fn rejects_alias_when_tmp_is_itself_a_bind_mount() {
fn masks_alias_when_tmp_is_itself_a_bind_mount() {
let mounts = b"1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 /backing/tmp /tmp rw - ext4 disk rw\n";
assert!(
let expected = BTreeSet::from([
PathBuf::from("/tmp/codex-daemon-1000"),
PathBuf::from("/backing/tmp/codex-daemon-1000"),
]);
assert_eq!(
check_mounts(
Path::new("/tmp/codex-daemon-1000"),
"0:1",
Some("2"),
mounts
)
.is_err()
.unwrap(),
expected
);
// A hidden deeper mount must not override the actual /tmp backing location.
let hidden = [
@@ -104,14 +114,15 @@ fn rejects_alias_when_tmp_is_itself_a_bind_mount() {
b"3 1 0:1 /tmp/codex-daemon-1000 /tmp/codex-daemon-1000 rw - ext4 disk rw\n",
]
.concat();
assert!(
assert_eq!(
check_mounts(
Path::new("/tmp/codex-daemon-1000"),
"0:1",
Some("2"),
&hidden
)
.is_err()
.unwrap(),
expected
);
}
@@ -161,7 +172,7 @@ fn rejects_mount_hidden_by_an_ancestor_overmount() {
#[test_case("/tmp/systemd-private-service/tmp", "/"; "tmp on root filesystem")]
#[test_case("/systemd-private-service/tmp", "/tmp"; "tmp on separate filesystem")]
fn accepts_private_tmp_bind_but_rejects_exposed_aliases(root: &str, parent: &str) {
fn accepts_private_tmp_bind_and_maskable_aliases(root: &str, parent: &str) {
let directory = Path::new("/tmp/codex-daemon-1000");
let mounts =
format!("1 0 0:1 / {parent} rw - ext4 disk rw\n2 1 0:1 {root} /tmp rw - ext4 disk rw\n");
@@ -169,29 +180,48 @@ fn accepts_private_tmp_bind_but_rejects_exposed_aliases(root: &str, parent: &str
// PrivateTmp remains ambiguous when neither fdinfo nor statx supplies an ID.
assert!(check_mounts(directory, "0:1", /*mount_id*/ None, mounts.as_bytes()).is_err());
// A real alias beneath /tmp is not hidden by the private /tmp mount.
for (alias_root, destination) in [
(root.to_owned(), "/tmp/exposed"),
// A real alias beneath /tmp needs a mask; a direct socket alias remains unsupported.
for (alias_root, destination, can_mask) in [
(root.to_owned(), "/tmp/exposed", true),
(
format!("{root}/codex-daemon-1000/rpc.sock"),
"/tmp/alias.sock",
false,
),
("/".to_owned(), "/host"),
("/".to_owned(), "/host", true),
] {
let exposed = format!("{mounts}3 2 0:1 {alias_root} {destination} rw - ext4 disk rw\n");
assert!(check_mounts(directory, "0:1", Some("2"), exposed.as_bytes()).is_err());
assert_eq!(
check_mounts(directory, "0:1", Some("2"), exposed.as_bytes()).is_ok(),
can_mask
);
}
}
#[test]
fn masked_wslg_alias_does_not_allow_other_exposed_aliases() {
fn masked_wslg_alias_does_not_skip_other_socket_masks() {
let mounts = "1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 / /mnt/wslg/distro rw - ext4 disk rw\n";
let directory = Path::new("/tmp/codex-daemon-1000");
let mask = Some(Path::new(crate::bwrap::WSLG_DISTRO_ROOT));
let exposed = format!("{mounts}3 1 0:1 /tmp /host-tmp rw - ext4 disk rw\n");
for mount_id in [Some("1"), None] {
assert!(check_mounts(directory, "0:1", mount_id, mounts.as_bytes()).is_err());
assert!(super::check_mounts(directory, "0:1", mount_id, mounts.as_bytes(), mask).is_ok());
assert!(super::check_mounts(directory, "0:1", mount_id, exposed.as_bytes(), mask).is_err());
assert_eq!(
check_mounts(directory, "0:1", mount_id, mounts.as_bytes()).unwrap(),
BTreeSet::from([
directory.to_path_buf(),
PathBuf::from("/mnt/wslg/distro/tmp/codex-daemon-1000"),
])
);
assert_eq!(
super::check_mounts(directory, "0:1", mount_id, mounts.as_bytes(), mask).unwrap(),
BTreeSet::from([directory.to_path_buf()])
);
assert_eq!(
super::check_mounts(directory, "0:1", mount_id, exposed.as_bytes(), mask).unwrap(),
BTreeSet::from([
directory.to_path_buf(),
PathBuf::from("/host-tmp/codex-daemon-1000"),
])
);
}
}
@@ -72,39 +72,10 @@ fn inserts_bwrap_argv0_before_command_separator() {
/*supports_argv0*/ true,
"/tmp/codex-arg0-session/codex-linux-sandbox".to_string(),
);
let daemon_directory = codex_uds::shared_daemon_socket_directory()
.unwrap()
.display()
.to_string();
let separator = argv.iter().position(|arg| arg == "--").unwrap();
assert_eq!(
argv,
vec![
"bwrap".to_string(),
"--new-session".to_string(),
"--die-with-parent".to_string(),
"--ro-bind".to_string(),
"/".to_string(),
"/".to_string(),
"--dev".to_string(),
"/dev".to_string(),
"--perms".to_string(),
"000".to_string(),
"--tmpfs".to_string(),
daemon_directory.clone(),
"--remount-ro".to_string(),
daemon_directory,
"--unshare-user".to_string(),
"--unshare-pid".to_string(),
"--unshare-ipc".to_string(),
"--proc".to_string(),
"/proc".to_string(),
"--cap-drop".to_string(),
"ALL".to_string(),
"--argv0".to_string(),
"codex-linux-sandbox".to_string(),
"--".to_string(),
"/bin/true".to_string(),
]
&argv[separator - 2..],
["--argv0", "codex-linux-sandbox", "--", "/bin/true"]
);
}
@@ -3,6 +3,7 @@ use pretty_assertions::assert_eq;
use std::os::fd::AsRawFd;
use std::os::unix::net::UnixListener;
use std::os::unix::net::UnixStream;
use std::path::PathBuf;
#[tokio::test]
async fn private_tmp_mount_preserves_daemon_socket_isolation() {
@@ -102,7 +103,54 @@ fn private_tmp_fixture() {
"--nocapture",
"--test-threads=1",
])
.env("CODEX_TEST_DAEMON_SOCKET", &endpoint);
.env("CODEX_TEST_DAEMON_SOCKET", &endpoint)
.env("TMPDIR", "/tmp");
let output = command.output().unwrap();
assert_eq!(output.status.code(), Some(0), "{output:?}");
assert!(String::from_utf8_lossy(&output.stdout).contains("private-tmp-isolated"));
// Devboxes expose their temporary volume through both /tmp and /build/tmp.
// An ancestor alias must receive the same socket mask without hiding other files.
std::fs::create_dir("/tmp/build").unwrap();
let mount = std::process::Command::new("mount")
.args(["--bind", "/tmp", "/tmp/build"])
.output()
.unwrap();
assert!(mount.status.success(), "{mount:?}");
let alias_endpoint = PathBuf::from("/tmp/build")
.join(root.file_name().unwrap())
.join("rpc.sock");
UnixStream::connect(&alias_endpoint).expect("host can reach ancestor alias");
command
.env("CODEX_TEST_DAEMON_ALIAS_SOCKET", &alias_endpoint)
.env("CODEX_TEST_OTHER_ALIAS_SOCKET", "/tmp/build/other.sock");
let output = command.output().unwrap();
assert_eq!(output.status.code(), Some(0), "{output:?}");
assert!(String::from_utf8_lossy(&output.stdout).contains("private-tmp-isolated"));
// Rebinding the synthetic-mount registry must preserve masks for aliases
// beneath it, even when the surrounding /tmp is writable.
let registry_alias = PathBuf::from(format!(
"/tmp/codex-bwrap-synthetic-mount-targets-{}/view",
unsafe { libc::geteuid() }
));
std::fs::create_dir_all(&registry_alias).unwrap();
let mount = std::process::Command::new("mount")
.args(["--bind", "/tmp"])
.arg(&registry_alias)
.output()
.unwrap();
assert!(mount.status.success(), "{mount:?}");
let alias_endpoint = registry_alias
.join(root.file_name().unwrap())
.join("rpc.sock");
UnixStream::connect(&alias_endpoint).expect("host can reach registry alias");
command
.env("CODEX_TEST_DAEMON_ALIAS_SOCKET", &alias_endpoint)
.env(
"CODEX_TEST_OTHER_ALIAS_SOCKET",
registry_alias.join("other.sock"),
);
let output = command.output().unwrap();
assert_eq!(output.status.code(), Some(0), "{output:?}");
assert!(String::from_utf8_lossy(&output.stdout).contains("private-tmp-isolated"));
@@ -133,6 +181,14 @@ fn private_tmp_client() {
println!("private-tmp-client-started");
assert!(UnixStream::connect(endpoint).is_err(), "daemon reachable");
UnixStream::connect("/tmp/other.sock").expect("unrelated socket reachable");
if let Some(alias) = std::env::var_os("CODEX_TEST_DAEMON_ALIAS_SOCKET") {
assert!(
UnixStream::connect(alias).is_err(),
"daemon alias reachable"
);
UnixStream::connect(std::env::var_os("CODEX_TEST_OTHER_ALIAS_SOCKET").unwrap())
.expect("unrelated alias socket reachable");
}
println!("private-tmp-isolated");
}