mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Allow unrelated namespace mounts in Linux sandbox socket checks (#46535)
## Why Namespace mounts can have roots such as `mnt:[inode]` or `net:[inode]` that are not filesystem paths. Treating every mount root as a path rejects these unrelated mounts and prevents sandbox startup. ## What changed Parse mount roots as paths only for the daemon socket's filesystem. Continue validating every mount destination and checking mount ancestry, nested mounts, and socket aliases. ## Testing Add regression cases for namespace mounts, invalid destinations, and socket aliases, both with and without a mount ID. Extend the socket-isolation integration fixture with an unrelated network namespace mount to exercise successful startup and rejection of a real socket alias. GitOrigin-RevId: 9a1d2a69e5bd3f8d09b1de7cf998ee6d9c6bb980
This commit is contained in:
@@ -62,8 +62,13 @@ fn check_mounts(
|
||||
let [id, parent, mount_device, root, destination] = fields.as_slice() else {
|
||||
return Err(invalid());
|
||||
};
|
||||
let root = mount_path(root)?;
|
||||
let destination = mount_path(destination)?;
|
||||
// Only roots on the socket filesystem can identify aliases. Other
|
||||
// filesystems can use non-path roots such as nsfs `mnt:[inode]`, but
|
||||
// their destinations still matter for ancestry and nested-mount checks.
|
||||
let root = (*mount_device == device.as_bytes())
|
||||
.then(|| mount_path(root))
|
||||
.transpose()?;
|
||||
mounts.push((*id, *parent, *mount_device, root, destination));
|
||||
}
|
||||
let (location, containing_mount) = if let Some(mount_id) = mount_id {
|
||||
@@ -77,6 +82,7 @@ fn check_mounts(
|
||||
if *mount_device != device.as_bytes() {
|
||||
return Err(invalid());
|
||||
}
|
||||
let root = root.as_ref().ok_or_else(invalid)?;
|
||||
let relative = directory.strip_prefix(destination).map_err(|_| invalid())?;
|
||||
let mut current = Some(selected);
|
||||
let mut visible_child: Option<&Path> = None;
|
||||
@@ -106,8 +112,8 @@ fn check_mounts(
|
||||
// on the backing location, and do not assume any aliases are hidden.
|
||||
let locations: BTreeSet<_> = mounts
|
||||
.iter()
|
||||
.filter(|(_, _, mount_device, ..)| *mount_device == device.as_bytes())
|
||||
.filter_map(|(_, _, _, root, destination)| {
|
||||
let root = root.as_ref()?;
|
||||
directory
|
||||
.strip_prefix(destination)
|
||||
.ok()
|
||||
@@ -119,10 +125,10 @@ fn check_mounts(
|
||||
}
|
||||
(locations.into_iter().next().ok_or_else(invalid)?, None)
|
||||
};
|
||||
for (id, _, mount_device, root, destination) in &mounts {
|
||||
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 *mount_device == device.as_bytes() {
|
||||
let alias = if let Some(root) = root {
|
||||
if let Ok(relative) = location.strip_prefix(root) {
|
||||
Some(destination.join(relative))
|
||||
} else if root.starts_with(&location) {
|
||||
|
||||
@@ -41,6 +41,51 @@ fn rejects_only_mounts_that_compromise_the_directory(root: &str, destination: &s
|
||||
}
|
||||
}
|
||||
|
||||
#[test_case("0:2", "mnt:[4026532835]", "/run/snapd/ns/example.mnt", Ok(()); "unrelated mount namespace")]
|
||||
#[test_case("0:2", "net:[4026531840]", "/run/netns/example", Ok(()); "unrelated network namespace")]
|
||||
#[test_case("0:2", "mnt:[4026532835]", "/tmp/codex-daemon-1000/ns", Err(io::ErrorKind::PermissionDenied); "nested namespace mount")]
|
||||
#[test_case("0:1", "mnt:[4026532835]", "/run/snapd/ns/example.mnt", Err(io::ErrorKind::Other); "non-path root on socket filesystem")]
|
||||
#[test_case("0:2", "mnt:[4026532835]", "relative/ns", Err(io::ErrorKind::Other); "relative destination")]
|
||||
#[test_case("0:2", "mnt:[4026532835]", "/run/snapd/ns/\\invalid", Err(io::ErrorKind::Other); "invalid destination escape")]
|
||||
fn validates_namespace_mounts_by_device_and_destination(
|
||||
device: &str,
|
||||
root: &str,
|
||||
destination: &str,
|
||||
expected: Result<(), io::ErrorKind>,
|
||||
) {
|
||||
let mounts = format!(
|
||||
"1 0 0:1 / / rw - ext4 disk rw\n2 1 {device} {root} {destination} rw - nsfs nsfs rw\n"
|
||||
);
|
||||
for mount_id in [Some("1"), None] {
|
||||
assert_eq!(
|
||||
check_mounts(
|
||||
Path::new("/tmp/codex-daemon-1000"),
|
||||
"0:1",
|
||||
mount_id,
|
||||
mounts.as_bytes()
|
||||
)
|
||||
.map_err(|error| error.kind()),
|
||||
expected,
|
||||
"mount_id: {mount_id:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unrelated_namespace_mount_does_not_hide_a_socket_alias() {
|
||||
let mounts = b"1 0 0:1 / / rw - ext4 disk rw\n\
|
||||
2 1 0:2 net:[4026531840] /run/netns/example rw - nsfs nsfs rw\n\
|
||||
3 1 0:1 /tmp/codex-daemon-1000 /alias rw - ext4 disk rw\n";
|
||||
for mount_id in [Some("1"), None] {
|
||||
assert_eq!(
|
||||
check_mounts(Path::new("/tmp/codex-daemon-1000"), "0:1", mount_id, mounts)
|
||||
.map_err(|error| error.kind()),
|
||||
Err(io::ErrorKind::PermissionDenied),
|
||||
"mount_id: {mount_id:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_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";
|
||||
|
||||
@@ -65,6 +65,15 @@ fn private_tmp_fixture() {
|
||||
if !mount.status.success() {
|
||||
std::process::exit(/*code*/ 77);
|
||||
}
|
||||
// Namespace mounts have non-path roots such as `net:[inode]`. An unrelated
|
||||
// one must not stop startup or disable the socket-isolation checks below.
|
||||
let namespace = "/tmp/network-namespace";
|
||||
std::fs::File::create(namespace).unwrap();
|
||||
let mount = std::process::Command::new("mount")
|
||||
.args(["--bind", "/proc/self/ns/net", namespace])
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(mount.status.success(), "{mount:?}");
|
||||
let root = codex_uds::prepare_shared_daemon_socket_directory().unwrap();
|
||||
let endpoint = root.join("rpc.sock");
|
||||
let _daemon = UnixListener::bind(&endpoint).unwrap();
|
||||
|
||||
Reference in New Issue
Block a user