From 89d7eb9202f77b7a62efb9ac47a246a407272bbe Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 1 Oct 2026 15:01:58 -0400 Subject: [PATCH 1/7] fix(sandbox): answer getpeername from the kernel for directly connected sockets getpeername(2) was always answered by writing the sockaddr into the workload's address space from the broker. On kernels without SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV the broker runs in LegacyReadOnly mode, where that cross-process write fails closed with EOPNOTSUPP, so getpeername failed for every socket. For SocketState::Local and SocketState::AcceptedLocal the descriptor is genuinely connected to the recorded peer, so the kernel's own answer is identical to the broker's. Respond with SECCOMP_USER_NOTIF_FLAG_CONTINUE for those states and let the kernel write the sockaddr in the workload's own address space. That needs no cross-process task-memory write and therefore works on kernels before 5.19. SocketState::Connected keeps the existing substitution. Those descriptors are connected to a loopback relay rather than to the destination the workload requested, so the kernel's answer would leak the relay address. A regression test pins that distinction. This is the path libuv-based runtimes depend on: libuv passes a null peer address to accept4(2) and resolves the peer lazily through getpeername(2) when the application reads remoteAddress. Refs #4058 Signed-off-by: Russell Bryant --- .../openshell-sandbox/src/network_broker.rs | 105 +++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/openshell-sandbox/src/network_broker.rs b/crates/openshell-sandbox/src/network_broker.rs index 56c857ce9c..4db17c1364 100644 --- a/crates/openshell-sandbox/src/network_broker.rs +++ b/crates/openshell-sandbox/src/network_broker.rs @@ -1580,8 +1580,19 @@ fn get_peer_name( return listener.respond_continue(notification.id); }; let peer = match entry.state() { + // The relay path leaves the workload's descriptor connected to a + // loopback relay, so the kernel's peer is the relay address rather + // than the destination the workload asked for. The broker must keep + // writing `original_peer` itself. SocketState::Connected { original_peer } => *original_peer, - SocketState::Local { peer } | SocketState::AcceptedLocal { peer } => *peer, + // These descriptors really are connected to the recorded peer, so the + // kernel's own answer is identical to the broker's. Letting the kernel + // write it keeps the sockaddr store inside the workload's address + // space, which needs no cross-process task-memory write and therefore + // works on kernels without SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV. + SocketState::Local { .. } | SocketState::AcceptedLocal { .. } => { + return listener.respond_continue(notification.id); + } _ => return Err(io::Error::from_raw_os_error(libc::ENOTCONN)), }; write_socket_addr( @@ -2041,6 +2052,98 @@ mod tests { assert_eq!(error.raw_os_error(), Some(libc::EOPNOTSUPP)); } + /// Stage one registered TCP socket in `state` and return the registry + /// alongside the descriptor number the workload would see. + fn registry_with_state(state: SocketState) -> (Mutex, OwnedFd) { + let metadata = SocketMetadata { + family: InetFamily::V4, + kind: InetKind::Tcp, + close_on_exec: true, + nonblocking: false, + creator_generation: 1, + }; + // SAFETY: a successful socket call returns one new owned descriptor. + let fd = unsafe { libc::socket(libc::AF_INET, libc::SOCK_STREAM, 0) }; + assert!(fd >= 0, "socket: {}", io::Error::last_os_error()); + // SAFETY: successful socket returned one owned descriptor. + let socket = unsafe { OwnedFd::from_raw_fd(fd) }; + let installed = duplicate_close_on_exec(fd).unwrap(); + let mut registry = SocketRegistry::new(1, 2).unwrap(); + let tentative = registry.stage(socket, metadata).unwrap(); + registry.commit_with_state(tentative, state).unwrap(); + (Mutex::new(registry), installed) + } + + /// A `NotificationListener` whose descriptor is not a seccomp listener, so + /// any ioctl fails. Only useful for discriminating *which* path was taken. + fn fake_legacy_listener() -> NotificationListener { + // SAFETY: dup returns a new descriptor or a negative error. + let dup = unsafe { libc::dup(libc::STDERR_FILENO) }; + assert!(dup >= 0, "dup stderr"); + NotificationListener::from_fd_with_mode( + // SAFETY: successful dup returned a new owned descriptor. + unsafe { OwnedFd::from_raw_fd(dup) }, + ListenerMode::LegacyReadOnly, + ) + } + + fn getpeername_notification(fd: RawFd) -> Notification { + Notification { + id: 1, + // SAFETY: gettid has no arguments or side effects. + tid: u32::try_from(unsafe { libc::syscall(libc::SYS_gettid) }).unwrap(), + syscall: i32::try_from(libc::SYS_getpeername).unwrap(), + args: [u64::try_from(fd).unwrap(), 0x1000, 0x2000, 0, 0, 0], + } + } + + #[test] + fn legacy_getpeername_continues_for_locally_connected_sockets() { + // The descriptor really is connected to `peer`, so the kernel's own + // answer equals the broker's. Responding CONTINUE lets the kernel + // write the sockaddr in the workload's own address space, which needs + // no WAIT_KILLABLE_RECV and therefore works on kernels < 5.19. + let peer: SocketAddr = "127.0.0.1:8080".parse().unwrap(); + for (label, state) in [ + ("AcceptedLocal", SocketState::AcceptedLocal { peer }), + ("Local", SocketState::Local { peer }), + ] { + let (registry, installed) = registry_with_state(state); + let listener = fake_legacy_listener(); + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .expect_err("the fake listener cannot complete any ioctl"); + assert_ne!( + error.raw_os_error(), + Some(libc::EOPNOTSUPP), + "{label} must not route through the fail-closed task-memory write" + ); + } + } + + #[test] + fn legacy_getpeername_still_substitutes_the_original_peer_for_relayed_sockets() { + // A `Connected` descriptor is connected to a loopback relay, not to + // the destination the workload asked for. CONTINUE here would hand the + // workload the relay's ephemeral address, so the broker must keep + // writing `original_peer` itself -- and keep failing closed when it + // cannot. + let (registry, installed) = registry_with_state(SocketState::Connected { + original_peer: "203.0.113.7:443".parse().unwrap(), + }); + let listener = fake_legacy_listener(); + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .expect_err("legacy listener must reject socket-address writes"); + assert_eq!(error.raw_os_error(), Some(libc::EOPNOTSUPP)); + } + #[test] fn relay_rejects_descriptor_replaced_after_policy_decision() { let metadata = SocketMetadata { From 2cde3117a8a2d116831972ce8744be4f10c72224 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 1 Oct 2026 16:17:52 -0400 Subject: [PATCH 2/7] feat(sandbox): restore peer addresses on kernels without WAIT_KILLABLE_RECV Kernels before Linux 5.19 lack SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV, so the broker cannot hold a notified workload thread in a kill-only wait and cannot safely write into workload memory. It fails closed with EOPNOTSUPP on every mediated syscall whose result is an output buffer. accept(fd, &addr, &len) has exactly that shape, so server workloads on RHEL 9.x and RHCOS see EOPNOTSUPP where they expect a connection. Add openshell-accept-shim, a freestanding preloadable library that rewrites an address-bearing accept into accept4(fd, NULL, NULL, flags) followed by getpeername on the accepted descriptor. The first call has no output buffer, so the broker injects the descriptor without touching workload memory; the second is answered with SECCOMP_USER_NOTIF_FLAG_CONTINUE, so the kernel writes the address into the caller's own buffer. Because the workload writes its own memory, the cross-process TOCTOU that WAIT_KILLABLE_RECV exists to close does not apply. The sandbox installs the object at /run/openshell-compat only when the listener reports writes disabled, and composes LD_PRELOAD so a workload's own value is preserved. Installation failure is non-fatal and logged as an OCSF config state change; the sandbox keeps the existing fail-closed behavior. Landlock gates open independently of the file mode, so a world-readable object is not by itself reachable: without an explicit admission the loader reports "cannot open shared object file" and silently drops the shim. Workload children are launched from more than one place -- the sandbox entrypoint and sandbox exec through the boundary -- and each prepares Landlock with its own runtime paths. Admit the object and its directory inside prepare_child_sandbox, the single production entry point, so a launch path cannot set LD_PRELOAD without also letting the loader open what it points at. Resolve the shim's C compiler through the cc crate so the standard CC_, TARGET_CC, and CC overrides apply, along with the per-target wrappers cargo-zigbuild installs when release binaries are cross-compiled from a non-Linux host. Verify the emitted object's ELF machine against CARGO_CFG_TARGET_ARCH and fail the build on a mismatch: a misresolved compiler otherwise produces a valid object for the wrong architecture, which the loader reports as "cannot open shared object file" and is indistinguishable from a policy denial. This is a compatibility aid, not a security control. Nothing in OpenShell trusts its output: loopback and authorization decisions continue to use the kernel's peer address from the broker's own accept4. A workload that unsets LD_PRELOAD, links statically, or issues raw syscalls gains nothing it did not already have. Coverage follows from LD_PRELOAD interposing symbols rather than syscalls: Bun and CPython are covered, Node.js does not need it because libuv passes a null address and resolves the peer lazily through getpeername, and Go and statically linked binaries remain unsupported for address-bearing accept. getpeername is fixed for every runtime because the kernel answers it. The object is built freestanding with -nostdlib so one build per architecture loads under both glibc and musl, carries no DT_NEEDED entry and no text relocations, and leaves __errno_location as its sole undefined symbol. Add an e2e test that asserts what the workload observes rather than how the sandbox arranges it: a listener accepts a connection from a client in the same sandbox and must report the client's address, not EOPNOTSUPP and not its own. It passes unchanged on both kernel generations. Signed-off-by: Russell Bryant --- AGENTS.md | 1 + Cargo.lock | 9 + crates/openshell-accept-shim/Cargo.toml | 23 ++ crates/openshell-accept-shim/README.md | 142 +++++++ crates/openshell-accept-shim/build.rs | 99 +++++ .../openshell-accept-shim/src/accept_shim.c | 131 +++++++ crates/openshell-accept-shim/src/lib.rs | 291 ++++++++++++++ .../tests/shim_behavior.rs | 361 ++++++++++++++++++ crates/openshell-sandbox/Cargo.toml | 1 + crates/openshell-sandbox/src/boundary_exec.rs | 17 + .../openshell-sandbox/src/boundary_server.rs | 46 +++ crates/openshell-sandbox/src/child_env.rs | 76 +++- .../openshell-sandbox/src/network_broker.rs | 6 + crates/openshell-sandbox/src/process.rs | 159 +++++++- docs/about/support-matrix.mdx | 53 ++- docs/kubernetes/openshift.mdx | 21 +- e2e/rust/tests/peer_address.rs | 120 ++++++ scripts/update_license_headers.py | 2 + 18 files changed, 1535 insertions(+), 23 deletions(-) create mode 100644 crates/openshell-accept-shim/Cargo.toml create mode 100644 crates/openshell-accept-shim/README.md create mode 100644 crates/openshell-accept-shim/build.rs create mode 100644 crates/openshell-accept-shim/src/accept_shim.c create mode 100644 crates/openshell-accept-shim/src/lib.rs create mode 100644 crates/openshell-accept-shim/tests/shim_behavior.rs create mode 100644 e2e/rust/tests/peer_address.rs diff --git a/AGENTS.md b/AGENTS.md index b049c28f50..4d18e8dc77 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,7 @@ Do not rely on this file for a full inventory. The detailed public and contribut | `crates/openshell-server/` | Gateway server | Control-plane API, sandbox lifecycle, auth boundary | | `crates/openshell-sandbox/` | Sandbox runtime | Capability-free workload launcher, process identity, and seccomp-mediated I/O | | `crates/openshell-supervisor/` | Supervisor runtime | Gateway session, policy evaluation, credentials, and upstream networking | +| `crates/openshell-accept-shim/` | Peer-address compatibility shim | Preloadable library that rewrites address-bearing `accept` for seccomp listeners without `WAIT_KILLABLE_RECV` | | `crates/openshell-binary-identity/` | Binary identity | Shared trusted procfs executable identity resolution for isolation backends | | `crates/openshell-isolation-interface/` | Isolation backend interface | RFC 0012 `IsolationBackend` trait and types; the supervisor-facing runtime contract | | `crates/openshell-sandbox-backend/` | OpenShell sandbox backend | `OpenShellRuntimeBackend` and the authenticated OpenShell Sandbox Protocol shared with `openshell-sandbox` | diff --git a/Cargo.lock b/Cargo.lock index c6173fee09..8185b06606 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3949,6 +3949,14 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "openshell-accept-shim" +version = "0.0.0" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "openshell-binary-identity" version = "0.0.0" @@ -4543,6 +4551,7 @@ dependencies = [ "libc", "miette", "nix 0.29.0", + "openshell-accept-shim", "openshell-binary-identity", "openshell-core", "openshell-isolation-interface", diff --git a/crates/openshell-accept-shim/Cargo.toml b/crates/openshell-accept-shim/Cargo.toml new file mode 100644 index 0000000000..9c081fc466 --- /dev/null +++ b/crates/openshell-accept-shim/Cargo.toml @@ -0,0 +1,23 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +[package] +name = "openshell-accept-shim" +description = "Preloadable peer-address shim for seccomp listeners without WAIT_KILLABLE_RECV" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +build = "build.rs" + +[dependencies] +libc = "0.2" + +[build-dependencies] +# Resolves the target C compiler through the standard cross-compilation +# conventions, including the per-target wrappers cargo-zigbuild installs. +cc = "1" + +[lints] +workspace = true diff --git a/crates/openshell-accept-shim/README.md b/crates/openshell-accept-shim/README.md new file mode 100644 index 0000000000..ae921f84fd --- /dev/null +++ b/crates/openshell-accept-shim/README.md @@ -0,0 +1,142 @@ + + +# openshell-accept-shim + +Peer-address compatibility library for seccomp listeners that lack +`SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV`. + +The crate carries three things: the freestanding C source for the library, a +build script that compiles and embeds it, and the Rust helpers the sandbox uses +to materialize it and compose `LD_PRELOAD`. + +## Why it exists + +`SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` arrived in Linux 5.19. Without it the +broker cannot hold a notified workload thread in a kill-only wait, so it cannot +safely write into workload memory and fails closed with `EOPNOTSUPP` on every +mediated syscall whose result is an output buffer. `accept(fd, &addr, &len)` is +exactly that shape, so a server workload on RHEL 9.x or RHCOS sees `EOPNOTSUPP` +where it expects a connection. + +The library rewrites that call into two syscalls the broker can satisfy in this +mode: + +1. `accept4(fd, NULL, NULL, flags)` — no output buffer, so the broker injects + the accepted descriptor without touching workload memory. +2. `getpeername(accepted, addr, addrlen)` — the broker answers this with + `SECCOMP_USER_NOTIF_FLAG_CONTINUE` for a directly connected socket, so the + *kernel* writes the address into the caller's buffer. + +Both halves are required. Step 2 depends on the broker's `CONTINUE` for +`getpeername`; without it, step 2 fails closed for the same reason step 1 did. + +Because the workload writes its own memory, the cross-process TOCTOU that +`WAIT_KILLABLE_RECV` exists to close does not apply here. + +## Not a security control + +Nothing in OpenShell trusts this library's output. + +The broker's loopback and authorization decisions use the kernel's own peer +address, obtained from the broker's own `accept4`. The library's result never +re-enters OpenShell's trust domain. A workload can unset `LD_PRELOAD`, link +statically, or issue raw syscalls, and gains nothing it did not already have — +it only loses the compatibility benefit. The enforcement boundary remains the +broker's fail-closed behavior. + +This is why the library's location on disk carries no privilege weight. + +## Coverage + +`LD_PRELOAD` interposes library symbols, not syscalls. + +| Runtime | Address-bearing `accept` | Reason | +| --- | --- | --- | +| Bun | Covered | Calls `accept4` through libc with a peer buffer | +| CPython | Covered | `sock_accept` passes a buffer, through libc | +| Node.js | Not needed | libuv always passes `NULL`; it resolves the peer lazily via `getpeername`, which the broker fix handles | +| Go | Not covered | `net` issues the syscall instruction directly | +| Static / `AT_SECURE` binaries | Not covered | No dynamic loader, or the preload is ignored | + +`getpeername` is fixed for every runtime, including Go, because the kernel +answers it. + +## Build invariants + +The source is freestanding C rather than a Rust `cdylib`, because a `cdylib` +would carry a `DT_NEEDED` entry on either glibc or musl and be unloadable in the +other. The build script compiles with `-shared -fPIC -O2 -nostdlib +-fno-stack-protector`. Four properties must hold, and a regression in any of +them is a bug: + +| Invariant | Why | +| --- | --- | +| No `DT_NEEDED` | One build per architecture loads under both glibc and musl | +| No `TEXTREL` | Avoids requiring SELinux `execmod`, the permission most likely denied to `container_t` | +| Exactly one undefined symbol, `__errno_location` | Both libcs export it; the loader resolves it from the already-loaded libc | +| Exactly two exported `FUNC` symbols, `accept` and `accept4` | Prevents accidental interposition of unrelated symbols such as `memcpy` | +| ELF machine matches the Rust target | A host object loads nowhere, and the loader reports it as `cannot open shared object file` — indistinguishable from a policy denial | + +Supported architectures are `x86_64` and `aarch64`; the source fails to compile +on anything else rather than silently producing a non-functional object. + +The build script resolves the C compiler through the `cc` crate, so the +standard `CC_`, `TARGET_CC`, and `CC` overrides apply, as do the +per-target wrappers `cargo-zigbuild` installs when the release binaries are +cross-compiled from a non-Linux host. It then checks the emitted object's ELF +header against `CARGO_CFG_TARGET_ARCH` and fails the build on a mismatch, +because a misresolved compiler otherwise produces a valid object for the wrong +architecture. + +The code performs no allocation, takes no locks, and cannot panic. When +`getpeername` fails on an already-accepted connection it reports a zero-length +address — what the kernel itself reports for an unnamed peer — rather than +leaking the descriptor or failing an accept that has already succeeded. + +## Installation + +`install_shim` materializes the embedded object at +`/run/openshell-compat/accept_shim.so`. The sandbox calls it during startup, +only when `listener.writes_disabled()` reports legacy mode. + +The path is constrained from both sides: + +- **Not under `/.openshell`.** The capability-free Landlock baseline grants each + top-level filesystem entry *except* the driver-owned `.openshell` hierarchy, + and a user ruleset can only narrow the baseline. A workload physically cannot + open a file there, so a library placed there could never be preloaded. +- **Not under the supervisor CA tmpfs.** That mount is `noexec`, so the loader + cannot map an object from it. + +`/run` satisfies both. On Docker and Podman it is part of the workload's own +writable, exec-capable rootfs. On Kubernetes the workload receives it as an +`emptyDir{medium: Memory}` tmpfs, mounted `rw,seclabel,relatime` with no +`noexec` and mode `1777`, so a non-root sandbox identity can create its own +subdirectory there. No compute driver needs a packaging change: the object is +embedded in the `openshell-sandbox` binary with `include_bytes!`. + +Installation creates the directory, writes through a temporary file, `rename`s +it into place, then seals both the file and the directory to `0o555`. Symlinked +directories and symlinked targets are refused rather than followed. Failure is +non-fatal and logged as an OCSF `Config State Change` event — the sandbox starts +without the library and keeps the broker's existing fail-closed behavior. + +`LD_PRELOAD` composition preserves any value the workload supplied and is +idempotent, so a child that re-inherits the variable and spawns its own child +does not accumulate duplicate entries. + +## Validation + +Validated on OpenShift 4.21 / RHCOS 9.6 (kernel `5.14.0-570.141.1.el9_6`, +SELinux enforcing), against the `emptyDir{medium: Memory}` mount the Kubernetes +workload actually receives, running as a non-root uid with +`readOnlyRootFilesystem: true`, all capabilities dropped, and the +`RuntimeDefault` seccomp profile: the object maps `r-xp` with zero AVC denials. + +When validating across architectures, check `e_machine` in the ELF header +before drawing conclusions. A wrong-architecture object produces +`cannot open shared object file` from the loader, which reads like an SELinux +denial and is not one. diff --git a/crates/openshell-accept-shim/build.rs b/crates/openshell-accept-shim/build.rs new file mode 100644 index 0000000000..684371ac45 --- /dev/null +++ b/crates/openshell-accept-shim/build.rs @@ -0,0 +1,99 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Compile the preloadable peer-address shim for Linux workload targets. +//! +//! The object is deliberately freestanding: `-nostdlib` keeps any `DT_NEEDED` +//! entry out of the result so a single per-architecture build loads under both +//! glibc and musl. `-fPIC` without text relocations also keeps `SELinux` from +//! requiring `execmod` on the materialized file. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +const SOURCE: &str = "src/accept_shim.c"; + +fn main() { + println!("cargo:rerun-if-changed={SOURCE}"); + + let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); + if target_os != "linux" { + // Other hosts still build and lint the workspace; the shim is only + // ever loaded inside a Linux workload. + return; + } + + let out_dir = PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR")); + let object = out_dir.join("accept_shim.so"); + + let mut command = compiler_command(); + command + .args([ + "-shared", + "-fPIC", + "-O2", + "-nostdlib", + "-fno-stack-protector", + // Undefined `__errno_location` is resolved from the workload's + // own libc at load time, so do not demand definitions here. + "-Wall", + "-Wextra", + "-Werror", + SOURCE, + "-o", + ]) + .arg(&object); + + let status = command + .status() + .unwrap_or_else(|error| panic!("run C compiler {command:?}: {error}")); + assert!(status.success(), "compile {SOURCE}: {status}"); + + verify_object_architecture(&object); + + println!("cargo:rustc-env=OPENSHELL_ACCEPT_SHIM={}", object.display()); +} + +/// Build the C compiler invocation for the target being compiled. +/// +/// Resolution is delegated to the `cc` crate so the standard overrides +/// (`CC_`, `TARGET_CC`, `CC`) and the per-target wrappers that +/// cross-build drivers such as `cargo-zigbuild` install are all honored. +/// Guessing a cross prefix instead would pick a binary that is frequently +/// absent on the build host. +fn compiler_command() -> Command { + match cc::Build::new().cargo_metadata(false).try_get_compiler() { + Ok(compiler) => compiler.to_command(), + Err(error) => panic!("locate a C compiler for the shim: {error}"), + } +} + +/// Fail the build when the emitted object does not match the Rust target. +/// +/// A host compiler reached through a misconfigured `CC` produces a valid +/// object for the wrong architecture. The workload's loader then reports +/// `cannot open shared object file`, which is indistinguishable from a +/// sandbox policy denial, so catch the mismatch here instead. +fn verify_object_architecture(object: &Path) { + let expected: u16 = match std::env::var("CARGO_CFG_TARGET_ARCH") + .unwrap_or_default() + .as_str() + { + "x86_64" => 0x3e, // EM_X86_64 + "aarch64" => 0xb7, // EM_AARCH64 + other => panic!("unsupported architecture for the accept shim: {other}"), + }; + + let bytes = std::fs::read(object).expect("read the compiled shim"); + assert!(bytes.len() > 20, "compiled shim is not an ELF object"); + assert_eq!( + &bytes[0..4], + b"\x7fELF", + "compiled shim is not an ELF object" + ); + let machine = u16::from_le_bytes([bytes[18], bytes[19]]); + assert_eq!( + machine, expected, + "compiled shim targets ELF machine {machine:#x}, expected {expected:#x}" + ); +} diff --git a/crates/openshell-accept-shim/src/accept_shim.c b/crates/openshell-accept-shim/src/accept_shim.c new file mode 100644 index 0000000000..c59e84935b --- /dev/null +++ b/crates/openshell-accept-shim/src/accept_shim.c @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Peer-address shim for seccomp listeners without WAIT_KILLABLE_RECV. +// +// On kernels that reject SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV the broker +// cannot safely write into workload memory, so it fails closed on any +// mediated syscall whose result is an output buffer. `accept`/`accept4` with +// a non-NULL address argument is exactly that shape, and the workload sees +// EOPNOTSUPP instead of a connection. +// +// This object is preloaded into the workload and rewrites the request into +// two syscalls the broker can satisfy in that mode: +// +// 1. accept4(fd, NULL, NULL, flags) — no output buffer, so the broker +// injects the accepted descriptor without touching workload memory. +// 2. getpeername(accepted, addr, addrlen) — the broker answers this for a +// directly connected socket with SECCOMP_USER_NOTIF_FLAG_CONTINUE, so +// the kernel itself stores the address into the caller's buffer. +// +// Both halves are required: without the broker's CONTINUE for `getpeername` +// step 2 fails closed for the same reason step 1 did. +// +// Raw syscalls are used rather than dlsym(RTLD_NEXT, ...) so the object needs +// no DT_NEEDED entry and no loader-visible libc dependency. One build per +// architecture therefore loads correctly under both glibc and musl. The only +// undefined symbol is `__errno_location`, which both libcs export and the +// dynamic linker resolves from the already-loaded libc at relocation time. +// +// Interposing here covers runtimes that call these functions through the +// PLT (CPython, Node, Bun, and anything else dynamically linked against +// libc). It cannot cover statically linked binaries or programs that issue +// the syscall instruction directly; those remain subject to the broker's +// fail-closed behavior. + +typedef unsigned int shim_socklen_t; + +extern int *__errno_location(void); + +#if defined(__x86_64__) +#define SHIM_NR_ACCEPT 43 +#define SHIM_NR_GETPEERNAME 52 +#define SHIM_NR_ACCEPT4 288 + +static long shim_syscall3(long number, long a0, long a1, long a2) { + long result; + __asm__ volatile("syscall" + : "=a"(result) + : "a"(number), "D"(a0), "S"(a1), "d"(a2) + : "rcx", "r11", "memory"); + return result; +} + +static long shim_syscall4(long number, long a0, long a1, long a2, long a3) { + long result; + register long r10 __asm__("r10") = a3; + __asm__ volatile("syscall" + : "=a"(result) + : "a"(number), "D"(a0), "S"(a1), "d"(a2), "r"(r10) + : "rcx", "r11", "memory"); + return result; +} + +#elif defined(__aarch64__) +#define SHIM_NR_ACCEPT 202 +#define SHIM_NR_GETPEERNAME 205 +#define SHIM_NR_ACCEPT4 242 + +static long shim_syscall4(long number, long a0, long a1, long a2, long a3) { + register long x8 __asm__("x8") = number; + register long x0 __asm__("x0") = a0; + register long x1 __asm__("x1") = a1; + register long x2 __asm__("x2") = a2; + register long x3 __asm__("x3") = a3; + __asm__ volatile("svc #0" + : "+r"(x0) + : "r"(x1), "r"(x2), "r"(x3), "r"(x8) + : "memory"); + return x0; +} + +static long shim_syscall3(long number, long a0, long a1, long a2) { + return shim_syscall4(number, a0, a1, a2, 0); +} + +#else +#error "openshell-accept-shim supports x86_64 and aarch64 only" +#endif + +// Translate a raw syscall return into the libc convention: negative values +// carry -errno, which the caller expects in errno with a -1 return. +static int shim_finish(long result) { + if (result < 0 && result >= -4095) { + *__errno_location() = (int)-result; + return -1; + } + return (int)result; +} + +static int shim_accept4(int sockfd, void *addr, shim_socklen_t *addrlen, + int flags) { + // Without an output buffer the broker's existing path already works, so + // forward unchanged and preserve its exact semantics. + if (addr == 0 || addrlen == 0) { + return shim_finish( + shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags)); + } + + long accepted = shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags); + if (accepted < 0) { + return shim_finish(accepted); + } + + // The connection is already established; a failure to report its address + // must not leak the descriptor or fail the accept. Report a zero-length + // address instead, which is the same thing the kernel reports for an + // unnamed peer, rather than leaving the caller's buffer undefined. + if (shim_syscall3(SHIM_NR_GETPEERNAME, accepted, (long)addr, + (long)addrlen) < 0) { + *addrlen = 0; + } + return (int)accepted; +} + +int accept4(int sockfd, void *addr, shim_socklen_t *addrlen, int flags) { + return shim_accept4(sockfd, addr, addrlen, flags); +} + +int accept(int sockfd, void *addr, shim_socklen_t *addrlen) { + return shim_accept4(sockfd, addr, addrlen, 0); +} diff --git a/crates/openshell-accept-shim/src/lib.rs b/crates/openshell-accept-shim/src/lib.rs new file mode 100644 index 0000000000..8b51c90c1b --- /dev/null +++ b/crates/openshell-accept-shim/src/lib.rs @@ -0,0 +1,291 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Preloadable peer-address shim for legacy seccomp listeners. +//! +//! See `src/accept_shim.c` and this crate's README for why the shim exists. +//! The compiled object is embedded here so the sandbox can materialize it +//! into a workload whose image it does not control. + +/// File name used when the shim is materialized into a workload. +pub const FILE_NAME: &str = "accept_shim.so"; + +/// Environment variable the workload's dynamic loader reads. +pub const PRELOAD_ENV: &str = "LD_PRELOAD"; + +/// The compiled shared object for this build's target architecture. +#[cfg(target_os = "linux")] +pub const SHIM_OBJECT: &[u8] = include_bytes!(env!("OPENSHELL_ACCEPT_SHIM")); + +/// Directory the shim is materialized into inside the workload. +/// +/// This deliberately sits outside the driver-owned `/.openshell` hierarchy, +/// which the capability-free Landlock baseline never exposes to a workload. +/// The dynamic loader must be able to open and map the object, so it also has +/// to live outside the `noexec` tmpfs mounts that carry supervisor material. +pub const RUNTIME_DIR: &str = "/run/openshell-compat"; + +/// Materialize an object into `directory` as a read-only executable file. +/// +/// The target rootfs is writable by the workload, so every component is +/// checked for symlink redirection and the file is created with `O_NOFOLLOW` +/// before being renamed into place. The shim is a compatibility aid rather +/// than a security control — a workload can always decline to load it — but +/// installing it must still never write through a path the workload chose. +pub fn install_object_at( + directory: &std::path::Path, + contents: &[u8], +) -> Result { + use std::io::Write as _; + use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + + match std::fs::symlink_metadata(directory) { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(format!( + "shim directory is a symlink: {}", + directory.display() + )); + } + Ok(metadata) if !metadata.is_dir() => { + return Err(format!( + "shim directory is not a directory: {}", + directory.display() + )); + } + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + std::fs::create_dir_all(directory).map_err(|error| { + format!("create shim directory {}: {error}", directory.display()) + })?; + } + Err(error) => { + return Err(format!( + "inspect shim directory {}: {error}", + directory.display() + )); + } + } + // Writable for the install itself; tightened to read-only below. The + // sandbox is not necessarily root, so the owner write bit is required + // here even on a freshly created directory. + std::fs::set_permissions(directory, std::fs::Permissions::from_mode(0o755)) + .map_err(|error| format!("set shim directory permissions: {error}"))?; + + let path = directory.join(FILE_NAME); + if let Ok(metadata) = std::fs::symlink_metadata(&path) + && metadata.file_type().is_symlink() + { + return Err(format!("shim path is a symlink: {}", path.display())); + } + + let temporary = path.with_extension("tmp"); + if let Ok(metadata) = std::fs::symlink_metadata(&temporary) { + if !metadata.is_file() || metadata.file_type().is_symlink() { + return Err(format!( + "refusing unsafe temporary shim path: {}", + temporary.display() + )); + } + std::fs::remove_file(&temporary) + .map_err(|error| format!("remove stale temporary shim: {error}"))?; + } + + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o555) + .custom_flags(libc::O_NOFOLLOW) + .open(&temporary) + .map_err(|error| format!("create temporary shim: {error}"))?; + if let Err(error) = file + .write_all(contents) + .and_then(|()| file.sync_all()) + .and_then(|()| file.set_permissions(std::fs::Permissions::from_mode(0o555))) + .and_then(|()| std::fs::rename(&temporary, &path)) + { + let _ = std::fs::remove_file(&temporary); + return Err(format!("install shim: {error}")); + } + // Traversable and readable by every workload identity, writable by none. + std::fs::set_permissions(directory, std::fs::Permissions::from_mode(0o555)) + .map_err(|error| format!("seal shim directory permissions: {error}"))?; + Ok(path) +} + +/// Materialize the embedded shim into [`RUNTIME_DIR`]. +#[cfg(target_os = "linux")] +pub fn install_shim() -> Result { + install_object_at(std::path::Path::new(RUNTIME_DIR), SHIM_OBJECT) +} + +/// Compose an `LD_PRELOAD` value that keeps any workload-supplied entries. +/// +/// The shim is placed first so it wins symbol resolution, and an existing +/// value is preserved rather than replaced: a workload that relies on its own +/// preload must keep working. +#[must_use] +pub fn compose_preload(shim_path: &str, existing: Option<&str>) -> String { + match existing.map(str::trim).filter(|value| !value.is_empty()) { + Some(existing) if preload_contains(existing, shim_path) => existing.to_string(), + Some(existing) => format!("{shim_path}:{existing}"), + None => shim_path.to_string(), + } +} + +/// Whether an `LD_PRELOAD` value already lists a path. +/// +/// The loader separates entries by colon or whitespace, so both are honored. +fn preload_contains(value: &str, path: &str) -> bool { + value + .split([':', ' ', '\t']) + .any(|entry| !entry.is_empty() && entry == path) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt as _; + + /// The object is produced by a C compiler chosen at build time, so a + /// misresolved cross-compiler yields a host-architecture object that the + /// workload's loader rejects with a message that reads like a policy + /// denial. Pin the shape of what actually got embedded. + #[cfg(target_os = "linux")] + #[test] + fn the_embedded_object_is_a_shared_object_for_the_build_target() { + let expected_machine: u16 = if cfg!(target_arch = "x86_64") { + 0x3e // EM_X86_64 + } else if cfg!(target_arch = "aarch64") { + 0xb7 // EM_AARCH64 + } else { + panic!("unsupported architecture for the accept shim"); + }; + + let header = SHIM_OBJECT; + assert!(header.len() > 20, "object is too short to be an ELF file"); + assert_eq!(&header[0..4], b"\x7fELF", "not an ELF object"); + assert_eq!(header[4], 2, "expected ELFCLASS64"); + assert_eq!(header[5], 1, "expected little-endian ELF"); + assert_eq!( + u16::from_le_bytes([header[16], header[17]]), + 3, + "expected ET_DYN; a non-shared object cannot be preloaded" + ); + assert_eq!( + u16::from_le_bytes([header[18], header[19]]), + expected_machine, + "object architecture does not match the build target" + ); + } + + fn scratch_dir(name: &str) -> std::path::PathBuf { + let base = std::env::temp_dir().join(format!("openshell-accept-shim-{name}")); + let _ = std::fs::remove_dir_all(&base); + base + } + + #[test] + fn installing_creates_a_read_only_executable_object() { + let directory = scratch_dir("install"); + let installed = install_object_at(&directory, b"shim-bytes").expect("install"); + + assert_eq!(installed, directory.join(FILE_NAME)); + assert_eq!(std::fs::read(&installed).expect("read"), b"shim-bytes"); + // The workload must be able to map it executable but never rewrite it. + let mode = std::fs::metadata(&installed) + .expect("stat") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o555); + } + + #[test] + fn installing_twice_replaces_the_previous_object() { + // A sandbox restart re-materializes into a directory that may already + // hold a previous generation of the shim. + let directory = scratch_dir("reinstall"); + install_object_at(&directory, b"old").expect("first install"); + let installed = install_object_at(&directory, b"new").expect("second install"); + + assert_eq!(std::fs::read(&installed).expect("read"), b"new"); + assert_eq!( + std::fs::metadata(&installed) + .expect("stat") + .permissions() + .mode() + & 0o777, + 0o555 + ); + } + + #[test] + fn a_symlinked_directory_is_refused() { + // The directory lives on a workload-writable rootfs, so a redirect + // must never be followed into a path the workload chose. + let base = scratch_dir("symlink-dir"); + std::fs::create_dir_all(base.join("real")).expect("create real"); + let link = base.join("link"); + std::os::unix::fs::symlink(base.join("real"), &link).expect("symlink"); + + let error = install_object_at(&link, b"shim-bytes").expect_err("must refuse"); + assert!(error.contains("symlink"), "unexpected error: {error}"); + } + + #[test] + fn a_symlinked_target_file_is_refused() { + let directory = scratch_dir("symlink-file"); + std::fs::create_dir_all(&directory).expect("create"); + let target = directory.join("elsewhere"); + std::fs::write(&target, b"victim").expect("write victim"); + std::os::unix::fs::symlink(&target, directory.join(FILE_NAME)).expect("symlink"); + + let error = install_object_at(&directory, b"shim-bytes").expect_err("must refuse"); + assert!(error.contains("symlink"), "unexpected error: {error}"); + assert_eq!(std::fs::read(&target).expect("read"), b"victim"); + } + + #[test] + fn preload_is_the_only_entry_when_the_workload_set_none() { + assert_eq!(compose_preload("/run/shim.so", None), "/run/shim.so"); + assert_eq!(compose_preload("/run/shim.so", Some("")), "/run/shim.so"); + assert_eq!(compose_preload("/run/shim.so", Some(" ")), "/run/shim.so"); + } + + #[test] + fn workload_supplied_preloads_are_kept_after_the_shim() { + assert_eq!( + compose_preload("/run/shim.so", Some("/opt/jemalloc.so")), + "/run/shim.so:/opt/jemalloc.so" + ); + assert_eq!( + compose_preload("/run/shim.so", Some("/a.so:/b.so")), + "/run/shim.so:/a.so:/b.so" + ); + } + + #[test] + fn an_already_listed_shim_is_not_added_twice() { + // Children re-inherit the composed value; repeated spawns must not + // grow LD_PRELOAD without bound. + assert_eq!( + compose_preload("/run/shim.so", Some("/run/shim.so")), + "/run/shim.so" + ); + assert_eq!( + compose_preload("/run/shim.so", Some("/run/shim.so:/b.so")), + "/run/shim.so:/b.so" + ); + assert_eq!( + compose_preload("/run/shim.so", Some("/a.so /run/shim.so")), + "/a.so /run/shim.so" + ); + } + + #[test] + fn a_path_that_merely_shares_a_prefix_is_not_treated_as_present() { + assert_eq!( + compose_preload("/run/shim.so", Some("/run/shim.so.bak")), + "/run/shim.so:/run/shim.so.bak" + ); + } +} diff --git a/crates/openshell-accept-shim/tests/shim_behavior.rs b/crates/openshell-accept-shim/tests/shim_behavior.rs new file mode 100644 index 0000000000..a13371fb9d --- /dev/null +++ b/crates/openshell-accept-shim/tests/shim_behavior.rs @@ -0,0 +1,361 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Behavioral tests for the compiled shim object. +//! +//! The shim replaces one address-bearing `accept` with `accept4(NULL)` plus +//! `getpeername`. That rewrite is only safe if it is indistinguishable from a +//! plain `accept` for the workload, so these tests load the real object and +//! compare it against libc on an ordinary socket. No seccomp broker is +//! involved: the broker is what makes the rewrite *necessary*, not what makes +//! it *correct*, and leaving it out lets these run on any Linux host. + +#![cfg(target_os = "linux")] +// Loading and calling the object under test is inherently unsafe: it is a +// shared library resolved at run time and invoked through raw pointers. +#![allow(unsafe_code)] + +use std::ffi::{CStr, CString}; +use std::io::{Error, Read as _, Write as _}; +use std::mem::{size_of, transmute}; +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, TcpListener, TcpStream}; +use std::os::fd::AsRawFd as _; +use std::process::id as process_id; +use std::ptr::null_mut; +use std::sync::OnceLock; + +use libc::{ + AF_INET, AF_INET6, EINVAL, RTLD_LOCAL, RTLD_NOW, c_int, c_void, sockaddr, sockaddr_in, + sockaddr_in6, sockaddr_storage, socklen_t, +}; + +use openshell_accept_shim::{SHIM_OBJECT, install_object_at}; + +type Accept4Fn = unsafe extern "C" fn(c_int, *mut sockaddr, *mut socklen_t, c_int) -> c_int; + +type AcceptFn = unsafe extern "C" fn(c_int, *mut sockaddr, *mut socklen_t) -> c_int; + +/// The shim loaded the way a workload loads it: through the dynamic loader. +struct Shim { + accept4: Accept4Fn, + accept: AcceptFn, +} + +impl Shim { + fn load() -> Self { + let directory = + std::env::temp_dir().join(format!("openshell-shim-behavior-{}", process_id())); + let _ = std::fs::remove_dir_all(&directory); + let path = install_object_at(&directory, SHIM_OBJECT).expect("install shim object"); + + let c_path = CString::new(path.as_os_str().as_encoded_bytes()) + .expect("shim path has no interior NUL"); + // RTLD_NOW so an unresolved symbol fails here rather than at the call + // site; the object's sole undefined symbol is `__errno_location`, + // which the already-loaded libc provides. + let handle = unsafe { libc::dlopen(c_path.as_ptr(), RTLD_NOW | RTLD_LOCAL) }; + assert!(!handle.is_null(), "dlopen: {}", dl_error()); + + Self { + accept4: unsafe { transmute::<*mut c_void, Accept4Fn>(symbol(handle, c"accept4")) }, + accept: unsafe { transmute::<*mut c_void, AcceptFn>(symbol(handle, c"accept")) }, + } + } +} + +fn symbol(handle: *mut c_void, name: &CStr) -> *mut c_void { + let resolved = unsafe { libc::dlsym(handle, name.as_ptr()) }; + assert!( + !resolved.is_null(), + "dlsym {}: {}", + name.to_string_lossy(), + dl_error() + ); + resolved +} + +fn dl_error() -> String { + let message = unsafe { libc::dlerror() }; + if message.is_null() { + return "no error reported".to_string(); + } + unsafe { CStr::from_ptr(message) } + .to_string_lossy() + .into_owned() +} + +fn shim() -> &'static Shim { + static SHIM: OnceLock = OnceLock::new(); + SHIM.get_or_init(Shim::load) +} + +/// A listener with one pending connection from a known local address. +struct Pending { + listener: TcpListener, + client: TcpStream, +} + +fn pending_connection(bind: SocketAddr) -> Pending { + let listener = TcpListener::bind(bind).expect("bind listener"); + let client = TcpStream::connect(listener.local_addr().expect("local addr")).expect("connect"); + Pending { listener, client } +} + +/// What the kernel reports for `fd`'s peer, with no truncation. +fn kernel_peer(fd: c_int) -> (Vec, socklen_t) { + let mut storage = [0u8; size_of::()]; + let mut length = storage.len() as socklen_t; + let result = + unsafe { libc::getpeername(fd, storage.as_mut_ptr().cast::(), &raw mut length) }; + assert_eq!(result, 0, "getpeername: {}", Error::last_os_error()); + (storage[..length as usize].to_vec(), length) +} + +/// Accept through the shim into a sentinel-filled buffer of `capacity` bytes. +/// +/// Returns the accepted descriptor, the whole buffer, and the `addrlen` the +/// shim reported, so a caller can check both what was written and what was +/// left alone. +fn shim_accept4(listener: &TcpListener, capacity: socklen_t) -> (c_int, Vec, socklen_t) { + const SENTINEL: u8 = 0xAA; + let mut buffer = vec![SENTINEL; size_of::()]; + let mut length = capacity; + let accepted = unsafe { + (shim().accept4)( + listener.as_raw_fd(), + buffer.as_mut_ptr().cast::(), + &raw mut length, + 0, + ) + }; + assert!(accepted >= 0, "shim accept4: {}", Error::last_os_error()); + (accepted, buffer, length) +} + +fn close(fd: c_int) { + assert_eq!(unsafe { libc::close(fd) }, 0, "close accepted descriptor"); +} + +#[test] +fn the_reported_peer_matches_the_kernel_for_ipv4() { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let expected_port = pending + .client + .local_addr() + .expect("client local addr") + .port(); + + let capacity = size_of::() as socklen_t; + let (accepted, buffer, length) = shim_accept4(&pending.listener, capacity); + let (kernel, kernel_length) = kernel_peer(accepted); + + assert_eq!(length, kernel_length); + assert_eq!(&buffer[..length as usize], &kernel[..]); + + // The whole point of the rewrite: the caller learns the *client's* + // ephemeral port, not the listener's. + let family = u16::from_ne_bytes([buffer[0], buffer[1]]); + assert_eq!(c_int::from(family), AF_INET); + let port = u16::from_be_bytes([buffer[2], buffer[3]]); + assert_eq!(port, expected_port); + assert_ne!( + port, + pending.listener.local_addr().expect("listener addr").port() + ); + + close(accepted); +} + +#[test] +fn the_reported_peer_matches_the_kernel_for_ipv6() { + let pending = pending_connection(SocketAddr::from((Ipv6Addr::LOCALHOST, 0))); + let expected_port = pending + .client + .local_addr() + .expect("client local addr") + .port(); + + let capacity = size_of::() as socklen_t; + let (accepted, buffer, length) = shim_accept4(&pending.listener, capacity); + let (kernel, kernel_length) = kernel_peer(accepted); + + assert_eq!(length, kernel_length); + assert_eq!(&buffer[..length as usize], &kernel[..]); + + let family = u16::from_ne_bytes([buffer[0], buffer[1]]); + assert_eq!(c_int::from(family), AF_INET6); + let port = u16::from_be_bytes([buffer[2], buffer[3]]); + assert_eq!(port, expected_port); + + close(accepted); +} + +#[test] +fn a_short_buffer_truncates_and_still_reports_the_full_length() { + // Linux signals truncation by returning an `addrlen` larger than the one + // supplied. A caller that trusts the returned length would read past its + // own buffer if the shim reported the truncated length instead. + let full = size_of::() as socklen_t; + + for capacity in [0, 4, 8, full - 1] { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let (accepted, buffer, length) = shim_accept4(&pending.listener, capacity); + let (kernel, _) = kernel_peer(accepted); + + assert_eq!( + length, full, + "capacity {capacity} must report the untruncated length" + ); + assert_eq!( + &buffer[..capacity as usize], + &kernel[..capacity as usize], + "capacity {capacity} wrote the wrong prefix" + ); + assert!( + buffer[capacity as usize..].iter().all(|byte| *byte == 0xAA), + "capacity {capacity} wrote past the caller's buffer" + ); + + close(accepted); + } +} + +#[test] +fn an_oversized_buffer_is_filled_only_to_the_address_length() { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let oversized = size_of::() as socklen_t; + + let (accepted, buffer, length) = shim_accept4(&pending.listener, oversized); + + assert_eq!(length, size_of::() as socklen_t); + assert!( + buffer[length as usize..].iter().all(|byte| *byte == 0xAA), + "bytes beyond the address must be left alone" + ); + + close(accepted); +} + +#[test] +fn a_v4_mapped_peer_is_reported_as_the_kernel_reports_it() { + // A dual-stack listener reports IPv4 clients as v4-mapped IPv6. The shim + // must not normalize that, or a workload's own address parsing diverges + // from what it would see without the shim. + let listener = TcpListener::bind(SocketAddr::from((Ipv6Addr::UNSPECIFIED, 0))) + .expect("bind dual-stack listener"); + let port = listener.local_addr().expect("local addr").port(); + let Ok(client) = TcpStream::connect(SocketAddr::from((Ipv4Addr::LOCALHOST, port))) else { + // A host with `bindv6only` set has no dual-stack listener to test. + return; + }; + let expected_port = client.local_addr().expect("client local addr").port(); + + let capacity = size_of::() as socklen_t; + let (accepted, buffer, length) = shim_accept4(&listener, capacity); + let (kernel, kernel_length) = kernel_peer(accepted); + + assert_eq!(length, kernel_length); + assert_eq!(&buffer[..length as usize], &kernel[..]); + + let family = u16::from_ne_bytes([buffer[0], buffer[1]]); + assert_eq!(c_int::from(family), AF_INET6); + assert_eq!(u16::from_be_bytes([buffer[2], buffer[3]]), expected_port); + + close(accepted); +} + +#[test] +fn a_null_address_still_accepts_the_connection() { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + + let accepted = + unsafe { (shim().accept4)(pending.listener.as_raw_fd(), null_mut(), null_mut(), 0) }; + + assert!( + accepted >= 0, + "shim accept4 with no address: {}", + Error::last_os_error() + ); + close(accepted); +} + +#[test] +fn the_accepted_descriptor_carries_the_connection() { + // The rewrite returns a descriptor from a second syscall. If it ever + // returned the wrong one, addresses could still look right while the + // stream was unusable. + let mut pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + + let capacity = size_of::() as socklen_t; + let (accepted, _, _) = shim_accept4(&pending.listener, capacity); + + pending.client.write_all(b"ping").expect("client write"); + let written = unsafe { libc::write(accepted, c"pong".as_ptr().cast(), 4) }; + assert_eq!(written, 4, "write to accepted descriptor"); + + let mut received = [0u8; 4]; + let read = unsafe { libc::read(accepted, received.as_mut_ptr().cast(), 4) }; + assert_eq!(read, 4, "read from accepted descriptor"); + assert_eq!(&received, b"ping"); + + let mut echoed = [0u8; 4]; + pending.client.read_exact(&mut echoed).expect("client read"); + assert_eq!(&echoed, b"pong"); + + close(accepted); +} + +#[test] +fn the_accept_entry_point_behaves_like_accept4_without_flags() { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let expected_port = pending + .client + .local_addr() + .expect("client local addr") + .port(); + + let mut buffer = vec![0xAAu8; size_of::()]; + let mut length = size_of::() as socklen_t; + let accepted = unsafe { + (shim().accept)( + pending.listener.as_raw_fd(), + buffer.as_mut_ptr().cast::(), + &raw mut length, + ) + }; + + assert!(accepted >= 0, "shim accept: {}", Error::last_os_error()); + assert_eq!(u16::from_be_bytes([buffer[2], buffer[3]]), expected_port); + + close(accepted); +} + +#[test] +fn a_failing_accept_reports_the_kernel_error() { + // `shim_finish` must translate a raw negative return into `-1` plus + // `errno`; a workload that sees the raw value instead would treat a + // failure as a valid descriptor. + let not_a_listener = TcpStream::connect( + TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))) + .expect("bind") + .local_addr() + .expect("addr"), + ); + let Ok(stream) = not_a_listener else { + return; + }; + + let mut buffer = vec![0u8; size_of::()]; + let mut length = buffer.len() as socklen_t; + let result = unsafe { + (shim().accept4)( + stream.as_raw_fd(), + buffer.as_mut_ptr().cast::(), + &raw mut length, + 0, + ) + }; + + assert_eq!(result, -1, "accept on a connected socket must fail"); + assert_eq!(Error::last_os_error().raw_os_error(), Some(EINVAL)); +} diff --git a/crates/openshell-sandbox/Cargo.toml b/crates/openshell-sandbox/Cargo.toml index 50d91f0abf..81ee353a0d 100644 --- a/crates/openshell-sandbox/Cargo.toml +++ b/crates/openshell-sandbox/Cargo.toml @@ -26,6 +26,7 @@ perf-harness = [] openshell-core = { path = "../openshell-core", default-features = false } openshell-binary-identity = { path = "../openshell-binary-identity" } openshell-isolation-interface = { path = "../openshell-isolation-interface" } +openshell-accept-shim = { path = "../openshell-accept-shim" } openshell-sandbox-backend = { path = "../openshell-sandbox-backend" } openshell-ocsf = { path = "../openshell-ocsf" } openshell-policy = { path = "../openshell-policy" } diff --git a/crates/openshell-sandbox/src/boundary_exec.rs b/crates/openshell-sandbox/src/boundary_exec.rs index da35294c06..ac551b28ba 100644 --- a/crates/openshell-sandbox/src/boundary_exec.rs +++ b/crates/openshell-sandbox/src/boundary_exec.rs @@ -188,6 +188,23 @@ impl LocalBoundaryExec { command.env(key, value); } } + // Last, so the shim composes with whichever LD_PRELOAD the workload + // would otherwise have received rather than being overwritten by it. + if let Some(shim) = crate::child_env::preload_shim() { + let inherited = spec + .env + .iter() + .rev() + .find(|(key, _)| key == openshell_accept_shim::PRELOAD_ENV) + .map(|(_, value)| value.as_str()) + .or_else(|| { + self.user_environment + .get(openshell_accept_shim::PRELOAD_ENV) + .map(String::as_str) + }); + let (key, value) = crate::child_env::preload_env_var(shim, inherited); + command.env(key, value); + } if let Some(workdir) = spec.workdir.as_deref().or(self.base_workdir.as_deref()) { command.current_dir(workdir); } diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index f84f98d424..cbc4a9512a 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -228,6 +228,7 @@ mod linux { } let (launcher, listener) = openshell_isolation_interface::linux::workload_launcher::start() .map_err(|error| format!("start sandbox workload launcher: {error}"))?; + crate::child_env::set_preload_shim(install_peer_address_shim(&listener)); let protected_control_port = match &config.listener { BoundaryListenerConfig::TlsTcp { address, .. } => Some(address.port()), BoundaryListenerConfig::Unix { .. } | BoundaryListenerConfig::Vsock { .. } => None, @@ -248,6 +249,51 @@ mod linux { serve(&config.listener, runtime) } + /// Materialize the peer-address shim when the kernel's seccomp listener + /// leaves broker output writes disabled. + /// + /// Without `WAIT_KILLABLE_RECV` the broker cannot write into workload + /// memory, so `accept`/`accept4` with an address buffer fails closed. The + /// shim rewrites those calls into an `accept4(NULL)` plus `getpeername` + /// pair that the broker can serve. It is a compatibility aid, not a + /// security control, so a failure to install it is not fatal: the sandbox + /// still runs with the broker's existing fail-closed behavior. + fn install_peer_address_shim( + listener: &openshell_isolation_interface::linux::seccomp_notify::NotificationListener, + ) -> Option { + if !listener.writes_disabled() { + return None; + } + match openshell_accept_shim::install_shim() { + Ok(path) => { + openshell_ocsf::ocsf_emit!( + openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) + .severity(openshell_ocsf::SeverityId::Informational) + .status(openshell_ocsf::StatusId::Success) + .state(openshell_ocsf::StateId::Enabled, "legacy_read_only") + .message(format!( + "Installed peer-address shim for legacy seccomp listener [path:{}]", + path.display() + )) + .build() + ); + Some(path) + } + Err(error) => { + openshell_ocsf::ocsf_emit!( + openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) + .severity(openshell_ocsf::SeverityId::Medium) + .status(openshell_ocsf::StatusId::Failure) + .message(format!( + "Peer-address shim unavailable; accept with an address buffer stays unsupported [error:{error}]" + )) + .build() + ); + None + } + } + } + fn make_boundary_nondumpable() -> Result<(), String> { // SAFETY: PR_SET_DUMPABLE accepts one scalar flag. The sandbox keeps // bootstrap and protected-channel keys in memory after this point. diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index 50549a7439..81f89ecaff 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -1,7 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use std::path::Path; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; pub fn tls_env_vars( ca_cert_path: &Path, @@ -21,12 +23,84 @@ pub fn tls_env_vars( ] } +/// Shim materialized for this sandbox, or `None` when the kernel's seccomp +/// listener supports `WAIT_KILLABLE_RECV` and the broker can answer +/// `accept`/`accept4` directly. +static PRELOAD_SHIM: OnceLock> = OnceLock::new(); + +/// Record the materialized shim path once, during sandbox startup. +pub fn set_preload_shim(path: Option) { + let _ = PRELOAD_SHIM.set(path); +} + +/// Path of the materialized shim, if this sandbox installed one. +pub fn preload_shim() -> Option<&'static Path> { + PRELOAD_SHIM.get()?.as_deref() +} + +/// Build the `LD_PRELOAD` entry for a child, preserving any value the +/// workload asked for. +/// +/// Children re-inherit this value when they spawn their own children, so the +/// composition must be idempotent rather than prepending on every exec. +pub fn preload_env_var(shim_path: &Path, inherited: Option<&str>) -> (&'static str, String) { + ( + openshell_accept_shim::PRELOAD_ENV, + openshell_accept_shim::compose_preload(&shim_path.display().to_string(), inherited), + ) +} + +/// Resolve the `LD_PRELOAD` a child would inherit where the environment is +/// not cleared: an explicit workload value wins over the sandbox's own. +#[allow(clippy::implicit_hasher)] +pub fn inherited_preload(user_environment: &HashMap) -> Option { + user_environment + .get(openshell_accept_shim::PRELOAD_ENV) + .cloned() + .or_else(|| std::env::var(openshell_accept_shim::PRELOAD_ENV).ok()) +} + #[cfg(test)] mod tests { use super::*; use std::process::Command; use std::process::Stdio; + #[test] + fn inherited_preload_prefers_the_workload_value() { + let mut user_environment = HashMap::new(); + user_environment.insert("LD_PRELOAD".to_string(), "/opt/jemalloc.so".to_string()); + assert_eq!( + inherited_preload(&user_environment).as_deref(), + Some("/opt/jemalloc.so") + ); + assert_eq!(inherited_preload(&HashMap::new()).as_deref(), None); + } + + #[test] + fn preload_env_var_keeps_a_workload_supplied_value() { + let (key, value) = preload_env_var(Path::new("/run/openshell-compat/accept_shim.so"), None); + assert_eq!(key, "LD_PRELOAD"); + assert_eq!(value, "/run/openshell-compat/accept_shim.so"); + + let (_, value) = preload_env_var( + Path::new("/run/openshell-compat/accept_shim.so"), + Some("/opt/jemalloc.so"), + ); + assert_eq!( + value, + "/run/openshell-compat/accept_shim.so:/opt/jemalloc.so" + ); + } + + #[test] + fn preload_env_var_does_not_grow_across_nested_spawns() { + let shim = Path::new("/run/openshell-compat/accept_shim.so"); + let (_, first) = preload_env_var(shim, None); + let (_, second) = preload_env_var(shim, Some(&first)); + assert_eq!(first, second); + } + #[test] fn apply_tls_env_sets_node_and_bundle_paths() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/crates/openshell-sandbox/src/network_broker.rs b/crates/openshell-sandbox/src/network_broker.rs index 4db17c1364..77837091f6 100644 --- a/crates/openshell-sandbox/src/network_broker.rs +++ b/crates/openshell-sandbox/src/network_broker.rs @@ -1265,6 +1265,12 @@ fn accept_and_inject( nonblocking: flags & libc::SOCK_NONBLOCK != 0, creator_generation: u64::from(notification.tid), }; + // Hold this guard across both `add_fd_and_send` and `commit_with_state` + // below. `add_fd_and_send` resumes the workload, which may immediately call + // `getpeername` on the descriptor it just received -- the sequence the + // legacy-mode preload issues on every accept. That handler resolves against + // this same registry, so releasing the lock before the commit would let it + // observe the socket as unregistered. let mut registry = lock(registry); let notifying_fd = raw_fd(notification.args[0])?; if registry diff --git a/crates/openshell-sandbox/src/process.rs b/crates/openshell-sandbox/src/process.rs index 595ceac29e..8424ba2ad4 100644 --- a/crates/openshell-sandbox/src/process.rs +++ b/crates/openshell-sandbox/src/process.rs @@ -72,11 +72,26 @@ pub(crate) fn prepare_child_sandbox( workdir: Option<&str>, runtime_read_only: &[PathBuf], ) -> Result> { - let effective_policy = policy_with_runtime_read_only(policy, runtime_read_only); + let runtime_read_only = + effective_runtime_read_only(runtime_read_only, child_env::preload_shim()); + let effective_policy = policy_with_runtime_read_only(policy, &runtime_read_only); let prepared = sandbox::linux::prepare_capability_free(&effective_policy, workdir)?; Ok(Some(prepared)) } +/// Combine a launch path's own runtime paths with the preloaded shim's. +/// +/// Both the sandbox entrypoint and `sandbox exec` set `LD_PRELOAD`, and each +/// supplies a different set of runtime paths. Admitting the shim here rather +/// than at each call site means a launch path cannot set `LD_PRELOAD` without +/// also letting the loader open what it points at. +#[cfg(target_os = "linux")] +fn effective_runtime_read_only(runtime_read_only: &[PathBuf], shim: Option<&Path>) -> Vec { + let mut paths = runtime_read_only.to_vec(); + paths.extend(shim_runtime_read_only_paths(shim)); + paths +} + #[cfg(target_os = "linux")] fn policy_with_runtime_read_only( policy: &SandboxPolicy, @@ -105,6 +120,27 @@ pub(crate) fn ca_runtime_read_only_paths(ca_paths: Option<&(PathBuf, PathBuf)>) paths } +/// Paths the dynamic loader must reach to honor the preloaded shim. +/// +/// `LD_PRELOAD` is resolved by the loader inside the workload, after Landlock +/// is enforced. The object is installed world-readable, but Landlock gates +/// `open` independently of the file mode, so without an explicit admission the +/// loader reports `cannot open shared object file` and silently drops the +/// shim. The directory is admitted alongside the object because the loader +/// resolves the path through it. +#[cfg(target_os = "linux")] +pub(crate) fn shim_runtime_read_only_paths(shim: Option<&Path>) -> Vec { + let Some(object) = shim else { + return Vec::new(); + }; + let mut paths = Vec::with_capacity(2); + if let Some(directory) = object.parent() { + paths.push(directory.to_path_buf()); + } + paths.push(object.to_path_buf()); + paths +} + const SUPERVISOR_ONLY_ENV_VARS: &[&str] = &[ openshell_core::sandbox_env::OCI_IMAGE_USER, openshell_core::sandbox_env::SANDBOX_UID, @@ -505,6 +541,12 @@ impl ProcessHandle { } } + if let Some(shim) = child_env::preload_shim() { + let inherited = child_env::inherited_preload(&configured_user_environment()); + let (key, value) = child_env::preload_env_var(shim, inherited.as_deref()); + cmd.env(key, value); + } + // Probe Landlock availability and emit OCSF logs from the parent // process where the tracing subscriber is functional. The child's // pre_exec context cannot reliably emit structured logs. @@ -670,6 +712,12 @@ impl ProcessHandle { } } + if let Some(shim) = child_env::preload_shim() { + let inherited = child_env::inherited_preload(&configured_user_environment()); + let (key, value) = child_env::preload_env_var(shim, inherited.as_deref()); + cmd.env(key, value); + } + // Create a dedicated session for PTY children and a dedicated process // group for pipe children so attachment signals target only the // canonical workload tree. @@ -1513,6 +1561,115 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + fn shim_runtime_paths_admit_the_object_and_its_directory() { + assert!( + shim_runtime_read_only_paths(None).is_empty(), + "no shim installed means nothing extra to admit" + ); + + let object = PathBuf::from(format!( + "{}/{}", + openshell_accept_shim::RUNTIME_DIR, + openshell_accept_shim::FILE_NAME + )); + assert_eq!( + shim_runtime_read_only_paths(Some(&object)), + vec![ + PathBuf::from(openshell_accept_shim::RUNTIME_DIR), + object.clone() + ], + "the loader resolves the object through its directory, so both must be admitted" + ); + } + + /// Workload children are launched from more than one place: the sandbox + /// entrypoint, and `sandbox exec` through the boundary. Every one of them + /// sets `LD_PRELOAD`, so every one of them needs the loader to be able to + /// open the shim. Composing the admission with the caller's own runtime + /// paths keeps a launch path from silently omitting it. + #[cfg(target_os = "linux")] + #[test] + fn runtime_read_only_composition_admits_caller_paths_and_the_shim() { + let certificate = PathBuf::from("/run/openshell-ca/ca.pem"); + let object = PathBuf::from(format!( + "{}/{}", + openshell_accept_shim::RUNTIME_DIR, + openshell_accept_shim::FILE_NAME + )); + + assert_eq!( + effective_runtime_read_only(&[certificate.clone()], None), + vec![certificate.clone()], + "without a shim the caller's paths pass through unchanged" + ); + + assert_eq!( + effective_runtime_read_only(&[certificate.clone()], Some(&object)), + vec![ + certificate, + PathBuf::from(openshell_accept_shim::RUNTIME_DIR), + object, + ], + "the shim must be admitted alongside whatever the launch path supplied" + ); + } + + /// The dynamic loader opens the shim as the workload user, after Landlock + /// is enforced. A world-readable mode is not sufficient on its own. + #[cfg(target_os = "linux")] + #[test] + #[allow(unsafe_code)] + fn preloaded_shim_remains_readable_after_landlock_for_non_root_workload() { + let root = tempfile::tempdir_in("/tmp").unwrap(); + std::fs::set_permissions(root.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + let shim_directory = root.path().join("openshell-compat"); + std::fs::create_dir(&shim_directory).unwrap(); + let object = shim_directory.join(openshell_accept_shim::FILE_NAME); + std::fs::write(&object, openshell_accept_shim::SHIM_OBJECT).unwrap(); + std::fs::set_permissions(&object, std::fs::Permissions::from_mode(0o555)).unwrap(); + // Tighten the directory only after the object exists, matching how the + // installer leaves it and keeping the test runnable as a normal user. + std::fs::set_permissions(&shim_directory, std::fs::Permissions::from_mode(0o555)).unwrap(); + let denied = root.path().join("not-authorized"); + std::fs::write(&denied, b"unrelated").unwrap(); + std::fs::set_permissions(&denied, std::fs::Permissions::from_mode(0o444)).unwrap(); + + let mut policy = policy_with_process(ProcessPolicy::default()); + policy.landlock = LandlockPolicy { + compatibility: openshell_core::policy::LandlockCompatibility::HardRequirement, + }; + let runtime_paths = shim_runtime_read_only_paths(Some(&object)); + let Ok(Some(prepared)) = prepare_child_sandbox(&policy, None, &runtime_paths) else { + return; + }; + + match unsafe { fork() }.expect("fork should succeed") { + ForkResult::Child => { + let dropped = if nix::unistd::geteuid().is_root() { + unsafe { + libc::setgroups(0, std::ptr::null()) == 0 + && libc::setgid(42_235) == 0 + && libc::setuid(42_234) == 0 + } + } else { + true + }; + let valid = dropped + && sandbox::linux::enforce(prepared).is_ok() + && std::fs::read(&object).is_ok() + && std::fs::read(&denied).is_err(); + unsafe { libc::_exit(i32::from(!valid)) }; + } + ForkResult::Parent { child } => assert_eq!( + waitpid(child, None).expect("waitpid should succeed"), + WaitStatus::Exited(child, 0), + "Landlock must admit the preloaded shim so the loader can open it" + ), + } + } + #[tokio::test] async fn inject_provider_env_skips_supervisor_identity_material() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/docs/about/support-matrix.mdx b/docs/about/support-matrix.mdx index 9cace1a708..c88ab266f2 100644 --- a/docs/about/support-matrix.mdx +++ b/docs/about/support-matrix.mdx @@ -184,22 +184,49 @@ kernels — notably RHEL 9.x and RHCOS, which ship a 5.14 kernel — the sandbox cannot install a kill-only listener, so it falls back to a plain listener and runs in a **legacy read-only** cancellation mode. The sandbox starts and enforces the full isolation boundary (Landlock, the outer NetworkPolicy fence, -DNS and TCP authorization); the only difference is that the broker refuses the -mediated operations that write results back into workload memory, failing them -closed with `EOPNOTSUPP`: +DNS and TCP authorization); the difference is that the broker cannot write +mediated results back into workload memory. -- `getpeername`; -- `accept` / `accept4` **when a non-null peer-address argument is supplied** - (a null address argument still works); -- `sendmmsg` paths that write per-message lengths back to the caller. +The sandbox compensates for the two operations that server workloads depend on. +`getpeername` is answered by the kernel itself on sockets that are genuinely +connected to the peer the broker recorded, so it returns the true peer address +in this mode. For `accept` and `accept4` with a non-null peer-address argument, +the sandbox preloads a small compatibility library into the workload that +rewrites the call into a null-address `accept4` followed by `getpeername`, and +fills the caller's buffer in the workload's own address space. + +`sendmmsg` paths that write per-message lengths back to the caller still fail +closed with `EOPNOTSUPP`. Socket creation, `connect`, `bind`, `listen`, `sendto`, and `sendmsg` are -unaffected. Outbound-oriented workloads generally run unchanged; server -workloads whose accept wrappers request the peer address will see `EOPNOTSUPP` -until the node runs a kernel that provides `WAIT_KILLABLE_RECV` (Linux 5.19+, or -a distribution backport). The selected mode is reported in the sandbox -qualification output as `seccomp_listener_mode` (`killable` or -`legacy_read_only`). +unaffected. The selected mode is reported in the sandbox qualification output +as `seccomp_listener_mode` (`killable` or `legacy_read_only`). + +#### What the compatibility library covers + +The library is injected with `LD_PRELOAD`, so it interposes library symbols +rather than system calls. It reaches runtimes that reach `accept` through libc +and does not reach runtimes that issue the system call directly. + +| Runtime | Address-bearing `accept` | `getpeername` | +| --- | --- | --- | +| Node.js | Not needed — libuv passes a null address | Supported | +| Bun | Supported | Supported | +| CPython | Supported | Supported | +| Go | Not supported — `net` issues the system call directly | Supported | +| Statically linked binaries | Not supported — no dynamic loader | Supported | + +`getpeername` is supported everywhere because the kernel answers it, with no +dependence on the preload. + +A workload that sets its own `LD_PRELOAD` keeps it; the sandbox composes its +entry with the existing value rather than replacing it. The library is a +compatibility aid, not a security control: a workload that removes it gains no +access it did not already have, because the broker's authorization decisions use +the kernel's own view of each socket and never the library's output. + +Run a Linux 5.19 or newer kernel — or a distribution backport that provides +`WAIT_KILLABLE_RECV` — to avoid this mode and its remaining gaps entirely. On macOS, these kernel modules run inside the Docker Desktop Linux VM, not on the host kernel. diff --git a/docs/kubernetes/openshift.mdx b/docs/kubernetes/openshift.mdx index c6189b1ebb..cea81923b8 100644 --- a/docs/kubernetes/openshift.mdx +++ b/docs/kubernetes/openshift.mdx @@ -24,15 +24,20 @@ OpenShell fails sandbox startup when either capability-free runtime probe fails. OpenShift nodes run RHCOS, which currently ships a RHEL 9.x kernel (5.14). That kernel predates `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` (Linux 5.19), so the sandbox starts in a reduced **legacy read-only** cancellation mode. Isolation is -unchanged, but the broker fails closed with `EOPNOTSUPP` on the mediated -operations that write results back into workload memory — `getpeername`, -`accept`/`accept4` with a non-null peer-address argument, and `sendmmsg` -per-message length write-backs. Outbound-oriented workloads run unchanged; -server workloads that read the peer address on accept need a node kernel with -`WAIT_KILLABLE_RECV` (Linux 5.19+, or a distribution backport). See the +unchanged, but the broker cannot write mediated results back into workload +memory. + +`getpeername` and peer-address `accept`/`accept4` are compensated for, so Node, +Bun, and CPython servers read the correct peer address on this kernel. Go +servers and statically linked binaries still receive `EOPNOTSUPP` from an +address-bearing `accept`, and `sendmmsg` per-message length write-backs still +fail closed. See the [support matrix](/about/support-matrix#legacy-read-only-mode-kernels-before-linux-519) -for the full behavior; the selected mode is reported as `seccomp_listener_mode` -in the sandbox qualification output. +for the full behavior and the per-runtime coverage table; the selected mode is +reported as `seccomp_listener_mode` in the sandbox qualification output. + +To remove the remaining gaps, run nodes on a kernel that provides +`WAIT_KILLABLE_RECV` (Linux 5.19+, or a distribution backport). ## Prerequisites diff --git a/e2e/rust/tests/peer_address.rs b/e2e/rust/tests/peer_address.rs new file mode 100644 index 0000000000..7697acbf1a --- /dev/null +++ b/e2e/rust/tests/peer_address.rs @@ -0,0 +1,120 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Verify that a workload accepting a connection learns who connected. +//! +//! `accept(fd, &addr, &len)` asks the kernel to write the peer address into +//! the caller's buffer. On kernels without +//! `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` the sandbox's seccomp broker +//! cannot write into workload memory and fails closed, so the workload sees +//! `EOPNOTSUPP` instead of a connection. RHEL 9 and its derivatives ship such +//! a kernel, and server frameworks that read the peer address are unusable +//! there. +//! +//! This test is implementation-agnostic: it asserts what the workload +//! observes, not how the sandbox arranges for it. On a kernel that supports +//! task-memory writes the broker answers directly; on one that does not, a +//! preloaded shim rewrites the call. Either way the peer must be reported +//! correctly. + +#![cfg(feature = "e2e")] + +use openshell_e2e::harness::sandbox::SandboxGuard; + +/// Python script that connects to its own listener and reports the peer. +/// +/// Both ends live inside the sandbox, which keeps the test independent of +/// port forwarding and exercises the directly connected socket case. The +/// script reports `errno` rather than raising so a fail-closed broker +/// produces a diagnosable result instead of a bare non-zero exit. +fn peer_address_script() -> &'static str { + r#" +import json, socket + +result = {} +server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +try: + server.bind(("127.0.0.1", 0)) + server.listen(1) + result["listen_port"] = server.getsockname()[1] + + client = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + client.settimeout(10) + client.connect(("127.0.0.1", result["listen_port"])) + result["client_port"] = client.getsockname()[1] + + # The call under test: socket.accept() passes an address buffer through + # libc, which is the shape a fail-closed broker rejects. + conn, peer = server.accept() + result["accept_peer_host"] = peer[0] + result["accept_peer_port"] = peer[1] + result["getpeername_port"] = conn.getpeername()[1] + + # Prove the accepted descriptor is the live connection, not just a + # plausible-looking number. + conn.sendall(b"ping") + result["echo"] = client.recv(4).decode() + result["accept"] = "ok" +except OSError as e: + result["accept"] = f"error:{e.errno}:{e}" + +print(json.dumps(result), flush=True) +"# +} + +/// A workload that accepts a connection must learn the connecting peer's +/// address, and that address must be the client's, not the listener's. +#[tokio::test] +async fn accept_reports_the_connecting_peer() { + let guard = SandboxGuard::create(&["--", "python3", "-c", peer_address_script()]) + .await + .expect("sandbox create"); + + let json_line = guard + .create_output + .lines() + .find(|l| l.contains("\"accept\"")) + .unwrap_or_else(|| panic!("no accept JSON in output:\n{}", guard.create_output)); + + let parsed: serde_json::Value = serde_json::from_str(json_line.trim()) + .unwrap_or_else(|e| panic!("failed to parse JSON '{json_line}': {e}")); + + let outcome = parsed["accept"].as_str().unwrap(); + assert_eq!( + outcome, "ok", + "accept() with a peer-address buffer failed. errno 95 (EOPNOTSUPP) means the \ + seccomp broker fell back to its fail-closed path and no compatibility shim \ + covered the call.\nFull output:\n{}", + guard.create_output + ); + + let listen_port = parsed["listen_port"].as_u64().unwrap(); + let client_port = parsed["client_port"].as_u64().unwrap(); + let accept_port = parsed["accept_peer_port"].as_u64().unwrap(); + let getpeername_port = parsed["getpeername_port"].as_u64().unwrap(); + + assert_eq!( + accept_port, client_port, + "accept() reported peer port {accept_port}, but the client is bound to {client_port}." + ); + // Reporting the listener's own address is the specific wrong answer a + // broker produces when it substitutes the socket it knows about. + assert_ne!( + accept_port, listen_port, + "accept() reported the listener's own port {listen_port} as the peer." + ); + assert_eq!( + parsed["accept_peer_host"].as_str().unwrap(), + "127.0.0.1", + "unexpected peer host for a loopback connection" + ); + assert_eq!( + getpeername_port, client_port, + "getpeername() on the accepted socket reported {getpeername_port}, expected {client_port}." + ); + assert_eq!( + parsed["echo"].as_str().unwrap(), + "ping", + "the accepted descriptor did not carry the connection" + ); +} diff --git a/scripts/update_license_headers.py b/scripts/update_license_headers.py index 48d7810171..5ec50bd1fb 100755 --- a/scripts/update_license_headers.py +++ b/scripts/update_license_headers.py @@ -38,6 +38,8 @@ COMMENT_STYLES: dict[str, str] = { ".rs": "//", ".proto": "//", + ".c": "//", + ".h": "//", ".py": "#", ".sh": "#", ".toml": "#", From 9d8842601ebd8285b881124d072ebdd80f97c309 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 1 Oct 2026 20:19:19 -0400 Subject: [PATCH 3/7] fix(sandbox): preserve shim cancellation and Windows builds Signed-off-by: Russell Bryant --- crates/openshell-accept-shim/README.md | 11 +- .../openshell-accept-shim/src/accept_shim.c | 32 +++++- crates/openshell-accept-shim/src/lib.rs | 7 ++ .../tests/fixtures/cancellation.c | 108 ++++++++++++++++++ .../tests/shim_behavior.rs | 77 +++++++++++-- 5 files changed, 216 insertions(+), 19 deletions(-) create mode 100644 crates/openshell-accept-shim/tests/fixtures/cancellation.c diff --git a/crates/openshell-accept-shim/README.md b/crates/openshell-accept-shim/README.md index ae921f84fd..59fcc66332 100644 --- a/crates/openshell-accept-shim/README.md +++ b/crates/openshell-accept-shim/README.md @@ -76,7 +76,7 @@ them is a bug: | --- | --- | | No `DT_NEEDED` | One build per architecture loads under both glibc and musl | | No `TEXTREL` | Avoids requiring SELinux `execmod`, the permission most likely denied to `container_t` | -| Exactly one undefined symbol, `__errno_location` | Both libcs export it; the loader resolves it from the already-loaded libc | +| Required `__errno_location` and weak `pthread_setcanceltype` references | Resolve from the workload's libc or already-loaded libpthread without adding a loader dependency | | Exactly two exported `FUNC` symbols, `accept` and `accept4` | Prevents accidental interposition of unrelated symbols such as `memcpy` | | ELF machine matches the Rust target | A host object loads nowhere, and the loader reports it as `cannot open shared object file` — indistinguishable from a policy denial | @@ -96,6 +96,15 @@ The code performs no allocation, takes no locks, and cannot panic. When address — what the kernel itself reports for an unnamed peer — rather than leaking the descriptor or failing an accept that has already succeeded. +The blocking accept phase preserves libc's pthread cancellation behavior by +temporarily switching the caller to asynchronous cancellation, then restoring +its previous cancellation type before querying the peer. Disabled cancellation +remains disabled. The pthread symbol is weak so a single-threaded workload on +older glibc does not need to load libpthread just to use the shim. + +Unix installation helpers and their tests are gated with `cfg(unix)`. The +preload-composition helpers also compile in the Windows workspace checks. + ## Installation `install_shim` materializes the embedded object at diff --git a/crates/openshell-accept-shim/src/accept_shim.c b/crates/openshell-accept-shim/src/accept_shim.c index c59e84935b..f5ae45b3d0 100644 --- a/crates/openshell-accept-shim/src/accept_shim.c +++ b/crates/openshell-accept-shim/src/accept_shim.c @@ -23,9 +23,10 @@ // // Raw syscalls are used rather than dlsym(RTLD_NEXT, ...) so the object needs // no DT_NEEDED entry and no loader-visible libc dependency. One build per -// architecture therefore loads correctly under both glibc and musl. The only -// undefined symbol is `__errno_location`, which both libcs export and the -// dynamic linker resolves from the already-loaded libc at relocation time. +// architecture therefore loads correctly under both glibc and musl. Both +// export `__errno_location`. The weak `pthread_setcanceltype` reference also +// resolves from libc or an already-loaded libpthread, but does not require +// loading libpthread in a single-threaded workload on older glibc. // // Interposing here covers runtimes that call these functions through the // PLT (CPython, Node, Bun, and anything else dynamically linked against @@ -36,6 +37,10 @@ typedef unsigned int shim_socklen_t; extern int *__errno_location(void); +extern int pthread_setcanceltype(int, int *) __attribute__((weak)); + +// glibc and musl both use 1 for PTHREAD_CANCEL_ASYNCHRONOUS. +#define SHIM_CANCEL_ASYNCHRONOUS 1 #if defined(__x86_64__) #define SHIM_NR_ACCEPT 43 @@ -97,16 +102,31 @@ static int shim_finish(long result) { return (int)result; } +// libc's accept wrappers are cancellation points. Temporarily enabling +// asynchronous cancellation gives the raw blocking syscall the same behavior, +// including cancellation already pending on entry. Preserve cancellation state +// (a disabled caller stays disabled) and restore the caller's type immediately +// after the syscall, before reporting the peer address. +static long shim_cancellable_accept4(int sockfd, int flags) { + int old_type = 0; + int changed = pthread_setcanceltype != 0 && + pthread_setcanceltype(SHIM_CANCEL_ASYNCHRONOUS, &old_type) == 0; + long accepted = shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags); + if (changed) { + pthread_setcanceltype(old_type, 0); + } + return accepted; +} + static int shim_accept4(int sockfd, void *addr, shim_socklen_t *addrlen, int flags) { // Without an output buffer the broker's existing path already works, so // forward unchanged and preserve its exact semantics. if (addr == 0 || addrlen == 0) { - return shim_finish( - shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags)); + return shim_finish(shim_cancellable_accept4(sockfd, flags)); } - long accepted = shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags); + long accepted = shim_cancellable_accept4(sockfd, flags); if (accepted < 0) { return shim_finish(accepted); } diff --git a/crates/openshell-accept-shim/src/lib.rs b/crates/openshell-accept-shim/src/lib.rs index 8b51c90c1b..a5fa38253a 100644 --- a/crates/openshell-accept-shim/src/lib.rs +++ b/crates/openshell-accept-shim/src/lib.rs @@ -32,6 +32,7 @@ pub const RUNTIME_DIR: &str = "/run/openshell-compat"; /// before being renamed into place. The shim is a compatibility aid rather /// than a security control — a workload can always decline to load it — but /// installing it must still never write through a path the workload chose. +#[cfg(unix)] pub fn install_object_at( directory: &std::path::Path, contents: &[u8], @@ -144,6 +145,7 @@ fn preload_contains(value: &str, path: &str) -> bool { #[cfg(test)] mod tests { use super::*; + #[cfg(unix)] use std::os::unix::fs::PermissionsExt as _; /// The object is produced by a C compiler chosen at build time, so a @@ -178,6 +180,7 @@ mod tests { ); } + #[cfg(unix)] fn scratch_dir(name: &str) -> std::path::PathBuf { let base = std::env::temp_dir().join(format!("openshell-accept-shim-{name}")); let _ = std::fs::remove_dir_all(&base); @@ -185,6 +188,7 @@ mod tests { } #[test] + #[cfg(unix)] fn installing_creates_a_read_only_executable_object() { let directory = scratch_dir("install"); let installed = install_object_at(&directory, b"shim-bytes").expect("install"); @@ -200,6 +204,7 @@ mod tests { } #[test] + #[cfg(unix)] fn installing_twice_replaces_the_previous_object() { // A sandbox restart re-materializes into a directory that may already // hold a previous generation of the shim. @@ -219,6 +224,7 @@ mod tests { } #[test] + #[cfg(unix)] fn a_symlinked_directory_is_refused() { // The directory lives on a workload-writable rootfs, so a redirect // must never be followed into a path the workload chose. @@ -232,6 +238,7 @@ mod tests { } #[test] + #[cfg(unix)] fn a_symlinked_target_file_is_refused() { let directory = scratch_dir("symlink-file"); std::fs::create_dir_all(&directory).expect("create"); diff --git a/crates/openshell-accept-shim/tests/fixtures/cancellation.c b/crates/openshell-accept-shim/tests/fixtures/cancellation.c new file mode 100644 index 0000000000..86917028a8 --- /dev/null +++ b/crates/openshell-accept-shim/tests/fixtures/cancellation.c @@ -0,0 +1,108 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Run in its own C process: pthread cancellation must not unwind Rust frames. +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static int listener; +static int use_accept4; +static int with_address; +static int disable_cancellation; +static atomic_int ready; +static atomic_int go; + +static void *worker(void *unused) { + (void)unused; + if (disable_cancellation && + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0) { + abort(); + } + atomic_store(&ready, 1); + // Stage cancellation before accept, without passing a cancellation point. + while (!atomic_load(&go)) { + sched_yield(); + } + struct sockaddr_storage peer; + socklen_t length = sizeof(peer); + struct sockaddr *address = with_address ? (struct sockaddr *)&peer : NULL; + socklen_t *address_length = with_address ? &length : NULL; + int accepted = use_accept4 ? accept4(listener, address, address_length, 0) + : accept(listener, address, address_length); + if (!disable_cancellation || accepted < 0) { + abort(); + } + close(accepted); + int old_type; + int old_state; + if (pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &old_type) != 0 || + old_type != PTHREAD_CANCEL_DEFERRED || + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_state) != 0 || + old_state != PTHREAD_CANCEL_DISABLE) { + abort(); + } + // The shim must preserve disabled cancellation and restore the type. + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); + pthread_testcancel(); + abort(); +} + +int main(int argc, char **argv) { + if (argc != 4) { + return 2; + } + use_accept4 = atoi(argv[1]); + with_address = atoi(argv[2]); + disable_cancellation = atoi(argv[3]); + listener = socket(AF_UNIX, SOCK_STREAM, 0); + struct sockaddr_un address = {.sun_family = AF_UNIX}; + snprintf(address.sun_path + 1, sizeof(address.sun_path) - 1, + "openshell-cancellation-%ld", (long)getpid()); + if (listener < 0 || bind(listener, (struct sockaddr *)&address, + sizeof(address)) != 0 || listen(listener, 1) != 0) { + perror("listener"); + return 1; + } + pthread_t thread; + if (pthread_create(&thread, NULL, worker, NULL) != 0) { + return 1; + } + while (!atomic_load(&ready)) { + sched_yield(); + } + if (pthread_cancel(thread) != 0) { + return 1; + } + int client = -1; + if (disable_cancellation) { + client = socket(AF_UNIX, SOCK_STREAM, 0); + if (client < 0 || connect(client, (struct sockaddr *)&address, + sizeof(address)) != 0) { + perror("connect"); + return 1; + } + } + atomic_store(&go, 1); + struct timespec deadline; + clock_gettime(CLOCK_REALTIME, &deadline); + deadline.tv_sec += 2; + void *result; + int error = pthread_timedjoin_np(thread, &result, &deadline); + if (error != 0 || result != PTHREAD_CANCELED) { + fprintf(stderr, "join error=%d; expected a cancelled worker\n", error); + return 1; + } + if (client >= 0) { + close(client); + } + close(listener); + return 0; +} diff --git a/crates/openshell-accept-shim/tests/shim_behavior.rs b/crates/openshell-accept-shim/tests/shim_behavior.rs index a13371fb9d..4fc49c0664 100644 --- a/crates/openshell-accept-shim/tests/shim_behavior.rs +++ b/crates/openshell-accept-shim/tests/shim_behavior.rs @@ -51,8 +51,8 @@ impl Shim { let c_path = CString::new(path.as_os_str().as_encoded_bytes()) .expect("shim path has no interior NUL"); // RTLD_NOW so an unresolved symbol fails here rather than at the call - // site; the object's sole undefined symbol is `__errno_location`, - // which the already-loaded libc provides. + // site; libc provides the required `__errno_location`, while the weak + // pthread reference resolves from libc or an already-loaded libpthread. let handle = unsafe { libc::dlopen(c_path.as_ptr(), RTLD_NOW | RTLD_LOCAL) }; assert!(!handle.is_null(), "dlopen: {}", dl_error()); @@ -102,9 +102,11 @@ fn pending_connection(bind: SocketAddr) -> Pending { } /// What the kernel reports for `fd`'s peer, with no truncation. +// The kernel copies sockaddr bytes; no typed Rust access requires alignment. +#[allow(clippy::cast_ptr_alignment)] fn kernel_peer(fd: c_int) -> (Vec, socklen_t) { let mut storage = [0u8; size_of::()]; - let mut length = storage.len() as socklen_t; + let mut length = socklen_t::try_from(storage.len()).unwrap(); let result = unsafe { libc::getpeername(fd, storage.as_mut_ptr().cast::(), &raw mut length) }; assert_eq!(result, 0, "getpeername: {}", Error::last_os_error()); @@ -116,6 +118,8 @@ fn kernel_peer(fd: c_int) -> (Vec, socklen_t) { /// Returns the accepted descriptor, the whole buffer, and the `addrlen` the /// shim reported, so a caller can check both what was written and what was /// left alone. +// The kernel copies sockaddr bytes; no typed Rust access requires alignment. +#[allow(clippy::cast_ptr_alignment)] fn shim_accept4(listener: &TcpListener, capacity: socklen_t) -> (c_int, Vec, socklen_t) { const SENTINEL: u8 = 0xAA; let mut buffer = vec![SENTINEL; size_of::()]; @@ -145,7 +149,7 @@ fn the_reported_peer_matches_the_kernel_for_ipv4() { .expect("client local addr") .port(); - let capacity = size_of::() as socklen_t; + let capacity = socklen_t::try_from(size_of::()).unwrap(); let (accepted, buffer, length) = shim_accept4(&pending.listener, capacity); let (kernel, kernel_length) = kernel_peer(accepted); @@ -175,7 +179,7 @@ fn the_reported_peer_matches_the_kernel_for_ipv6() { .expect("client local addr") .port(); - let capacity = size_of::() as socklen_t; + let capacity = socklen_t::try_from(size_of::()).unwrap(); let (accepted, buffer, length) = shim_accept4(&pending.listener, capacity); let (kernel, kernel_length) = kernel_peer(accepted); @@ -195,7 +199,7 @@ fn a_short_buffer_truncates_and_still_reports_the_full_length() { // Linux signals truncation by returning an `addrlen` larger than the one // supplied. A caller that trusts the returned length would read past its // own buffer if the shim reported the truncated length instead. - let full = size_of::() as socklen_t; + let full = socklen_t::try_from(size_of::()).unwrap(); for capacity in [0, 4, 8, full - 1] { let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); @@ -223,11 +227,14 @@ fn a_short_buffer_truncates_and_still_reports_the_full_length() { #[test] fn an_oversized_buffer_is_filled_only_to_the_address_length() { let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); - let oversized = size_of::() as socklen_t; + let oversized = socklen_t::try_from(size_of::()).unwrap(); let (accepted, buffer, length) = shim_accept4(&pending.listener, oversized); - assert_eq!(length, size_of::() as socklen_t); + assert_eq!( + length, + socklen_t::try_from(size_of::()).unwrap() + ); assert!( buffer[length as usize..].iter().all(|byte| *byte == 0xAA), "bytes beyond the address must be left alone" @@ -250,7 +257,7 @@ fn a_v4_mapped_peer_is_reported_as_the_kernel_reports_it() { }; let expected_port = client.local_addr().expect("client local addr").port(); - let capacity = size_of::() as socklen_t; + let capacity = socklen_t::try_from(size_of::()).unwrap(); let (accepted, buffer, length) = shim_accept4(&listener, capacity); let (kernel, kernel_length) = kernel_peer(accepted); @@ -286,7 +293,7 @@ fn the_accepted_descriptor_carries_the_connection() { // stream was unusable. let mut pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); - let capacity = size_of::() as socklen_t; + let capacity = socklen_t::try_from(size_of::()).unwrap(); let (accepted, _, _) = shim_accept4(&pending.listener, capacity); pending.client.write_all(b"ping").expect("client write"); @@ -306,6 +313,8 @@ fn the_accepted_descriptor_carries_the_connection() { } #[test] +// The kernel copies sockaddr bytes; no typed Rust access requires alignment. +#[allow(clippy::cast_ptr_alignment)] fn the_accept_entry_point_behaves_like_accept4_without_flags() { let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); let expected_port = pending @@ -315,7 +324,7 @@ fn the_accept_entry_point_behaves_like_accept4_without_flags() { .port(); let mut buffer = vec![0xAAu8; size_of::()]; - let mut length = size_of::() as socklen_t; + let mut length = socklen_t::try_from(size_of::()).unwrap(); let accepted = unsafe { (shim().accept)( pending.listener.as_raw_fd(), @@ -331,6 +340,8 @@ fn the_accept_entry_point_behaves_like_accept4_without_flags() { } #[test] +// The kernel copies sockaddr bytes; no typed Rust access requires alignment. +#[allow(clippy::cast_ptr_alignment)] fn a_failing_accept_reports_the_kernel_error() { // `shim_finish` must translate a raw negative return into `-1` plus // `errno`; a workload that sees the raw value instead would treat a @@ -346,7 +357,7 @@ fn a_failing_accept_reports_the_kernel_error() { }; let mut buffer = vec![0u8; size_of::()]; - let mut length = buffer.len() as socklen_t; + let mut length = socklen_t::try_from(buffer.len()).unwrap(); let result = unsafe { (shim().accept4)( stream.as_raw_fd(), @@ -359,3 +370,45 @@ fn a_failing_accept_reports_the_kernel_error() { assert_eq!(result, -1, "accept on a connected socket must fail"); assert_eq!(Error::last_os_error().raw_os_error(), Some(EINVAL)); } + +#[test] +fn preloaded_accept_preserves_pthread_cancellation() { + let directory = + std::env::temp_dir().join(format!("openshell-shim-cancellation-{}", process_id())); + std::fs::create_dir_all(&directory).expect("helper directory"); + let object = install_object_at(&directory.join("shim"), SHIM_OBJECT).expect("install shim"); + let executable = directory.join("cancellation"); + let status = std::process::Command::new("cc") + .args(["-std=c11", "-Wall", "-Wextra", "-Werror", "-pthread"]) + .arg(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/cancellation.c" + )) + .arg("-o") + .arg(&executable) + .status() + .expect("compile cancellation helper"); + assert!(status.success(), "compile cancellation helper: {status}"); + + for accept4 in [false, true] { + for address in [false, true] { + for disabled in [false, true] { + let args = [accept4, address, disabled].map(|flag| if flag { "1" } else { "0" }); + // Pin the expected libc behavior before testing interposition. + for preload in [false, true] { + let mut command = std::process::Command::new(&executable); + command.args(args).env_remove("LD_PRELOAD"); + if preload { + command.env("LD_PRELOAD", &object); + } + let output = command.output().expect("run cancellation helper"); + assert!( + output.status.success(), + "accept4={accept4} address={address} disabled={disabled} preload={preload}: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + } + } + } +} From de27081b33a188a99b19b85beaa04f511d501c80 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 1 Oct 2026 20:39:20 -0400 Subject: [PATCH 4/7] docs(sandbox): clarify legacy getpeername support Signed-off-by: Russell Bryant --- docs/about/support-matrix.mdx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/about/support-matrix.mdx b/docs/about/support-matrix.mdx index c88ab266f2..e1dff0dc25 100644 --- a/docs/about/support-matrix.mdx +++ b/docs/about/support-matrix.mdx @@ -208,7 +208,7 @@ The library is injected with `LD_PRELOAD`, so it interposes library symbols rather than system calls. It reaches runtimes that reach `accept` through libc and does not reach runtimes that issue the system call directly. -| Runtime | Address-bearing `accept` | `getpeername` | +| Runtime | Address-bearing `accept` | `getpeername` on directly connected sockets | | --- | --- | --- | | Node.js | Not needed — libuv passes a null address | Supported | | Bun | Supported | Supported | @@ -216,8 +216,10 @@ and does not reach runtimes that issue the system call directly. | Go | Not supported — `net` issues the system call directly | Supported | | Statically linked binaries | Not supported — no dynamic loader | Supported | -`getpeername` is supported everywhere because the kernel answers it, with no -dependence on the preload. +For directly connected sockets, including accepted sockets, `getpeername` works +across runtimes without the preload. Relayed outbound sockets still require the +broker to write the original peer address into workload memory, so `getpeername` +returns `EOPNOTSUPP` for those sockets in legacy read-only mode. A workload that sets its own `LD_PRELOAD` keeps it; the sandbox composes its entry with the existing value rather than replacing it. The library is a From 15498e497f2859707374f35cd65fd2a88a244f9a Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Fri, 2 Oct 2026 15:20:08 -0400 Subject: [PATCH 5/7] fix(sandbox): allow legacy outbound peer queries Signed-off-by: Russell Bryant --- crates/openshell-accept-shim/README.md | 9 +- .../openshell-sandbox/src/network_broker.rs | 141 ++- docs/about/support-matrix.mdx | 11 +- docs/kubernetes/openshift.mdx | 5 +- docs/superpowers/accept-shim-slides.html | 898 ++++++++++++++++++ e2e/reproducers/accept-peer.md | 176 ++++ e2e/reproducers/accept-peer.py | 34 + e2e/reproducers/outbound-tcp-echo.yaml | 56 ++ .../outbound-tcp-policy.template.yaml | 19 + e2e/reproducers/outbound-tcp.md | 158 +++ e2e/reproducers/outbound-tcp.py | 54 ++ 11 files changed, 1543 insertions(+), 18 deletions(-) create mode 100644 docs/superpowers/accept-shim-slides.html create mode 100644 e2e/reproducers/accept-peer.md create mode 100644 e2e/reproducers/accept-peer.py create mode 100644 e2e/reproducers/outbound-tcp-echo.yaml create mode 100644 e2e/reproducers/outbound-tcp-policy.template.yaml create mode 100644 e2e/reproducers/outbound-tcp.md create mode 100644 e2e/reproducers/outbound-tcp.py diff --git a/crates/openshell-accept-shim/README.md b/crates/openshell-accept-shim/README.md index 59fcc66332..e384166442 100644 --- a/crates/openshell-accept-shim/README.md +++ b/crates/openshell-accept-shim/README.md @@ -61,8 +61,13 @@ This is why the library's location on disk carries no privilege weight. | Go | Not covered | `net` issues the syscall instruction directly | | Static / `AT_SECURE` binaries | Not covered | No dynamic loader, or the preload is ignored | -`getpeername` is fixed for every runtime, including Go, because the kernel -answers it. +On directly connected sockets, `getpeername` reports the true peer for every +runtime, including Go, because the kernel answers it. In legacy mode the +broker also continues `getpeername` on relayed outbound sockets, allowing the +query to succeed with the loopback relay's address rather than the upstream +destination. This fallback does not need the shim and does not change outbound +authorization. Modern listeners still substitute the original upstream address +through the broker's safe task-memory write path. ## Build invariants diff --git a/crates/openshell-sandbox/src/network_broker.rs b/crates/openshell-sandbox/src/network_broker.rs index 77837091f6..9636f69ed7 100644 --- a/crates/openshell-sandbox/src/network_broker.rs +++ b/crates/openshell-sandbox/src/network_broker.rs @@ -1588,8 +1588,14 @@ fn get_peer_name( let peer = match entry.state() { // The relay path leaves the workload's descriptor connected to a // loopback relay, so the kernel's peer is the relay address rather - // than the destination the workload asked for. The broker must keep - // writing `original_peer` itself. + // than the destination the workload asked for. Legacy mode cannot + // safely substitute the original destination in workload memory. + // Return the kernel's relay address instead of failing the query; + // connection authorization does not depend on this reported address. + SocketState::Connected { .. } if listener.writes_disabled() => { + return listener.respond_continue(notification.id); + } + // Modern listeners can safely preserve transparent peer reporting. SocketState::Connected { original_peer } => *original_peer, // These descriptors really are connected to the recorded peer, so the // kernel's own answer is identical to the broker's. Letting the kernel @@ -2082,14 +2088,14 @@ mod tests { /// A `NotificationListener` whose descriptor is not a seccomp listener, so /// any ioctl fails. Only useful for discriminating *which* path was taken. - fn fake_legacy_listener() -> NotificationListener { + fn fake_listener(mode: ListenerMode) -> NotificationListener { // SAFETY: dup returns a new descriptor or a negative error. let dup = unsafe { libc::dup(libc::STDERR_FILENO) }; assert!(dup >= 0, "dup stderr"); NotificationListener::from_fd_with_mode( // SAFETY: successful dup returned a new owned descriptor. unsafe { OwnedFd::from_raw_fd(dup) }, - ListenerMode::LegacyReadOnly, + mode, ) } @@ -2115,7 +2121,7 @@ mod tests { ("Local", SocketState::Local { peer }), ] { let (registry, installed) = registry_with_state(state); - let listener = fake_legacy_listener(); + let listener = fake_listener(ListenerMode::LegacyReadOnly); let error = get_peer_name( ®istry, &listener, @@ -2131,23 +2137,134 @@ mod tests { } #[test] - fn legacy_getpeername_still_substitutes_the_original_peer_for_relayed_sockets() { + fn legacy_getpeername_continues_for_relayed_outbound_sockets() { // A `Connected` descriptor is connected to a loopback relay, not to // the destination the workload asked for. CONTINUE here would hand the - // workload the relay's ephemeral address, so the broker must keep - // writing `original_peer` itself -- and keep failing closed when it - // cannot. + // workload the relay's ephemeral address. Legacy mode accepts that + // compatibility tradeoff rather than failing the query altogether. let (registry, installed) = registry_with_state(SocketState::Connected { original_peer: "203.0.113.7:443".parse().unwrap(), }); - let listener = fake_legacy_listener(); + let listener = fake_listener(ListenerMode::LegacyReadOnly); let error = get_peer_name( ®istry, &listener, getpeername_notification(installed.as_raw_fd()), ) - .expect_err("legacy listener must reject socket-address writes"); - assert_eq!(error.raw_os_error(), Some(libc::EOPNOTSUPP)); + .expect_err("the fake listener cannot complete the CONTINUE ioctl"); + assert_eq!(error.raw_os_error(), Some(libc::ENOTTY)); + } + + #[test] + fn killable_getpeername_still_substitutes_the_original_outbound_peer() { + let (registry, installed) = registry_with_state(SocketState::Connected { + original_peer: "203.0.113.7:443".parse().unwrap(), + }); + let listener = fake_listener(ListenerMode::Killable); + // The notification's invalid output-length pointer must be read on + // the substitution path. CONTINUE would instead fail with ENOTTY on + // this fake listener, so EFAULT proves modern mode keeps emulating. + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .expect_err("invalid workload output-length pointer"); + assert_eq!(error.raw_os_error(), Some(libc::EFAULT)); + } + + #[test] + fn legacy_getpeername_rejects_unconnected_sockets() { + let (registry, installed) = registry_with_state(SocketState::Created); + let listener = fake_listener(ListenerMode::LegacyReadOnly); + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .expect_err("unconnected socket has no peer"); + assert_eq!(error.raw_os_error(), Some(libc::ENOTCONN)); + } + + fn mediated_outbound_peer(mode: ListenerMode) -> io::Result { + use openshell_isolation_interface::linux::seccomp_notify::install_listener; + + let relay = TcpListener::bind("127.0.0.1:0").unwrap(); + let stream = TcpStream::connect(relay.local_addr().unwrap()).unwrap(); + let (_upstream, _) = relay.accept().unwrap(); + let mut registry = SocketRegistry::new(1, 2).unwrap(); + let tentative = registry + .stage( + stream.try_clone().unwrap().into(), + SocketMetadata { + family: InetFamily::V4, + kind: InetKind::Tcp, + close_on_exec: true, + nonblocking: false, + creator_generation: 1, + }, + ) + .unwrap(); + registry + .commit_with_state( + tentative, + SocketState::Connected { + original_peer: "203.0.113.7:443".parse().unwrap(), + }, + ) + .unwrap(); + let (sender, receiver) = std::sync::mpsc::channel(); + let workload = std::thread::spawn(move || { + let listener = install_listener(&[libc::SYS_getpeername]).unwrap(); + if mode == ListenerMode::Killable && listener.writes_disabled() { + sender.send(None).unwrap(); + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "kernel lacks WAIT_KILLABLE_RECV", + )); + } + sender.send(Some(listener)).unwrap(); + stream.peer_addr() + }); + let Some(installed) = receiver.recv_timeout(Duration::from_secs(5)).unwrap() else { + return workload.join().unwrap(); + }; + // SAFETY: installed owns a live descriptor; dup returns a distinct fd. + let fd = unsafe { libc::dup(installed.as_raw_fd()) }; + assert!(fd >= 0); + let listener = NotificationListener::from_fd_with_mode( + // SAFETY: successful dup returned one new owned descriptor. + unsafe { OwnedFd::from_raw_fd(fd) }, + mode, + ); + let notification = listener.receive().unwrap(); + if let Err(error) = get_peer_name(&Mutex::new(registry), &listener, notification) { + listener + .respond_errno(notification.id, error_to_errno(&error)) + .unwrap(); + } + workload.join().unwrap() + } + + #[test] + fn legacy_outbound_getpeername_returns_the_kernel_relay_address() { + let peer = mediated_outbound_peer(ListenerMode::LegacyReadOnly) + .expect("legacy outbound getpeername succeeds"); + assert!(peer.ip().is_loopback()); + assert_ne!(peer.port(), 0); + } + + #[test] + fn killable_outbound_getpeername_returns_the_original_destination() { + let result = mediated_outbound_peer(ListenerMode::Killable); + if let Err(error) = &result { + if error.kind() == io::ErrorKind::Unsupported { + eprintln!("skipping modern peer substitution: {error}"); + return; + } + } + let peer = result.expect("modern outbound getpeername succeeds"); + assert_eq!(peer, "203.0.113.7:443".parse().unwrap()); } #[test] diff --git a/docs/about/support-matrix.mdx b/docs/about/support-matrix.mdx index e1dff0dc25..3ee561ec6c 100644 --- a/docs/about/support-matrix.mdx +++ b/docs/about/support-matrix.mdx @@ -217,9 +217,14 @@ and does not reach runtimes that issue the system call directly. | Statically linked binaries | Not supported — no dynamic loader | Supported | For directly connected sockets, including accepted sockets, `getpeername` works -across runtimes without the preload. Relayed outbound sockets still require the -broker to write the original peer address into workload memory, so `getpeername` -returns `EOPNOTSUPP` for those sockets in legacy read-only mode. +across runtimes without the preload. In legacy read-only mode, `getpeername` +also succeeds on relayed outbound sockets, but returns the loopback relay's +address and port rather than the original upstream destination. The kernel +writes this result into the caller's buffer; the broker does not write into +workload memory. Clients that require the upstream address may still be +incompatible. Connection authorization remains based on the requested +destination. On kernels with `WAIT_KILLABLE_RECV`, the broker continues to +report the original upstream destination. A workload that sets its own `LD_PRELOAD` keeps it; the sandbox composes its entry with the existing value rather than replacing it. The library is a diff --git a/docs/kubernetes/openshift.mdx b/docs/kubernetes/openshift.mdx index cea81923b8..d4237445d1 100644 --- a/docs/kubernetes/openshift.mdx +++ b/docs/kubernetes/openshift.mdx @@ -30,7 +30,10 @@ memory. `getpeername` and peer-address `accept`/`accept4` are compensated for, so Node, Bun, and CPython servers read the correct peer address on this kernel. Go servers and statically linked binaries still receive `EOPNOTSUPP` from an -address-bearing `accept`, and `sendmmsg` per-message length write-backs still +address-bearing `accept`. For relayed outbound connections, `getpeername` +succeeds but reports the loopback relay's address and port instead of the +upstream destination. Clients that depend on the upstream peer address may +still be incompatible. `sendmmsg` per-message length write-backs still fail closed. See the [support matrix](/about/support-matrix#legacy-read-only-mode-kernels-before-linux-519) for the full behavior and the per-runtime coverage table; the selected mode is diff --git a/docs/superpowers/accept-shim-slides.html b/docs/superpowers/accept-shim-slides.html new file mode 100644 index 0000000000..af17613abb --- /dev/null +++ b/docs/superpowers/accept-shim-slides.html @@ -0,0 +1,898 @@ + + + + + +accept() in the OpenShell sandbox + + + + + + +
+ + +
+
OpenShell sandbox
+

What happens when a sandboxed
agent calls accept()

+
+ …and why it stops working on kernels older than 5.19. +
+
→ / space to advance  ·  ← to go back  ·  f for fullscreen
+
+ + +
+
+

The cast

+
The agent never touches the network. Every socket syscall is trapped by a seccomp filter, and the broker decides what happens next.
+
+
+ + + + Sandboxed agent + unprivileged process + socket() · bind() + listen() · accept() + + + + Kernel + + seccomp filter + traps socket syscalls + + + + Broker + openshell-sandbox + holds the real socket + applies policy + + + + Network + peers + + + + syscall + + notify + + real I/O + +
+
+ + +
+
+

First, accept() with no sandbox

+
A process waits on a listening socket. When a connection arrives, the kernel hands back a brand-new file descriptor for that one connection.
+
+
+ + + Any process + holds listening fd 3 + + + Kernel + accept queue + + 1 pending connection + + + Remote peer + connects in + + + accept(3) + + + + + → fd 7 + + The return value is a new connected socket. Everything that follows is about who is allowed to produce it. + +
+
+ + +
+
+

Now accept() inside the sandbox

+
The agent asks. The broker is the one that actually accepts — then hands the descriptor over.
+
+
+ + + + Kernel + + seccomp filter + user notification + + + Agent + + + Broker + + + Peer + connecting in + + + + + accept() + + + + + + ⏸ thread parked + + + + + notify + + + + + accept4() + + policy: allow / deny + broker now holds fd 12 + + + + + ADDFD_SEND + + resumes → fd 7 + + +
+
1
The agent calls accept() on what it believes is its listening socket.
+
2
The filter traps the syscall. The kernel parks the agent's thread and hands a notification to the broker.
+
3
The broker reads the notification: which thread, which syscall, which arguments.
+
4
The broker does its own accept4() on the real listener, applies policy, and ends up holding the connected socket.
+
5
The broker injects that descriptor into the agent's fd table and wakes the thread. accept() returns a number the agent never opened.
+
+
+
+ + +
+
+

Four ways the broker can answer

+
Three of them stay entirely on the broker's side of the process boundary. One does not.
+
+
+
+
+

Inject a descriptor

+

ADDFD_SEND — the kernel adds an fd the broker already owns to the agent's table, then returns it. Used in step 5.

+
+
+

Return a value

+

Hand back a plain integer or an errno. The kernel delivers it as the syscall's return value.

+
+
+

Let it through

+

CONTINUE — tell the kernel to just run the real syscall as if the filter had allowed it.

+
+
+
+
+

…or write into the agent's memory

+

Some syscalls return data through a caller-supplied output buffer, not through the return value. To emulate one of those, the broker has to reach across the process boundary and write into the agent's address space — process_vm_writev or /proc/pid/mem.

+
+
+
This last one is the only option that depends on where the agent's thread is right now.
+
+
+ + +
+
+

Why that write is dangerous

+
It is only safe if the agent's thread is guaranteed to still be parked at the syscall boundary.
+
+
+ + + Agent thread + + Broker + + time → + + + + + parked + + + + + starts the write + + + + + + ⚡ + signal + wakes, unwinds + + + + + stack reused + that address is something else now + + + + + + write lands + + corruption + + +
+
1
The agent's thread is parked inside the syscall, waiting for the broker's answer.
+
2
The broker begins writing the result into a buffer on that thread's stack.
+
3
A non-fatal signal arrives — SIGCHLD, SIGWINCH, job control, a Go runtime preemption. The thread wakes and the syscall unwinds.
+
4
The agent runs on. That stack region now holds something completely different.
+
5
The broker's write lands anyway. A cross-process TOCTOU, and a memory-corruption primitive handed to the thing we are sandboxing.
+
+
+
+ + +
+
+

WAIT_KILLABLE_RECV closes that window

+
A seccomp filter flag. It changes what kind of wait the notified thread sits in.
+
+
+
+
+

Plain listener

+

Interruptible wait — any signal can end it.

+ + + parked thread + + SIGCHLD + + resumes + the race on the previous slide is possible + +
+
+

WAIT_KILLABLE_RECV

+

Kill-only wait — only a fatal signal ends it.

+ + + parked thread + + + SIGCHLD + ✕ + + SIGKILL + the thread cannot move out from under the broker + +
+
+
+ Added in Linux 5.19. The guarantee kicks in once the broker has actually fetched the notification — + from that moment the thread is parked until the broker answers, or the process dies. + With it, the broker can safely write into agent memory. That is the whole point of the flag. +
+
+
+ + +
+
+

On older kernels we fall back

+
The flag is rejected with EINVAL, so we install a plain listener and give up the one capability that needed it.
+
+
+ + + seccomp(NEW_LISTENER + | WAIT_KILLABLE_RECV) + + + + ok + + ListenerMode::Killable + full mediation, memory writes allowed + + + + EINVAL + + seccomp(NEW_LISTENER) + retry, no extra flags + + + + LegacyReadOnly + sandbox still boots + + + + every memory write + → EOPNOTSUPP + +
+ Fail closed, not open — we refuse the write rather than race it. Input mediation and fd injection are unaffected.
+ RHEL 9.x and RHCOS ship 5.14, and the flag was not backported. These are not exotic hosts. +
+
+
+ + +
+
+

accept() is exactly the wrong shape

+
Its peer address is not a return value. It is an output buffer, in the agent's memory, that somebody has to fill in.
+
+
+ + + + + + accept(lfd, &addr, &len) + + agent memory + + addr + must be filled in + + process boundary + + + + Broker + + + + ✕ + + + cross-process write → refused → EOPNOTSUPP + + + accept4(lfd, NULL, NULL, 0) + + agent memory + nothing to fill + + + Broker + + + ADDFD_SEND + return value only + + + no buffer, no problem — works today + +
+
+ + +
+
+

So some runtimes simply stop working

+
Whether your server survives on a 5.14 host comes down to one argument your language runtime chose for you.
+
+
+
+ + + + + + + + +
RuntimeWhat it passes to acceptOn a pre-5.19 host
CPythonsock_accept() passes a sockaddr buffer✕ EOPNOTSUPP
Bunaccept4() with a peer buffer✕ EOPNOTSUPP
Gonet: raw syscall, with a peer buffer✕ EOPNOTSUPP
Node.jslibuv passes NULL, resolves the peer later✓ works
+
+
+ The symptom an agent author actually sees: a server workload on RHEL 9.x or RHCOS gets EOPNOTSUPP where it expects a connection. + Nothing in the policy denied it. The sandbox just cannot safely produce the answer. +
+
+
+ + +
+
+

The fix: let the kernel do the write

+
A preloaded shim splits the one syscall the broker cannot serve into two it can.
+
+
+ + + + + + 1 + get the connection + accept4(fd, NULL, NULL, flags) + + Agent + Kernel + Broker + + + no buffer + + notify + + ADDFD_SEND + + fd appears + + + no agent memory written + + + + + + + 2 + get the address + getpeername(newfd, &addr, &len) + + Agent + Kernel + Broker + + + has a buffer + + notify + + CONTINUE + + kernel writes &addr + + + the agent's own memory, written by the kernel + + + + The agent writes its own memory, so the race does not exist — and both halves are required. + + +
+
1
Drop the buffer. The broker injects the descriptor, exactly as in the normal flow.
+
2
Ask for the address separately. The broker answers CONTINUE, so the kernel runs the real getpeername() and fills the buffer.
+
✓
accept() returns a descriptor and a peer address, on a kernel that cannot support it directly.
+
+
+
+ + +
+
+

How the rewrite gets in there

+
A tiny shared object, preloaded into the workload, that interposes two libc symbols.
+
+
+
+
+

LD_PRELOAD

+

The sandbox sets LD_PRELOAD=/run/openshell-compat/accept_shim.so, preserving anything the workload already set. The dynamic loader resolves accept and accept4 to the shim first.

+
+
+

Freestanding C, not Rust

+

Built -nostdlib with no DT_NEEDED, so one object per architecture loads under both glibc and musl. Exactly two exported symbols, so nothing else gets interposed by accident.

+
+
+
+
+

Shipped inside the sandbox binary

+

Embedded with include_bytes! and written to disk at startup — only when the listener reports legacy mode. No compute driver needs a packaging change.

+
+
+

Best effort

+

If installation fails, it is logged and the sandbox starts anyway. You are back to the fail-closed behavior you already had, not worse.

+
+
+
+
+ + +
+
+

What it does and does not reach

+
LD_PRELOAD interposes library symbols, not syscalls. That bounds the fix precisely.
+
+
+
+
+ + + + + + + + + +
RuntimeAddress-bearing accept
CPython✓ fixed  goes through libc
Bun✓ fixed  goes through libc
Node.js— never broke
Go✕ not reached  emits the syscall itself
Static / AT_SECURE✕ not reached  no loader, or preload ignored
+
+ On directly connected sockets, including accepted sockets, getpeername works for every runtime, Go included. The broker answers CONTINUE; the kernel writes the true peer address. Relayed outbound sockets are different. +
+
+
+

Not a security control

+

Nothing in OpenShell trusts what this library returns.

+

The broker's loopback and authorization decisions use the kernel's peer address, from the broker's own accept4().

+

A workload can unset LD_PRELOAD, link statically, or issue raw syscalls. It gains nothing it did not already have — it only loses the compatibility benefit.

+
+
+
+
+ + +
+
+

Outbound has no true answer to give

+
The relay means the kernel's honest answer is not the address the workload asked for.
+
+
+ + workload asks to connect to 203.0.113.7:443 + + Workload TCP fd + a real kernel socket + + + Loopback relay + 127.0.0.1:<port> + + + Supervisor + opens the upstream TCP fd + + + Upstream server + 203.0.113.7:443 + The kernel's honest answer is the relay address. Only the broker knows the upstream one. + +
+
+

Accepted sockets: the kernel is correct

+

The injected fd really is connected to the accepted peer. The broker can let the kernel fill the buffer in the workload's own address space.

+
+
+

Outbound relay: only the broker knows

+

The broker remembers original_peer. Reporting it means writing into workload memory — and without WAIT_KILLABLE_RECV that write fails closed with EOPNOTSUPP.

+
+
+
The shim cannot help here. It interposes accept and accept4, and no rewrite makes the kernel report a peer this socket is not connected to. So the choice is between a correct answer and a successful one.
+
+
+ + +
+
+

Before: TCP worked, introspection didn'treplaced

+
The behavior we are replacing, measured on OpenShift 4.21.34 · RHCOS kernel 5.14.0-570.141.1.el9_6.x86_64.
+
+
+
+
+ + + + + + + + +
OperationObserved result
connect()✓ 9 connections
getpeername()✕ errno 95 on all 9
Send and receive after the error✓ 4 bytes + 256 KiB each
Repeat without LD_PRELOAD✓ TCP still works
+
Initial workload and both exec runs exited 0. The echo server saw the supervisor's IP, confirming traffic passed through the outbound relay.
+
+
+

Why we changed it

+

A client requiring a successful getpeername() could abort, even though the connection could send and receive data.

+

Our baseline probe caught errno 95 and continued. A library propagating that error could fail the whole request.

+

A kernel with WAIT_KILLABLE_RECV (Linux 5.19+ or a backport) enables the broker's required address write. The next slide shows the implemented legacy fallback.

+
+
+
Probe: e2e/reproducers/outbound-tcp.py · Three connections per run: initial workload, exec, and exec without preload. This checks raw TCP, not TLS or every client library.
+
+
+ + +
+
+

After: answer CONTINUE anywayimplemented

+
Legacy kernels only. We trade an answer we cannot deliver for one we can.
+
+
+
+
+

Before the fallback

+

The broker tried to write original_peer into workload memory, could not, and failed closed.

+

getpeername() → errno 95
+ no address at all

+
+
+

After, on a legacy kernel

+

The broker responds CONTINUE. The kernel fills the buffer in the workload's own memory — safely, as on the accept path.

+

getpeername() → 127.0.0.1:42317
+ the relay, not the upstream

+
+
+
+ + + + + + + + +
What the client does with the addressEffect of the change
Calls it only to confirm the socket is connected✓ now succeeds
Logs or displays it⚠ shows the relay
Compares it to an expected host, or uses it for an ACL✕ wrong decision
Caches it to reconnect later✕ wrong decision
+
+
+ Modern kernels still get the original upstream address. OpenShell authorization stays based on the requested destination. + OpenShift: 9/9 peer queries and echoes passed, including exec without preload. Real seccomp/TCP tests verify both modes; clients needing the upstream address can still be incompatible. +
+
+
+ + +
+

The whole thing, in six lines

+
+
    +
  1. The broker accepts connections on the agent's behalf and injects the descriptor back.
    2. +
  2. Emulating a syscall that returns data through a buffer means writing into another process's memory.
    3. +
  3. WAIT_KILLABLE_RECV (Linux 5.19+) is what makes that write safe. Without it we fail closed.
    4. +
  4. accept(fd, &addr, &len) is exactly that shape, so servers break on RHEL 9.x and RHCOS.
    5. +
  5. For accepted sockets, a preloaded shim splits it into accept4(…, NULL, NULL) + getpeername(), so the kernel writes the address instead.
    6. +
  6. On legacy kernels, outbound getpeername() now answers CONTINUE and returns the relay address — succeeding with a different answer rather than failing with none.
    7. +
+
TCP data transfer already works on the legacy kernel. Peer queries now succeed too; clients that require the original upstream address may still fail.
+
+
+ +
+
+
+
+ + + + diff --git a/e2e/reproducers/accept-peer.md b/e2e/reproducers/accept-peer.md new file mode 100644 index 0000000000..0eb8a5e756 --- /dev/null +++ b/e2e/reproducers/accept-peer.md @@ -0,0 +1,176 @@ + + +# OpenShift peer-address reproducer + +This procedure validates [PR #4087](https://github.com/NVIDIA/OpenShell/pull/4087). +The standard-library-only [Python reproducer](accept-peer.py) connects a client +to a loopback listener inside an OpenShell sandbox. It checks that both +`accept()` and `getpeername()` return the client's address, rather than the +listener's, and that the accepted socket delivers `ping`. + +## Prerequisites and deployment + +Use an OpenShift cluster whose kernel lacks `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` +(the tested RHCOS kernel is listed below). Have `oc`, `helm`, `mise`, the local +CLI, and the upstream Agent Sandbox controller installed. Authenticate to the +cluster and its development image registry. Run commands from the repository root. + +Record the cluster and source version: + +```shell +git rev-parse HEAD +oc get clusterversion version -o jsonpath='{.status.desired.version}{"\n"}' +oc get nodes -o custom-columns=NAME:.metadata.name,KERNEL:.status.nodeInfo.kernelVersion +``` + +Build and stage the sandbox runtime from the checkout, then package it with +the repository's image build task. For a Mac host and x86-64 cluster, use: + +```shell +ulimit -n 8192 +mise exec -- env RUSTC_WRAPPER= cargo zigbuild --release --locked \ + --target x86_64-unknown-linux-musl -p openshell-sandbox +install -m 0755 target/x86_64-unknown-linux-musl/release/openshell-sandbox \ + deploy/docker/.build/prebuilt-binaries/amd64/openshell-sandbox +PREBUILT_AUTO_STAGE=0 DOCKER_PLATFORM=linux/amd64 IMAGE_TAG=pr4087-de27081b3 \ + mise run docker:build:sandbox +``` + +Push this uniquely tagged image to a registry the cluster can pull. This run +used Podman and the OpenShift development registry, with `--tls-verify=false` +for that registry's certificate: + +```shell +REGISTRY_HOST="$(oc -n openshift-image-registry get route default-route -o jsonpath='{.spec.host}')" +podman push --tls-verify=false localhost/openshell/sandbox:pr4087-de27081b3 \ + "docker://${REGISTRY_HOST}/openshell-images/sandbox:pr4087-de27081b3" +``` + +Create a separate test deployment using the existing development gateway's +values and gateway/supervisor images. Inspect those values before reuse; this +run used plaintext transport through a local port-forward, no public Route, +and `allowUnauthenticatedUsers: true`. Keep this configuration scoped to the +test deployment. Only the sandbox runtime was rebuilt for this run. + +```shell +helm get values openshell -n openshell -o yaml > /tmp/pr4087-existing-values.yaml +oc create namespace openshell-pr4087-test +oc -n openshell-images policy add-role-to-group system:image-puller \ + system:serviceaccounts:openshell-pr4087-test +helm install openshell deploy/helm/openshell -n openshell-pr4087-test \ + -f /tmp/pr4087-existing-values.yaml \ + --set sandboxRuntime.image.tag=pr4087-de27081b3 \ + --set server.otlp.endpoint= --wait --timeout 5m +oc -n openshell-pr4087-test port-forward svc/openshell 18088:8080 +``` + +Keep the port-forward running; execute subsequent commands in another terminal. +The reused values point all runtime images at +`image-registry.openshift-image-registry.svc:5000/openshell-images` and apply +`deploy/helm/openshell/ci/values-openshift-scc.yaml`'s security settings. +No privileged SCC grant was added. + +## Run the reproducer + +Use a Python image and pass the small script directly; no upload, package +installation, external connection, or forwarded application port is needed. +`target/debug/openshell` is the prebuilt local CLI used in this run. + +```shell +CLI=target/debug/openshell +ENDPOINT=http://127.0.0.1:18088 +PYTHON_IMAGE=ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 + +"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ + --name accept-peer-pr4087 --from "$PYTHON_IMAGE" --no-tty \ + -- python3 -c "$(cat e2e/reproducers/accept-peer.py)" +``` + +Expect exit status 0 and JSON containing `"result": "PASS"`, identical +`client`, `accept_peer`, and `peer` addresses, and +`"preload": "/run/openshell-compat/accept_shim.so"`. Ports vary per run. + +The initial command exits and the sandbox becomes Completed. To check the +separate exec launch path, create a sandbox that runs the same check and then +stays alive for ten minutes: + +```shell +"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ + --name peer-exec-pr4087 --from "$PYTHON_IMAGE" --detach \ + -- python3 -u -c "$(cat e2e/reproducers/accept-peer.py) +import time +time.sleep(600)" + +"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ + --no-tty --no-login-shell --timeout 30 peer-exec-pr4087 \ + -- python3 -c "$(cat e2e/reproducers/accept-peer.py)" +``` + +Expect the same PASS output and exit status 0. For a negative control, start +Python through `env` so its dynamic loader does not receive the preload: + +```shell +"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ + --no-tty --no-login-shell --timeout 30 peer-exec-pr4087 \ + -- env -u LD_PRELOAD python3 -c "$(cat e2e/reproducers/accept-peer.py)" +``` + +On the affected kernel, expect exit status 1 and +`OSError: [Errno 95] Operation not supported` at `server.accept()`. This control +demonstrates that the shim covers the failing call; it is not a comparison +against an independently built base-branch image. On kernels supporting +`WAIT_KILLABLE_RECV`, this control can succeed. + +Check the actual workload binary and admission profile: + +```shell +oc -n openshell-pr4087-test exec default--accept-peer-pr4087 -c agent \ + -- sha256sum /.openshell/runtime/openshell-sandbox +oc -n openshell-pr4087-test get pod default--accept-peer-pr4087 \ + -o jsonpath='{.metadata.annotations.openshift\.io/scc}{"\n"}{.spec.containers[0].securityContext}{"\n"}{.spec.securityContext}{"\n"}' +``` + +## Observed results + +Run on 2026-10-02 against source commit +`de27081b3` (PR head). OpenShift **4.21.34**, node +`control-plane-cluster-48kkt-1`, kernel **5.14.0-570.141.1.el9_6.x86_64**. + +| Check | Result | +| --- | --- | +| Initial command | PASS; client, accept peer, and getpeername ports all 50372 | +| Persistent sandbox's initial command | PASS; all three ports 39438 | +| `sandbox exec` | PASS; all three ports 53330 | +| Exec with preload unset | Expected failure, errno 95 at `accept()`, exit 1 | +| Workload admission | `restricted-v2`; UID/GID 1000780000; all capabilities dropped; privilege escalation disabled; `RuntimeDefault` seccomp; SELinux level `s0:c28,c12` | +| Binary provenance | Deployed binary SHA-256 matched the freshly built local binary | + +Recorded runtime binary SHA-256: +`dc004b17fd02c4bc0cb8208850b1dc29dcfa7a76ab80536f5ce8dc0eaf91333c`. + +Recorded image digests in `openshell-images`: + +- Sandbox: `sha256:8d43dd76b5936386eaee5978d346d79bcc4a89aaee59b1c28ff7330df15d36a7`. +- Gateway (reused): `sha256:83c7fde9ae6a197ab6c7c668086946beafc0eac99f2ea169ff1003fbaca9896d`. +- Supervisor (reused): `sha256:a4404edd399429fe4cc543a923b011d78d82e6e4531172cb8ad7bf9fcc57b6b2`. + +This test covers dynamically linked CPython on x86-64 and both workload launch +paths. It does not establish support for Go, static binaries, other +architectures, or every runtime. The sandbox logged an unrelated warning that +the runtime cgroup's `pids.max` is unlimited. + +## Cleanup + +Remove the test release, namespace, and the scoped registry pull grant. Stop +the port-forward with Ctrl-C. The uniquely tagged registry image remains +available for reruns. + +```shell +helm uninstall openshell -n openshell-pr4087-test +oc delete namespace openshell-pr4087-test --wait=true --timeout=120s +oc -n openshell-images policy remove-role-from-group system:image-puller \ + system:serviceaccounts:openshell-pr4087-test +``` diff --git a/e2e/reproducers/accept-peer.py b/e2e/reproducers/accept-peer.py new file mode 100644 index 0000000000..5be6a52c7d --- /dev/null +++ b/e2e/reproducers/accept-peer.py @@ -0,0 +1,34 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Minimal loopback accept/getpeername check; run inside an OpenShell sandbox.""" + +import json +import os +import socket + +socket.setdefaulttimeout(10) +with socket.socket() as server, socket.socket() as client: + server.bind(("127.0.0.1", 0)) + server.listen(1) + client.connect(server.getsockname()) + expected = client.getsockname() + conn, accepted_peer = server.accept() + with conn: + peer = conn.getpeername() + conn.sendall(b"ping") + assert client.recv(4) == b"ping" + assert peer == expected, (peer, expected) + assert accepted_peer == expected, (accepted_peer, expected) + assert peer[1] != server.getsockname()[1] + print( + json.dumps( + { + "result": "PASS", + "client": expected, + "accept_peer": accepted_peer, + "peer": peer, + "preload": os.environ.get("LD_PRELOAD", ""), + } + ) + ) diff --git a/e2e/reproducers/outbound-tcp-echo.yaml b/e2e/reproducers/outbound-tcp-echo.yaml new file mode 100644 index 0000000000..9b80a66e46 --- /dev/null +++ b/e2e/reproducers/outbound-tcp-echo.yaml @@ -0,0 +1,56 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +apiVersion: v1 +kind: Pod +metadata: + name: echo + namespace: openshell-outbound-test + labels: + app: outbound-tcp-echo +spec: + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + containers: + - name: echo + image: ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: [ALL] + command: [python3, -u, -c] + args: + - | + import socket, threading + def echo(conn, peer): + total = 0 + with conn: + while data := conn.recv(65536): + total += len(data) + conn.sendall(data) + print(f"peer={peer} echoed_bytes={total}", flush=True) + with socket.socket() as server: + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.bind(("0.0.0.0", 5432)) + server.listen() + print("READY", flush=True) + while True: + conn, peer = server.accept() + threading.Thread(target=echo, args=(conn, peer), daemon=True).start() + readinessProbe: + tcpSocket: + port: 5432 + periodSeconds: 2 +--- +apiVersion: v1 +kind: Service +metadata: + name: echo + namespace: openshell-outbound-test +spec: + selector: + app: outbound-tcp-echo + ports: + - port: 5432 + targetPort: 5432 diff --git a/e2e/reproducers/outbound-tcp-policy.template.yaml b/e2e/reproducers/outbound-tcp-policy.template.yaml new file mode 100644 index 0000000000..64d4d769bc --- /dev/null +++ b/e2e/reproducers/outbound-tcp-policy.template.yaml @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +version: 1 +filesystem_policy: + include_workdir: true + read_only: [/usr, /lib, /proc, /dev/urandom, /etc] + read_write: [/tmp, /dev/null] +landlock: + compatibility: best_effort +network_policies: + echo: + name: outbound-tcp-echo + endpoints: + - host: echo.openshell-outbound-test.svc.cluster.local + port: 5432 + protocol: tcp + allowed_ips: ["ECHO_IP/32"] + binaries: + - path: "/**" diff --git a/e2e/reproducers/outbound-tcp.md b/e2e/reproducers/outbound-tcp.md new file mode 100644 index 0000000000..e7bbc5937d --- /dev/null +++ b/e2e/reproducers/outbound-tcp.md @@ -0,0 +1,158 @@ + + +# Outbound TCP on the legacy kernel + +[outbound-tcp.py](outbound-tcp.py) opens three outbound connections to a separate +TCP echo Pod. On each connection it calls `getpeername()`, records any error, +then sends and receives both 4 bytes and 256 KiB. It compares every echoed byte +with the original payload. Pass `--expect-relay` to require a successful peer +query that returns a loopback address with a nonzero port. Without that flag, +the script records peer-address errors so it can also reproduce the baseline. + +## Test procedure + +Use an OpenShell Kubernetes gateway whose sandbox namespace is +`openshell-outbound-test`, with the PR-head sandbox runtime deployed. The +[peer-address procedure](accept-peer.md) describes building and deploying that +runtime. Build from the checkout containing the fallback, and use image tag +`pr4087-outbound-fallback` in the build, push, and Helm commands instead of +the baseline tag `pr4087-de27081b3`. For this test, use namespace +`openshell-outbound-test` and local +gateway forwarding port `18089`. Standard chart deployments require working +gateway and workspace PVC provisioning. The recorded run used ephemeral +gateway database and workspace volumes in its temporary test deployment; +the workload's seccomp, capabilities, and network restrictions stayed enabled. + +Start the echo fixture and scope the network policy to its exact Service IP: + +```shell +oc apply -f e2e/reproducers/outbound-tcp-echo.yaml +oc -n openshell-outbound-test wait --for=condition=Ready pod/echo --timeout=90s +ECHO_IP="$(oc -n openshell-outbound-test get service echo -o jsonpath='{.spec.clusterIP}')" +sed "s/ECHO_IP/${ECHO_IP}/g" e2e/reproducers/outbound-tcp-policy.template.yaml \ + > /tmp/pr4087-outbound-policy.yaml +oc -n openshell-outbound-test port-forward svc/openshell 18089:8080 +``` + +Keep the port-forward running and execute the following in another terminal: + +```shell +CLI=target/debug/openshell +ENDPOINT=http://127.0.0.1:18089 +HOST=echo.openshell-outbound-test.svc.cluster.local +PYTHON_IMAGE=ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 + +"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ + --name outbound3-pr4087 --policy /tmp/pr4087-outbound-policy.yaml \ + --from "$PYTHON_IMAGE" --no-tty \ + -- python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay +``` + +Expect `connect`, `getpeername`, and both `echo` stages to report PASS for +attempts 0, 1, and 2, with exit status 0. In legacy mode the peer address is the +loopback relay, not the echo Service's IP. For a before/after comparison, run +the probe without `--expect-relay` against the earlier runtime image first; +the recorded baseline below shows errno 95. Then deploy the fallback runtime +and run the strict check above. + +Check the exec launch path, then repeat without the accept shim: + +```shell +"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ + --name outbound-exec --policy /tmp/pr4087-outbound-policy.yaml \ + --from "$PYTHON_IMAGE" --detach \ + -- python3 -c 'import time; time.sleep(600)' + +"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ + --no-tty --no-login-shell --timeout 60 outbound-exec \ + -- python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay + +"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ + --no-tty --no-login-shell --timeout 60 outbound-exec \ + -- env -u LD_PRELOAD python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay +``` + +Both exec commands should produce the same data-transfer results. Check the +server logs and supervisor IPs to verify the relay path: + +```shell +oc -n openshell-outbound-test logs echo | rg 'echoed_bytes=262148' +oc -n openshell-outbound-test get pods -l openshell.ai/boundary-role=supervisor \ + -o custom-columns=NAME:.metadata.name,IP:.status.podIP +``` + +The server should record three nonempty connections per invocation, each +echoing 262148 bytes. Their peer IP should be the supervisor's. Zero-byte +connections from the node are the echo Pod's readiness probes. + +## Recorded baseline before the fallback + +The earlier behavior was validated on 2026-10-02, OpenShift **4.21.34**, kernel +**5.14.0-570.141.1.el9_6.x86_64**, using PR-head sandbox commit `de27081b3`. +The deployed binary matched SHA-256 +`dc004b17fd02c4bc0cb8208850b1dc29dcfa7a76ab80536f5ce8dc0eaf91333c`. +Gateway and supervisor image digests were the reused images recorded in +[the peer-address report](accept-peer.md#observed-results). + +| Invocation | Connections | Echoes | `getpeername()` | Exit | +| --- | --- | --- | --- | --- | +| Initial workload | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | +| `sandbox exec` | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | +| Exec without `LD_PRELOAD` | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | + +The first invocation's echo server saw supervisor `10.232.1.68` as its peer. +Supervisor logs recorded `ALLOWED` decisions for Python connecting to the echo +host on port 5432. The workload was admitted under `restricted-v2`, ran as UID/GID +1000780000, dropped all capabilities, and disabled privilege escalation. + +The baseline established that outbound TCP data transfer works even when the +peer query fails. The fallback changes that query to `CONTINUE`: the kernel +now reports the loopback relay's address. Clients that require the original +upstream address may still be incompatible. This probe does not validate TLS, +Internet routing, or application-client compatibility. + +## Fallback regression validation + +Native ARM64 Linux tests run as a non-root user verified the fallback with a +real seccomp listener and TCP connection: legacy mode returned a loopback peer +with a nonzero port, and modern mode returned the original upstream destination. +Additional tests covered local sockets, unconnected sockets, and the modern +substitution path. All 196 sandbox library tests passed. + +The fallback was also validated on 2026-10-02 on the same OpenShift 4.21.34 +cluster and kernel 5.14.0-570.141.1.el9_6.x86_64, with `--expect-relay`: + +| Invocation | Connections | Echoes | `getpeername()` | Exit | +| --- | --- | --- | --- | --- | +| Initial workload | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | +| `sandbox exec` | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | +| Exec without `LD_PRELOAD` | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | + +The runtime image was +`image-registry.openshift-image-registry.svc:5000/openshell-images/sandbox:pr4087-outbound-fallback`, +digest `sha256:4afb424c54d158ad06ab39c9b47dfea7c1815d58acf5dd404d1377aba36c6b59`. +The deployed binary matched SHA-256 +`544abc2524174e8fa0cb4a452b5c6d76c2bcbce77c923cace4860a74579c0bc8`. +The echo server recorded nine connections carrying 262148 bytes each, from +supervisor IPs `10.232.1.80` (initial workload) and `10.232.1.78` (exec). +Supervisor logs recorded policy `ALLOWED` decisions for the echo destination. +The workload used `restricted-v2`, UID/GID 1000790000, all capabilities dropped, +and privilege escalation disabled. Gateway and supervisor images were unchanged. + +## Cleanup + +Delete the named test sandboxes before removing a temporary gateway. Stop the +port-forward with Ctrl-C. Remove the namespace and pull grant only if they +were created for this test: + +```shell +"$CLI" sandbox delete --gateway-endpoint "$ENDPOINT" outbound3-pr4087 outbound-exec +oc delete -f e2e/reproducers/outbound-tcp-echo.yaml +helm uninstall openshell -n openshell-outbound-test +oc delete namespace openshell-outbound-test --wait=true --timeout=120s +oc -n openshell-images policy remove-role-from-group system:image-puller \ + system:serviceaccounts:openshell-outbound-test +``` diff --git a/e2e/reproducers/outbound-tcp.py b/e2e/reproducers/outbound-tcp.py new file mode 100644 index 0000000000..225cb91485 --- /dev/null +++ b/e2e/reproducers/outbound-tcp.py @@ -0,0 +1,54 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Check outbound relay data transfer independently of peer-address reporting.""" + +import argparse +import ipaddress +import json +import socket + +parser = argparse.ArgumentParser(description=__doc__) +parser.add_argument("host") +parser.add_argument("port", type=int) +parser.add_argument("--expect-relay", action="store_true") +args = parser.parse_args() +for attempt in range(3): + with socket.create_connection((args.host, args.port), timeout=10) as conn: + print( + json.dumps({"stage": "connect", "attempt": attempt, "result": "PASS"}), + flush=True, + ) + try: + peer = {"result": "PASS", "address": conn.getpeername()} + except OSError as error: + peer = {"result": "ERROR", "errno": error.errno, "message": str(error)} + print( + json.dumps({"stage": "getpeername", "attempt": attempt, **peer}), flush=True + ) + if args.expect_relay: + assert peer["result"] == "PASS", peer + assert ipaddress.ip_address(peer["address"][0]).is_loopback, peer + assert peer["address"][1] != 0, peer + # Exchange bytes after querying the peer, including after a baseline error. + for size in (4, 262144): + payload = b"ping" * (size // 4) + conn.sendall(payload) + received = bytearray() + while len(received) < size: + chunk = conn.recv(min(65536, size - len(received))) + if not chunk: + raise RuntimeError("unexpected EOF from echo server") + received.extend(chunk) + assert received == payload + print( + json.dumps( + { + "stage": "echo", + "attempt": attempt, + "bytes": size, + "result": "PASS", + } + ), + flush=True, + ) From e26aaba76ffd29c956316abbdb58f54b6e3e97ff Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Fri, 2 Oct 2026 15:48:54 -0400 Subject: [PATCH 6/7] docs: remove presentation slides from source control Signed-off-by: Russell Bryant --- docs/superpowers/accept-shim-slides.html | 898 ----------------------- 1 file changed, 898 deletions(-) delete mode 100644 docs/superpowers/accept-shim-slides.html diff --git a/docs/superpowers/accept-shim-slides.html b/docs/superpowers/accept-shim-slides.html deleted file mode 100644 index af17613abb..0000000000 --- a/docs/superpowers/accept-shim-slides.html +++ /dev/null @@ -1,898 +0,0 @@ - - - - - -accept() in the OpenShell sandbox - - - - - - -
- - -
-
OpenShell sandbox
-

What happens when a sandboxed
agent calls accept()

-
- …and why it stops working on kernels older than 5.19. -
-
→ / space to advance  ·  ← to go back  ·  f for fullscreen
-
- - -
-
-

The cast

-
The agent never touches the network. Every socket syscall is trapped by a seccomp filter, and the broker decides what happens next.
-
-
- - - - Sandboxed agent - unprivileged process - socket() · bind() - listen() · accept() - - - - Kernel - - seccomp filter - traps socket syscalls - - - - Broker - openshell-sandbox - holds the real socket - applies policy - - - - Network - peers - - - - syscall - - notify - - real I/O - -
-
- - -
-
-

First, accept() with no sandbox

-
A process waits on a listening socket. When a connection arrives, the kernel hands back a brand-new file descriptor for that one connection.
-
-
- - - Any process - holds listening fd 3 - - - Kernel - accept queue - - 1 pending connection - - - Remote peer - connects in - - - accept(3) - - - - - → fd 7 - - The return value is a new connected socket. Everything that follows is about who is allowed to produce it. - -
-
- - -
-
-

Now accept() inside the sandbox

-
The agent asks. The broker is the one that actually accepts — then hands the descriptor over.
-
-
- - - - Kernel - - seccomp filter - user notification - - - Agent - - - Broker - - - Peer - connecting in - - - - - accept() - - - - - - ⏸ thread parked - - - - - notify - - - - - accept4() - - policy: allow / deny - broker now holds fd 12 - - - - - ADDFD_SEND - - resumes → fd 7 - - -
-
1
The agent calls accept() on what it believes is its listening socket.
-
2
The filter traps the syscall. The kernel parks the agent's thread and hands a notification to the broker.
-
3
The broker reads the notification: which thread, which syscall, which arguments.
-
4
The broker does its own accept4() on the real listener, applies policy, and ends up holding the connected socket.
-
5
The broker injects that descriptor into the agent's fd table and wakes the thread. accept() returns a number the agent never opened.
-
-
-
- - -
-
-

Four ways the broker can answer

-
Three of them stay entirely on the broker's side of the process boundary. One does not.
-
-
-
-
-

Inject a descriptor

-

ADDFD_SEND — the kernel adds an fd the broker already owns to the agent's table, then returns it. Used in step 5.

-
-
-

Return a value

-

Hand back a plain integer or an errno. The kernel delivers it as the syscall's return value.

-
-
-

Let it through

-

CONTINUE — tell the kernel to just run the real syscall as if the filter had allowed it.

-
-
-
-
-

…or write into the agent's memory

-

Some syscalls return data through a caller-supplied output buffer, not through the return value. To emulate one of those, the broker has to reach across the process boundary and write into the agent's address space — process_vm_writev or /proc/pid/mem.

-
-
-
This last one is the only option that depends on where the agent's thread is right now.
-
-
- - -
-
-

Why that write is dangerous

-
It is only safe if the agent's thread is guaranteed to still be parked at the syscall boundary.
-
-
- - - Agent thread - - Broker - - time → - - - - - parked - - - - - starts the write - - - - - - ⚡ - signal - wakes, unwinds - - - - - stack reused - that address is something else now - - - - - - write lands - - corruption - - -
-
1
The agent's thread is parked inside the syscall, waiting for the broker's answer.
-
2
The broker begins writing the result into a buffer on that thread's stack.
-
3
A non-fatal signal arrives — SIGCHLD, SIGWINCH, job control, a Go runtime preemption. The thread wakes and the syscall unwinds.
-
4
The agent runs on. That stack region now holds something completely different.
-
5
The broker's write lands anyway. A cross-process TOCTOU, and a memory-corruption primitive handed to the thing we are sandboxing.
-
-
-
- - -
-
-

WAIT_KILLABLE_RECV closes that window

-
A seccomp filter flag. It changes what kind of wait the notified thread sits in.
-
-
-
-
-

Plain listener

-

Interruptible wait — any signal can end it.

- - - parked thread - - SIGCHLD - - resumes - the race on the previous slide is possible - -
-
-

WAIT_KILLABLE_RECV

-

Kill-only wait — only a fatal signal ends it.

- - - parked thread - - - SIGCHLD - ✕ - - SIGKILL - the thread cannot move out from under the broker - -
-
-
- Added in Linux 5.19. The guarantee kicks in once the broker has actually fetched the notification — - from that moment the thread is parked until the broker answers, or the process dies. - With it, the broker can safely write into agent memory. That is the whole point of the flag. -
-
-
- - -
-
-

On older kernels we fall back

-
The flag is rejected with EINVAL, so we install a plain listener and give up the one capability that needed it.
-
-
- - - seccomp(NEW_LISTENER - | WAIT_KILLABLE_RECV) - - - - ok - - ListenerMode::Killable - full mediation, memory writes allowed - - - - EINVAL - - seccomp(NEW_LISTENER) - retry, no extra flags - - - - LegacyReadOnly - sandbox still boots - - - - every memory write - → EOPNOTSUPP - -
- Fail closed, not open — we refuse the write rather than race it. Input mediation and fd injection are unaffected.
- RHEL 9.x and RHCOS ship 5.14, and the flag was not backported. These are not exotic hosts. -
-
-
- - -
-
-

accept() is exactly the wrong shape

-
Its peer address is not a return value. It is an output buffer, in the agent's memory, that somebody has to fill in.
-
-
- - - - - - accept(lfd, &addr, &len) - - agent memory - - addr - must be filled in - - process boundary - - - - Broker - - - - ✕ - - - cross-process write → refused → EOPNOTSUPP - - - accept4(lfd, NULL, NULL, 0) - - agent memory - nothing to fill - - - Broker - - - ADDFD_SEND - return value only - - - no buffer, no problem — works today - -
-
- - -
-
-

So some runtimes simply stop working

-
Whether your server survives on a 5.14 host comes down to one argument your language runtime chose for you.
-
-
-
- - - - - - - - -
RuntimeWhat it passes to acceptOn a pre-5.19 host
CPythonsock_accept() passes a sockaddr buffer✕ EOPNOTSUPP
Bunaccept4() with a peer buffer✕ EOPNOTSUPP
Gonet: raw syscall, with a peer buffer✕ EOPNOTSUPP
Node.jslibuv passes NULL, resolves the peer later✓ works
-
-
- The symptom an agent author actually sees: a server workload on RHEL 9.x or RHCOS gets EOPNOTSUPP where it expects a connection. - Nothing in the policy denied it. The sandbox just cannot safely produce the answer. -
-
-
- - -
-
-

The fix: let the kernel do the write

-
A preloaded shim splits the one syscall the broker cannot serve into two it can.
-
-
- - - - - - 1 - get the connection - accept4(fd, NULL, NULL, flags) - - Agent - Kernel - Broker - - - no buffer - - notify - - ADDFD_SEND - - fd appears - - - no agent memory written - - - - - - - 2 - get the address - getpeername(newfd, &addr, &len) - - Agent - Kernel - Broker - - - has a buffer - - notify - - CONTINUE - - kernel writes &addr - - - the agent's own memory, written by the kernel - - - - The agent writes its own memory, so the race does not exist — and both halves are required. - - -
-
1
Drop the buffer. The broker injects the descriptor, exactly as in the normal flow.
-
2
Ask for the address separately. The broker answers CONTINUE, so the kernel runs the real getpeername() and fills the buffer.
-
✓
accept() returns a descriptor and a peer address, on a kernel that cannot support it directly.
-
-
-
- - -
-
-

How the rewrite gets in there

-
A tiny shared object, preloaded into the workload, that interposes two libc symbols.
-
-
-
-
-

LD_PRELOAD

-

The sandbox sets LD_PRELOAD=/run/openshell-compat/accept_shim.so, preserving anything the workload already set. The dynamic loader resolves accept and accept4 to the shim first.

-
-
-

Freestanding C, not Rust

-

Built -nostdlib with no DT_NEEDED, so one object per architecture loads under both glibc and musl. Exactly two exported symbols, so nothing else gets interposed by accident.

-
-
-
-
-

Shipped inside the sandbox binary

-

Embedded with include_bytes! and written to disk at startup — only when the listener reports legacy mode. No compute driver needs a packaging change.

-
-
-

Best effort

-

If installation fails, it is logged and the sandbox starts anyway. You are back to the fail-closed behavior you already had, not worse.

-
-
-
-
- - -
-
-

What it does and does not reach

-
LD_PRELOAD interposes library symbols, not syscalls. That bounds the fix precisely.
-
-
-
-
- - - - - - - - - -
RuntimeAddress-bearing accept
CPython✓ fixed  goes through libc
Bun✓ fixed  goes through libc
Node.js— never broke
Go✕ not reached  emits the syscall itself
Static / AT_SECURE✕ not reached  no loader, or preload ignored
-
- On directly connected sockets, including accepted sockets, getpeername works for every runtime, Go included. The broker answers CONTINUE; the kernel writes the true peer address. Relayed outbound sockets are different. -
-
-
-

Not a security control

-

Nothing in OpenShell trusts what this library returns.

-

The broker's loopback and authorization decisions use the kernel's peer address, from the broker's own accept4().

-

A workload can unset LD_PRELOAD, link statically, or issue raw syscalls. It gains nothing it did not already have — it only loses the compatibility benefit.

-
-
-
-
- - -
-
-

Outbound has no true answer to give

-
The relay means the kernel's honest answer is not the address the workload asked for.
-
-
- - workload asks to connect to 203.0.113.7:443 - - Workload TCP fd - a real kernel socket - - - Loopback relay - 127.0.0.1:<port> - - - Supervisor - opens the upstream TCP fd - - - Upstream server - 203.0.113.7:443 - The kernel's honest answer is the relay address. Only the broker knows the upstream one. - -
-
-

Accepted sockets: the kernel is correct

-

The injected fd really is connected to the accepted peer. The broker can let the kernel fill the buffer in the workload's own address space.

-
-
-

Outbound relay: only the broker knows

-

The broker remembers original_peer. Reporting it means writing into workload memory — and without WAIT_KILLABLE_RECV that write fails closed with EOPNOTSUPP.

-
-
-
The shim cannot help here. It interposes accept and accept4, and no rewrite makes the kernel report a peer this socket is not connected to. So the choice is between a correct answer and a successful one.
-
-
- - -
-
-

Before: TCP worked, introspection didn'treplaced

-
The behavior we are replacing, measured on OpenShift 4.21.34 · RHCOS kernel 5.14.0-570.141.1.el9_6.x86_64.
-
-
-
-
- - - - - - - - -
OperationObserved result
connect()✓ 9 connections
getpeername()✕ errno 95 on all 9
Send and receive after the error✓ 4 bytes + 256 KiB each
Repeat without LD_PRELOAD✓ TCP still works
-
Initial workload and both exec runs exited 0. The echo server saw the supervisor's IP, confirming traffic passed through the outbound relay.
-
-
-

Why we changed it

-

A client requiring a successful getpeername() could abort, even though the connection could send and receive data.

-

Our baseline probe caught errno 95 and continued. A library propagating that error could fail the whole request.

-

A kernel with WAIT_KILLABLE_RECV (Linux 5.19+ or a backport) enables the broker's required address write. The next slide shows the implemented legacy fallback.

-
-
-
Probe: e2e/reproducers/outbound-tcp.py · Three connections per run: initial workload, exec, and exec without preload. This checks raw TCP, not TLS or every client library.
-
-
- - -
-
-

After: answer CONTINUE anywayimplemented

-
Legacy kernels only. We trade an answer we cannot deliver for one we can.
-
-
-
-
-

Before the fallback

-

The broker tried to write original_peer into workload memory, could not, and failed closed.

-

getpeername() → errno 95
- no address at all

-
-
-

After, on a legacy kernel

-

The broker responds CONTINUE. The kernel fills the buffer in the workload's own memory — safely, as on the accept path.

-

getpeername() → 127.0.0.1:42317
- the relay, not the upstream

-
-
-
- - - - - - - - -
What the client does with the addressEffect of the change
Calls it only to confirm the socket is connected✓ now succeeds
Logs or displays it⚠ shows the relay
Compares it to an expected host, or uses it for an ACL✕ wrong decision
Caches it to reconnect later✕ wrong decision
-
-
- Modern kernels still get the original upstream address. OpenShell authorization stays based on the requested destination. - OpenShift: 9/9 peer queries and echoes passed, including exec without preload. Real seccomp/TCP tests verify both modes; clients needing the upstream address can still be incompatible. -
-
-
- - -
-

The whole thing, in six lines

-
-
    -
  1. The broker accepts connections on the agent's behalf and injects the descriptor back.
    2. -
  2. Emulating a syscall that returns data through a buffer means writing into another process's memory.
    3. -
  3. WAIT_KILLABLE_RECV (Linux 5.19+) is what makes that write safe. Without it we fail closed.
    4. -
  4. accept(fd, &addr, &len) is exactly that shape, so servers break on RHEL 9.x and RHCOS.
    5. -
  5. For accepted sockets, a preloaded shim splits it into accept4(…, NULL, NULL) + getpeername(), so the kernel writes the address instead.
    6. -
  6. On legacy kernels, outbound getpeername() now answers CONTINUE and returns the relay address — succeeding with a different answer rather than failing with none.
    7. -
-
TCP data transfer already works on the legacy kernel. Peer queries now succeed too; clients that require the original upstream address may still fail.
-
-
- -
-
-
-
- - - - From bdd15bb89e4d3dc4363571e7b2fe5b571bec9311 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Fri, 2 Oct 2026 16:25:14 -0400 Subject: [PATCH 7/7] fix(sandbox): harden legacy accept compatibility Replace workload-owned shim paths with sealed anonymous objects isolated per launch. Keep descriptors close-on-exec in the boundary and enable inheritance only in the intended child. Preserve effective preload environment overrides without introducing a Landlock user ruleset. Delegate cancellation to libc's accept4 wrapper, close descriptors on peer lookup failure, and return ECONNABORTED for reset peers or EFAULT for invalid output pointers. Reject unsupported raw accepts before consuming clients and allow kernel peer queries on connected DNS sockets. Legacy outbound peer queries intentionally retain the documented loopback relay-address fallback. Add target-compiler fixtures and ELF, peer-error, legacy broker, and concurrent launch regressions. Update compatibility documentation and remove PR-specific reproducer artifacts. Signed-off-by: Russell Bryant --- Cargo.lock | 1 + crates/openshell-accept-shim/Cargo.toml | 3 + crates/openshell-accept-shim/README.md | 109 ++++---- crates/openshell-accept-shim/build.rs | 43 ++- .../openshell-accept-shim/src/accept_shim.c | 192 +++++++++---- crates/openshell-accept-shim/src/lib.rs | 253 +++++++----------- .../tests/fixtures/other_preload.c | 10 + .../tests/shim_behavior.rs | 176 ++++++++++-- crates/openshell-sandbox/src/boundary_exec.rs | 18 +- crates/openshell-sandbox/src/child_env.rs | 235 ++++++++++++++-- .../openshell-sandbox/src/network_broker.rs | 66 ++++- crates/openshell-sandbox/src/process.rs | 171 ++---------- docs/about/support-matrix.mdx | 7 + e2e/reproducers/accept-peer.md | 176 ------------ e2e/reproducers/accept-peer.py | 34 --- e2e/reproducers/outbound-tcp-echo.yaml | 56 ---- .../outbound-tcp-policy.template.yaml | 19 -- e2e/reproducers/outbound-tcp.md | 158 ----------- e2e/reproducers/outbound-tcp.py | 54 ---- 19 files changed, 799 insertions(+), 982 deletions(-) create mode 100644 crates/openshell-accept-shim/tests/fixtures/other_preload.c delete mode 100644 e2e/reproducers/accept-peer.md delete mode 100644 e2e/reproducers/accept-peer.py delete mode 100644 e2e/reproducers/outbound-tcp-echo.yaml delete mode 100644 e2e/reproducers/outbound-tcp-policy.template.yaml delete mode 100644 e2e/reproducers/outbound-tcp.md delete mode 100644 e2e/reproducers/outbound-tcp.py diff --git a/Cargo.lock b/Cargo.lock index 8185b06606..79663e0c30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3955,6 +3955,7 @@ version = "0.0.0" dependencies = [ "cc", "libc", + "object", ] [[package]] diff --git a/crates/openshell-accept-shim/Cargo.toml b/crates/openshell-accept-shim/Cargo.toml index 9c081fc466..df215d492b 100644 --- a/crates/openshell-accept-shim/Cargo.toml +++ b/crates/openshell-accept-shim/Cargo.toml @@ -19,5 +19,8 @@ libc = "0.2" # conventions, including the per-target wrappers cargo-zigbuild installs. cc = "1" +[dev-dependencies] +object = { version = "0.37.3", default-features = false, features = ["read_core", "elf", "std"] } + [lints] workspace = true diff --git a/crates/openshell-accept-shim/README.md b/crates/openshell-accept-shim/README.md index e384166442..44f81af3ba 100644 --- a/crates/openshell-accept-shim/README.md +++ b/crates/openshell-accept-shim/README.md @@ -47,8 +47,6 @@ statically, or issue raw syscalls, and gains nothing it did not already have — it only loses the compatibility benefit. The enforcement boundary remains the broker's fail-closed behavior. -This is why the library's location on disk carries no privilege weight. - ## Coverage `LD_PRELOAD` interposes library symbols, not syscalls. @@ -81,7 +79,7 @@ them is a bug: | --- | --- | | No `DT_NEEDED` | One build per architecture loads under both glibc and musl | | No `TEXTREL` | Avoids requiring SELinux `execmod`, the permission most likely denied to `container_t` | -| Required `__errno_location` and weak `pthread_setcanceltype` references | Resolve from the workload's libc or already-loaded libpthread without adding a loader dependency | +| Required `__errno_location` and `dl_iterate_phdr` references | Resolve from glibc or musl without a loader dependency | | Exactly two exported `FUNC` symbols, `accept` and `accept4` | Prevents accidental interposition of unrelated symbols such as `memcpy` | | ELF machine matches the Rust target | A host object loads nowhere, and the loader reports it as `cannot open shared object file` — indistinguishable from a policy denial | @@ -96,61 +94,56 @@ header against `CARGO_CFG_TARGET_ARCH` and fails the build on a mismatch, because a misresolved compiler otherwise produces a valid object for the wrong architecture. -The code performs no allocation, takes no locks, and cannot panic. When -`getpeername` fails on an already-accepted connection it reports a zero-length -address — what the kernel itself reports for an unnamed peer — rather than -leaking the descriptor or failing an accept that has already succeeded. - -The blocking accept phase preserves libc's pthread cancellation behavior by -temporarily switching the caller to asynchronous cancellation, then restoring -its previous cancellation type before querying the peer. Disabled cancellation -remains disabled. The pthread symbol is weak so a single-threaded workload on -older glibc does not need to load libpthread just to use the shim. - -Unix installation helpers and their tests are gated with `cfg(unix)`. The -preload-composition helpers also compile in the Windows workspace checks. - -## Installation - -`install_shim` materializes the embedded object at -`/run/openshell-compat/accept_shim.so`. The sandbox calls it during startup, -only when `listener.writes_disabled()` reports legacy mode. - -The path is constrained from both sides: - -- **Not under `/.openshell`.** The capability-free Landlock baseline grants each - top-level filesystem entry *except* the driver-owned `.openshell` hierarchy, - and a user ruleset can only narrow the baseline. A workload physically cannot - open a file there, so a library placed there could never be preloaded. -- **Not under the supervisor CA tmpfs.** That mount is `noexec`, so the loader - cannot map an object from it. - -`/run` satisfies both. On Docker and Podman it is part of the workload's own -writable, exec-capable rootfs. On Kubernetes the workload receives it as an -`emptyDir{medium: Memory}` tmpfs, mounted `rw,seclabel,relatime` with no -`noexec` and mode `1777`, so a non-root sandbox identity can create its own -subdirectory there. No compute driver needs a packaging change: the object is -embedded in the `openshell-sandbox` binary with `include_bytes!`. - -Installation creates the directory, writes through a temporary file, `rename`s -it into place, then seals both the file and the directory to `0o555`. Symlinked -directories and symlinked targets are refused rather than followed. Failure is -non-fatal and logged as an OCSF `Config State Change` event — the sandbox starts -without the library and keeps the broker's existing fail-closed behavior. - -`LD_PRELOAD` composition preserves any value the workload supplied and is -idempotent, so a child that re-inherits the variable and spawns its own child -does not accumulate duplicate entries. +The shim resolves libc's `accept4` from the already-loaded ELF image that +provides `__errno_location`, using `dl_iterate_phdr`. Delegating the blocking +phase preserves libc's cancellation handling, including the accepted-fd race +addressed by glibc BZ #12683. Peer lookup and error-path close use raw syscalls, +which are not cancellation points. A failed peer lookup closes the descriptor +and returns the kernel error; `ENOTCONN` becomes `ECONNABORTED` so a reset peer +does not produce a successful accept with an unusable address. Invalid output +pointers return `EFAULT` without userspace dereferences. + +## Installation and child environments + +The sandbox probes installation only for a legacy read-only listener. It +creates a sealed executable memfd instead of writing into the image's `/run`. +This works with a non-root identity, a read-only rootfs, and noexec temporary +mounts. There are no workload-selected pathname components or symlinks. Write, +grow, shrink, and seal seals prevent changing its bytes. The boundary's private +probe descriptor is close-on-exec. + +Each entrypoint or exec launch gets a fresh sealed memfd inode. Its descriptor +remains close-on-exec in the boundary and is made inheritable only in its own +forked child. The loader opens `/proc/self/fd/N`. +A workload can close or chmod its own inode, but cannot replace the object or +change permissions on the inode used by a later operator exec session. +Anonymous inodes need no extra Landlock admission, so the shim does not create +a restrictive user ruleset when the authored policy has none. + +Installation checks executable mapping before setting `LD_PRELOAD`. It requests +`MFD_EXEC` where supported and falls back to the pre-6.3 ABI on older kernels. +A host that forbids executable memfds keeps the broker's fail-closed behavior; +installation failure is non-fatal and logged. + +Both launch paths compose the shim after all environment sources have been +applied, preserving the final provider, workload, or per-session override. +Directly launched ELF binaries for a different class or architecture do not +receive the shim. Descendants inherit ordinary `LD_PRELOAD` semantics: a child +that closes the descriptor, hides `/proc`, or invokes a foreign-architecture +loader must remove the shim entry itself. The boundary cannot check arbitrary +descendant execs, and those loaders may otherwise emit a preload warning. +Static and secure-execution loaders do not use the shim. ## Validation -Validated on OpenShift 4.21 / RHCOS 9.6 (kernel `5.14.0-570.141.1.el9_6`, -SELinux enforcing), against the `emptyDir{medium: Memory}` mount the Kubernetes -workload actually receives, running as a non-root uid with -`readOnlyRootFilesystem: true`, all capabilities dropped, and the -`RuntimeDefault` seccomp profile: the object maps `r-xp` with zero AVC denials. - -When validating across architectures, check `e_machine` in the ELF header -before drawing conclusions. A wrong-architecture object produces -`cannot open shared object file` from the loader, which reads like an SELinux -denial and is not one. +Tests check the embedded target-compiler output for ELF target, no `DT_NEEDED`, +no text relocations, a non-executable stack, exactly `accept` and `accept4` +exports, and the expected libc references. Behavioral tests cover IPv4, IPv6, +truncation, reset peers, invalid length pointers, accepted streams, and pthread +cancellation, including coexistence with a provider preload defining `accept4`. +C fixtures use the same resolved target compiler as the shim. + +Sandbox tests exercise a forced legacy listener with the real broker and +preloaded CPython: a rejected raw address-bearing accept must leave its client +queued for a subsequent shimmed accept. They also check connected DNS peer +queries and shim access under unrestricted and restricted Landlock policies. diff --git a/crates/openshell-accept-shim/build.rs b/crates/openshell-accept-shim/build.rs index 684371ac45..7ff9518511 100644 --- a/crates/openshell-accept-shim/build.rs +++ b/crates/openshell-accept-shim/build.rs @@ -49,6 +49,47 @@ fn main() { .unwrap_or_else(|error| panic!("run C compiler {command:?}: {error}")); assert!(status.success(), "compile {SOURCE}: {status}"); + println!("cargo:rerun-if-changed=tests/fixtures/cancellation.c"); + let helper = out_dir.join("cancellation"); + let status = compiler_command() + .args([ + "-std=c11", + "-Wall", + "-Wextra", + "-Werror", + "-pthread", + "-lc", + "tests/fixtures/cancellation.c", + "-o", + ]) + .arg(&helper) + .status() + .expect("compile cancellation helper"); + assert!(status.success(), "compile cancellation helper: {status}"); + println!( + "cargo:rustc-env=OPENSHELL_CANCELLATION_HELPER={}", + helper.display() + ); + + println!("cargo:rerun-if-changed=tests/fixtures/other_preload.c"); + let other = out_dir.join("other_preload.so"); + let status = compiler_command() + .args([ + "-shared", + "-fPIC", + "-nostdlib", + "tests/fixtures/other_preload.c", + "-o", + ]) + .arg(&other) + .status() + .expect("compile other preload fixture"); + assert!(status.success(), "compile other preload fixture: {status}"); + println!( + "cargo:rustc-env=OPENSHELL_OTHER_PRELOAD={}", + other.display() + ); + verify_object_architecture(&object); println!("cargo:rustc-env=OPENSHELL_ACCEPT_SHIM={}", object.display()); @@ -62,7 +103,7 @@ fn main() { /// Guessing a cross prefix instead would pick a binary that is frequently /// absent on the build host. fn compiler_command() -> Command { - match cc::Build::new().cargo_metadata(false).try_get_compiler() { + match cc::Build::new().try_get_compiler() { Ok(compiler) => compiler.to_command(), Err(error) => panic!("locate a C compiler for the shim: {error}"), } diff --git a/crates/openshell-accept-shim/src/accept_shim.c b/crates/openshell-accept-shim/src/accept_shim.c index f5ae45b3d0..eb21820ffa 100644 --- a/crates/openshell-accept-shim/src/accept_shim.c +++ b/crates/openshell-accept-shim/src/accept_shim.c @@ -21,31 +21,133 @@ // Both halves are required: without the broker's CONTINUE for `getpeername` // step 2 fails closed for the same reason step 1 did. // -// Raw syscalls are used rather than dlsym(RTLD_NEXT, ...) so the object needs -// no DT_NEEDED entry and no loader-visible libc dependency. One build per -// architecture therefore loads correctly under both glibc and musl. Both -// export `__errno_location`. The weak `pthread_setcanceltype` reference also -// resolves from libc or an already-loaded libpthread, but does not require -// loading libpthread in a single-threaded workload on older glibc. -// -// Interposing here covers runtimes that call these functions through the -// PLT (CPython, Node, Bun, and anything else dynamically linked against -// libc). It cannot cover statically linked binaries or programs that issue -// the syscall instruction directly; those remain subject to the broker's -// fail-closed behavior. +// Delegate the blocking call to libc's accept4 wrapper. Its cancellation +// assembly distinguishes a cancelled syscall from one that already returned +// an fd (glibc BZ #12683). Switching to asynchronous cancellation around a raw +// syscall cannot make that distinction and can leak the accepted descriptor. +// Resolve the wrapper from loaded ELF objects without dlsym: older glibc puts +// dlsym in libdl, while dl_iterate_phdr is provided by both glibc and musl. +// ELF64 ABI definitions: keeping this source header-free lets the release +// compiler build it with -nostdlib, without selecting a libc sysroot. +typedef unsigned long size_t; +typedef unsigned long ElfAddr; +typedef struct { + unsigned int p_type, p_flags; + unsigned long p_offset, p_vaddr, p_paddr, p_filesz, p_memsz, p_align; +} ElfPhdr; +typedef struct { + long d_tag; + union { unsigned long d_ptr, d_val; } d_un; +} ElfDyn; +typedef struct { + unsigned int st_name; + unsigned char st_info, st_other; + unsigned short st_shndx; + unsigned long st_value, st_size; +} ElfSym; +// The first four fields are the stable dl_iterate_phdr ABI on glibc and musl. +struct dl_phdr_info { + ElfAddr dlpi_addr; + const char *dlpi_name; + const ElfPhdr *dlpi_phdr; + unsigned short dlpi_phnum; +}; +extern int dl_iterate_phdr(int (*)(struct dl_phdr_info *, size_t, void *), void *); +#define PT_LOAD 1 +#define PT_DYNAMIC 2 +#define DT_NULL 0 +#define DT_HASH 4 +#define DT_STRTAB 5 +#define DT_SYMTAB 6 +#define DT_GNU_HASH 0x6ffffef5 +#define SHN_UNDEF 0 +#define STT_FUNC 2 typedef unsigned int shim_socklen_t; - extern int *__errno_location(void); -extern int pthread_setcanceltype(int, int *) __attribute__((weak)); +int accept4(int, void *, shim_socklen_t *, int); +typedef int (*accept4_fn)(int, void *, shim_socklen_t *, int); +static accept4_fn libc_accept4; + +static unsigned long dynamic_pointer(unsigned long base, unsigned long value) { + // glibc relocates these pointers; musl leaves them relative to the DSO. + return value < base ? base + value : value; +} + +static int find_accept4(struct dl_phdr_info *info, size_t size, void *data) { + (void)size; + (void)data; + // Resolve only inside the DSO providing libc's errno accessor. Selecting + // an arbitrary provider preload's accept4 can recurse back through accept + // and need not preserve libc cancellation semantics. + int is_libc = 0; + for (unsigned int i = 0; i < info->dlpi_phnum; ++i) { + const ElfPhdr *segment = info->dlpi_phdr + i; + unsigned long start = info->dlpi_addr + segment->p_vaddr; + if (segment->p_type == PT_LOAD && (unsigned long)__errno_location >= start && + (unsigned long)__errno_location - start < segment->p_memsz) is_libc = 1; + } + if (!is_libc) return 0; + const ElfDyn *dynamic = 0; + for (unsigned int i = 0; i < info->dlpi_phnum; ++i) { + if (info->dlpi_phdr[i].p_type == PT_DYNAMIC) { + dynamic = (const ElfDyn *)(info->dlpi_addr + info->dlpi_phdr[i].p_vaddr); + break; + } + } + if (!dynamic) return 0; + const ElfSym *symbols = 0; + const char *strings = 0; + const unsigned int *hash = 0; + const unsigned int *gnu_hash = 0; + for (; dynamic->d_tag != DT_NULL; ++dynamic) { + unsigned long pointer = dynamic_pointer(info->dlpi_addr, dynamic->d_un.d_ptr); + if (dynamic->d_tag == DT_SYMTAB) symbols = (const ElfSym *)pointer; + if (dynamic->d_tag == DT_STRTAB) strings = (const char *)pointer; + if (dynamic->d_tag == DT_HASH) hash = (const unsigned int *)pointer; + if (dynamic->d_tag == DT_GNU_HASH) gnu_hash = (const unsigned int *)pointer; + } + if (!symbols || !strings) return 0; + unsigned int count = 0; + if (hash) { + count = hash[1]; + } else if (gnu_hash) { + // The last nonempty GNU hash bucket ends at the highest symbol index. + const unsigned int *buckets = (const unsigned int *) + ((const ElfAddr *)(gnu_hash + 4) + gnu_hash[2]); + const unsigned int *chains = buckets + gnu_hash[0]; + unsigned int last = 0; + for (unsigned int i = 0; i < gnu_hash[0]; ++i) + if (buckets[i] > last) last = buckets[i]; + if (last) { + count = last; + while (!(chains[count - gnu_hash[1]] & 1)) ++count; + ++count; + } + } + for (unsigned int i = 0; i < count; ++i) { + const ElfSym *symbol = symbols + i; + if (symbol->st_shndx == SHN_UNDEF || (symbol->st_info & 15) != STT_FUNC) + continue; + const char *name = strings + symbol->st_name; + const char wanted[] = "accept4"; + unsigned int j = 0; + while (name[j] && name[j] == wanted[j]) ++j; + if (name[j] != wanted[j]) continue; + accept4_fn candidate = (accept4_fn)(info->dlpi_addr + symbol->st_value); + libc_accept4 = candidate; + return 1; + } + return 0; +} -// glibc and musl both use 1 for PTHREAD_CANCEL_ASYNCHRONOUS. -#define SHIM_CANCEL_ASYNCHRONOUS 1 +__attribute__((constructor)) static void resolve_accept4(void) { + dl_iterate_phdr(find_accept4, 0); +} #if defined(__x86_64__) -#define SHIM_NR_ACCEPT 43 +#define SHIM_NR_CLOSE 3 #define SHIM_NR_GETPEERNAME 52 -#define SHIM_NR_ACCEPT4 288 static long shim_syscall3(long number, long a0, long a1, long a2) { long result; @@ -56,20 +158,9 @@ static long shim_syscall3(long number, long a0, long a1, long a2) { return result; } -static long shim_syscall4(long number, long a0, long a1, long a2, long a3) { - long result; - register long r10 __asm__("r10") = a3; - __asm__ volatile("syscall" - : "=a"(result) - : "a"(number), "D"(a0), "S"(a1), "d"(a2), "r"(r10) - : "rcx", "r11", "memory"); - return result; -} - #elif defined(__aarch64__) -#define SHIM_NR_ACCEPT 202 +#define SHIM_NR_CLOSE 57 #define SHIM_NR_GETPEERNAME 205 -#define SHIM_NR_ACCEPT4 242 static long shim_syscall4(long number, long a0, long a1, long a2, long a3) { register long x8 __asm__("x8") = number; @@ -102,42 +193,27 @@ static int shim_finish(long result) { return (int)result; } -// libc's accept wrappers are cancellation points. Temporarily enabling -// asynchronous cancellation gives the raw blocking syscall the same behavior, -// including cancellation already pending on entry. Preserve cancellation state -// (a disabled caller stays disabled) and restore the caller's type immediately -// after the syscall, before reporting the peer address. -static long shim_cancellable_accept4(int sockfd, int flags) { - int old_type = 0; - int changed = pthread_setcanceltype != 0 && - pthread_setcanceltype(SHIM_CANCEL_ASYNCHRONOUS, &old_type) == 0; - long accepted = shim_syscall4(SHIM_NR_ACCEPT4, sockfd, 0, 0, flags); - if (changed) { - pthread_setcanceltype(old_type, 0); - } - return accepted; -} - static int shim_accept4(int sockfd, void *addr, shim_socklen_t *addrlen, int flags) { // Without an output buffer the broker's existing path already works, so // forward unchanged and preserve its exact semantics. - if (addr == 0 || addrlen == 0) { - return shim_finish(shim_cancellable_accept4(sockfd, flags)); + if (!libc_accept4) { + return shim_finish(-95); // EOPNOTSUPP: unsupported dynamic loader. } - - long accepted = shim_cancellable_accept4(sockfd, flags); + int accepted = libc_accept4(sockfd, 0, 0, flags); if (accepted < 0) { - return shim_finish(accepted); + return accepted; } - // The connection is already established; a failure to report its address - // must not leak the descriptor or fail the accept. Report a zero-length - // address instead, which is the same thing the kernel reports for an - // unnamed peer, rather than leaving the caller's buffer undefined. - if (shim_syscall3(SHIM_NR_GETPEERNAME, accepted, (long)addr, - (long)addrlen) < 0) { - *addrlen = 0; + if (addr != 0) { + long result = shim_syscall3(SHIM_NR_GETPEERNAME, accepted, (long)addr, + (long)addrlen); + if (result < 0) { + // Never dereference output pointers in userspace. The kernel + // reports EFAULT, and a reset queued peer may report ENOTCONN. + shim_syscall3(SHIM_NR_CLOSE, accepted, 0, 0); + return shim_finish(result == -107 ? -103 : result); // ENOTCONN -> ECONNABORTED + } } return (int)accepted; } diff --git a/crates/openshell-accept-shim/src/lib.rs b/crates/openshell-accept-shim/src/lib.rs index a5fa38253a..4409f8fae2 100644 --- a/crates/openshell-accept-shim/src/lib.rs +++ b/crates/openshell-accept-shim/src/lib.rs @@ -17,106 +17,87 @@ pub const PRELOAD_ENV: &str = "LD_PRELOAD"; #[cfg(target_os = "linux")] pub const SHIM_OBJECT: &[u8] = include_bytes!(env!("OPENSHELL_ACCEPT_SHIM")); -/// Directory the shim is materialized into inside the workload. +/// Probe and retain a private shim object in the boundary. /// -/// This deliberately sits outside the driver-owned `/.openshell` hierarchy, -/// which the capability-free Landlock baseline never exposes to a workload. -/// The dynamic loader must be able to open and map the object, so it also has -/// to live outside the `noexec` tmpfs mounts that carry supervisor material. -pub const RUNTIME_DIR: &str = "/run/openshell-compat"; +/// This descriptor is +/// close-on-exec; workloads receive their own anonymous inode via +/// [`create_child_shim`], so changes to file permissions cannot affect later +/// sessions. Anonymous inodes need no Landlock user-ruleset admission. +#[cfg(target_os = "linux")] +pub fn install_shim() -> Result { + use std::os::fd::AsRawFd as _; + use std::sync::OnceLock; + static OBJECT: OnceLock = OnceLock::new(); + if OBJECT.get().is_none() { + let _ = OBJECT.set(create_object(true)?); + } + Ok(std::path::PathBuf::from(format!( + "/proc/self/fd/{}", + OBJECT.get().expect("installed object").as_raw_fd() + ))) +} -/// Materialize an object into `directory` as a read-only executable file. +/// Create a fresh sealed object for one workload launch. /// -/// The target rootfs is writable by the workload, so every component is -/// checked for symlink redirection and the file is created with `O_NOFOLLOW` -/// before being renamed into place. The shim is a compatibility aid rather -/// than a security control — a workload can always decline to load it — but -/// installing it must still never write through a path the workload chose. -#[cfg(unix)] -pub fn install_object_at( - directory: &std::path::Path, - contents: &[u8], -) -> Result { - use std::io::Write as _; - use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; +/// The descriptor is +/// close-on-exec in the boundary: the caller must retain the File until spawn +/// and clear CLOEXEC only in the forked child, so concurrent launches cannot +/// inherit and change permissions on one another's pending objects. +#[cfg(target_os = "linux")] +pub fn create_child_shim() -> Result { + create_object(true) +} - match std::fs::symlink_metadata(directory) { - Ok(metadata) if metadata.file_type().is_symlink() => { - return Err(format!( - "shim directory is a symlink: {}", - directory.display() - )); - } - Ok(metadata) if !metadata.is_dir() => { - return Err(format!( - "shim directory is not a directory: {}", - directory.display() - )); - } - Ok(_) => {} - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - std::fs::create_dir_all(directory).map_err(|error| { - format!("create shim directory {}: {error}", directory.display()) - })?; - } - Err(error) => { - return Err(format!( - "inspect shim directory {}: {error}", - directory.display() - )); - } +#[cfg(target_os = "linux")] +#[allow(unsafe_code)] +fn create_object(close_on_exec: bool) -> Result { + use std::io::Write as _; + use std::os::fd::FromRawFd as _; + let flags = libc::MFD_ALLOW_SEALING | if close_on_exec { libc::MFD_CLOEXEC } else { 0 }; + // MFD_EXEC overrides vm.memfd_noexec=1 on 6.3+. Older kernels reject + // that flag; retry with the original executable memfd ABI. + let mut fd = unsafe { libc::memfd_create(c"openshell-accept-shim".as_ptr(), flags | 0x10) }; + if fd < 0 && std::io::Error::last_os_error().raw_os_error() == Some(libc::EINVAL) { + fd = unsafe { libc::memfd_create(c"openshell-accept-shim".as_ptr(), flags) }; } - // Writable for the install itself; tightened to read-only below. The - // sandbox is not necessarily root, so the owner write bit is required - // here even on a freshly created directory. - std::fs::set_permissions(directory, std::fs::Permissions::from_mode(0o755)) - .map_err(|error| format!("set shim directory permissions: {error}"))?; - - let path = directory.join(FILE_NAME); - if let Ok(metadata) = std::fs::symlink_metadata(&path) - && metadata.file_type().is_symlink() - { - return Err(format!("shim path is a symlink: {}", path.display())); + if fd < 0 { + return Err(format!( + "create shim memfd: {}", + std::io::Error::last_os_error() + )); } - - let temporary = path.with_extension("tmp"); - if let Ok(metadata) = std::fs::symlink_metadata(&temporary) { - if !metadata.is_file() || metadata.file_type().is_symlink() { - return Err(format!( - "refusing unsafe temporary shim path: {}", - temporary.display() - )); - } - std::fs::remove_file(&temporary) - .map_err(|error| format!("remove stale temporary shim: {error}"))?; + let mut file = unsafe { std::fs::File::from_raw_fd(fd) }; + file.write_all(SHIM_OBJECT) + .map_err(|error| format!("write shim memfd: {error}"))?; + let seals = libc::F_SEAL_WRITE | libc::F_SEAL_GROW | libc::F_SEAL_SHRINK | libc::F_SEAL_SEAL; + if unsafe { libc::fcntl(fd, libc::F_ADD_SEALS, seals) } < 0 { + return Err(format!( + "seal shim memfd: {}", + std::io::Error::last_os_error() + )); } - - let mut file = std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .mode(0o555) - .custom_flags(libc::O_NOFOLLOW) - .open(&temporary) - .map_err(|error| format!("create temporary shim: {error}"))?; - if let Err(error) = file - .write_all(contents) - .and_then(|()| file.sync_all()) - .and_then(|()| file.set_permissions(std::fs::Permissions::from_mode(0o555))) - .and_then(|()| std::fs::rename(&temporary, &path)) - { - let _ = std::fs::remove_file(&temporary); - return Err(format!("install shim: {error}")); + // Check executable mapping before exporting LD_PRELOAD: some hosts + // prohibit execution of anonymous files even when creation succeeds. + let mapping = unsafe { + libc::mmap( + std::ptr::null_mut(), + SHIM_OBJECT.len(), + libc::PROT_READ | libc::PROT_EXEC, + libc::MAP_PRIVATE, + fd, + 0, + ) + }; + if mapping == libc::MAP_FAILED { + return Err(format!( + "map executable shim: {}", + std::io::Error::last_os_error() + )); } - // Traversable and readable by every workload identity, writable by none. - std::fs::set_permissions(directory, std::fs::Permissions::from_mode(0o555)) - .map_err(|error| format!("seal shim directory permissions: {error}"))?; - Ok(path) -} - -/// Materialize the embedded shim into [`RUNTIME_DIR`]. -#[cfg(target_os = "linux")] -pub fn install_shim() -> Result { - install_object_at(std::path::Path::new(RUNTIME_DIR), SHIM_OBJECT) + unsafe { + libc::munmap(mapping, SHIM_OBJECT.len()); + } + Ok(file) } /// Compose an `LD_PRELOAD` value that keeps any workload-supplied entries. @@ -145,8 +126,6 @@ fn preload_contains(value: &str, path: &str) -> bool { #[cfg(test)] mod tests { use super::*; - #[cfg(unix)] - use std::os::unix::fs::PermissionsExt as _; /// The object is produced by a C compiler chosen at build time, so a /// misresolved cross-compiler yields a host-architecture object that the @@ -180,75 +159,35 @@ mod tests { ); } - #[cfg(unix)] - fn scratch_dir(name: &str) -> std::path::PathBuf { - let base = std::env::temp_dir().join(format!("openshell-accept-shim-{name}")); - let _ = std::fs::remove_dir_all(&base); - base - } - - #[test] - #[cfg(unix)] - fn installing_creates_a_read_only_executable_object() { - let directory = scratch_dir("install"); - let installed = install_object_at(&directory, b"shim-bytes").expect("install"); - - assert_eq!(installed, directory.join(FILE_NAME)); - assert_eq!(std::fs::read(&installed).expect("read"), b"shim-bytes"); - // The workload must be able to map it executable but never rewrite it. - let mode = std::fs::metadata(&installed) - .expect("stat") - .permissions() - .mode(); - assert_eq!(mode & 0o777, 0o555); - } - #[test] - #[cfg(unix)] - fn installing_twice_replaces_the_previous_object() { - // A sandbox restart re-materializes into a directory that may already - // hold a previous generation of the shim. - let directory = scratch_dir("reinstall"); - install_object_at(&directory, b"old").expect("first install"); - let installed = install_object_at(&directory, b"new").expect("second install"); - - assert_eq!(std::fs::read(&installed).expect("read"), b"new"); + #[cfg(target_os = "linux")] + fn installed_object_cannot_be_replaced_or_rewritten() { + use std::io::Write as _; + let path = install_shim().expect("install sealed object"); + assert_eq!(std::fs::read(&path).unwrap(), SHIM_OBJECT); + let mut file = std::fs::OpenOptions::new().write(true).open(&path).unwrap(); assert_eq!( - std::fs::metadata(&installed) - .expect("stat") - .permissions() - .mode() - & 0o777, - 0o555 + file.write(b"replacement").unwrap_err().raw_os_error(), + Some(libc::EPERM) ); + assert_eq!( + file.set_len(0).unwrap_err().raw_os_error(), + Some(libc::EPERM) + ); + assert_eq!(install_shim().unwrap(), path); } #[test] - #[cfg(unix)] - fn a_symlinked_directory_is_refused() { - // The directory lives on a workload-writable rootfs, so a redirect - // must never be followed into a path the workload chose. - let base = scratch_dir("symlink-dir"); - std::fs::create_dir_all(base.join("real")).expect("create real"); - let link = base.join("link"); - std::os::unix::fs::symlink(base.join("real"), &link).expect("symlink"); - - let error = install_object_at(&link, b"shim-bytes").expect_err("must refuse"); - assert!(error.contains("symlink"), "unexpected error: {error}"); - } - - #[test] - #[cfg(unix)] - fn a_symlinked_target_file_is_refused() { - let directory = scratch_dir("symlink-file"); - std::fs::create_dir_all(&directory).expect("create"); - let target = directory.join("elsewhere"); - std::fs::write(&target, b"victim").expect("write victim"); - std::os::unix::fs::symlink(&target, directory.join(FILE_NAME)).expect("symlink"); - - let error = install_object_at(&directory, b"shim-bytes").expect_err("must refuse"); - assert!(error.contains("symlink"), "unexpected error: {error}"); - assert_eq!(std::fs::read(&target).expect("read"), b"victim"); + #[cfg(target_os = "linux")] + fn one_child_cannot_change_the_next_launch_object() { + use std::os::unix::fs::PermissionsExt as _; + let first = create_child_shim().unwrap(); + first + .set_permissions(std::fs::Permissions::from_mode(0o000)) + .unwrap(); + let second = create_child_shim().unwrap(); + assert_ne!(second.metadata().unwrap().permissions().mode() & 0o444, 0); + assert_eq!(std::fs::read(install_shim().unwrap()).unwrap(), SHIM_OBJECT); } #[test] diff --git a/crates/openshell-accept-shim/tests/fixtures/other_preload.c b/crates/openshell-accept-shim/tests/fixtures/other_preload.c new file mode 100644 index 0000000000..eb12d7961a --- /dev/null +++ b/crates/openshell-accept-shim/tests/fixtures/other_preload.c @@ -0,0 +1,10 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// A provider preload must not be mistaken for libc's cancellation wrapper. +// Resolving this accept4 from the shim would recurse through shimmed accept. +extern int accept(int, void *, unsigned int *); +int accept4(int fd, void *address, unsigned int *length, int flags) { + (void)flags; + return accept(fd, address, length); +} diff --git a/crates/openshell-accept-shim/tests/shim_behavior.rs b/crates/openshell-accept-shim/tests/shim_behavior.rs index 4fc49c0664..918a86a4e8 100644 --- a/crates/openshell-accept-shim/tests/shim_behavior.rs +++ b/crates/openshell-accept-shim/tests/shim_behavior.rs @@ -20,6 +20,8 @@ use std::io::{Error, Read as _, Write as _}; use std::mem::{size_of, transmute}; use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, TcpListener, TcpStream}; use std::os::fd::AsRawFd as _; +use std::os::unix::fs::PermissionsExt as _; +use std::os::unix::process::CommandExt as _; use std::process::id as process_id; use std::ptr::null_mut; use std::sync::OnceLock; @@ -29,7 +31,7 @@ use libc::{ sockaddr_in6, sockaddr_storage, socklen_t, }; -use openshell_accept_shim::{SHIM_OBJECT, install_object_at}; +use openshell_accept_shim::install_shim; type Accept4Fn = unsafe extern "C" fn(c_int, *mut sockaddr, *mut socklen_t, c_int) -> c_int; @@ -43,10 +45,7 @@ struct Shim { impl Shim { fn load() -> Self { - let directory = - std::env::temp_dir().join(format!("openshell-shim-behavior-{}", process_id())); - let _ = std::fs::remove_dir_all(&directory); - let path = install_object_at(&directory, SHIM_OBJECT).expect("install shim object"); + let path = install_shim().expect("install shim object"); let c_path = CString::new(path.as_os_str().as_encoded_bytes()) .expect("shim path has no interior NUL"); @@ -376,30 +375,48 @@ fn preloaded_accept_preserves_pthread_cancellation() { let directory = std::env::temp_dir().join(format!("openshell-shim-cancellation-{}", process_id())); std::fs::create_dir_all(&directory).expect("helper directory"); - let object = install_object_at(&directory.join("shim"), SHIM_OBJECT).expect("install shim"); + let file = openshell_accept_shim::create_child_shim().expect("child shim"); + let object = format!("/proc/self/fd/{}", file.as_raw_fd()); let executable = directory.join("cancellation"); - let status = std::process::Command::new("cc") - .args(["-std=c11", "-Wall", "-Wextra", "-Werror", "-pthread"]) - .arg(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/fixtures/cancellation.c" - )) - .arg("-o") - .arg(&executable) - .status() - .expect("compile cancellation helper"); - assert!(status.success(), "compile cancellation helper: {status}"); + let other_preload = directory.join("other_preload.so"); + std::fs::write( + &other_preload, + include_bytes!(env!("OPENSHELL_OTHER_PRELOAD")), + ) + .unwrap(); + std::fs::write( + &executable, + include_bytes!(env!("OPENSHELL_CANCELLATION_HELPER")), + ) + .unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); for accept4 in [false, true] { for address in [false, true] { for disabled in [false, true] { let args = [accept4, address, disabled].map(|flag| if flag { "1" } else { "0" }); // Pin the expected libc behavior before testing interposition. - for preload in [false, true] { + for preload in [0, 1, 2] { let mut command = std::process::Command::new(&executable); + let descriptor = file.as_raw_fd(); + unsafe { + command.pre_exec(move || { + if libc::fcntl(descriptor, libc::F_SETFD, 0) < 0 { + return Err(Error::last_os_error()); + } + Ok(()) + }); + } command.args(args).env_remove("LD_PRELOAD"); - if preload { - command.env("LD_PRELOAD", &object); + if preload > 0 { + command.env( + "LD_PRELOAD", + if preload == 1 { + object.clone() + } else { + format!("{object}:{}", other_preload.display()) + }, + ); } let output = command.output().expect("run cancellation helper"); assert!( @@ -411,4 +428,123 @@ fn preloaded_accept_preserves_pthread_cancellation() { } } } + std::fs::remove_dir_all(directory).unwrap(); +} + +#[test] +fn a_reset_peer_never_succeeds_with_an_empty_address() { + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let linger = libc::linger { + l_onoff: 1, + l_linger: 0, + }; + assert_eq!( + unsafe { + libc::setsockopt( + pending.client.as_raw_fd(), + libc::SOL_SOCKET, + libc::SO_LINGER, + (&raw const linger).cast(), + socklen_t::try_from(size_of::()).unwrap(), + ) + }, + 0 + ); + let peer = pending.client.local_addr().unwrap(); + drop(pending.client); + let mut storage = [0u8; size_of::()]; + let mut length = socklen_t::try_from(storage.len()).unwrap(); + let result = unsafe { + (shim().accept4)( + pending.listener.as_raw_fd(), + storage.as_mut_ptr().cast(), + &raw mut length, + 0, + ) + }; + let error = Error::last_os_error().raw_os_error(); + if result >= 0 { + close(result); + // Older kernels can retain the peer after a queued reset. Success + // must still contain the address CPython expects to unpack. + assert_eq!(length as usize, size_of::()); + assert_eq!(u16::from_be_bytes([storage[2], storage[3]]), peer.port()); + } else { + assert_eq!(result, -1); + assert_eq!(error, Some(libc::ECONNABORTED)); + } +} + +#[test] +fn a_bad_length_pointer_returns_efault() { + // Isolate the old shim's invalid userspace write from the test runner. + const CHILD: &str = "OPENSHELL_SHIM_BAD_POINTER_CHILD"; + if std::env::var_os(CHILD).is_none() { + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "a_bad_length_pointer_returns_efault"]) + .env(CHILD, "1") + .status() + .unwrap(); + assert!( + status.success(), + "bad pointer must return EFAULT, not crash" + ); + return; + } + let pending = pending_connection(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))); + let mut storage = [0u8; size_of::()]; + let result = unsafe { + (shim().accept4)( + pending.listener.as_raw_fd(), + storage.as_mut_ptr().cast(), + std::ptr::dangling_mut(), + 0, + ) + }; + assert_eq!(result, -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EFAULT)); +} + +#[test] +fn embedded_object_has_no_loader_dependencies_or_unsafe_relocations() { + use object::{Object as _, ObjectSection as _, ObjectSymbol as _}; + let bytes = openshell_accept_shim::SHIM_OBJECT; + let elf = object::File::parse(bytes).unwrap(); + let dynamic = elf.section_by_name(".dynamic").unwrap().data().unwrap(); + for entry in dynamic.chunks_exact(16) { + let tag = u64::from_le_bytes(entry[..8].try_into().unwrap()); + let value = u64::from_le_bytes(entry[8..].try_into().unwrap()); + assert_ne!(tag, 1, "DT_NEEDED adds a libc dependency"); + assert_ne!(tag, 22, "DT_TEXTREL needs SELinux execmod"); + if tag == 30 { + assert_eq!(value & 4, 0, "DF_TEXTREL"); + } + } + let mut exports = elf + .dynamic_symbols() + .filter(|symbol| symbol.is_definition() && symbol.kind() == object::SymbolKind::Text) + .map(|symbol| symbol.name().unwrap()) + .collect::>(); + exports.sort_unstable(); + assert_eq!(exports, ["accept", "accept4"]); + let mut imports = elf + .dynamic_symbols() + .filter(object::ObjectSymbol::is_undefined) + .map(|symbol| symbol.name().unwrap()) + .filter(|name| !name.is_empty()) + .collect::>(); + imports.sort_unstable(); + assert_eq!(imports, ["__errno_location", "dl_iterate_phdr"]); + let offset = usize::try_from(u64::from_le_bytes(bytes[32..40].try_into().unwrap())).unwrap(); + let stride = u16::from_le_bytes(bytes[54..56].try_into().unwrap()) as usize; + let count = u16::from_le_bytes(bytes[56..58].try_into().unwrap()) as usize; + let stack = (0..count) + .map(|index| &bytes[offset + index * stride..][..stride]) + .find(|header| u32::from_le_bytes(header[..4].try_into().unwrap()) == 0x6474_e551) + .expect("GNU_STACK header"); + assert_eq!( + u32::from_le_bytes(stack[4..8].try_into().unwrap()) & 1, + 0, + "shim must not require an executable stack" + ); } diff --git a/crates/openshell-sandbox/src/boundary_exec.rs b/crates/openshell-sandbox/src/boundary_exec.rs index ac551b28ba..65d375ee54 100644 --- a/crates/openshell-sandbox/src/boundary_exec.rs +++ b/crates/openshell-sandbox/src/boundary_exec.rs @@ -188,26 +188,10 @@ impl LocalBoundaryExec { command.env(key, value); } } - // Last, so the shim composes with whichever LD_PRELOAD the workload - // would otherwise have received rather than being overwritten by it. - if let Some(shim) = crate::child_env::preload_shim() { - let inherited = spec - .env - .iter() - .rev() - .find(|(key, _)| key == openshell_accept_shim::PRELOAD_ENV) - .map(|(_, value)| value.as_str()) - .or_else(|| { - self.user_environment - .get(openshell_accept_shim::PRELOAD_ENV) - .map(String::as_str) - }); - let (key, value) = crate::child_env::preload_env_var(shim, inherited); - command.env(key, value); - } if let Some(workdir) = spec.workdir.as_deref().or(self.base_workdir.as_deref()) { command.current_dir(workdir); } + crate::child_env::apply_preload(&mut command, false); Ok(command) } diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index 81f89ecaff..1f44d0d8e2 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -1,8 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use std::collections::HashMap; -use std::path::{Path, PathBuf}; +use std::path::Path; +#[cfg(target_os = "linux")] +use std::path::PathBuf; +#[cfg(target_os = "linux")] use std::sync::OnceLock; pub fn tls_env_vars( @@ -26,14 +28,17 @@ pub fn tls_env_vars( /// Shim materialized for this sandbox, or `None` when the kernel's seccomp /// listener supports `WAIT_KILLABLE_RECV` and the broker can answer /// `accept`/`accept4` directly. +#[cfg(target_os = "linux")] static PRELOAD_SHIM: OnceLock> = OnceLock::new(); /// Record the materialized shim path once, during sandbox startup. +#[cfg(target_os = "linux")] pub fn set_preload_shim(path: Option) { let _ = PRELOAD_SHIM.set(path); } /// Path of the materialized shim, if this sandbox installed one. +#[cfg(target_os = "linux")] pub fn preload_shim() -> Option<&'static Path> { PRELOAD_SHIM.get()?.as_deref() } @@ -43,6 +48,7 @@ pub fn preload_shim() -> Option<&'static Path> { /// /// Children re-inherit this value when they spawn their own children, so the /// composition must be idempotent rather than prepending on every exec. +#[cfg(any(target_os = "linux", test))] pub fn preload_env_var(shim_path: &Path, inherited: Option<&str>) -> (&'static str, String) { ( openshell_accept_shim::PRELOAD_ENV, @@ -50,14 +56,131 @@ pub fn preload_env_var(shim_path: &Path, inherited: Option<&str>) -> (&'static s ) } -/// Resolve the `LD_PRELOAD` a child would inherit where the environment is -/// not cleared: an explicit workload value wins over the sandbox's own. -#[allow(clippy::implicit_hasher)] -pub fn inherited_preload(user_environment: &HashMap) -> Option { - user_environment - .get(openshell_accept_shim::PRELOAD_ENV) - .cloned() - .or_else(|| std::env::var(openshell_accept_shim::PRELOAD_ENV).ok()) +/// Compose after all child environment sources have been applied. Reading the +/// command's effective override preserves provider and per-session precedence. +pub fn apply_preload(command: &mut std::process::Command, inherit_environment: bool) { + #[cfg(target_os = "linux")] + if preload_shim().is_some() { + apply_preload_for_child(command, inherit_environment); + } + #[cfg(not(target_os = "linux"))] + let _ = (command, inherit_environment); +} + +#[cfg(target_os = "linux")] +pub(crate) fn apply_preload_for_child( + command: &mut std::process::Command, + inherit_environment: bool, +) { + use std::os::fd::AsRawFd as _; + use std::os::unix::process::CommandExt as _; + if !compatible_executable(command) { + return; + } + let file = match openshell_accept_shim::create_child_shim() { + Ok(file) => file, + Err(error) => { + openshell_ocsf::ocsf_emit!( + openshell_ocsf::ConfigStateChangeBuilder::new(openshell_ocsf::ctx::ctx()) + .severity(openshell_ocsf::SeverityId::Medium) + .status(openshell_ocsf::StatusId::Failure) + .message(format!( + "Peer-address shim unavailable for child [error:{error}]" + )) + .build() + ); + return; + } + }; + let path = PathBuf::from(format!("/proc/self/fd/{}", file.as_raw_fd())); + apply_preload_at(command, &path, inherit_environment); + // CLOEXEC stays set in the shared parent fd table. Clear it only in this + // forked child; other launches' pending objects close on its exec. + #[allow(unsafe_code)] + unsafe { + command.pre_exec(move || { + if libc::fcntl(file.as_raw_fd(), libc::F_SETFD, 0) < 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } +} + +#[cfg(any(target_os = "linux", test))] +fn compatible_executable(command: &std::process::Command) -> bool { + // The shim is ELF64 for this runtime's architecture. Do not inject it + // into a directly launched foreign ELF executable. Scripts and commands + // resolved by a shell retain normal loader inheritance semantics. + let program = command.get_program(); + let executable = if Path::new(program).components().count() > 1 { + Some( + command + .get_current_dir() + .unwrap_or_else(|| Path::new(".")) + .join(program), + ) + } else { + let path = command + .get_envs() + .find(|(key, _)| *key == "PATH") + .and_then(|(_, value)| value.map(std::ffi::OsStr::to_os_string)) + .or_else(|| std::env::var_os("PATH")); + path.and_then(|path| { + std::env::split_paths(&path) + .map(|directory| directory.join(program)) + .find(|path| path.is_file()) + }) + }; + if let Some(executable) = executable { + let mut header = [0u8; 20]; + if read_executable_header(&executable, &mut header).is_ok() + && &header[..4] == b"\x7fELF" + && (header[4] != 2 + || header[5] != 1 + || u16::from_le_bytes([header[18], header[19]]) + != if cfg!(target_arch = "aarch64") { + 183 + } else { + 62 + }) + { + return false; + } + } + true +} + +#[cfg(any(target_os = "linux", test))] +fn read_executable_header(path: &Path, header: &mut [u8; 20]) -> std::io::Result<()> { + use std::io::Read as _; + let mut options = std::fs::OpenOptions::new(); + options.read(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.custom_flags(libc::O_NONBLOCK); + } + let mut file = options.open(path)?; + if !file.metadata()?.is_file() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "not a regular executable", + )); + } + file.read_exact(header) +} + +#[cfg(any(target_os = "linux", test))] +fn apply_preload_at(command: &mut std::process::Command, shim: &Path, inherit_environment: bool) { + let key = std::ffi::OsStr::new(openshell_accept_shim::PRELOAD_ENV); + let inherited = match command.get_envs().find(|(name, _)| *name == key) { + Some((_, value)) => value.map(|value| value.to_string_lossy().into_owned()), + None if !inherit_environment => None, + None => std::env::var(openshell_accept_shim::PRELOAD_ENV).ok(), + }; + let (key, value) = preload_env_var(shim, inherited.as_deref()); + command.env(key, value); } #[cfg(test)] @@ -66,15 +189,95 @@ mod tests { use std::process::Command; use std::process::Stdio; + #[cfg(target_os = "linux")] + #[test] + fn concurrent_launches_inherit_only_their_own_shim() { + let script = r" +import os +objects = [] +for name in os.listdir('/proc/self/fd'): + try: + if os.readlink('/proc/self/fd/' + name).startswith('/memfd:openshell-accept-shim'): + objects.append(name) + except FileNotFoundError: + pass +assert len(objects) == 1, objects +os.chmod(os.environ['LD_PRELOAD'], 0) +print('isolated launch') +"; + let mut first = Command::new("python3"); + first.args(["-c", script]); + apply_preload_for_child(&mut first, false); + let mut second = Command::new("python3"); + second.args(["-c", script]); + apply_preload_for_child(&mut second, false); + let paths = [&first, &second].map(|command| { + PathBuf::from( + command + .get_envs() + .find(|(key, _)| *key == "LD_PRELOAD") + .unwrap() + .1 + .unwrap(), + ) + }); + for command in [&mut first, &mut second] { + let output = command.output().unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"isolated launch\n"); + } + drop(first); + drop(second); + for path in paths { + assert!(!path.exists(), "parent must release its descriptor"); + } + } + + #[test] + fn preload_composes_with_the_final_command_override() { + let mut command = Command::new("/usr/bin/env"); + command.env("LD_PRELOAD", "/user.so"); + command.env("LD_PRELOAD", "/provider.so"); + apply_preload_at(&mut command, Path::new("/shim.so"), false); + let preload = command + .get_envs() + .find(|(key, _)| *key == "LD_PRELOAD") + .unwrap() + .1 + .unwrap(); + assert_eq!(preload, "/shim.so:/provider.so"); + } + + #[test] + fn foreign_elf_does_not_receive_the_shim() { + use std::io::Write as _; + let mut executable = tempfile::NamedTempFile::new().unwrap(); + let mut header = [0u8; 20]; + header[..4].copy_from_slice(b"\x7fELF"); + header[4] = 1; // ELFCLASS32 + header[5] = 1; + executable.write_all(&header).unwrap(); + let command = Command::new(executable.path()); + assert!(!compatible_executable(&command)); + } + + #[cfg(unix)] #[test] - fn inherited_preload_prefers_the_workload_value() { - let mut user_environment = HashMap::new(); - user_environment.insert("LD_PRELOAD".to_string(), "/opt/jemalloc.so".to_string()); + fn executable_probe_does_not_wait_for_a_fifo_writer() { + let directory = tempfile::tempdir().unwrap(); + let fifo = directory.path().join("program"); + nix::unistd::mkfifo(&fifo, nix::sys::stat::Mode::S_IRUSR).unwrap(); assert_eq!( - inherited_preload(&user_environment).as_deref(), - Some("/opt/jemalloc.so") + read_executable_header(&fifo, &mut [0; 20]) + .unwrap_err() + .kind(), + std::io::ErrorKind::InvalidInput ); - assert_eq!(inherited_preload(&HashMap::new()).as_deref(), None); } #[test] diff --git a/crates/openshell-sandbox/src/network_broker.rs b/crates/openshell-sandbox/src/network_broker.rs index 9636f69ed7..0dbfae237e 100644 --- a/crates/openshell-sandbox/src/network_broker.rs +++ b/crates/openshell-sandbox/src/network_broker.rs @@ -1120,6 +1120,10 @@ fn accept_socket( let source = duplicate_close_on_exec(entry.retained_preconnect()?.as_raw_fd())?; (entry.identity().inode, entry.metadata(), source) }; + // Reject unsupported output before consuming a queued client connection. + if listener.writes_disabled() && notification.args[1] != 0 { + return Err(io::Error::from_raw_os_error(libc::EOPNOTSUPP)); + } let slot = acquire_pending_accept_slot(&active_accepts)?; let worker_listener = Arc::clone(&listener); std::thread::Builder::new() @@ -1602,7 +1606,10 @@ fn get_peer_name( // write it keeps the sockaddr store inside the workload's address // space, which needs no cross-process task-memory write and therefore // works on kernels without SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV. - SocketState::Local { .. } | SocketState::AcceptedLocal { .. } => { + SocketState::Local { .. } + | SocketState::AcceptedLocal { .. } + | SocketState::DnsTcp { .. } + | SocketState::DnsUdp { .. } => { return listener.respond_continue(notification.id); } _ => return Err(io::Error::from_raw_os_error(libc::ENOTCONN)), @@ -2119,6 +2126,8 @@ mod tests { for (label, state) in [ ("AcceptedLocal", SocketState::AcceptedLocal { peer }), ("Local", SocketState::Local { peer }), + ("DnsTcp", SocketState::DnsTcp { relay: peer }), + ("DnsUdp", SocketState::DnsUdp { relay: peer }), ] { let (registry, installed) = registry_with_state(state); let listener = fake_listener(ListenerMode::LegacyReadOnly); @@ -2128,9 +2137,9 @@ mod tests { getpeername_notification(installed.as_raw_fd()), ) .expect_err("the fake listener cannot complete any ioctl"); - assert_ne!( + assert_eq!( error.raw_os_error(), - Some(libc::EOPNOTSUPP), + Some(libc::ENOTTY), "{label} must not route through the fail-closed task-memory write" ); } @@ -2257,11 +2266,11 @@ mod tests { #[test] fn killable_outbound_getpeername_returns_the_original_destination() { let result = mediated_outbound_peer(ListenerMode::Killable); - if let Err(error) = &result { - if error.kind() == io::ErrorKind::Unsupported { - eprintln!("skipping modern peer substitution: {error}"); - return; - } + if let Err(error) = &result + && error.kind() == io::ErrorKind::Unsupported + { + eprintln!("skipping modern peer substitution: {error}"); + return; } let peer = result.expect("modern outbound getpeername succeeds"); assert_eq!(peer, "203.0.113.7:443".parse().unwrap()); @@ -2535,6 +2544,47 @@ mod tests { client.join().expect("join client").expect("Unix client"); } + #[test] + fn legacy_preloaded_accept_preserves_a_client_rejected_by_raw_accept() { + let (launcher, installed) = + openshell_isolation_interface::linux::workload_launcher::start().unwrap(); + let descriptor = duplicate_close_on_exec(installed.as_raw_fd()).unwrap(); + let listener = + NotificationListener::from_fd_with_mode(descriptor, ListenerMode::LegacyReadOnly); + let _broker = NetworkBroker::start_for_test(listener).unwrap(); + let mut command = std::process::Command::new("python3"); + crate::child_env::apply_preload_for_child(&mut command, false); + command.args([ + "-c", + r" +import ctypes, errno, socket, platform +s = socket.socket() +s.bind(('127.0.0.1', 0)) +s.listen(4) +c = socket.socket() +c.connect(s.getsockname()) +libc = ctypes.CDLL(None, use_errno=True) +address = ctypes.create_string_buffer(128) +length = ctypes.c_uint(128) +number = 288 if platform.machine() == 'x86_64' else 242 +result = libc.syscall(number, s.fileno(), ctypes.byref(address), ctypes.byref(length), 0) +assert result == -1 and ctypes.get_errno() == errno.EOPNOTSUPP +conn, peer = s.accept() +assert peer == c.getsockname() == conn.getpeername() +conn.sendall(b'ping') +assert c.recv(4) == b'ping' +print('legacy accept passed', flush=True) +", + ]); + let output = launcher.execute(move || command.output()).unwrap().unwrap(); + assert!( + output.status.success(), + "legacy preload: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(output.stdout, b"legacy accept passed\n"); + } + #[test] fn accepted_loopback_stream_is_registered_for_notified_operations() { let (launcher, listener) = openshell_isolation_interface::linux::workload_launcher::start() diff --git a/crates/openshell-sandbox/src/process.rs b/crates/openshell-sandbox/src/process.rs index 8424ba2ad4..89c0490736 100644 --- a/crates/openshell-sandbox/src/process.rs +++ b/crates/openshell-sandbox/src/process.rs @@ -72,26 +72,11 @@ pub(crate) fn prepare_child_sandbox( workdir: Option<&str>, runtime_read_only: &[PathBuf], ) -> Result> { - let runtime_read_only = - effective_runtime_read_only(runtime_read_only, child_env::preload_shim()); - let effective_policy = policy_with_runtime_read_only(policy, &runtime_read_only); + let effective_policy = policy_with_runtime_read_only(policy, runtime_read_only); let prepared = sandbox::linux::prepare_capability_free(&effective_policy, workdir)?; Ok(Some(prepared)) } -/// Combine a launch path's own runtime paths with the preloaded shim's. -/// -/// Both the sandbox entrypoint and `sandbox exec` set `LD_PRELOAD`, and each -/// supplies a different set of runtime paths. Admitting the shim here rather -/// than at each call site means a launch path cannot set `LD_PRELOAD` without -/// also letting the loader open what it points at. -#[cfg(target_os = "linux")] -fn effective_runtime_read_only(runtime_read_only: &[PathBuf], shim: Option<&Path>) -> Vec { - let mut paths = runtime_read_only.to_vec(); - paths.extend(shim_runtime_read_only_paths(shim)); - paths -} - #[cfg(target_os = "linux")] fn policy_with_runtime_read_only( policy: &SandboxPolicy, @@ -120,27 +105,6 @@ pub(crate) fn ca_runtime_read_only_paths(ca_paths: Option<&(PathBuf, PathBuf)>) paths } -/// Paths the dynamic loader must reach to honor the preloaded shim. -/// -/// `LD_PRELOAD` is resolved by the loader inside the workload, after Landlock -/// is enforced. The object is installed world-readable, but Landlock gates -/// `open` independently of the file mode, so without an explicit admission the -/// loader reports `cannot open shared object file` and silently drops the -/// shim. The directory is admitted alongside the object because the loader -/// resolves the path through it. -#[cfg(target_os = "linux")] -pub(crate) fn shim_runtime_read_only_paths(shim: Option<&Path>) -> Vec { - let Some(object) = shim else { - return Vec::new(); - }; - let mut paths = Vec::with_capacity(2); - if let Some(directory) = object.parent() { - paths.push(directory.to_path_buf()); - } - paths.push(object.to_path_buf()); - paths -} - const SUPERVISOR_ONLY_ENV_VARS: &[&str] = &[ openshell_core::sandbox_env::OCI_IMAGE_USER, openshell_core::sandbox_env::SANDBOX_UID, @@ -541,11 +505,7 @@ impl ProcessHandle { } } - if let Some(shim) = child_env::preload_shim() { - let inherited = child_env::inherited_preload(&configured_user_environment()); - let (key, value) = child_env::preload_env_var(shim, inherited.as_deref()); - cmd.env(key, value); - } + child_env::apply_preload(cmd.as_std_mut(), true); // Probe Landlock availability and emit OCSF logs from the parent // process where the tracing subscriber is functional. The child's @@ -712,12 +672,6 @@ impl ProcessHandle { } } - if let Some(shim) = child_env::preload_shim() { - let inherited = child_env::inherited_preload(&configured_user_environment()); - let (key, value) = child_env::preload_env_var(shim, inherited.as_deref()); - cmd.env(key, value); - } - // Create a dedicated session for PTY children and a dedicated process // group for pipe children so attachment signals target only the // canonical workload tree. @@ -1561,112 +1515,29 @@ mod tests { } } - #[cfg(target_os = "linux")] - #[test] - fn shim_runtime_paths_admit_the_object_and_its_directory() { - assert!( - shim_runtime_read_only_paths(None).is_empty(), - "no shim installed means nothing extra to admit" - ); - - let object = PathBuf::from(format!( - "{}/{}", - openshell_accept_shim::RUNTIME_DIR, - openshell_accept_shim::FILE_NAME - )); - assert_eq!( - shim_runtime_read_only_paths(Some(&object)), - vec![ - PathBuf::from(openshell_accept_shim::RUNTIME_DIR), - object.clone() - ], - "the loader resolves the object through its directory, so both must be admitted" - ); - } - - /// Workload children are launched from more than one place: the sandbox - /// entrypoint, and `sandbox exec` through the boundary. Every one of them - /// sets `LD_PRELOAD`, so every one of them needs the loader to be able to - /// open the shim. Composing the admission with the caller's own runtime - /// paths keeps a launch path from silently omitting it. - #[cfg(target_os = "linux")] - #[test] - fn runtime_read_only_composition_admits_caller_paths_and_the_shim() { - let certificate = PathBuf::from("/run/openshell-ca/ca.pem"); - let object = PathBuf::from(format!( - "{}/{}", - openshell_accept_shim::RUNTIME_DIR, - openshell_accept_shim::FILE_NAME - )); - - assert_eq!( - effective_runtime_read_only(&[certificate.clone()], None), - vec![certificate.clone()], - "without a shim the caller's paths pass through unchanged" - ); - - assert_eq!( - effective_runtime_read_only(&[certificate.clone()], Some(&object)), - vec![ - certificate, - PathBuf::from(openshell_accept_shim::RUNTIME_DIR), - object, - ], - "the shim must be admitted alongside whatever the launch path supplied" - ); - } - - /// The dynamic loader opens the shim as the workload user, after Landlock - /// is enforced. A world-readable mode is not sufficient on its own. #[cfg(target_os = "linux")] #[test] #[allow(unsafe_code)] - fn preloaded_shim_remains_readable_after_landlock_for_non_root_workload() { - let root = tempfile::tempdir_in("/tmp").unwrap(); - std::fs::set_permissions(root.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); - let shim_directory = root.path().join("openshell-compat"); - std::fs::create_dir(&shim_directory).unwrap(); - let object = shim_directory.join(openshell_accept_shim::FILE_NAME); - std::fs::write(&object, openshell_accept_shim::SHIM_OBJECT).unwrap(); - std::fs::set_permissions(&object, std::fs::Permissions::from_mode(0o555)).unwrap(); - // Tighten the directory only after the object exists, matching how the - // installer leaves it and keeping the test runnable as a normal user. - std::fs::set_permissions(&shim_directory, std::fs::Permissions::from_mode(0o555)).unwrap(); - let denied = root.path().join("not-authorized"); - std::fs::write(&denied, b"unrelated").unwrap(); - std::fs::set_permissions(&denied, std::fs::Permissions::from_mode(0o444)).unwrap(); - - let mut policy = policy_with_process(ProcessPolicy::default()); - policy.landlock = LandlockPolicy { - compatibility: openshell_core::policy::LandlockCompatibility::HardRequirement, - }; - let runtime_paths = shim_runtime_read_only_paths(Some(&object)); - let Ok(Some(prepared)) = prepare_child_sandbox(&policy, None, &runtime_paths) else { - return; - }; - - match unsafe { fork() }.expect("fork should succeed") { - ForkResult::Child => { - let dropped = if nix::unistd::geteuid().is_root() { - unsafe { - libc::setgroups(0, std::ptr::null()) == 0 - && libc::setgid(42_235) == 0 - && libc::setuid(42_234) == 0 - } - } else { - true - }; - let valid = dropped - && sandbox::linux::enforce(prepared).is_ok() - && std::fs::read(&object).is_ok() - && std::fs::read(&denied).is_err(); - unsafe { libc::_exit(i32::from(!valid)) }; + fn sealed_shim_does_not_create_a_user_landlock_ruleset() { + let object = openshell_accept_shim::install_shim().unwrap(); + for restricted in [false, true] { + let mut policy = policy_with_process(ProcessPolicy::default()); + policy.filesystem.include_workdir = false; + if restricted { + policy.filesystem.read_only = vec![PathBuf::from("/usr"), PathBuf::from("/lib")]; + } + let prepared = prepare_child_sandbox(&policy, None, &[]).unwrap().unwrap(); + match unsafe { fork() }.unwrap() { + ForkResult::Child => { + let valid = sandbox::linux::enforce(prepared).is_ok() + && std::fs::read(&object).is_ok() + && std::fs::read("/usr/bin/env").is_ok(); + unsafe { libc::_exit(i32::from(!valid)) }; + } + ForkResult::Parent { child } => { + assert_eq!(waitpid(child, None).unwrap(), WaitStatus::Exited(child, 0)); + } } - ForkResult::Parent { child } => assert_eq!( - waitpid(child, None).expect("waitpid should succeed"), - WaitStatus::Exited(child, 0), - "Landlock must admit the preloaded shim so the loader can open it" - ), } } diff --git a/docs/about/support-matrix.mdx b/docs/about/support-matrix.mdx index 3ee561ec6c..62dabf931c 100644 --- a/docs/about/support-matrix.mdx +++ b/docs/about/support-matrix.mdx @@ -195,6 +195,13 @@ the sandbox preloads a small compatibility library into the workload that rewrites the call into a null-address `accept4` followed by `getpeername`, and fills the caller's buffer in the workload's own address space. +The compatibility library returns `ECONNABORTED` for a reset queued client. +It uses a separate sealed anonymous file for each launch and does not require +a writable `/run`. Descendants must retain its inherited descriptor and access +to `/proc/self/fd`. Remove the shim entry from `LD_PRELOAD` before a descendant +closes the descriptor or starts a foreign-architecture loader to avoid a +loader warning. + `sendmmsg` paths that write per-message lengths back to the caller still fail closed with `EOPNOTSUPP`. diff --git a/e2e/reproducers/accept-peer.md b/e2e/reproducers/accept-peer.md deleted file mode 100644 index 0eb8a5e756..0000000000 --- a/e2e/reproducers/accept-peer.md +++ /dev/null @@ -1,176 +0,0 @@ - - -# OpenShift peer-address reproducer - -This procedure validates [PR #4087](https://github.com/NVIDIA/OpenShell/pull/4087). -The standard-library-only [Python reproducer](accept-peer.py) connects a client -to a loopback listener inside an OpenShell sandbox. It checks that both -`accept()` and `getpeername()` return the client's address, rather than the -listener's, and that the accepted socket delivers `ping`. - -## Prerequisites and deployment - -Use an OpenShift cluster whose kernel lacks `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` -(the tested RHCOS kernel is listed below). Have `oc`, `helm`, `mise`, the local -CLI, and the upstream Agent Sandbox controller installed. Authenticate to the -cluster and its development image registry. Run commands from the repository root. - -Record the cluster and source version: - -```shell -git rev-parse HEAD -oc get clusterversion version -o jsonpath='{.status.desired.version}{"\n"}' -oc get nodes -o custom-columns=NAME:.metadata.name,KERNEL:.status.nodeInfo.kernelVersion -``` - -Build and stage the sandbox runtime from the checkout, then package it with -the repository's image build task. For a Mac host and x86-64 cluster, use: - -```shell -ulimit -n 8192 -mise exec -- env RUSTC_WRAPPER= cargo zigbuild --release --locked \ - --target x86_64-unknown-linux-musl -p openshell-sandbox -install -m 0755 target/x86_64-unknown-linux-musl/release/openshell-sandbox \ - deploy/docker/.build/prebuilt-binaries/amd64/openshell-sandbox -PREBUILT_AUTO_STAGE=0 DOCKER_PLATFORM=linux/amd64 IMAGE_TAG=pr4087-de27081b3 \ - mise run docker:build:sandbox -``` - -Push this uniquely tagged image to a registry the cluster can pull. This run -used Podman and the OpenShift development registry, with `--tls-verify=false` -for that registry's certificate: - -```shell -REGISTRY_HOST="$(oc -n openshift-image-registry get route default-route -o jsonpath='{.spec.host}')" -podman push --tls-verify=false localhost/openshell/sandbox:pr4087-de27081b3 \ - "docker://${REGISTRY_HOST}/openshell-images/sandbox:pr4087-de27081b3" -``` - -Create a separate test deployment using the existing development gateway's -values and gateway/supervisor images. Inspect those values before reuse; this -run used plaintext transport through a local port-forward, no public Route, -and `allowUnauthenticatedUsers: true`. Keep this configuration scoped to the -test deployment. Only the sandbox runtime was rebuilt for this run. - -```shell -helm get values openshell -n openshell -o yaml > /tmp/pr4087-existing-values.yaml -oc create namespace openshell-pr4087-test -oc -n openshell-images policy add-role-to-group system:image-puller \ - system:serviceaccounts:openshell-pr4087-test -helm install openshell deploy/helm/openshell -n openshell-pr4087-test \ - -f /tmp/pr4087-existing-values.yaml \ - --set sandboxRuntime.image.tag=pr4087-de27081b3 \ - --set server.otlp.endpoint= --wait --timeout 5m -oc -n openshell-pr4087-test port-forward svc/openshell 18088:8080 -``` - -Keep the port-forward running; execute subsequent commands in another terminal. -The reused values point all runtime images at -`image-registry.openshift-image-registry.svc:5000/openshell-images` and apply -`deploy/helm/openshell/ci/values-openshift-scc.yaml`'s security settings. -No privileged SCC grant was added. - -## Run the reproducer - -Use a Python image and pass the small script directly; no upload, package -installation, external connection, or forwarded application port is needed. -`target/debug/openshell` is the prebuilt local CLI used in this run. - -```shell -CLI=target/debug/openshell -ENDPOINT=http://127.0.0.1:18088 -PYTHON_IMAGE=ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 - -"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ - --name accept-peer-pr4087 --from "$PYTHON_IMAGE" --no-tty \ - -- python3 -c "$(cat e2e/reproducers/accept-peer.py)" -``` - -Expect exit status 0 and JSON containing `"result": "PASS"`, identical -`client`, `accept_peer`, and `peer` addresses, and -`"preload": "/run/openshell-compat/accept_shim.so"`. Ports vary per run. - -The initial command exits and the sandbox becomes Completed. To check the -separate exec launch path, create a sandbox that runs the same check and then -stays alive for ten minutes: - -```shell -"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ - --name peer-exec-pr4087 --from "$PYTHON_IMAGE" --detach \ - -- python3 -u -c "$(cat e2e/reproducers/accept-peer.py) -import time -time.sleep(600)" - -"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ - --no-tty --no-login-shell --timeout 30 peer-exec-pr4087 \ - -- python3 -c "$(cat e2e/reproducers/accept-peer.py)" -``` - -Expect the same PASS output and exit status 0. For a negative control, start -Python through `env` so its dynamic loader does not receive the preload: - -```shell -"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ - --no-tty --no-login-shell --timeout 30 peer-exec-pr4087 \ - -- env -u LD_PRELOAD python3 -c "$(cat e2e/reproducers/accept-peer.py)" -``` - -On the affected kernel, expect exit status 1 and -`OSError: [Errno 95] Operation not supported` at `server.accept()`. This control -demonstrates that the shim covers the failing call; it is not a comparison -against an independently built base-branch image. On kernels supporting -`WAIT_KILLABLE_RECV`, this control can succeed. - -Check the actual workload binary and admission profile: - -```shell -oc -n openshell-pr4087-test exec default--accept-peer-pr4087 -c agent \ - -- sha256sum /.openshell/runtime/openshell-sandbox -oc -n openshell-pr4087-test get pod default--accept-peer-pr4087 \ - -o jsonpath='{.metadata.annotations.openshift\.io/scc}{"\n"}{.spec.containers[0].securityContext}{"\n"}{.spec.securityContext}{"\n"}' -``` - -## Observed results - -Run on 2026-10-02 against source commit -`de27081b3` (PR head). OpenShift **4.21.34**, node -`control-plane-cluster-48kkt-1`, kernel **5.14.0-570.141.1.el9_6.x86_64**. - -| Check | Result | -| --- | --- | -| Initial command | PASS; client, accept peer, and getpeername ports all 50372 | -| Persistent sandbox's initial command | PASS; all three ports 39438 | -| `sandbox exec` | PASS; all three ports 53330 | -| Exec with preload unset | Expected failure, errno 95 at `accept()`, exit 1 | -| Workload admission | `restricted-v2`; UID/GID 1000780000; all capabilities dropped; privilege escalation disabled; `RuntimeDefault` seccomp; SELinux level `s0:c28,c12` | -| Binary provenance | Deployed binary SHA-256 matched the freshly built local binary | - -Recorded runtime binary SHA-256: -`dc004b17fd02c4bc0cb8208850b1dc29dcfa7a76ab80536f5ce8dc0eaf91333c`. - -Recorded image digests in `openshell-images`: - -- Sandbox: `sha256:8d43dd76b5936386eaee5978d346d79bcc4a89aaee59b1c28ff7330df15d36a7`. -- Gateway (reused): `sha256:83c7fde9ae6a197ab6c7c668086946beafc0eac99f2ea169ff1003fbaca9896d`. -- Supervisor (reused): `sha256:a4404edd399429fe4cc543a923b011d78d82e6e4531172cb8ad7bf9fcc57b6b2`. - -This test covers dynamically linked CPython on x86-64 and both workload launch -paths. It does not establish support for Go, static binaries, other -architectures, or every runtime. The sandbox logged an unrelated warning that -the runtime cgroup's `pids.max` is unlimited. - -## Cleanup - -Remove the test release, namespace, and the scoped registry pull grant. Stop -the port-forward with Ctrl-C. The uniquely tagged registry image remains -available for reruns. - -```shell -helm uninstall openshell -n openshell-pr4087-test -oc delete namespace openshell-pr4087-test --wait=true --timeout=120s -oc -n openshell-images policy remove-role-from-group system:image-puller \ - system:serviceaccounts:openshell-pr4087-test -``` diff --git a/e2e/reproducers/accept-peer.py b/e2e/reproducers/accept-peer.py deleted file mode 100644 index 5be6a52c7d..0000000000 --- a/e2e/reproducers/accept-peer.py +++ /dev/null @@ -1,34 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Minimal loopback accept/getpeername check; run inside an OpenShell sandbox.""" - -import json -import os -import socket - -socket.setdefaulttimeout(10) -with socket.socket() as server, socket.socket() as client: - server.bind(("127.0.0.1", 0)) - server.listen(1) - client.connect(server.getsockname()) - expected = client.getsockname() - conn, accepted_peer = server.accept() - with conn: - peer = conn.getpeername() - conn.sendall(b"ping") - assert client.recv(4) == b"ping" - assert peer == expected, (peer, expected) - assert accepted_peer == expected, (accepted_peer, expected) - assert peer[1] != server.getsockname()[1] - print( - json.dumps( - { - "result": "PASS", - "client": expected, - "accept_peer": accepted_peer, - "peer": peer, - "preload": os.environ.get("LD_PRELOAD", ""), - } - ) - ) diff --git a/e2e/reproducers/outbound-tcp-echo.yaml b/e2e/reproducers/outbound-tcp-echo.yaml deleted file mode 100644 index 9b80a66e46..0000000000 --- a/e2e/reproducers/outbound-tcp-echo.yaml +++ /dev/null @@ -1,56 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -apiVersion: v1 -kind: Pod -metadata: - name: echo - namespace: openshell-outbound-test - labels: - app: outbound-tcp-echo -spec: - securityContext: - runAsNonRoot: true - seccompProfile: - type: RuntimeDefault - containers: - - name: echo - image: ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: [ALL] - command: [python3, -u, -c] - args: - - | - import socket, threading - def echo(conn, peer): - total = 0 - with conn: - while data := conn.recv(65536): - total += len(data) - conn.sendall(data) - print(f"peer={peer} echoed_bytes={total}", flush=True) - with socket.socket() as server: - server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - server.bind(("0.0.0.0", 5432)) - server.listen() - print("READY", flush=True) - while True: - conn, peer = server.accept() - threading.Thread(target=echo, args=(conn, peer), daemon=True).start() - readinessProbe: - tcpSocket: - port: 5432 - periodSeconds: 2 ---- -apiVersion: v1 -kind: Service -metadata: - name: echo - namespace: openshell-outbound-test -spec: - selector: - app: outbound-tcp-echo - ports: - - port: 5432 - targetPort: 5432 diff --git a/e2e/reproducers/outbound-tcp-policy.template.yaml b/e2e/reproducers/outbound-tcp-policy.template.yaml deleted file mode 100644 index 64d4d769bc..0000000000 --- a/e2e/reproducers/outbound-tcp-policy.template.yaml +++ /dev/null @@ -1,19 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -version: 1 -filesystem_policy: - include_workdir: true - read_only: [/usr, /lib, /proc, /dev/urandom, /etc] - read_write: [/tmp, /dev/null] -landlock: - compatibility: best_effort -network_policies: - echo: - name: outbound-tcp-echo - endpoints: - - host: echo.openshell-outbound-test.svc.cluster.local - port: 5432 - protocol: tcp - allowed_ips: ["ECHO_IP/32"] - binaries: - - path: "/**" diff --git a/e2e/reproducers/outbound-tcp.md b/e2e/reproducers/outbound-tcp.md deleted file mode 100644 index e7bbc5937d..0000000000 --- a/e2e/reproducers/outbound-tcp.md +++ /dev/null @@ -1,158 +0,0 @@ - - -# Outbound TCP on the legacy kernel - -[outbound-tcp.py](outbound-tcp.py) opens three outbound connections to a separate -TCP echo Pod. On each connection it calls `getpeername()`, records any error, -then sends and receives both 4 bytes and 256 KiB. It compares every echoed byte -with the original payload. Pass `--expect-relay` to require a successful peer -query that returns a loopback address with a nonzero port. Without that flag, -the script records peer-address errors so it can also reproduce the baseline. - -## Test procedure - -Use an OpenShell Kubernetes gateway whose sandbox namespace is -`openshell-outbound-test`, with the PR-head sandbox runtime deployed. The -[peer-address procedure](accept-peer.md) describes building and deploying that -runtime. Build from the checkout containing the fallback, and use image tag -`pr4087-outbound-fallback` in the build, push, and Helm commands instead of -the baseline tag `pr4087-de27081b3`. For this test, use namespace -`openshell-outbound-test` and local -gateway forwarding port `18089`. Standard chart deployments require working -gateway and workspace PVC provisioning. The recorded run used ephemeral -gateway database and workspace volumes in its temporary test deployment; -the workload's seccomp, capabilities, and network restrictions stayed enabled. - -Start the echo fixture and scope the network policy to its exact Service IP: - -```shell -oc apply -f e2e/reproducers/outbound-tcp-echo.yaml -oc -n openshell-outbound-test wait --for=condition=Ready pod/echo --timeout=90s -ECHO_IP="$(oc -n openshell-outbound-test get service echo -o jsonpath='{.spec.clusterIP}')" -sed "s/ECHO_IP/${ECHO_IP}/g" e2e/reproducers/outbound-tcp-policy.template.yaml \ - > /tmp/pr4087-outbound-policy.yaml -oc -n openshell-outbound-test port-forward svc/openshell 18089:8080 -``` - -Keep the port-forward running and execute the following in another terminal: - -```shell -CLI=target/debug/openshell -ENDPOINT=http://127.0.0.1:18089 -HOST=echo.openshell-outbound-test.svc.cluster.local -PYTHON_IMAGE=ghcr.io/astral-sh/uv:0.12.17-python3.12-trixie-slim@sha256:9a59bb7206905ccaae4f7dab222fbac47c125a21e5fc16f43f427cd6c940ade3 - -"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ - --name outbound3-pr4087 --policy /tmp/pr4087-outbound-policy.yaml \ - --from "$PYTHON_IMAGE" --no-tty \ - -- python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay -``` - -Expect `connect`, `getpeername`, and both `echo` stages to report PASS for -attempts 0, 1, and 2, with exit status 0. In legacy mode the peer address is the -loopback relay, not the echo Service's IP. For a before/after comparison, run -the probe without `--expect-relay` against the earlier runtime image first; -the recorded baseline below shows errno 95. Then deploy the fallback runtime -and run the strict check above. - -Check the exec launch path, then repeat without the accept shim: - -```shell -"$CLI" sandbox create --gateway-endpoint "$ENDPOINT" \ - --name outbound-exec --policy /tmp/pr4087-outbound-policy.yaml \ - --from "$PYTHON_IMAGE" --detach \ - -- python3 -c 'import time; time.sleep(600)' - -"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ - --no-tty --no-login-shell --timeout 60 outbound-exec \ - -- python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay - -"$CLI" sandbox exec --gateway-endpoint "$ENDPOINT" \ - --no-tty --no-login-shell --timeout 60 outbound-exec \ - -- env -u LD_PRELOAD python3 -c "$(cat e2e/reproducers/outbound-tcp.py)" "$HOST" 5432 --expect-relay -``` - -Both exec commands should produce the same data-transfer results. Check the -server logs and supervisor IPs to verify the relay path: - -```shell -oc -n openshell-outbound-test logs echo | rg 'echoed_bytes=262148' -oc -n openshell-outbound-test get pods -l openshell.ai/boundary-role=supervisor \ - -o custom-columns=NAME:.metadata.name,IP:.status.podIP -``` - -The server should record three nonempty connections per invocation, each -echoing 262148 bytes. Their peer IP should be the supervisor's. Zero-byte -connections from the node are the echo Pod's readiness probes. - -## Recorded baseline before the fallback - -The earlier behavior was validated on 2026-10-02, OpenShift **4.21.34**, kernel -**5.14.0-570.141.1.el9_6.x86_64**, using PR-head sandbox commit `de27081b3`. -The deployed binary matched SHA-256 -`dc004b17fd02c4bc0cb8208850b1dc29dcfa7a76ab80536f5ce8dc0eaf91333c`. -Gateway and supervisor image digests were the reused images recorded in -[the peer-address report](accept-peer.md#observed-results). - -| Invocation | Connections | Echoes | `getpeername()` | Exit | -| --- | --- | --- | --- | --- | -| Initial workload | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | -| `sandbox exec` | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | -| Exec without `LD_PRELOAD` | 3 successful | 4 bytes and 256 KiB matched on each | errno 95 on all 3 | 0 | - -The first invocation's echo server saw supervisor `10.232.1.68` as its peer. -Supervisor logs recorded `ALLOWED` decisions for Python connecting to the echo -host on port 5432. The workload was admitted under `restricted-v2`, ran as UID/GID -1000780000, dropped all capabilities, and disabled privilege escalation. - -The baseline established that outbound TCP data transfer works even when the -peer query fails. The fallback changes that query to `CONTINUE`: the kernel -now reports the loopback relay's address. Clients that require the original -upstream address may still be incompatible. This probe does not validate TLS, -Internet routing, or application-client compatibility. - -## Fallback regression validation - -Native ARM64 Linux tests run as a non-root user verified the fallback with a -real seccomp listener and TCP connection: legacy mode returned a loopback peer -with a nonzero port, and modern mode returned the original upstream destination. -Additional tests covered local sockets, unconnected sockets, and the modern -substitution path. All 196 sandbox library tests passed. - -The fallback was also validated on 2026-10-02 on the same OpenShift 4.21.34 -cluster and kernel 5.14.0-570.141.1.el9_6.x86_64, with `--expect-relay`: - -| Invocation | Connections | Echoes | `getpeername()` | Exit | -| --- | --- | --- | --- | --- | -| Initial workload | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | -| `sandbox exec` | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | -| Exec without `LD_PRELOAD` | 3 successful | 4 bytes and 256 KiB matched on each | loopback address and nonzero port on all 3 | 0 | - -The runtime image was -`image-registry.openshift-image-registry.svc:5000/openshell-images/sandbox:pr4087-outbound-fallback`, -digest `sha256:4afb424c54d158ad06ab39c9b47dfea7c1815d58acf5dd404d1377aba36c6b59`. -The deployed binary matched SHA-256 -`544abc2524174e8fa0cb4a452b5c6d76c2bcbce77c923cace4860a74579c0bc8`. -The echo server recorded nine connections carrying 262148 bytes each, from -supervisor IPs `10.232.1.80` (initial workload) and `10.232.1.78` (exec). -Supervisor logs recorded policy `ALLOWED` decisions for the echo destination. -The workload used `restricted-v2`, UID/GID 1000790000, all capabilities dropped, -and privilege escalation disabled. Gateway and supervisor images were unchanged. - -## Cleanup - -Delete the named test sandboxes before removing a temporary gateway. Stop the -port-forward with Ctrl-C. Remove the namespace and pull grant only if they -were created for this test: - -```shell -"$CLI" sandbox delete --gateway-endpoint "$ENDPOINT" outbound3-pr4087 outbound-exec -oc delete -f e2e/reproducers/outbound-tcp-echo.yaml -helm uninstall openshell -n openshell-outbound-test -oc delete namespace openshell-outbound-test --wait=true --timeout=120s -oc -n openshell-images policy remove-role-from-group system:image-puller \ - system:serviceaccounts:openshell-outbound-test -``` diff --git a/e2e/reproducers/outbound-tcp.py b/e2e/reproducers/outbound-tcp.py deleted file mode 100644 index 225cb91485..0000000000 --- a/e2e/reproducers/outbound-tcp.py +++ /dev/null @@ -1,54 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Check outbound relay data transfer independently of peer-address reporting.""" - -import argparse -import ipaddress -import json -import socket - -parser = argparse.ArgumentParser(description=__doc__) -parser.add_argument("host") -parser.add_argument("port", type=int) -parser.add_argument("--expect-relay", action="store_true") -args = parser.parse_args() -for attempt in range(3): - with socket.create_connection((args.host, args.port), timeout=10) as conn: - print( - json.dumps({"stage": "connect", "attempt": attempt, "result": "PASS"}), - flush=True, - ) - try: - peer = {"result": "PASS", "address": conn.getpeername()} - except OSError as error: - peer = {"result": "ERROR", "errno": error.errno, "message": str(error)} - print( - json.dumps({"stage": "getpeername", "attempt": attempt, **peer}), flush=True - ) - if args.expect_relay: - assert peer["result"] == "PASS", peer - assert ipaddress.ip_address(peer["address"][0]).is_loopback, peer - assert peer["address"][1] != 0, peer - # Exchange bytes after querying the peer, including after a baseline error. - for size in (4, 262144): - payload = b"ping" * (size // 4) - conn.sendall(payload) - received = bytearray() - while len(received) < size: - chunk = conn.recv(min(65536, size - len(received))) - if not chunk: - raise RuntimeError("unexpected EOF from echo server") - received.extend(chunk) - assert received == payload - print( - json.dumps( - { - "stage": "echo", - "attempt": attempt, - "bytes": size, - "result": "PASS", - } - ), - flush=True, - )