From 9eecc1c140040f8525a0b53d4b46a4285a62a7c9 Mon Sep 17 00:00:00 2001 From: hushen <190065939+918154429@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:56:33 +0800 Subject: [PATCH 1/2] fix(daemon): fail closed when a peer cannot be pinned --- CHANGELOG.md | 7 + crates/sysknife-daemon/src/dispatcher.rs | 161 ++++++++++++++++++----- 2 files changed, 133 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a8b8a47..dcc3cf28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ Releases before `0.2.5` predate the public launch; their notes live in the ## [Unreleased] +### Fixed + +- Keep supplementary groups out of caller authorization when `SO_PEERPIDFD` + cannot pin the peer, including an already-reaped peer or fd exhaustion. + Only `ENOPROTOOPT` retains the older-kernel best-effort path; every other + failure keeps only the primary GID captured by `SO_PEERCRED` (#250). + ## [0.13.1] — 2026-09-05 The last digit moves. No shipped code changed: `crates/**`, `apps/*/src/**` and diff --git a/crates/sysknife-daemon/src/dispatcher.rs b/crates/sysknife-daemon/src/dispatcher.rs index 1f52b002..ad16b17c 100644 --- a/crates/sysknife-daemon/src/dispatcher.rs +++ b/crates/sysknife-daemon/src/dispatcher.rs @@ -431,15 +431,33 @@ struct JobResult { #[cfg(target_os = "linux")] const SO_PEERPIDFD: libc::c_int = 77; +#[cfg(target_os = "linux")] +enum PeerPin { + Pinned(OwnedFd), + Unsupported, + Unpinnable, +} + +#[cfg(target_os = "linux")] +impl PeerPin { + fn from_error(error: &std::io::Error) -> Self { + if error.raw_os_error() == Some(libc::ENOPROTOOPT) { + Self::Unsupported + } else { + Self::Unpinnable + } + } +} + /// Obtain a pidfd pinned to the process that opened this connection, via /// `SO_PEERPIDFD`. Unlike `pidfd_open(pid)`, this has no PID-based lookup and so /// no reuse race — the kernel pins the actual peer captured at `connect()`. /// -/// Returns `None` when the kernel does not support the option (e.g. Ubuntu -/// 22.04's 5.15 kernel), in which case the caller falls back to the best-effort -/// `/proc/{pid}` path. +/// Only `ENOPROTOOPT` means the kernel does not support the option (e.g. +/// Ubuntu 22.04's 5.15 kernel). Other failures, including `EINVAL` for a +/// reaped peer and fd exhaustion, must not enable the best-effort `/proc` path. #[cfg(target_os = "linux")] -fn peer_pidfd(stream: &UnixStream) -> Option { +fn peer_pidfd(stream: &UnixStream) -> PeerPin { let mut fd: libc::c_int = -1; let mut len = std::mem::size_of::() as libc::socklen_t; // SAFETY: getsockopt writes at most `len` bytes into `fd` (a c_int) and @@ -453,12 +471,48 @@ fn peer_pidfd(stream: &UnixStream) -> Option { &mut len, ) }; - if rc != 0 || fd < 0 { - return None; + if rc != 0 { + return PeerPin::from_error(&std::io::Error::last_os_error()); + } + if fd < 0 { + return PeerPin::Unpinnable; } // SAFETY: getsockopt returned a fresh, owned fd; take sole ownership so it is // closed on drop. - Some(unsafe { OwnedFd::from_raw_fd(fd) }) + PeerPin::Pinned(unsafe { OwnedFd::from_raw_fd(fd) }) +} + +/// The pin is acquired before this function; the injected reader keeps the +/// ordering and the unpinnable-peer case testable without forcing PID reuse. +#[cfg(target_os = "linux")] +fn groups_for_pinned_peer( + pid: u32, + pin: PeerPin, + read_groups: impl FnOnce() -> Vec, +) -> Vec { + match pin { + PeerPin::Unsupported => read_groups(), + PeerPin::Unpinnable => { + eprintln!( + "[sysknife-daemon] WARNING: cannot pin peer PID {pid}; ignoring supplementary \ + groups and using the SO_PEERCRED primary GID only" + ); + Vec::new() + } + PeerPin::Pinned(fd) => { + let groups = read_groups(); + if pidfd_peer_still_live(&fd) { + groups + } else { + eprintln!( + "[sysknife-daemon] WARNING: peer PID {pid} was reaped while resolving its \ + groups (possible PID reuse); ignoring supplementary groups and using the \ + SO_PEERCRED primary GID only" + ); + Vec::new() + } + } + } } /// Returns `true` while the pinned peer process has not yet been reaped — i.e. @@ -507,10 +561,24 @@ fn pidfd_peer_still_live(pidfd: &OwnedFd) -> bool { /// Linux 6.5+ (Ubuntu 24.04 / 26.04) we pin the peer with a pidfd via /// `peer_pidfd` and, after reading `/proc`, confirm the pinned process was not /// reaped during the read; if it was, the supplementary set is untrustworthy and -/// is dropped, keeping only the race-free primary GID. On older kernels (e.g. -/// Ubuntu 22.04) the pidfd is unavailable and the read is best-effort, as before -/// — no worse than the previous behavior. +/// is dropped, keeping only the race-free primary GID. Only `ENOPROTOOPT` +/// disables the pin check and permits a best-effort read on older kernels +/// (e.g. Ubuntu 22.04). Every other pin failure, including `EINVAL` for an +/// already-reaped peer, skips the supplementary read and keeps the primary GID. pub fn resolve_caller(stream: &UnixStream) -> CallerAttribution { + // Pin before any /proc read. The kernel's peer identity does not depend on + // the PID lookup performed later, even if the peer has already exited. + resolve_caller_with_pin( + stream, + #[cfg(target_os = "linux")] + peer_pidfd(stream), + ) +} + +fn resolve_caller_with_pin( + stream: &UnixStream, + #[cfg(target_os = "linux")] pin: PeerPin, +) -> CallerAttribution { let (pid, primary_gid, uid) = match stream.peer_cred() { Ok(cred) => { let pid = match cred.pid() { @@ -538,30 +606,13 @@ pub fn resolve_caller(stream: &UnixStream) -> CallerAttribution { } }; - // Pin the connecting peer *before* the /proc read so PID reuse can be - // detected afterward. None on kernels without SO_PEERPIDFD (< 6.5). - #[cfg(target_os = "linux")] - let peer_fd = peer_pidfd(stream); - // Read /etc/group once and build a lookup map — avoids N+1 file reads when // a process has many supplementary groups (one read per GID in the old code). let gid_map = read_gid_map(); - let mut groups = groups_for_pid(pid, &gid_map); - - // If the pinned peer was reaped while we read /proc, its PID may have been - // recycled and the supplementary groups belong to a different process — drop - // them and fall back to the race-free primary GID only. #[cfg(target_os = "linux")] - if let Some(ref fd) = peer_fd { - if !pidfd_peer_still_live(fd) { - eprintln!( - "[sysknife-daemon] WARNING: peer PID {pid} was reaped while resolving its \ - groups (possible PID reuse); ignoring supplementary groups and using the \ - SO_PEERCRED primary GID only" - ); - groups.clear(); - } - } + let mut groups = groups_for_pinned_peer(pid, pin, || groups_for_pid(pid, &gid_map)); + #[cfg(not(target_os = "linux"))] + let mut groups = groups_for_pid(pid, &gid_map); // Include the primary GID from SO_PEERCRED. It is not listed in the // supplementary Groups: line so must be resolved and added explicitly. @@ -3298,14 +3349,54 @@ mod tests { let (a, _b) = UnixStream::pair().unwrap(); // On kernels with SO_PEERPIDFD the pinned peer is this very test process, // which is obviously still alive, so the liveness check must return true. - // On older kernels peer_pidfd returns None and there is nothing to assert - // (the fallback path is exercised by `resolve_caller` below). - if let Some(fd) = peer_pidfd(&a) { - assert!( + // Only an unsupported option may take the compatibility path. + match peer_pidfd(&a) { + PeerPin::Pinned(fd) => assert!( pidfd_peer_still_live(&fd), "the connecting (self) process must read as live" - ); + ), + PeerPin::Unsupported => {} + PeerPin::Unpinnable => panic!("a live peer must not silently lose its pin"), + } + } + + #[cfg(target_os = "linux")] + #[test] + fn peer_pin_only_unsupported_option_allows_best_effort_groups() { + assert!(matches!( + PeerPin::from_error(&std::io::Error::from_raw_os_error(libc::ENOPROTOOPT)), + PeerPin::Unsupported + )); + for errno in [ + libc::EINVAL, + libc::ESRCH, + libc::EMFILE, + libc::ENFILE, + libc::EPERM, + ] { + let pin = PeerPin::from_error(&std::io::Error::from_raw_os_error(errno)); + assert!(matches!(pin, PeerPin::Unpinnable), "errno {errno}"); + let groups = groups_for_pinned_peer(123, pin, || { + panic!("an unpinnable peer must not read a potentially recycled PID") + }); + assert_eq!(highest_role_from_groups(groups), CallerRole::Observer); } + let groups = groups_for_pinned_peer(123, PeerPin::Unsupported, || { + vec!["sysknife-admin".to_string()] + }); + assert_eq!(highest_role_from_groups(groups), CallerRole::Admin); + } + + #[cfg(target_os = "linux")] + #[tokio::test] + async fn unpinnable_peer_keeps_only_its_primary_group() { + let (a, _b) = UnixStream::pair().unwrap(); + let cred = a.peer_cred().unwrap(); + let gid_map = read_gid_map(); + let expected = highest_role_from_groups(gid_map.get(&cred.gid())); + let caller = resolve_caller_with_pin(&a, PeerPin::Unpinnable); + assert_eq!(caller.role(), expected); + assert_eq!(caller.principal(), CallerPrincipal::Uid(cred.uid())); } /// The uid must come from the kernel, not from anything the peer says. A From e4cf823a8b24a7fec0588082390c3e03a271a9f7 Mon Sep 17 00:00:00 2001 From: Vladimir Rotariu Date: Mon, 7 Sep 2026 14:26:59 -0600 Subject: [PATCH 2/2] test(baseline): move workspace count 1,843 -> 1,845 for the peer-pin tests #382 adds two dispatcher tests (errno classification, and an unpinnable peer keeping only its primary group). The published baseline and the three prose claims move with them. The CHANGELOG conflict came from me populating [Unreleased] after this PR was opened; resolved here, keeping both entries. --- README.md | 2 +- docs/distro-support.md | 2 +- docs/introduction.md | 2 +- tests/evidence/workspace-tests.json | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7023ee98..e6ab4db2 100644 --- a/README.md +++ b/README.md @@ -314,7 +314,7 @@ milestone. | **Every Ubuntu LTS validated** — 22.04, 24.04 and 26.04 all at 79/79, each with a replay twin that reproduces it | ✅ | | Telegram approval interface | 📋 roadmap | -**1,843 Rust tests and 72 frontend tests** form the current deterministic +**1,845 Rust tests and 72 frontend tests** form the current deterministic release baseline. ## Configure your LLM diff --git a/docs/distro-support.md b/docs/distro-support.md index e2db1db2..415d35bf 100644 --- a/docs/distro-support.md +++ b/docs/distro-support.md @@ -82,7 +82,7 @@ family and the atomic story family are implemented and covered by the workspace suite. What is missing is a way to put the helpers somewhere the daemon's own grants already point. -The deterministic workspace baseline is 1,843 Rust tests plus 72 frontend +The deterministic workspace baseline is 1,845 Rust tests plus 72 frontend tests. Those tests verify action construction, policy, approval, storage, and UI behavior, but they do not replace a real distribution VM run. diff --git a/docs/introduction.md b/docs/introduction.md index b7106963..f26a54d7 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -141,7 +141,7 @@ flow. ## Status -190 typed actions · 1,843 Rust tests + 72 frontend tests · MIT +190 typed actions · 1,845 Rust tests + 72 frontend tests · MIT SysKnife is the reference implementation of the [LACS specification](https://github.com/lacs-project/specification) — a diff --git a/tests/evidence/workspace-tests.json b/tests/evidence/workspace-tests.json index 710c27da..0a166507 100644 --- a/tests/evidence/workspace-tests.json +++ b/tests/evidence/workspace-tests.json @@ -5,13 +5,13 @@ }, "commit": { "frontend_tests": "656b399035ee5fc81bd949e871daee65b4a1f3c4", - "tests": "019bf4761addcdf265ba9e5df55920a0202f38bd" + "tests": "5bef5e6c48f73d23d10404b72901be9a920bc46f" }, "frontend_tests": 72, "measured_at": { "frontend_tests": "2026-08-05T09:23:15-06:00", - "tests": "2026-09-07T14:09:57-06:00" + "tests": "2026-09-07T14:24:31-06:00" }, - "tests": 1843, + "tests": 1845, "version": 1 }