Skip to content

Commit bdd15bb

Browse files
committed
fix(sandbox): harden legacy accept compatibility
Replace workload-owned shim paths with sealed anonymous objects isolated per launch. Keep descriptors close-on-exec in the boundary and enable inheritance only in the intended child. Preserve effective preload environment overrides without introducing a Landlock user ruleset. Delegate cancellation to libc's accept4 wrapper, close descriptors on peer lookup failure, and return ECONNABORTED for reset peers or EFAULT for invalid output pointers. Reject unsupported raw accepts before consuming clients and allow kernel peer queries on connected DNS sockets. Legacy outbound peer queries intentionally retain the documented loopback relay-address fallback. Add target-compiler fixtures and ELF, peer-error, legacy broker, and concurrent launch regressions. Update compatibility documentation and remove PR-specific reproducer artifacts. Signed-off-by: Russell Bryant <rbryant@redhat.com>
1 parent e26aaba commit bdd15bb

19 files changed

Lines changed: 799 additions & 982 deletions

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/openshell-accept-shim/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,8 @@ libc = "0.2"
1919
# conventions, including the per-target wrappers cargo-zigbuild installs.
2020
cc = "1"
2121

22+
[dev-dependencies]
23+
object = { version = "0.37.3", default-features = false, features = ["read_core", "elf", "std"] }
24+
2225
[lints]
2326
workspace = true

‎crates/openshell-accept-shim/README.md‎

Lines changed: 51 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ statically, or issue raw syscalls, and gains nothing it did not already have —
4747
it only loses the compatibility benefit. The enforcement boundary remains the
4848
broker's fail-closed behavior.
4949

50-
This is why the library's location on disk carries no privilege weight.
51-
5250
## Coverage
5351

5452
`LD_PRELOAD` interposes library symbols, not syscalls.
@@ -81,7 +79,7 @@ them is a bug:
8179
| --- | --- |
8280
| No `DT_NEEDED` | One build per architecture loads under both glibc and musl |
8381
| No `TEXTREL` | Avoids requiring SELinux `execmod`, the permission most likely denied to `container_t` |
84-
| Required `__errno_location` and weak `pthread_setcanceltype` references | Resolve from the workload's libc or already-loaded libpthread without adding a loader dependency |
82+
| Required `__errno_location` and `dl_iterate_phdr` references | Resolve from glibc or musl without a loader dependency |
8583
| Exactly two exported `FUNC` symbols, `accept` and `accept4` | Prevents accidental interposition of unrelated symbols such as `memcpy` |
8684
| 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 |
8785

@@ -96,61 +94,56 @@ header against `CARGO_CFG_TARGET_ARCH` and fails the build on a mismatch,
9694
because a misresolved compiler otherwise produces a valid object for the wrong
9795
architecture.
9896

99-
The code performs no allocation, takes no locks, and cannot panic. When
100-
`getpeername` fails on an already-accepted connection it reports a zero-length
101-
address — what the kernel itself reports for an unnamed peer — rather than
102-
leaking the descriptor or failing an accept that has already succeeded.
103-
104-
The blocking accept phase preserves libc's pthread cancellation behavior by
105-
temporarily switching the caller to asynchronous cancellation, then restoring
106-
its previous cancellation type before querying the peer. Disabled cancellation
107-
remains disabled. The pthread symbol is weak so a single-threaded workload on
108-
older glibc does not need to load libpthread just to use the shim.
109-
110-
Unix installation helpers and their tests are gated with `cfg(unix)`. The
111-
preload-composition helpers also compile in the Windows workspace checks.
112-
113-
## Installation
114-
115-
`install_shim` materializes the embedded object at
116-
`/run/openshell-compat/accept_shim.so`. The sandbox calls it during startup,
117-
only when `listener.writes_disabled()` reports legacy mode.
118-
119-
The path is constrained from both sides:
120-
121-
- **Not under `/.openshell`.** The capability-free Landlock baseline grants each
122-
top-level filesystem entry *except* the driver-owned `.openshell` hierarchy,
123-
and a user ruleset can only narrow the baseline. A workload physically cannot
124-
open a file there, so a library placed there could never be preloaded.
125-
- **Not under the supervisor CA tmpfs.** That mount is `noexec`, so the loader
126-
cannot map an object from it.
127-
128-
`/run` satisfies both. On Docker and Podman it is part of the workload's own
129-
writable, exec-capable rootfs. On Kubernetes the workload receives it as an
130-
`emptyDir{medium: Memory}` tmpfs, mounted `rw,seclabel,relatime` with no
131-
`noexec` and mode `1777`, so a non-root sandbox identity can create its own
132-
subdirectory there. No compute driver needs a packaging change: the object is
133-
embedded in the `openshell-sandbox` binary with `include_bytes!`.
134-
135-
Installation creates the directory, writes through a temporary file, `rename`s
136-
it into place, then seals both the file and the directory to `0o555`. Symlinked
137-
directories and symlinked targets are refused rather than followed. Failure is
138-
non-fatal and logged as an OCSF `Config State Change` event — the sandbox starts
139-
without the library and keeps the broker's existing fail-closed behavior.
140-
141-
`LD_PRELOAD` composition preserves any value the workload supplied and is
142-
idempotent, so a child that re-inherits the variable and spawns its own child
143-
does not accumulate duplicate entries.
97+
The shim resolves libc's `accept4` from the already-loaded ELF image that
98+
provides `__errno_location`, using `dl_iterate_phdr`. Delegating the blocking
99+
phase preserves libc's cancellation handling, including the accepted-fd race
100+
addressed by glibc BZ #12683. Peer lookup and error-path close use raw syscalls,
101+
which are not cancellation points. A failed peer lookup closes the descriptor
102+
and returns the kernel error; `ENOTCONN` becomes `ECONNABORTED` so a reset peer
103+
does not produce a successful accept with an unusable address. Invalid output
104+
pointers return `EFAULT` without userspace dereferences.
105+
106+
## Installation and child environments
107+
108+
The sandbox probes installation only for a legacy read-only listener. It
109+
creates a sealed executable memfd instead of writing into the image's `/run`.
110+
This works with a non-root identity, a read-only rootfs, and noexec temporary
111+
mounts. There are no workload-selected pathname components or symlinks. Write,
112+
grow, shrink, and seal seals prevent changing its bytes. The boundary's private
113+
probe descriptor is close-on-exec.
114+
115+
Each entrypoint or exec launch gets a fresh sealed memfd inode. Its descriptor
116+
remains close-on-exec in the boundary and is made inheritable only in its own
117+
forked child. The loader opens `/proc/self/fd/N`.
118+
A workload can close or chmod its own inode, but cannot replace the object or
119+
change permissions on the inode used by a later operator exec session.
120+
Anonymous inodes need no extra Landlock admission, so the shim does not create
121+
a restrictive user ruleset when the authored policy has none.
122+
123+
Installation checks executable mapping before setting `LD_PRELOAD`. It requests
124+
`MFD_EXEC` where supported and falls back to the pre-6.3 ABI on older kernels.
125+
A host that forbids executable memfds keeps the broker's fail-closed behavior;
126+
installation failure is non-fatal and logged.
127+
128+
Both launch paths compose the shim after all environment sources have been
129+
applied, preserving the final provider, workload, or per-session override.
130+
Directly launched ELF binaries for a different class or architecture do not
131+
receive the shim. Descendants inherit ordinary `LD_PRELOAD` semantics: a child
132+
that closes the descriptor, hides `/proc`, or invokes a foreign-architecture
133+
loader must remove the shim entry itself. The boundary cannot check arbitrary
134+
descendant execs, and those loaders may otherwise emit a preload warning.
135+
Static and secure-execution loaders do not use the shim.
144136

145137
## Validation
146138

147-
Validated on OpenShift 4.21 / RHCOS 9.6 (kernel `5.14.0-570.141.1.el9_6`,
148-
SELinux enforcing), against the `emptyDir{medium: Memory}` mount the Kubernetes
149-
workload actually receives, running as a non-root uid with
150-
`readOnlyRootFilesystem: true`, all capabilities dropped, and the
151-
`RuntimeDefault` seccomp profile: the object maps `r-xp` with zero AVC denials.
152-
153-
When validating across architectures, check `e_machine` in the ELF header
154-
before drawing conclusions. A wrong-architecture object produces
155-
`cannot open shared object file` from the loader, which reads like an SELinux
156-
denial and is not one.
139+
Tests check the embedded target-compiler output for ELF target, no `DT_NEEDED`,
140+
no text relocations, a non-executable stack, exactly `accept` and `accept4`
141+
exports, and the expected libc references. Behavioral tests cover IPv4, IPv6,
142+
truncation, reset peers, invalid length pointers, accepted streams, and pthread
143+
cancellation, including coexistence with a provider preload defining `accept4`.
144+
C fixtures use the same resolved target compiler as the shim.
145+
146+
Sandbox tests exercise a forced legacy listener with the real broker and
147+
preloaded CPython: a rejected raw address-bearing accept must leave its client
148+
queued for a subsequent shimmed accept. They also check connected DNS peer
149+
queries and shim access under unrestricted and restricted Landlock policies.

‎crates/openshell-accept-shim/build.rs‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,47 @@ fn main() {
4949
.unwrap_or_else(|error| panic!("run C compiler {command:?}: {error}"));
5050
assert!(status.success(), "compile {SOURCE}: {status}");
5151

52+
println!("cargo:rerun-if-changed=tests/fixtures/cancellation.c");
53+
let helper = out_dir.join("cancellation");
54+
let status = compiler_command()
55+
.args([
56+
"-std=c11",
57+
"-Wall",
58+
"-Wextra",
59+
"-Werror",
60+
"-pthread",
61+
"-lc",
62+
"tests/fixtures/cancellation.c",
63+
"-o",
64+
])
65+
.arg(&helper)
66+
.status()
67+
.expect("compile cancellation helper");
68+
assert!(status.success(), "compile cancellation helper: {status}");
69+
println!(
70+
"cargo:rustc-env=OPENSHELL_CANCELLATION_HELPER={}",
71+
helper.display()
72+
);
73+
74+
println!("cargo:rerun-if-changed=tests/fixtures/other_preload.c");
75+
let other = out_dir.join("other_preload.so");
76+
let status = compiler_command()
77+
.args([
78+
"-shared",
79+
"-fPIC",
80+
"-nostdlib",
81+
"tests/fixtures/other_preload.c",
82+
"-o",
83+
])
84+
.arg(&other)
85+
.status()
86+
.expect("compile other preload fixture");
87+
assert!(status.success(), "compile other preload fixture: {status}");
88+
println!(
89+
"cargo:rustc-env=OPENSHELL_OTHER_PRELOAD={}",
90+
other.display()
91+
);
92+
5293
verify_object_architecture(&object);
5394

5495
println!("cargo:rustc-env=OPENSHELL_ACCEPT_SHIM={}", object.display());
@@ -62,7 +103,7 @@ fn main() {
62103
/// Guessing a cross prefix instead would pick a binary that is frequently
63104
/// absent on the build host.
64105
fn compiler_command() -> Command {
65-
match cc::Build::new().cargo_metadata(false).try_get_compiler() {
106+
match cc::Build::new().try_get_compiler() {
66107
Ok(compiler) => compiler.to_command(),
67108
Err(error) => panic!("locate a C compiler for the shim: {error}"),
68109
}

0 commit comments

Comments
 (0)