fix(sandbox): release broker-owned listening sockets (#4444)

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

crates/openshell-sandbox/README.md

@@ -9,3 +9,11 @@ socket identity, Landlock qualification, and the workload launch thread.
 the lower-level mechanisms used by that orchestration and the boundary server.
 Unsafe kernel ABI, process-launch, and descriptor-ownership operations stay in
 this runtime crate behind safe APIs where their contracts can be enforced.
+
+The network broker retains source socket descriptors only while preparing a
+workload endpoint. A successful connection or `listen` releases that reference;
+the kernel then tracks the lifetime through workload-owned descriptors. Native
+`accept` and repeated `listen` preserve the socket's loopback confinement. Closing
+the last workload reference to a listener removes it without waiting for registry
+collection. Broker source descriptors always use close-on-exec independently of
+the workload's requested descriptor flags.

crates/openshell-sandbox/src/linux/socket_registry.rs

@@ -119,10 +119,15 @@ impl SocketEntry {
     /// Replace the stable state. Callers perform policy and notification
     /// revalidation before invoking this commit primitive.
     pub fn set_state(&mut self, state: SocketState) {
+        if matches!(state, SocketState::Listening { .. }) {
+            // Native accept needs no broker reference. Only workload aliases
+            // should keep a listener alive after this transition.
+            self.release_preconnect();
+        }
         self.state = state;
     }
 
-    /// Close the temporary source descriptor after a connection commits.
+    /// Close the temporary source descriptor after a connection or listen commits.
     pub fn release_preconnect(&mut self) {
         self.retained_preconnect = None;
     }
@@ -268,10 +273,7 @@ impl SocketRegistry {
             ));
         }
         let identity = tentative.identity;
-        let retain_source = matches!(
-            state,
-            SocketState::Created | SocketState::Bound { .. } | SocketState::Listening { .. }
-        );
+        let retain_source = matches!(state, SocketState::Created | SocketState::Bound { .. });
         self.entries.insert(
             identity.inode,
             SocketEntry {
@@ -369,6 +371,24 @@ mod tests {
         }
     }
 
+    #[test]
+    fn listening_commit_does_not_retain_source() {
+        let mut registry = SocketRegistry::new(7, 1).unwrap();
+        let source = tcp_socket();
+        let installed = rustix::io::fcntl_dupfd_cloexec(&source, 3).unwrap();
+        let tentative = registry.stage(source, metadata()).unwrap();
+        let local = "127.0.0.1:12345".parse().unwrap();
+        registry
+            .commit_with_state(tentative, SocketState::Listening { local })
+            .unwrap();
+        let entry = registry
+            .resolve(std::process::id(), installed.as_raw_fd())
+            .unwrap();
+        assert_eq!(entry.state(), &SocketState::Listening { local });
+        assert!(entry.retained_preconnect().is_err());
+        assert_eq!(registry.retained_preconnect_count(), 0);
+    }
+
     #[test]
     fn tentative_entry_is_invisible_until_commit() {
         let mut registry = SocketRegistry::new(7, 1).unwrap();

crates/openshell-sandbox/src/network_broker.rs

@@ -677,14 +677,17 @@ fn create_socket(
     if let Err(error) = prepare_registry_for_socket(registry, descriptor_soft_limit) {
         return listener.respond_errno(notification.id, error_to_errno(&error));
     }
+    // The broker's copy must never survive workload exec, even when the
+    // injected descriptor is intentionally inheritable in the workload.
+    let source_kind = raw_kind | libc::SOCK_CLOEXEC;
     // SAFETY: arguments were reduced to the supported native INET matrix. A
     // successful call returns one newly owned descriptor.
-    let mut source = unsafe { libc::socket(domain, raw_kind, protocol) };
+    let mut source = unsafe { libc::socket(domain, source_kind, protocol) };
     if source < 0 && io::Error::last_os_error().raw_os_error() == Some(libc::EMFILE) {
         collect_closed_socket_entries(registry)?;
         // SAFETY: same validated native INET socket creation after reclaiming
         // broker-held descriptors for closed workload sockets.
-        source = unsafe { libc::socket(domain, raw_kind, protocol) };
+        source = unsafe { libc::socket(domain, source_kind, protocol) };
     }
     if source < 0 {
         return Err(io::Error::last_os_error());
@@ -1150,7 +1153,10 @@ fn repeated_connect_outcome(
     destination: SocketAddr,
 ) -> Option<i32> {
     match state {
-        SocketState::Created | SocketState::Bound { .. } | SocketState::Listening { .. } => None,
+        SocketState::Created | SocketState::Bound { .. } => None,
+        // A listening TCP socket cannot be connected, and no longer has a
+        // broker-owned descriptor on which to attempt it.
+        SocketState::Listening { .. } => Some(libc::EISCONN),
         // UDP connect replaces the association; repeating the same one succeeds.
         SocketState::DnsUdp { relay } if *relay == destination => Some(0),
         SocketState::Local { peer } if kind == InetKind::DnsUdp && *peer == destination => Some(0),
@@ -1268,6 +1274,9 @@ fn bind_socket(
     let bind_result = {
         let mut registry = lock(registry);
         let entry = registry.resolve_mut(notification.tid, fd)?;
+        if matches!(entry.state(), SocketState::Listening { .. }) {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         // A native bind is never restarted, but a notified one can be after a
         // signal. Report a repeat of the bind the broker completed as success.
         if entry.state() == &(SocketState::Bound { local }) {
@@ -1316,6 +1325,11 @@ fn listen_socket(
     let Ok(entry) = registry.resolve_mut(notification.tid, fd) else {
         return listener.respond_continue(notification.id);
     };
+    if matches!(entry.state(), SocketState::Listening { .. }) {
+        // Backlog changes and restarted notifications operate on the workload
+        // descriptor. Confinement is standing kernel state, as for accept.
+        return listener.respond_continue(notification.id);
+    }
     // listen(2) may be repeated natively, so a restart needs no special case.
     listener.validate_id(notification.id)?;
     // SAFETY: retained descriptor is the exact registered socket OFD.
@@ -1724,6 +1738,162 @@ mod tests {
     use super::*;
     use crate::linux::socket_confinement;
 
+    fn assert_listener_closed(address: SocketAddr, inode: u64) {
+        let table = if address.is_ipv4() {
+            "/proc/net/tcp"
+        } else {
+            "/proc/net/tcp6"
+        };
+        let inode = inode.to_string();
+        assert!(
+            !std::fs::read_to_string(table).unwrap().lines().any(|line| {
+                let fields: Vec<_> = line.split_whitespace().collect();
+                fields.get(3) == Some(&"0A") && fields.get(9) == Some(&inode.as_str())
+            }),
+            "closed listener remains in {table}"
+        );
+        assert_eq!(
+            TcpStream::connect_timeout(&address, Duration::from_secs(1))
+                .expect_err("closed listener accepted a connection")
+                .raw_os_error(),
+            Some(libc::ECONNREFUSED)
+        );
+        // Check disappearance and refusal before bind can trigger collection.
+        let _replacement = TcpListener::bind(address).expect("reuse listener port");
+    }
+
+    #[test]
+    fn listening_socket_lives_only_as_long_as_workload_aliases() {
+        for address in ["127.0.0.1:0", "[::1]:0"] {
+            let address: SocketAddr = address.parse().unwrap();
+            let (launcher, listener) = crate::linux::workload_launcher::start().unwrap();
+            let broker = NetworkBroker::start_for_test(listener).unwrap();
+            let (socket, alias, address, inode) = launcher
+                .execute(move || -> io::Result<_> {
+                    let socket = socket2::Socket::new(
+                        socket2::Domain::for_address(address),
+                        socket2::Type::STREAM,
+                        None,
+                    )?;
+                    socket.bind(&address.into())?;
+                    socket.listen(1)?;
+                    socket.listen(8)?;
+                    let address = socket.local_addr()?.as_socket().unwrap();
+                    assert_eq!(
+                        socket.bind(&address.into()).unwrap_err().raw_os_error(),
+                        Some(libc::EINVAL)
+                    );
+                    assert_eq!(
+                        socket.connect(&address.into()).unwrap_err().raw_os_error(),
+                        Some(libc::EISCONN)
+                    );
+                    let inode = crate::linux::proc_fd::socket_inode(
+                        std::process::id(),
+                        socket.as_raw_fd(),
+                    )?;
+                    Ok((socket.try_clone()?, socket, address, inode))
+                })
+                .unwrap()
+                .unwrap();
+            let registry = lock(&broker.registry);
+            let entry = registry
+                .resolve(std::process::id(), socket.as_raw_fd())
+                .unwrap();
+            assert!(matches!(entry.state(), SocketState::Listening { .. }));
+            assert!(entry.retained_preconnect().is_err());
+            drop(registry);
+            drop(socket);
+            let _client = TcpStream::connect_timeout(&address, Duration::from_secs(1)).unwrap();
+            drop(alias);
+            assert_listener_closed(address, inode);
+        }
+    }
+
+    #[test]
+    fn implicit_listen_releases_source_and_failed_listen_retains_it() {
+        let (launcher, listener) = crate::linux::workload_launcher::start().unwrap();
+        let broker = NetworkBroker::start_for_test(listener).unwrap();
+        let (address, inode, udp) = launcher
+            .execute(|| -> io::Result<_> {
+                let socket =
+                    socket2::Socket::new(socket2::Domain::IPV4, socket2::Type::STREAM, None)?;
+                socket.listen(1)?;
+                let address = socket.local_addr()?.as_socket().unwrap();
+                let inode =
+                    crate::linux::proc_fd::socket_inode(std::process::id(), socket.as_raw_fd())?;
+                drop(socket);
+                let udp = socket2::Socket::new(socket2::Domain::IPV4, socket2::Type::DGRAM, None)?;
+                assert_eq!(
+                    udp.listen(1).unwrap_err().raw_os_error(),
+                    Some(libc::EOPNOTSUPP)
+                );
+                Ok((address, inode, udp))
+            })
+            .unwrap()
+            .unwrap();
+        let registry = lock(&broker.registry);
+        let entry = registry
+            .resolve(std::process::id(), udp.as_raw_fd())
+            .unwrap();
+        assert_eq!(entry.state(), &SocketState::Created);
+        assert!(entry.retained_preconnect().is_ok());
+        drop(registry);
+        assert_listener_closed(address, inode);
+    }
+
+    #[test]
+    fn source_cloexec_is_independent_of_workload_inheritance() {
+        use rustix::io::{FdFlags, fcntl_getfd};
+        for close_on_exec in [false, true] {
+            let (launcher, listener) = crate::linux::workload_launcher::start().unwrap();
+            let broker = NetworkBroker::start_for_test(listener).unwrap();
+            let socket = launcher
+                .execute(move || -> io::Result<OwnedFd> {
+                    let kind =
+                        libc::SOCK_STREAM | if close_on_exec { libc::SOCK_CLOEXEC } else { 0 };
+                    // SAFETY: supported socket arguments; a successful result
+                    // transfers one injected descriptor into OwnedFd.
+                    let fd = unsafe { libc::socket(libc::AF_INET, kind, 0) };
+                    if fd < 0 {
+                        return Err(io::Error::last_os_error());
+                    }
+                    // SAFETY: the successful call returned one owned descriptor.
+                    Ok(unsafe { OwnedFd::from_raw_fd(fd) })
+                })
+                .unwrap()
+                .unwrap();
+            assert_eq!(
+                fcntl_getfd(&socket).unwrap().contains(FdFlags::CLOEXEC),
+                close_on_exec
+            );
+            let registry = lock(&broker.registry);
+            let entry = registry
+                .resolve(std::process::id(), socket.as_raw_fd())
+                .unwrap();
+            assert!(
+                fcntl_getfd(entry.retained_preconnect().unwrap())
+                    .unwrap()
+                    .contains(FdFlags::CLOEXEC)
+            );
+            let inode = entry.identity().inode;
+            drop(registry);
+            let output = std::process::Command::new("/bin/sh")
+                .args(["-c", "for fd in /proc/$$/fd/*; do readlink \"$fd\"; done"])
+                .output()
+                .unwrap();
+            let inherited = String::from_utf8(output.stdout)
+                .unwrap()
+                .lines()
+                .filter(|line| *line == format!("socket:[{inode}]"))
+                .count();
+            assert_eq!(
+                inherited,
+                usize::from(!close_on_exec),
+                "broker source leaked across exec"
+            );
+        }
+    }
+
     #[test]
     fn provider_files_are_opened_on_demand_and_replaced() {
         let (launcher, listener) =

e2e/rust/tests/listener_lifetime.rs

@@ -0,0 +1,154 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+#![cfg(feature = "e2e")]
+
+use openshell_e2e::harness::sandbox::SandboxGuard;
+
+const LIFETIME_PROBE: &str = r"
+import errno, json, os, socket, subprocess, sys
+
+def assert_closed(address, inode):
+    # Check before bind can trigger the broker's stale-descriptor collection.
+    for table in ('/proc/net/tcp', '/proc/net/tcp6'):
+        for line in open(table):
+            fields = line.split()
+            assert not (len(fields) > 9 and fields[3] == '0A' and fields[9] == str(inode)), line
+    with socket.socket() as client:
+        client.settimeout(2)
+        assert client.connect_ex(tuple(address)) == errno.ECONNREFUSED, address
+    with socket.socket() as replacement:
+        replacement.bind(tuple(address))
+
+def inode(sock):
+    return int(os.readlink('/proc/self/fd/' + str(sock.fileno()))[8:-1])
+
+with socket.socket() as bound:
+    bound.bind(('127.0.0.1', 0))
+    address = bound.getsockname()
+with socket.socket() as client:
+    client.settimeout(2)
+    assert client.connect_ex(address) == errno.ECONNREFUSED
+
+for host in ('127.0.0.1', '::1'):
+    family = socket.AF_INET if host == '127.0.0.1' else socket.AF_INET6
+    listener = socket.socket(family)
+    listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+    listener.bind((host, 0))
+    listener.listen(1)
+    listener.listen(8)
+    address, identity = listener.getsockname(), inode(listener)
+    for operation, expected in ((lambda: listener.bind(address), errno.EINVAL),
+                                (lambda: listener.connect(address), errno.EISCONN)):
+        try:
+            operation()
+            raise AssertionError('operation on listener unexpectedly succeeded')
+        except OSError as error:
+            assert error.errno == expected, error
+    alias = listener.dup()
+    alias.settimeout(2)
+    listener.close()
+    with socket.socket(family) as client:
+        client.settimeout(2)
+        client.connect(address)
+        accepted, _ = alias.accept()
+        with accepted:
+            accepted.sendall(b'ok')
+            assert client.recv(2) == b'ok'
+    alias.close()
+    table = '/proc/net/tcp' if family == socket.AF_INET else '/proc/net/tcp6'
+    for line in open(table):
+        fields = line.split()
+        assert not (len(fields) > 9 and fields[3] == '0A' and fields[9] == str(identity)), line
+    with socket.socket(family) as client:
+        client.settimeout(2)
+        assert client.connect_ex(address) == errno.ECONNREFUSED
+    with socket.socket(family) as replacement:
+        replacement.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        replacement.bind(address)
+
+# The child exits while still owning its listener. Its broker must not keep it alive.
+child = r'''
+import json, os, socket
+s = socket.socket()
+s.bind(('127.0.0.1', 0))
+s.listen()
+inode = int(os.readlink('/proc/self/fd/' + str(s.fileno()))[8:-1])
+print(json.dumps([s.getsockname(), inode]), flush=True)
+os._exit(0)
+'''
+result = subprocess.run([sys.executable, '-c', child], check=True, capture_output=True, text=True, timeout=10)
+address, identity = json.loads(result.stdout)
+assert_closed(address, identity)
+print(json.dumps([address, identity]))
+";
+
+#[tokio::test]
+async fn closing_or_exiting_listener_refuses_connections_and_releases_port() {
+    let mut sandbox = SandboxGuard::create(&[]).await.expect("create sandbox");
+    let output = sandbox
+        .exec(&["python3", "-c", LIFETIME_PROBE])
+        .await
+        .expect("listener lifetime probe");
+    let observer = r"
+import json, os, sys
+_, inode = json.loads(sys.argv[1])
+for fd in os.listdir('/proc/self/fd'):
+    try:
+        target = os.readlink('/proc/self/fd/' + fd)
+    except FileNotFoundError:
+        continue
+    assert target != 'socket:[' + str(inode) + ']', (fd, target)
+";
+    sandbox
+        .exec(&["python3", "-c", observer, output.trim()])
+        .await
+        .expect("subsequent exec must not inherit closed listener");
+    sandbox.cleanup().await;
+}
+
+#[tokio::test]
+async fn unrelated_exec_does_not_inherit_broker_preconnect_socket() {
+    // Request a non-CLOEXEC socket explicitly; Python's socket constructor
+    // otherwise masks broker inheritance bugs by requesting SOCK_CLOEXEC.
+    let owner = r"
+import ctypes, json, os, socket, time
+libc = ctypes.CDLL(None, use_errno=True)
+fd = libc.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
+assert fd >= 0, ctypes.get_errno()
+assert os.get_inheritable(fd)
+inode = int(os.readlink('/proc/self/fd/' + str(fd))[8:-1])
+with open('/sandbox/preconnect-inode.json', 'w') as f:
+    json.dump(inode, f)
+while True:
+    time.sleep(60)
+";
+    let mut sandbox = SandboxGuard::create_detached_main(&["python3", "-c", owner])
+        .await
+        .expect("create socket owner");
+    let observer = r"
+import json, os, time
+path = '/sandbox/preconnect-inode.json'
+for _ in range(100):
+    try:
+        with open(path) as f:
+            inode = json.load(f)
+        break
+    except (FileNotFoundError, json.JSONDecodeError):
+        time.sleep(0.1)
+else:
+    raise AssertionError('socket owner did not publish its inode')
+for fd in os.listdir('/proc/self/fd'):
+    try:
+        target = os.readlink('/proc/self/fd/' + fd)
+    except FileNotFoundError:
+        continue
+    assert target != 'socket:[' + str(inode) + ']', (fd, target)
+print('broker socket was not inherited')
+";
+    sandbox
+        .exec(&["python3", "-c", observer])
+        .await
+        .expect("unrelated exec inherited broker descriptor");
+    sandbox.cleanup().await;
+}