diff --git a/CHANGELOG.md b/CHANGELOG.md index 4183285a..68c397f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,11 @@ Releases before `0.2.5` predate the public launch; their notes live in the ### 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). + - **A release pin that lost its `version` field passed the version check** ([#378](https://github.com/lacs-project/sysknife/pull/378), closes [#368](https://github.com/lacs-project/sysknife/issues/368)). `check_release_versions.sh` piped its list of internal path dependencies @@ -42,7 +47,6 @@ Releases before `0.2.5` predate the public launch; their notes live in the - **The daemon's `sysknife-apt-pin-edit` lock arm was unreachable** ([#375](https://github.com/lacs-project/sysknife/pull/375), closes [#248](https://github.com/lacs-project/sysknife/issues/248)). - ## [0.13.1] โ€” 2026-09-05 The last digit moves. No shipped code changed: `crates/**`, `apps/*/src/**` and 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/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 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 }