Skip to content

Commit 28db08e

Browse files
authored
fix(sandbox): disable child core dumps (#821)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent 28e1ff7 commit 28db08e

File tree

5 files changed

+161
-4
lines changed

5 files changed

+161
-4
lines changed

architecture/sandbox.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,9 @@ Wraps `tokio::process::Child` + PID. Platform-specific `spawn()` methods delegat
12811281
1. `setpgid(0, 0)` if non-interactive (create new process group)
12821282
2. `setns(fd, CLONE_NEWNET)` to enter network namespace (Linux only)
12831283
3. `drop_privileges(policy)`: `initgroups()` -> `setgid()` -> `setuid()`
1284-
4. `sandbox::apply(policy, workdir)`: Landlock then seccomp
1284+
4. Disable core dumps with `setrlimit(RLIMIT_CORE, 0)` on Unix
1285+
5. Set `prctl(PR_SET_DUMPABLE, 0)` on Linux
1286+
6. `sandbox::apply(policy, workdir)`: Landlock then seccomp
12851287

12861288
### `drop_privileges()`
12871289

@@ -1297,6 +1299,8 @@ The ordering is significant: `initgroups`/`setgid` must happen before `setuid` b
12971299

12981300
Steps 3, 5, and 6 are defense-in-depth post-condition checks (CWE-250 / CERT POS37-C). All three syscalls (`geteuid`, `getegid`, `setuid`) are async-signal-safe, so they are safe to call in the `pre_exec` context. The checks add negligible overhead while guarding against hypothetical kernel-level defects that could cause `setuid`/`setgid` to return success without actually changing the effective IDs.
12991301

1302+
After the privilege drop, the child process also disables core dumps before Landlock and seccomp are applied. On all Unix targets it sets `RLIMIT_CORE=0`; on Linux it additionally sets `PR_SET_DUMPABLE=0`. This prevents crash artifacts from containing provider credentials, request payloads, or other sensitive in-memory data.
1303+
13001304
### `ProcessStatus`
13011305

13021306
Exit code is `code` if the process exited normally, or `128 + signal` if killed by a signal (standard Unix convention). Returns `-1` if neither is available.

crates/openshell-sandbox/src/process.rs

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,35 @@ fn scrub_sensitive_env(cmd: &mut Command) {
3434
cmd.env_remove(SSH_HANDSHAKE_SECRET_ENV);
3535
}
3636

37+
#[cfg(unix)]
38+
#[allow(unsafe_code)]
39+
pub(crate) fn harden_child_process() -> Result<()> {
40+
let core_limit = libc::rlimit {
41+
rlim_cur: 0,
42+
rlim_max: 0,
43+
};
44+
let rc = unsafe { libc::setrlimit(libc::RLIMIT_CORE, &core_limit) };
45+
if rc != 0 {
46+
return Err(miette::miette!(
47+
"Failed to disable core dumps: {}",
48+
std::io::Error::last_os_error()
49+
));
50+
}
51+
52+
#[cfg(target_os = "linux")]
53+
{
54+
let rc = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
55+
if rc != 0 {
56+
return Err(miette::miette!(
57+
"Failed to set PR_SET_DUMPABLE=0: {}",
58+
std::io::Error::last_os_error()
59+
));
60+
}
61+
}
62+
63+
Ok(())
64+
}
65+
3766
/// Handle to a running process.
3867
pub struct ProcessHandle {
3968
child: Child,
@@ -200,6 +229,8 @@ impl ProcessHandle {
200229
drop_privileges(&policy)
201230
.map_err(|err| std::io::Error::other(err.to_string()))?;
202231

232+
harden_child_process().map_err(|err| std::io::Error::other(err.to_string()))?;
233+
203234
// Phase 2 (as unprivileged user): Enforce the prepared
204235
// Landlock ruleset via restrict_self() + apply seccomp.
205236
// restrict_self() does not require root.
@@ -292,6 +323,8 @@ impl ProcessHandle {
292323
drop_privileges(&policy)
293324
.map_err(|err| std::io::Error::other(err.to_string()))?;
294325

326+
harden_child_process().map_err(|err| std::io::Error::other(err.to_string()))?;
327+
295328
sandbox::apply(&policy, workdir.as_deref())
296329
.map_err(|err| std::io::Error::other(err.to_string()))?;
297330

@@ -543,6 +576,12 @@ mod tests {
543576
use crate::policy::{
544577
FilesystemPolicy, LandlockPolicy, NetworkPolicy, ProcessPolicy, SandboxPolicy,
545578
};
579+
#[cfg(unix)]
580+
use nix::sys::wait::{WaitStatus, waitpid};
581+
#[cfg(unix)]
582+
use nix::unistd::{ForkResult, fork};
583+
#[cfg(unix)]
584+
use std::mem::size_of;
546585
use std::process::Stdio as StdStdio;
547586

548587
/// Helper to create a minimal `SandboxPolicy` with the given process policy.
@@ -660,6 +699,91 @@ mod tests {
660699
);
661700
}
662701

702+
#[cfg(unix)]
703+
#[allow(unsafe_code)]
704+
fn probe_hardened_child(probe: unsafe fn() -> i64) -> i64 {
705+
const HARDEN_FAILED: i64 = -2;
706+
707+
let mut fds = [0; 2];
708+
let pipe_rc = unsafe { libc::pipe(fds.as_mut_ptr()) };
709+
assert_eq!(
710+
pipe_rc,
711+
0,
712+
"pipe failed: {}",
713+
std::io::Error::last_os_error()
714+
);
715+
716+
match unsafe { fork() }.expect("fork should succeed") {
717+
ForkResult::Child => {
718+
unsafe { libc::close(fds[0]) };
719+
let value = match harden_child_process() {
720+
Ok(()) => unsafe { probe() },
721+
Err(_) => HARDEN_FAILED,
722+
};
723+
let bytes = value.to_ne_bytes();
724+
let written = unsafe { libc::write(fds[1], bytes.as_ptr().cast(), bytes.len()) };
725+
unsafe {
726+
libc::close(fds[1]);
727+
libc::_exit(if written == bytes.len() as isize {
728+
0
729+
} else {
730+
1
731+
});
732+
}
733+
}
734+
ForkResult::Parent { child } => {
735+
unsafe { libc::close(fds[1]) };
736+
let mut bytes = [0u8; size_of::<i64>()];
737+
let read = unsafe { libc::read(fds[0], bytes.as_mut_ptr().cast(), bytes.len()) };
738+
unsafe { libc::close(fds[0]) };
739+
assert_eq!(
740+
read as usize,
741+
bytes.len(),
742+
"expected {} probe bytes, got {}",
743+
bytes.len(),
744+
read
745+
);
746+
747+
match waitpid(child, None).expect("waitpid should succeed") {
748+
WaitStatus::Exited(_, 0) => {}
749+
status => panic!("probe child exited unexpectedly: {status:?}"),
750+
}
751+
752+
i64::from_ne_bytes(bytes)
753+
}
754+
}
755+
}
756+
757+
#[cfg(unix)]
758+
#[allow(unsafe_code)]
759+
unsafe fn core_dump_limit_is_zero_probe() -> i64 {
760+
let mut limit = std::mem::MaybeUninit::<libc::rlimit>::uninit();
761+
let rc = unsafe { libc::getrlimit(libc::RLIMIT_CORE, limit.as_mut_ptr()) };
762+
if rc != 0 {
763+
return -1;
764+
}
765+
let limit = unsafe { limit.assume_init() };
766+
i64::from(limit.rlim_cur == 0 && limit.rlim_max == 0)
767+
}
768+
769+
#[test]
770+
#[cfg(unix)]
771+
fn harden_child_process_disables_core_dumps() {
772+
assert_eq!(probe_hardened_child(core_dump_limit_is_zero_probe), 1);
773+
}
774+
775+
#[cfg(target_os = "linux")]
776+
#[allow(unsafe_code)]
777+
unsafe fn dumpable_flag_probe() -> i64 {
778+
unsafe { libc::prctl(libc::PR_GET_DUMPABLE, 0, 0, 0, 0) as i64 }
779+
}
780+
781+
#[test]
782+
#[cfg(target_os = "linux")]
783+
fn harden_child_process_marks_process_nondumpable() {
784+
assert_eq!(probe_hardened_child(dumpable_flag_probe), 0);
785+
}
786+
663787
#[tokio::test]
664788
async fn scrub_sensitive_env_removes_ssh_handshake_secret() {
665789
let mut cmd = Command::new("/usr/bin/env");

crates/openshell-sandbox/src/ssh.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,8 @@ mod unsafe_pty {
12591259
// Drop privileges. initgroups/setgid/setuid need /etc/group and
12601260
// /etc/passwd which would be blocked if Landlock were already enforced.
12611261
drop_privileges(policy).map_err(|err| std::io::Error::other(err.to_string()))?;
1262+
crate::process::harden_child_process()
1263+
.map_err(|err| std::io::Error::other(err.to_string()))?;
12621264

12631265
// Phase 2: Enforce the prepared Landlock ruleset + seccomp.
12641266
// restrict_self() does not require root.

docs/security/best-practices.mdx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ The sandbox process runs as a non-root user after explicit privilege dropping.
185185

186186
| Aspect | Detail |
187187
|---|---|
188-
| Default | `run_as_user: sandbox`, `run_as_group: sandbox`. The supervisor calls `setuid()`/`setgid()` with post-condition verification: confirms the effective UID/GID match the target and that `setuid(0)` fails (root cannot be re-acquired). |
188+
| Default | `run_as_user: sandbox`, `run_as_group: sandbox`. The supervisor calls `setuid()`/`setgid()` with post-condition verification, disables core dumps with `RLIMIT_CORE=0`, and on Linux sets `PR_SET_DUMPABLE=0`. |
189189
| What you can change | Set `run_as_user` and `run_as_group` in the `process` section. Validation rejects root (`root` or `0`). |
190190
| Risk if relaxed | Running as a higher-privilege user increases the impact of container escape vulnerabilities. |
191191
| Recommendation | Keep the `sandbox` user. Do not attempt to set root. |
@@ -208,8 +208,9 @@ This ordering is intentional: privilege dropping needs `/etc/group` and `/etc/pa
208208

209209
1. Network namespace entry (`setns`).
210210
2. Privilege drop (`initgroups` + `setgid` + `setuid`).
211-
3. Landlock filesystem restrictions.
212-
4. Seccomp socket domain filters.
211+
3. Core-dump hardening (`RLIMIT_CORE=0`, plus `PR_SET_DUMPABLE=0` on Linux).
212+
4. Landlock filesystem restrictions.
213+
5. Seccomp socket domain filters.
213214

214215
## Inference Controls
215216

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
#![cfg(feature = "e2e")]
5+
6+
use openshell_e2e::harness::sandbox::SandboxGuard;
7+
8+
#[tokio::test]
9+
async fn sandbox_processes_disable_core_dumps() {
10+
let mut sb = SandboxGuard::create(&[
11+
"--",
12+
"sh",
13+
"-lc",
14+
"test \"$(ulimit -c)\" = 0 && echo core-limit-ok",
15+
])
16+
.await
17+
.expect("sandbox create should succeed");
18+
19+
assert!(
20+
sb.create_output.contains("core-limit-ok"),
21+
"expected sandbox output to confirm core dumps are disabled:\n{}",
22+
sb.create_output,
23+
);
24+
25+
sb.cleanup().await;
26+
}

0 commit comments

Comments
 (0)