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..79663e0c30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3949,6 +3949,15 @@ 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", + "object", +] + [[package]] name = "openshell-binary-identity" version = "0.0.0" @@ -4543,6 +4552,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..df215d492b --- /dev/null +++ b/crates/openshell-accept-shim/Cargo.toml @@ -0,0 +1,26 @@ +# 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" + +[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 new file mode 100644 index 0000000000..44f81af3ba --- /dev/null +++ b/crates/openshell-accept-shim/README.md @@ -0,0 +1,149 @@ + + +# 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. + +## 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 | + +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 + +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` | +| 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 | + +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 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 + +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 new file mode 100644 index 0000000000..7ff9518511 --- /dev/null +++ b/crates/openshell-accept-shim/build.rs @@ -0,0 +1,140 @@ +// 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}"); + + 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()); +} + +/// 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().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..eb21820ffa --- /dev/null +++ b/crates/openshell-accept-shim/src/accept_shim.c @@ -0,0 +1,227 @@ +// 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. +// +// 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); +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; +} + +__attribute__((constructor)) static void resolve_accept4(void) { + dl_iterate_phdr(find_accept4, 0); +} + +#if defined(__x86_64__) +#define SHIM_NR_CLOSE 3 +#define SHIM_NR_GETPEERNAME 52 + +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; +} + +#elif defined(__aarch64__) +#define SHIM_NR_CLOSE 57 +#define SHIM_NR_GETPEERNAME 205 + +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 (!libc_accept4) { + return shim_finish(-95); // EOPNOTSUPP: unsupported dynamic loader. + } + int accepted = libc_accept4(sockfd, 0, 0, flags); + if (accepted < 0) { + return accepted; + } + + 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; +} + +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..4409f8fae2 --- /dev/null +++ b/crates/openshell-accept-shim/src/lib.rs @@ -0,0 +1,237 @@ +// 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")); + +/// Probe and retain a private shim object in the boundary. +/// +/// 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() + ))) +} + +/// Create a fresh sealed object for one workload launch. +/// +/// 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) +} + +#[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) }; + } + if fd < 0 { + return Err(format!( + "create shim memfd: {}", + std::io::Error::last_os_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() + )); + } + // 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() + )); + } + unsafe { + libc::munmap(mapping, SHIM_OBJECT.len()); + } + Ok(file) +} + +/// 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::*; + + /// 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" + ); + } + + #[test] + #[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!( + 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(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] + 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/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/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 new file mode 100644 index 0000000000..918a86a4e8 --- /dev/null +++ b/crates/openshell-accept-shim/tests/shim_behavior.rs @@ -0,0 +1,550 @@ +// 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::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; + +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::install_shim; + +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 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"); + // RTLD_NOW so an unresolved symbol fails here rather than at the call + // 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()); + + 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. +// 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 = 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()); + (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. +// 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::()]; + 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 = socklen_t::try_from(size_of::()).unwrap(); + 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 = socklen_t::try_from(size_of::()).unwrap(); + 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 = socklen_t::try_from(size_of::()).unwrap(); + + 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 = socklen_t::try_from(size_of::()).unwrap(); + + let (accepted, buffer, length) = shim_accept4(&pending.listener, oversized); + + 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" + ); + + 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 = socklen_t::try_from(size_of::()).unwrap(); + 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 = socklen_t::try_from(size_of::()).unwrap(); + 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] +// 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 + .client + .local_addr() + .expect("client local addr") + .port(); + + let mut buffer = vec![0xAAu8; size_of::()]; + let mut length = socklen_t::try_from(size_of::()).unwrap(); + 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] +// 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 + // 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 = socklen_t::try_from(buffer.len()).unwrap(); + 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)); +} + +#[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 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 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 [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 > 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!( + output.status.success(), + "accept4={accept4} address={address} disabled={disabled} preload={preload}: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + } + } + } + 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/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..65d375ee54 100644 --- a/crates/openshell-sandbox/src/boundary_exec.rs +++ b/crates/openshell-sandbox/src/boundary_exec.rs @@ -191,6 +191,7 @@ impl LocalBoundaryExec { 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/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..1f44d0d8e2 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -2,6 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 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( ca_cert_path: &Path, @@ -21,12 +25,285 @@ 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() +} + +/// 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. +#[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, + openshell_accept_shim::compose_preload(&shim_path.display().to_string(), inherited), + ) +} + +/// 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)] mod tests { use super::*; 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 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!( + read_executable_header(&fifo, &mut [0; 20]) + .unwrap_err() + .kind(), + std::io::ErrorKind::InvalidInput + ); + } + + #[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 56c857ce9c..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() @@ -1265,6 +1269,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 @@ -1580,8 +1590,28 @@ 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. 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, - 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 { .. } + | SocketState::DnsTcp { .. } + | SocketState::DnsUdp { .. } => { + return listener.respond_continue(notification.id); + } _ => return Err(io::Error::from_raw_os_error(libc::ENOTCONN)), }; write_socket_addr( @@ -2041,6 +2071,211 @@ 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_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) }, + mode, + ) + } + + 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 }), + ("DnsTcp", SocketState::DnsTcp { relay: peer }), + ("DnsUdp", SocketState::DnsUdp { relay: peer }), + ] { + let (registry, installed) = registry_with_state(state); + let listener = fake_listener(ListenerMode::LegacyReadOnly); + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .expect_err("the fake listener cannot complete any ioctl"); + assert_eq!( + error.raw_os_error(), + Some(libc::ENOTTY), + "{label} must not route through the fail-closed task-memory write" + ); + } + } + + #[test] + 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. 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_listener(ListenerMode::LegacyReadOnly); + let error = get_peer_name( + ®istry, + &listener, + getpeername_notification(installed.as_raw_fd()), + ) + .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 + && 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] fn relay_rejects_descriptor_replaced_after_policy_decision() { let metadata = SocketMetadata { @@ -2309,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 595ceac29e..89c0490736 100644 --- a/crates/openshell-sandbox/src/process.rs +++ b/crates/openshell-sandbox/src/process.rs @@ -505,6 +505,8 @@ impl ProcessHandle { } } + 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 // pre_exec context cannot reliably emit structured logs. @@ -1513,6 +1515,32 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + #[allow(unsafe_code)] + 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)); + } + } + } + } + #[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..62dabf931c 100644 --- a/docs/about/support-matrix.mdx +++ b/docs/about/support-matrix.mdx @@ -184,22 +184,63 @@ 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`: - -- `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. +DNS and TCP authorization); the difference is that the broker cannot write +mediated results back into workload memory. + +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. + +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`. 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` on directly connected sockets | +| --- | --- | --- | +| 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 | + +For directly connected sockets, including accepted sockets, `getpeername` works +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 +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..d4237445d1 100644 --- a/docs/kubernetes/openshift.mdx +++ b/docs/kubernetes/openshift.mdx @@ -24,15 +24,23 @@ 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`. 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; 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": "#",