Skip to content

Commit c6972af

Browse files
ZhiXiao-LinRoy Lin
andauthored
fix(guest): give TTY containers the exec path's cgroup + path-restriction setup (#11 follow-up) (#117)
* fix(guest): give TTY containers a per-container cgroup (#11 follow-up) The PTY path (CRI tty:true workloads) never created a ContainerCgroup, so tty containers escaped ALL resource limits — cpu.max, pids.max, memory.* were unenforced even though the pod requested them. (The seccomp/caps/no_new_privs gap was closed separately.) Create the per-container cgroup from the same A3S_SEC_* limit vars the exec path consumes (memory.max/low/swap, cpu.max/weight, pids.max) in the parent before the fork, and have the child join it (write its PID to cgroup.procs) FIRST — before chroot makes /sys/fs/cgroup unreachable — with the same async-signal-safe open+getpid+itoa+write used by the exec path. The handle lives for the connection so the cgroup dir is reclaimed once the child exits. Completes #11: tty containers now get the same cgroup + security confinement as the exec path. * fix(guest): apply MaskedPaths/ReadonlyPaths/readonly-rootfs on the PTY path (#11) The PTY path also skipped the container rootfs path-restrictions the exec path applies — a tty container's securityContext MaskedPaths (e.g. /proc/kcore), ReadonlyPaths and readOnlyRootFilesystem went unenforced. Apply them in the PTY parent before the fork (the child shares the mount namespace and chroots in), reusing the exec path's critest-validated apply_container_path_restrictions / remount_rootfs_readonly (now pub(crate)) so the behavior is identical. Best- effort, matching exec: a masked path that doesn't exist is skipped. Together with the cgroup commit this closes the remaining #11 surface: tty containers now get the same security confinement, cgroup limits, and rootfs path restrictions as the exec path. (parse/apply reuse the exec functions, so they inherit the exec path's conformance coverage; a CRI-tty-with-rootfs harness to measure masking directly is a follow-up — exec -t carries no rootfs.) * fix(guest): set up /proc + /dev on the PTY rootfs path too (#11) The PTY path with a rootfs (a CRI tty container main carries the container rootfs) never mounted the container pseudo-filesystems (/proc, /sys) or the standard /dev nodes — only the exec path did, per spawn. A tty container was therefore missing /proc and /dev entirely (many workloads read /proc/self/* or /dev/urandom and won't start), AND the masked-paths added in the previous commit were no-ops because /proc wasn't mounted to mask under. Mirror the exec path's full rootfs setup in the PTY parent before the fork (ensure_container_pseudo_filesystems + ensure_container_dev_nodes, now pub(crate)), in the exec path's order: pseudo-fs -> dev -> masked/readonly -> readonly-rootfs. All reuse the exec path's critest-validated functions, so the behavior is identical; idempotent and best-effort. With this the PTY path performs the SAME container setup as the exec path — closing the functionality gap and making the masked-path restrictions effective. --------- Co-authored-by: Roy Lin <roylin@a3s.box>
1 parent 9887858 commit c6972af

2 files changed

Lines changed: 102 additions & 5 deletions

File tree

src/guest/init/src/exec_server.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,7 +1518,7 @@ fn configure_child_process(
15181518
/// existing mount (detected by a differing `st_dev`) is left untouched, and any
15191519
/// failure is logged without aborting the exec.
15201520
#[cfg(target_os = "linux")]
1521-
fn ensure_container_pseudo_filesystems(rootfs: &str) {
1521+
pub(crate) fn ensure_container_pseudo_filesystems(rootfs: &str) {
15221522
use nix::mount::{mount, MsFlags};
15231523
use std::os::unix::fs::MetadataExt;
15241524

@@ -1559,7 +1559,7 @@ fn ensure_container_pseudo_filesystems(rootfs: &str) {
15591559
/// still holds `CAP_MKNOD` (before the privilege drop and chroot). Best-effort
15601560
/// and idempotent: an existing node is left as-is.
15611561
#[cfg(target_os = "linux")]
1562-
fn ensure_container_dev_nodes(rootfs: &str) {
1562+
pub(crate) fn ensure_container_dev_nodes(rootfs: &str) {
15631563
let dev = format!("{rootfs}/dev");
15641564
if let Err(e) = std::fs::create_dir_all(&dev) {
15651565
warn!("Failed to create container /dev {dev}: {e}");
@@ -1635,7 +1635,7 @@ fn parse_sec_int(env: &[String], prefix: &str) -> Option<i64> {
16351635

16361636
/// Parse a `':'`-separated absolute-path list from an `A3S_SEC_*` env entry.
16371637
#[cfg(target_os = "linux")]
1638-
fn parse_sec_path_list<'a>(env: &'a [String], prefix: &str) -> Vec<&'a str> {
1638+
pub(crate) fn parse_sec_path_list<'a>(env: &'a [String], prefix: &str) -> Vec<&'a str> {
16391639
env.iter()
16401640
.find_map(|entry| entry.strip_prefix(prefix))
16411641
.map(|value| value.split(':').filter(|path| !path.is_empty()).collect())
@@ -1654,7 +1654,7 @@ fn parse_sec_path_list<'a>(env: &'a [String], prefix: &str) -> Vec<&'a str> {
16541654
///
16551655
/// Best-effort: an absent path is skipped and any mount failure is logged.
16561656
#[cfg(target_os = "linux")]
1657-
fn apply_container_path_restrictions(rootfs: &str, masked: &[&str], readonly: &[&str]) {
1657+
pub(crate) fn apply_container_path_restrictions(rootfs: &str, masked: &[&str], readonly: &[&str]) {
16581658
use nix::mount::{mount, MsFlags};
16591659

16601660
use std::os::unix::fs::MetadataExt;
@@ -1790,7 +1790,7 @@ fn apply_container_path_restrictions(rootfs: &str, masked: &[&str], readonly: &[
17901790
/// rootfs that is already read-only is left untouched, so re-exec into the
17911791
/// container does not stack mounts.
17921792
#[cfg(target_os = "linux")]
1793-
fn remount_rootfs_readonly(rootfs: &str) {
1793+
pub(crate) fn remount_rootfs_readonly(rootfs: &str) {
17941794
use nix::mount::{mount, MsFlags};
17951795

17961796
if nix::sys::statvfs::statvfs(rootfs)

src/guest/init/src/pty_server.rs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,72 @@ fn handle_pty_connection(fd: std::os::fd::OwnedFd) -> Result<(), Box<dyn std::er
233233
#[cfg(not(target_os = "linux"))]
234234
let _ = (&sec_cap_drop, &sec_cap_keep, sec_no_new_privs);
235235

236+
// Create the per-container cgroup from the A3S_SEC_* limits so the TTY
237+
// workload is bounded by cpu.max / pids.max / memory.* like the exec path —
238+
// the PTY path previously created NO cgroup, so tty containers escaped every
239+
// resource limit. Created here in the parent (allocates); the child joins it
240+
// (writes its PID to cgroup.procs) before chroot. The handle lives for the
241+
// connection so the cgroup dir is removed once the child exits.
242+
#[cfg(target_os = "linux")]
243+
let parse_u64 = |prefix: &str| {
244+
request
245+
.env
246+
.iter()
247+
.find_map(|entry| entry.strip_prefix(prefix))
248+
.and_then(|value| value.trim().parse::<u64>().ok())
249+
};
250+
#[cfg(target_os = "linux")]
251+
let parse_i64 = |prefix: &str| {
252+
request
253+
.env
254+
.iter()
255+
.find_map(|entry| entry.strip_prefix(prefix))
256+
.and_then(|value| value.trim().parse::<i64>().ok())
257+
};
258+
#[cfg(target_os = "linux")]
259+
let _container_cgroup = crate::cgroup::ContainerCgroup::create(
260+
parse_u64("A3S_SEC_MEM_LIMIT="),
261+
parse_u64("A3S_SEC_MEM_LOW="),
262+
parse_i64("A3S_SEC_MEM_SWAP="),
263+
parse_i64("A3S_SEC_CPU_QUOTA="),
264+
parse_u64("A3S_SEC_CPU_PERIOD="),
265+
parse_u64("A3S_SEC_CPU_SHARES="),
266+
parse_u64("A3S_SEC_PIDS_LIMIT="),
267+
);
268+
#[cfg(target_os = "linux")]
269+
let cgroup_procs: Option<std::ffi::CString> = _container_cgroup
270+
.as_ref()
271+
.and_then(|cgroup| std::ffi::CString::new(cgroup.procs_path()).ok());
272+
273+
// Set up the container rootfs before the fork — the child shares this mount
274+
// namespace and chroots into it. The exec path does all of this per spawn;
275+
// the PTY path did NONE of it, so a tty container was missing /proc + /dev and
276+
// its securityContext path restrictions went unenforced. Mirror the exec path
277+
// exactly (same critest-validated functions), in the same order.
278+
#[cfg(target_os = "linux")]
279+
if let Some(ref rootfs) = request.rootfs {
280+
// /proc + /sys, then the standard /dev nodes — many workloads read
281+
// /proc/self/* or /dev/urandom and won't start without them. Idempotent.
282+
crate::exec_server::ensure_container_pseudo_filesystems(rootfs);
283+
crate::exec_server::ensure_container_dev_nodes(rootfs);
284+
// CRI MaskedPaths / ReadonlyPaths (best-effort; a path that doesn't exist
285+
// is skipped) — these need /proc + /sys mounted above.
286+
let masked = crate::exec_server::parse_sec_path_list(&request.env, "A3S_SEC_MASKED_PATHS=");
287+
let readonly =
288+
crate::exec_server::parse_sec_path_list(&request.env, "A3S_SEC_READONLY_PATHS=");
289+
if !masked.is_empty() || !readonly.is_empty() {
290+
crate::exec_server::apply_container_path_restrictions(rootfs, &masked, &readonly);
291+
}
292+
// readOnlyRootFilesystem — last, after /proc, /sys and inner mounts are up.
293+
if request
294+
.env
295+
.iter()
296+
.any(|entry| entry == "A3S_SEC_READONLY_ROOTFS=1")
297+
{
298+
crate::exec_server::remount_rootfs_readonly(rootfs);
299+
}
300+
}
301+
236302
// Step 2: Allocate PTY
237303
let pty = openpty(None, None)?;
238304
let master_fd = pty.master;
@@ -247,6 +313,37 @@ fn handle_pty_connection(fd: std::os::fd::OwnedFd) -> Result<(), Box<dyn std::er
247313
// Child: set up PTY slave as stdin/stdout/stderr, then exec
248314
drop(master_fd);
249315

316+
// Join the per-container cgroup FIRST, before chroot makes
317+
// /sys/fs/cgroup unreachable, so this process and everything it forks
318+
// is bounded from birth. Async-signal-safe: open + getpid + a
319+
// stack-only itoa + write + close, no allocation.
320+
#[cfg(target_os = "linux")]
321+
if let Some(ref procs) = cgroup_procs {
322+
let fd = unsafe { libc::open(procs.as_ptr(), libc::O_WRONLY) };
323+
if fd >= 0 {
324+
let mut buf = [0u8; 20];
325+
let mut i = buf.len();
326+
let mut n = unsafe { libc::getpid() } as u64;
327+
if n == 0 {
328+
i -= 1;
329+
buf[i] = b'0';
330+
}
331+
while n > 0 {
332+
i -= 1;
333+
buf[i] = b'0' + (n % 10) as u8;
334+
n /= 10;
335+
}
336+
unsafe {
337+
libc::write(
338+
fd,
339+
buf[i..].as_ptr() as *const libc::c_void,
340+
(buf.len() - i) as libc::size_t,
341+
);
342+
libc::close(fd);
343+
}
344+
}
345+
}
346+
250347
// Create new session (detach from controlling terminal)
251348
setsid().ok();
252349

0 commit comments

Comments
 (0)