Skip to content

Commit 12cec59

Browse files
authored
fix(sandbox): accept local connections natively on loopback-confined sockets (#4150)
* fix(sandbox): accept local connections natively on loopback-confined sockets On kernels without SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV (RHEL 9 / RHCOS 5.14), the broker cannot safely write an accepted peer address into workload memory, so accept/accept4 with a peer-address buffer failed with EOPNOTSUPP. Static binaries and Go servers, which issue the raw syscall, could not accept connections at all. Move local acceptance into the kernel and replace per-accept inspection with standing kernel confinement: - Bind every broker-created TCP/UDP socket to the loopback device before injection and verify the binding. Accepted sockets inherit it, so they can neither receive routed ingress nor emit routed egress. - Stop notifying accept/accept4 and remove the accept workers, the SIGUSR2 accept-interrupt monitor, and the 64-accept ceiling. - Continue getpeername natively for every descriptor except relayed connections. - Deny interface-selection socket options and MSG_FASTOPEN sends from scalar syscall arguments, so confinement does not depend on the capability state of the user namespace that owns the network namespace and covers unregistered descriptors. - Drop loopback-interface ingress on the non-loopback TCP control listener before it listens, so a workload cannot reach it by reconnecting a natively accepted socket. - Mark descriptors above stdio close-on-exec in every workload pre_exec path. - Require an active confinement probe at qualification and carry it as required authenticated audit evidence. Document OpenShift 4.19 as the minimum release: RHCOS kernels for 4.16 through 4.18 are built without Landlock. Closes #4058 Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): close review gaps in native local accept confinement - Reject a TCP boundary control listener on a loopback address, including IPv4-mapped loopback. Production drivers bind the unspecified address; a loopback listener would be reachable from workload sockets the broker does not track. - Extend the confinement probe to IPv6 and prove that an accepted socket keeps its loopback binding after an AF_UNSPEC disconnect. - Pin the accepted-peer contract with tests: a loopback-bound listener admits only clients in the sandbox network namespace, and workload sockets cannot bind a non-loopback source address. - Correct the support matrix: accepted connections are bounded by per-process descriptor limits, the runtime PID limit, and the sandbox memory limit, not by a broker limit. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(sandbox): replace unsafe socket calls with socket2 and rustix Socket confinement now uses socket2 for device binding and the ingress filter, and rustix for interface lookup and AF_UNSPEC disconnect, so the module contains no unsafe code. The control-listener filter is no longer locked: no safe API exposes SO_LOCK_FILTER, and the listener descriptor never leaves the trusted sandbox process, which marks every descriptor above stdio close-on-exec before running workload code. Tests added for native accept use rustix and socket2 instead of raw libc calls. rustix issues accept4 and getpeername as raw syscalls, so the direct-syscall test keeps its meaning. The close-on-exec sweep test runs in a re-executed test process instead of a forked child. The close_range(CLOSE_RANGE_CLOEXEC) syscall in the pre_exec hook remains the only unsafe added by this branch; neither rustix nor nix wraps it. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): allowlist workload socket families The workload seccomp filter denied only AF_PACKET, AF_BLUETOOTH, and AF_VSOCK, and the broker continued socket() for every non-INET family. Several protocol families, including AF_RXRPC, AF_SMC, and AF_KCM, carry traffic over kernel-owned sockets that the broker never creates and that are not bound to loopback. Allow only AF_UNIX, AF_NETLINK (still limited to NETLINK_ROUTE), and the brokered AF_INET/AF_INET6 families, and restrict socketpair(2) to AF_UNIX. Both decisions use the scalar domain argument. The broker independently refuses non-INET families other than AF_UNIX and AF_NETLINK with EAFNOSUPPORT. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(sandbox): remove legacy read-only mode After native local accept, only two broker paths still wrote into workload memory: getpeername on relayed connections and the per-message lengths of a first sendmmsg to the DNS relay. Legacy mode existed only to refuse those writes on kernels without WAIT_KILLABLE_RECV, and the getpeername refusal broke CPython TLS on RHEL 9. The broker now never writes workload memory: - getpeername is no longer mediated and reports the kernel peer on every kernel; relayed connections report the loopback relay address. - A first send to the DNS relay pins the broker's socket copy to the relay and continues the syscall, so the kernel performs the send and writes any per-message results. Remove the listener mode, the task-memory write path and probe, and the task_memory_write, cancellation, and task_memory_writes_disabled audit evidence fields. WAIT_KILLABLE_RECV is still used when available and is reported for diagnostics, but no mediation decision depends on it. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(sandbox): drop WAIT_KILLABLE_RECV and make handlers restart-safe The broker no longer writes workload memory, so killable notification waits only reduced how often a signal restarts a notified syscall. RHEL 9 kernels never had them, so the handlers must tolerate restarts anyway. Install the listener without the flag on every kernel instead of special-casing newer ones. Handlers now check that the notification is still live immediately before each side effect (bind, connect of the retained socket, listen, and the first DNS send), and answer a restarted operation the broker already completed as the kernel would: a repeated TCP connect returns EISCONN, repeating the same UDP association succeeds, a repeat of a completed bind succeeds, and a failed relay reports its errno. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): finish teardown only when every workload descendant has exited Teardown waited only for registered process groups to disappear. A root is unregistered once reaped, so a descendant that ignored SIGTERM could outlive it while termination reported success and never sent SIGKILL, violating the bounded-termination requirement. The sandbox now becomes a child subreaper when it is not PID 1, so orphaned descendants stay in its tree and are reaped. Termination waits until no live descendant remains, and every scanned process is signalled through a pidfd after confirming its start time, so a reused PID is never signalled. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): keep a frozen workload from resuming itself Freezing stops every workload process with SIGSTOP. tgkill and rt_tgsigqueueinfo were not mediated, and mediated kill, rt_sigqueueinfo, and tkill passed SIGCONT through, so a workload process that was not yet stopped could resume the others while the supervisor recovered. Mediate tgkill and rt_tgsigqueueinfo like tkill: refuse targets in the sandbox thread group, report a thread outside the named group as missing, and continue otherwise. The boundary marks the broker frozen before stopping the workload and clears it after resuming; while frozen, workload requests to send SIGCONT fail with EPERM. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): send mediated DNS datagrams from the broker socket The native UDP send path re-ran the workload's own sendmsg, so per-message ancillary data such as IP_PKTINFO rode along; only the loopback destination contained a routing override. The broker now reads the datagram and sends it from its retained, relay-connected socket, which it builds without ancillary data, so a per-message override cannot redirect the packet. It writes nothing back into workload memory. A send carrying control data is refused with EOPNOTSUPP rather than silently stripped. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): keep sandbox control variables out of the workload environment The canonical process inherits the sandbox environment so the image's own ENV reaches the workload, then removed only a denylist of credential variables. Other variables in the reserved OPENSHELL_ namespace, such as the serialized user environment and the log level, still reached the workload. Remove every inherited OPENSHELL_ variable before applying the declared environment, and restore OPENSHELL_SANDBOX=1. The image's ordinary ENV and declared variables are unaffected; declared variables cannot use the reserved namespace. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): keep /proc read-only for GPU workloads GPU mode granted read-write access to all of /proc so CUDA's cuInit could write thread names to /proc/<pid>/task/<tid>/comm. Keep /proc read-only. Open mediation now serves a writable open of the caller's own thread comm file: the broker opens it and injects the descriptor, with no syscall continued. The kernel accepts a comm write only from the target's own thread group, so a substituted path or reused thread ID cannot rename another process's thread. Any other /proc write is left to Landlock, which denies it. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): keep the broker socket for connected UDP DNS sockets Sending DNS datagrams from the broker's socket required the broker to keep its copy, but the connect paths to the relay and to loopback still released it. glibc connects the resolver socket and then sends A and AAAA together with sendmmsg, which is mediated, so resolution failed. Release the copy on those connect paths only for TCP. Add a regression test for connect followed by sendmmsg that fails fast rather than hanging. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): keep slow loopback connects from stalling mediation The loopback connect path held the registry lock while polling a TCP connect for up to five seconds on the single notification dispatcher. A workload connecting to a busy local listener stalled every other mediated syscall, including opens and signals. Connect a duplicate of the retained socket without holding the lock. A nonblocking socket gets the native EINPROGRESS and the kernel completes the handshake on the shared socket; a blocking socket waits on a bounded worker thread. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): contain network broker handler panics A panic in a notification handler unwound the single broker thread while the health flag still read healthy, so every later blocked workload syscall hung until the sandbox was killed. Run each dispatch under catch_unwind: a panicking handler fails only that syscall with EIO and the broker keeps mediating. If the broker thread ever exits, mark it unhealthy so dependent operations fail closed instead of blocking. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): reject ancillary data on DNS sends instead of broker-sending Sending mediated DNS datagrams from the broker's own socket required keeping a broker handle for each DNS socket's whole life, which regressed real name resolution through reclaim and resource accounting that the mock-based unit tests did not exercise. Revert to continuing the kernel send, but refuse a send that carries ancillary control data (msg_controllen != 0) at read time, so a per-message routing override such as IP_PKTINFO cannot ride a mediated DNS send. A loopback destination contains an override that races the check. Full broker-side UDP mediation is left to a separate change with deployment e2e. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): address review findings across the hardening changes Fixes: - The blocking local-connect worker no longer toggles O_NONBLOCK on the open file it shares with the workload; it waits with a plain connect. - tgkill and rt_tgsigqueueinfo are notified only when they send SIGCONT, so ordinary thread signals such as Go preemption stay in the kernel. The frozen flag is re-read immediately before delivery. - A shell redirect to the caller's own thread comm file works again; O_CREAT and O_TRUNC are no-ops there and O_EXCL returns EEXIST. - The teardown scan is a linear walk and fails closed when /proc cannot be read. Simplifications: - Remove tests that need infrastructure outside the repo or prove nothing: the topology harness tests, the static-server benchmark, the direct-syscall accept test, the comm rename test without Landlock, a serde-default test, and the test-only panic hook in setsockopt. - Drop the unused Failed connect outcome, the redundant read-back after binding to loopback, redundant OPENSHELL_SANDBOX settings, and stale or duplicated comments; reuse the reserved environment prefix constant. - Tighten the support matrix and OpenShift wording. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(cli): wait for input when piped exec stdin is nonblocking Processes that inherit the same stdin share one open file description, so any of them can make it nonblocking for all. `sandbox exec` then read no input yet, got EAGAIN, and failed with "Resource temporarily unavailable (os error 11)". Parallel e2e tests inherit the runner's stdin, which made credential_gating fail intermittently. The stdin reader now blocks in poll() until input or end of file arrives instead of treating EAGAIN as fatal, and it leaves the shared descriptor's flags alone. The e2e exec helper also stops inheriting the runner's stdin, since its commands take no input. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(sandbox): fix macOS lint and older-glibc test linking Import HashSet only in the Linux-only process scan, and call gettid through the raw syscall in a test, since the libc wrapper needs glibc 2.30 or newer. Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(sandbox): clarify socket peer address reporting Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent fb8f6c0 commit 12cec59

27 files changed

Lines changed: 2010 additions & 1380 deletions

File tree

‎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-cli/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ tracing-subscriber = { workspace = true }
8888
workspace = true
8989

9090
[target.'cfg(unix)'.dependencies]
91-
nix = { workspace = true }
91+
nix = { workspace = true, features = ["poll"] }
9292

9393
[dev-dependencies]
9494
# Tests import the example profiles from providers/ the way an operator

‎crates/openshell-cli/src/run.rs‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1884,7 +1884,7 @@ enum PipedStdin {
18841884
/// never waits on a thread parked in `read(2)`. The thread exits at EOF, on a
18851885
/// read error, or when the receiver is dropped.
18861886
fn spawn_piped_stdin_reader(
1887-
mut reader: impl Read + Send + 'static,
1887+
mut reader: impl PipedInput,
18881888
) -> tokio::sync::mpsc::Receiver<std::io::Result<Vec<u8>>> {
18891889
let (tx, rx) = tokio::sync::mpsc::channel::<std::io::Result<Vec<u8>>>(64);
18901890
std::thread::spawn(move || {
@@ -1893,6 +1893,16 @@ fn spawn_piped_stdin_reader(
18931893
match reader.read(&mut buf) {
18941894
Ok(0) => return,
18951895
Err(error) if error.kind() == ErrorKind::Interrupted => {}
1896+
// Processes that inherit the same stdin share its open file
1897+
// description, so another process may have made it
1898+
// nonblocking. Wait for input instead of failing.
1899+
#[cfg(unix)]
1900+
Err(error) if error.kind() == ErrorKind::WouldBlock => {
1901+
if let Err(error) = wait_until_readable(&reader) {
1902+
let _ = tx.blocking_send(Err(error));
1903+
return;
1904+
}
1905+
}
18961906
Err(error) => {
18971907
let _ = tx.blocking_send(Err(error));
18981908
return;
@@ -1908,6 +1918,30 @@ fn spawn_piped_stdin_reader(
19081918
rx
19091919
}
19101920

1921+
/// Input the piped-stdin reader accepts. On Unix it must expose a descriptor
1922+
/// so a nonblocking stream can be waited on.
1923+
#[cfg(unix)]
1924+
trait PipedInput: Read + std::os::fd::AsFd + Send + 'static {}
1925+
#[cfg(unix)]
1926+
impl<T: Read + std::os::fd::AsFd + Send + 'static> PipedInput for T {}
1927+
#[cfg(not(unix))]
1928+
trait PipedInput: Read + Send + 'static {}
1929+
#[cfg(not(unix))]
1930+
impl<T: Read + Send + 'static> PipedInput for T {}
1931+
1932+
/// Block until `reader` has input or reaches end of file.
1933+
#[cfg(unix)]
1934+
fn wait_until_readable(reader: &impl std::os::fd::AsFd) -> std::io::Result<()> {
1935+
use nix::poll::{PollFd, PollFlags, PollTimeout, poll};
1936+
let mut fds = [PollFd::new(reader.as_fd(), PollFlags::POLLIN)];
1937+
loop {
1938+
match poll(&mut fds, PollTimeout::NONE) {
1939+
Err(nix::errno::Errno::EINTR) => {}
1940+
result => return result.map(drop).map_err(std::io::Error::from),
1941+
}
1942+
}
1943+
}
1944+
19111945
/// Collect piped stdin until EOF or until `grace` elapses, whichever comes
19121946
/// first. Input beyond `limit` bytes is rejected with the upload hint.
19131947
async fn collect_piped_stdin(
@@ -8811,6 +8845,42 @@ mod tests {
88118845
);
88128846
}
88138847

8848+
#[cfg(unix)]
8849+
#[test]
8850+
fn piped_stdin_left_nonblocking_by_another_process_still_streams() {
8851+
// Processes that inherit the same stdin share one open file
8852+
// description, so any of them can make it nonblocking for all. A
8853+
// read with no input yet then fails with EAGAIN instead of waiting.
8854+
use std::os::fd::AsRawFd as _;
8855+
let (reader, mut writer) = std::io::pipe().expect("pipe");
8856+
nix::fcntl::fcntl(
8857+
reader.as_raw_fd(),
8858+
nix::fcntl::FcntlArg::F_SETFL(nix::fcntl::OFlag::O_NONBLOCK),
8859+
)
8860+
.expect("make the shared pipe nonblocking");
8861+
let runtime = exec_stdin_runtime();
8862+
let collected = runtime.block_on(super::collect_piped_stdin(
8863+
super::spawn_piped_stdin_reader(reader),
8864+
Duration::from_millis(100),
8865+
super::MAX_EXEC_STDIN_BYTES,
8866+
));
8867+
let super::PipedStdin::Open { prefix, mut rest } = collected.expect("collect") else {
8868+
panic!("an open pipe must start the command before EOF");
8869+
};
8870+
assert!(prefix.is_empty());
8871+
writer.write_all(b"late").unwrap();
8872+
drop(writer);
8873+
let next = runtime
8874+
.block_on(rest.recv())
8875+
.expect("late chunk")
8876+
.expect("read");
8877+
assert_eq!(next, b"late");
8878+
assert!(
8879+
runtime.block_on(rest.recv()).is_none(),
8880+
"EOF closes the channel"
8881+
);
8882+
}
8883+
88148884
#[test]
88158885
fn piped_stdin_over_the_limit_is_rejected_with_the_upload_hint() {
88168886
let (reader, mut writer) = std::io::pipe().expect("pipe");

‎crates/openshell-isolation-interface/Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ tokio = { workspace = true }
2222
libc = "0.2"
2323

2424
[target.'cfg(target_os = "linux")'.dependencies]
25-
rustix = { workspace = true, features = ["fs", "process"] }
25+
rustix = { workspace = true, features = ["fs", "net", "process"] }
26+
socket2 = { workspace = true, features = ["all"] }
2627

2728
[dev-dependencies]
2829
tokio = { workspace = true }

‎crates/openshell-isolation-interface/src/linux/child_seccomp.rs‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const SECCOMP_DATA_ARGS_OFFSET: u32 = 16;
2929
const X32_SYSCALL_BIT: u32 = 0x4000_0000;
3030

3131
const CLOSE_RANGE_UNSHARE_FLAG: u32 = 1 << 1;
32+
const CLOSE_RANGE_CLOEXEC_FLAG: u32 = 1 << 2;
3233
const F_SETOWN_COMMAND: u32 = 8;
3334
const F_SETSIG_COMMAND: u32 = 10;
3435
const F_SETOWN_EX_COMMAND: u32 = 15;
@@ -53,6 +54,7 @@ impl ChildHardeningProgram {
5354
/// The caller must invoke this from the post-fork child after all
5455
/// sandbox-wide TSYNC work and the launcher's `NEW_LISTENER` filter.
5556
pub fn install(&mut self) -> io::Result<()> {
57+
mark_inherited_descriptors_close_on_exec()?;
5658
set_no_new_privileges()?;
5759
let len = u16::try_from(self.instructions.len()).map_err(|_| {
5860
io::Error::new(
@@ -88,13 +90,43 @@ impl ChildHardeningProgram {
8890
}
8991
}
9092

93+
/// Mark every descriptor above stdio close-on-exec in the post-fork child.
94+
///
95+
/// Workloads receive INET sockets only through broker injection, which binds
96+
/// them to loopback first. A descriptor the sandbox process inherited from its
97+
/// container runtime, or opened without `O_CLOEXEC`, must never cross `exec`
98+
/// as an unconfined socket. The command's stdio is already installed on 0-2
99+
/// when `pre_exec` hooks run. This is a single async-signal-safe syscall.
100+
///
101+
/// # Errors
102+
///
103+
/// Returns the kernel error; kernels without `CLOSE_RANGE_CLOEXEC` (before
104+
/// Linux 5.11) fail closed.
105+
pub fn mark_inherited_descriptors_close_on_exec() -> io::Result<()> {
106+
// SAFETY: close_range takes scalar arguments and only sets FD_CLOEXEC.
107+
let result = unsafe {
108+
libc::syscall(
109+
libc::SYS_close_range,
110+
3_u32,
111+
u32::MAX,
112+
CLOSE_RANGE_CLOEXEC_FLAG,
113+
)
114+
};
115+
if result < 0 {
116+
Err(io::Error::last_os_error())
117+
} else {
118+
Ok(())
119+
}
120+
}
121+
91122
/// Build the same-UID workload self-protection program before `fork`.
92123
///
93124
/// `sandbox_tgid` is the sandbox PID as visible from its workload namespace.
94125
/// The filter blocks thread-targeting operations that name the trusted sandbox
95126
/// leader and blocks process-directed operations with the same target. The
96-
/// ordinary workload listener additionally mediates `kill`, `tkill`, and
97-
/// `rt_sigqueueinfo`: Linux accepts nonleader TIDs for these operations, so a
127+
/// ordinary workload listener additionally mediates `kill`, `tkill`,
128+
/// `rt_sigqueueinfo`, and `SIGCONT` sent with `tgkill` or
129+
/// `rt_tgsigqueueinfo`: Linux accepts nonleader TIDs for these operations, so a
98130
/// static TGID comparison alone cannot protect future sandbox worker threads.
99131
pub fn prepare(sandbox_tgid: u32) -> io::Result<ChildHardeningProgram> {
100132
if sandbox_tgid == 0 {
@@ -352,6 +384,48 @@ fn set_no_new_privileges() -> io::Result<()> {
352384
mod tests {
353385
use super::*;
354386

387+
#[test]
388+
fn inherited_sockets_are_marked_close_on_exec_but_stdio_is_not() {
389+
// The sweep changes every descriptor in the calling process, so run
390+
// it in a fresh copy of this test binary rather than the harness.
391+
const CHILD_MARKER: &str = "OPENSHELL_CLOEXEC_SWEEP_CHILD";
392+
if std::env::var_os(CHILD_MARKER).is_some() {
393+
let socket = rustix::net::socket(
394+
rustix::net::AddressFamily::INET,
395+
rustix::net::SocketType::STREAM,
396+
None,
397+
)
398+
.expect("inheritable socket");
399+
assert!(
400+
!rustix::io::fcntl_getfd(&socket)
401+
.unwrap()
402+
.contains(rustix::io::FdFlags::CLOEXEC)
403+
);
404+
mark_inherited_descriptors_close_on_exec().expect("sweep descriptors");
405+
assert!(
406+
rustix::io::fcntl_getfd(&socket)
407+
.unwrap()
408+
.contains(rustix::io::FdFlags::CLOEXEC)
409+
);
410+
assert!(
411+
!rustix::io::fcntl_getfd(io::stderr())
412+
.unwrap()
413+
.contains(rustix::io::FdFlags::CLOEXEC)
414+
);
415+
return;
416+
}
417+
let status = std::process::Command::new(std::env::current_exe().unwrap())
418+
.args([
419+
"--exact",
420+
"linux::child_seccomp::tests::inherited_sockets_are_marked_close_on_exec_but_stdio_is_not",
421+
"--nocapture",
422+
])
423+
.env(CHILD_MARKER, "1")
424+
.status()
425+
.expect("run isolated sweep test");
426+
assert!(status.success(), "isolated sweep test failed");
427+
}
428+
355429
#[test]
356430
fn rejects_zero_sandbox_tgid() {
357431
assert_eq!(

‎crates/openshell-isolation-interface/src/linux/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub mod landlock;
1111
pub mod proc_fd;
1212
pub mod process_signal;
1313
pub mod seccomp_notify;
14+
pub mod socket_confinement;
1415
pub mod socket_registry;
1516
pub mod task_memory;
1617
pub mod workload_launcher;

‎crates/openshell-isolation-interface/src/linux/process_signal.rs‎

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
use std::io;
1414
use std::os::fd::{AsRawFd, FromRawFd, OwnedFd};
15+
use std::sync::atomic::{AtomicBool, Ordering};
1516

1617
use crate::linux::seccomp_notify::{Notification, NotificationListener};
1718
use crate::linux::task_memory;
@@ -27,6 +28,7 @@ pub fn mediate_process_signal(
2728
listener: &NotificationListener,
2829
notification: Notification,
2930
sandbox_tgid: u32,
31+
workload_frozen: &AtomicBool,
3032
) -> io::Result<()> {
3133
listener.validate_id(notification.id)?;
3234
let target = scalar_int(notification.args[0]);
@@ -38,6 +40,7 @@ pub fn mediate_process_signal(
3840
libc::EINVAL
3941
}));
4042
}
43+
refuse_resume_while_frozen(signal, workload_frozen)?;
4144
let target = u32::try_from(target).map_err(|_| io::Error::from_raw_os_error(libc::ESRCH))?;
4245
let retained = retain_signal_target(target, sandbox_tgid)?;
4346
// SAFETY: all-zero siginfo consists of valid integer/pointer fields. A
@@ -64,6 +67,7 @@ pub fn mediate_process_signal(
6467
_ => return Err(io::Error::from_raw_os_error(libc::ENOSYS)),
6568
};
6669
listener.validate_id(notification.id)?;
70+
refuse_resume_while_frozen(signal, workload_frozen)?;
6771
// SAFETY: retained owns a live pidfd; info is null or a complete trusted
6872
// copy. The kernel targets that process object, never a reused numeric PID.
6973
let result = unsafe {
@@ -81,37 +85,62 @@ pub fn mediate_process_signal(
8185
listener.respond_value(notification.id, 0)
8286
}
8387

84-
/// Continue a positive-target `tkill` only when the target thread belongs to
85-
/// an untrusted workload process rather than the sandbox runtime itself.
88+
/// Continue a thread-directed signal (`tkill`, `tgkill`,
89+
/// `rt_tgsigqueueinfo`) aimed at an untrusted workload thread.
8690
///
8791
/// Continuing preserves Linux's thread-directed signal semantics, including
88-
/// the cancellation signal used by musl. A target that exits between the
89-
/// ownership check and continuation can only be reused inside the same PID
90-
/// namespace; the static child filter still rejects the sandbox leader.
92+
/// the cancellation signal used by musl. The static child filter rejects the
93+
/// sandbox leader as a `tgkill`/`rt_tgsigqueueinfo` group, and the kernel
94+
/// rejects a thread outside the named group, so those two are notified only
95+
/// for `SIGCONT`. A `tkill` names a bare thread, so its group is resolved here;
96+
/// a target reused between this check and continuation stays inside the same
97+
/// PID namespace.
9198
pub fn mediate_thread_signal(
9299
listener: &NotificationListener,
93100
notification: Notification,
94101
sandbox_tgid: u32,
102+
workload_frozen: &AtomicBool,
95103
) -> io::Result<()> {
96104
listener.validate_id(notification.id)?;
97-
let target = scalar_int(notification.args[0]);
98-
let signal = scalar_int(notification.args[1]);
99-
if target <= 0 || !(0..=64).contains(&signal) {
100-
return Err(io::Error::from_raw_os_error(if target <= 0 {
101-
libc::EPERM
102-
} else {
103-
libc::EINVAL
104-
}));
105-
}
106-
let target = u32::try_from(target).map_err(|_| io::Error::from_raw_os_error(libc::ESRCH))?;
107-
let target_group = thread_group_id(target)?;
108-
if target_group == sandbox_tgid || target_group == 0 {
109-
return Err(io::Error::from_raw_os_error(libc::EPERM));
105+
let signal = match i64::from(notification.syscall) {
106+
libc::SYS_tkill => {
107+
let target = scalar_int(notification.args[0]);
108+
let signal = scalar_int(notification.args[1]);
109+
if target <= 0 {
110+
return Err(io::Error::from_raw_os_error(libc::EPERM));
111+
}
112+
let target =
113+
u32::try_from(target).map_err(|_| io::Error::from_raw_os_error(libc::ESRCH))?;
114+
let target_group = thread_group_id(target)?;
115+
if target_group == sandbox_tgid || target_group == 0 {
116+
return Err(io::Error::from_raw_os_error(libc::EPERM));
117+
}
118+
signal
119+
}
120+
libc::SYS_tgkill | libc::SYS_rt_tgsigqueueinfo => scalar_int(notification.args[2]),
121+
_ => return Err(io::Error::from_raw_os_error(libc::ENOSYS)),
122+
};
123+
if !(0..=64).contains(&signal) {
124+
return Err(io::Error::from_raw_os_error(libc::EINVAL));
110125
}
111126
listener.validate_id(notification.id)?;
127+
refuse_resume_while_frozen(signal, workload_frozen)?;
112128
listener.respond_continue(notification.id)
113129
}
114130

131+
/// While the boundary has stopped the workload for supervisor recovery, a
132+
/// workload process that was not yet stopped must not resume the others.
133+
///
134+
/// The flag is read immediately before delivery. The freezer does not wait
135+
/// for in-flight notifications, so a signal already past this check when the
136+
/// freeze begins can still be delivered.
137+
fn refuse_resume_while_frozen(signal: i32, workload_frozen: &AtomicBool) -> io::Result<()> {
138+
if signal == libc::SIGCONT && workload_frozen.load(Ordering::Acquire) {
139+
return Err(io::Error::from_raw_os_error(libc::EPERM));
140+
}
141+
Ok(())
142+
}
143+
115144
fn scalar_int(value: u64) -> i32 {
116145
let bytes = value.to_ne_bytes();
117146
#[cfg(target_endian = "little")]
@@ -176,8 +205,13 @@ mod tests {
176205
.recv_timeout(std::time::Duration::from_secs(5))
177206
.unwrap();
178207
let notification = listener.receive().unwrap();
179-
let error =
180-
mediate_process_signal(&listener, notification, std::process::id()).unwrap_err();
208+
let error = mediate_process_signal(
209+
&listener,
210+
notification,
211+
std::process::id(),
212+
&AtomicBool::new(false),
213+
)
214+
.unwrap_err();
181215
assert_eq!(error.raw_os_error(), Some(libc::EPERM));
182216
listener
183217
.respond_errno(notification.id, libc::EPERM)

0 commit comments

Comments
 (0)