Skip to content

Commit ad26f69

Browse files
ZhiXiao-LinRoy Lin
andauthored
fix(security): confine CRI localhostProfile seccomp path to the seccomp root (#143)
LOW. `parse_localhost_seccomp_deny` did `std::fs::read_to_string(localhost_ref)` with no path confinement. `localhost_ref` is the pod's `securityContext.seccompProfile.localhostProfile` — attacker-settable through the CRI by anyone who can create pods — so it was an arbitrary host-file OPEN primitive (a path-traversal / file-existence oracle). Information disclosure is limited (the parse error is only warn-logged, never returned to the gRPC caller, and content only escapes as syscall names from valid OCI-seccomp JSON into the attacker's own container), hence LOW — but the missing guard is real. Fix: `confined_seccomp_path(localhost_ref, root)` rejects any `..` component and requires an absolute ref to lie within the seccomp root; a relative ref resolves under it. The root is injected (default /var/lib/kubelet/seccomp via `seccomp_profile_root()`, override A3S_BOX_SECCOMP_PROFILE_ROOT) so the check is testable without process-global env. An out-of-root/traversing ref is rejected and the caller falls back to RuntimeDefault (never unconfined). Tests: new confines_localhost_profile_to_root_and_rejects_traversal (rejects `../../etc/passwd`, `/etc/passwd`, prefix-confusion, etc.); the two existing parse_localhost_seccomp_deny tests now pass their tempdir as the root. Full a3s-box-cri lib suite (250) green; neuter-verified (disabling the root check makes the new test FAIL on /etc/passwd); fmt + clippy clean. Completes the untrusted-input security audit (4/4): after #141 (CRITICAL digest traversal), #142 (HIGH/MED bombs), #140 (HIGH whiteout deletion). Co-authored-by: Roy Lin <roylin@a3s.box>
1 parent 0e4ed61 commit ad26f69

2 files changed

Lines changed: 87 additions & 6 deletions

File tree

src/cri/src/runtime_service/mod.rs

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,48 @@ struct OciSeccompSyscall {
210210
action: String,
211211
}
212212

213+
/// The node's seccomp profile root — kubelet default, override with
214+
/// `A3S_BOX_SECCOMP_PROFILE_ROOT`.
215+
fn seccomp_profile_root() -> PathBuf {
216+
std::env::var("A3S_BOX_SECCOMP_PROFILE_ROOT")
217+
.map(PathBuf::from)
218+
.unwrap_or_else(|_| PathBuf::from("/var/lib/kubelet/seccomp"))
219+
}
220+
221+
/// Confine a CRI `localhostProfile` path to the seccomp profile `root` and reject
222+
/// `..` traversal.
223+
///
224+
/// `localhost_ref` comes from a pod's
225+
/// `securityContext.seccompProfile.localhostProfile` (attacker-settable through
226+
/// the CRI) and is read off the host disk — without confinement it is an
227+
/// arbitrary host-file open primitive. Mirrors kubelet/containerd semantics:
228+
/// profiles live under the configured seccomp root. A relative ref resolves under
229+
/// the root; an absolute ref must lie within it. `root` is injected so the check
230+
/// is testable without process-global env state.
231+
fn confined_seccomp_path(localhost_ref: &str, root: &std::path::Path) -> Result<PathBuf, String> {
232+
let r = std::path::Path::new(localhost_ref);
233+
if r.components()
234+
.any(|c| matches!(c, std::path::Component::ParentDir))
235+
{
236+
return Err(format!(
237+
"seccomp profile path contains '..': {localhost_ref}"
238+
));
239+
}
240+
let candidate = if r.is_absolute() {
241+
r.to_path_buf()
242+
} else {
243+
root.join(r)
244+
};
245+
if !candidate.starts_with(root) {
246+
return Err(format!(
247+
"seccomp profile {} is outside the seccomp root {}",
248+
candidate.display(),
249+
root.display()
250+
));
251+
}
252+
Ok(candidate)
253+
}
254+
213255
/// Parse a CRI localhost seccomp profile file and return the syscall names it
214256
/// blocks with `SCMP_ACT_ERRNO`/`SCMP_ACT_KILL*`.
215257
///
@@ -218,9 +260,13 @@ struct OciSeccompSyscall {
218260
/// profile (which would need a full allow-list compiled to BPF) is not yet
219261
/// supported; returns an error so the caller can fall back rather than silently
220262
/// run unconfined.
221-
fn parse_localhost_seccomp_deny(localhost_ref: &str) -> Result<Vec<String>, String> {
222-
let raw = std::fs::read_to_string(localhost_ref)
223-
.map_err(|e| format!("read seccomp profile {localhost_ref}: {e}"))?;
263+
fn parse_localhost_seccomp_deny(
264+
localhost_ref: &str,
265+
root: &std::path::Path,
266+
) -> Result<Vec<String>, String> {
267+
let path = confined_seccomp_path(localhost_ref, root)?;
268+
let raw = std::fs::read_to_string(&path)
269+
.map_err(|e| format!("read seccomp profile {}: {e}", path.display()))?;
224270
let profile: OciSeccompProfile =
225271
serde_json::from_str(&raw).map_err(|e| format!("parse seccomp profile: {e}"))?;
226272
if !matches!(
@@ -249,6 +295,38 @@ fn parse_localhost_seccomp_deny(localhost_ref: &str) -> Result<Vec<String>, Stri
249295
Ok(deny)
250296
}
251297

298+
#[cfg(test)]
299+
mod seccomp_confine_tests {
300+
use super::confined_seccomp_path;
301+
use std::path::Path;
302+
303+
#[test]
304+
fn confines_localhost_profile_to_root_and_rejects_traversal() {
305+
let root = Path::new("/var/lib/kubelet/seccomp");
306+
// In-root relative/absolute refs are allowed (the check is lexical, so
307+
// the files need not exist).
308+
assert!(confined_seccomp_path("audit.json", root).is_ok());
309+
assert!(confined_seccomp_path("profiles/audit.json", root).is_ok());
310+
assert!(confined_seccomp_path("/var/lib/kubelet/seccomp/audit.json", root).is_ok());
311+
312+
// SECURITY: a malicious localhostProfile must not open arbitrary host
313+
// files — traversal, out-of-root absolute paths, and prefix-confusion
314+
// are all rejected (caller then falls back to RuntimeDefault).
315+
for evil in [
316+
"../../../../etc/passwd",
317+
"sub/../../escape",
318+
"/etc/passwd",
319+
"/etc/shadow",
320+
"/var/lib/kubelet/seccomp-evil/x",
321+
] {
322+
assert!(
323+
confined_seccomp_path(evil, root).is_err(),
324+
"must reject malicious seccomp path: {evil}"
325+
);
326+
}
327+
}
328+
}
329+
252330
#[derive(Debug, Clone)]
253331
pub struct CriRuntimeOptions {
254332
pub default_agent_image: String,
@@ -1151,7 +1229,10 @@ impl RuntimeService for BoxRuntimeService {
11511229
// Read + parse the localhost profile (allow-default +
11521230
// ERRNO deny list) and pass the blocked syscalls to the
11531231
// guest, which compiles them into a BPF filter.
1154-
match parse_localhost_seccomp_deny(&profile.localhost_ref) {
1232+
match parse_localhost_seccomp_deny(
1233+
&profile.localhost_ref,
1234+
&seccomp_profile_root(),
1235+
) {
11551236
Ok(deny) => {
11561237
env.push(("A3S_SEC_SECCOMP_LOCALHOST".to_string(), deny.join(",")));
11571238
}

src/cri/src/runtime_service/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,7 +1955,7 @@ fn test_parse_localhost_seccomp_deny_allow_default() {
19551955
r#"{"defaultAction":"SCMP_ACT_ALLOW","syscalls":[{"names":["chmod","fchmodat"],"action":"SCMP_ACT_ERRNO"}]}"#,
19561956
)
19571957
.unwrap();
1958-
let deny = super::parse_localhost_seccomp_deny(path.to_str().unwrap()).unwrap();
1958+
let deny = super::parse_localhost_seccomp_deny(path.to_str().unwrap(), dir.path()).unwrap();
19591959
assert_eq!(deny, vec!["chmod".to_string(), "fchmodat".to_string()]);
19601960
}
19611961

@@ -1966,7 +1966,7 @@ fn test_parse_localhost_seccomp_deny_rejects_deny_by_default() {
19661966
std::fs::write(&path, r#"{"defaultAction":"SCMP_ACT_ERRNO","syscalls":[]}"#).unwrap();
19671967
// Deny-by-default profiles need a full allow-list; not supported -> Err so
19681968
// the caller falls back to RuntimeDefault rather than running unconfined.
1969-
assert!(super::parse_localhost_seccomp_deny(path.to_str().unwrap()).is_err());
1969+
assert!(super::parse_localhost_seccomp_deny(path.to_str().unwrap(), dir.path()).is_err());
19701970
}
19711971

19721972
#[tokio::test]

0 commit comments

Comments
 (0)