fix(privops): fail closed when getpeereid fails on the libnv socket (1.1.19, security)#221
Merged
Merged
Conversation
…1.1.19, security)
daemon/privops_listener.cpp left peerUid = 0 on a getpeereid failure
and proceeded to dispatch. With rootless_per_user enabled the
authorize-before-dispatch gate is `if (rootlessPerUser && operatorUid
> 0)`, so peerUid = 0 was treated as the admin/host-wide path and EVERY
per-tenant gate was skipped — a fail-open on identity loss (e.g. the
peer exiting between accept and getpeereid, or a kernel edge). The old
comment even asserted "getpeereid only fails on non-AF_UNIX sockets
which we don't accept", which is not guaranteed.
The listener now rejects the connection with 403 when getpeereid fails
AND rootless_per_user is on, instead of running the verb
unauthenticated. When per-user enforcement is off there is nothing to
gate (peerUid only fed the audit trail), so a failure there stays a
harmless no-op and the connection proceeds unchanged. A root operator
whose getpeereid SUCCEEDS with uid 0 is unaffected — that remains the
intended admin treatment; only a genuine getpeereid FAILURE is
rejected. errno is captured before applyConnectionRights (Capsicum)
can clobber it, for the diagnostic log.
Closes the "fail closed on identity loss" guardrail in
docs/trust-model.md (item 3). Note: this is daemon-side (crated),
FreeBSD-only, and not exercised by the automated lite CI (which links
only crate(1)); verified by careful review + the manual full FreeBSD
build. docs/trust-model.{md,uk.md} mark the guardrail done;
CHANGELOG [1.1.19] + --version bump.
https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
daemon/privops_listener.cppleftpeerUid = 0on agetpeereidfailure and proceeded to dispatch. Withrootless_per_userenabled the authorize-before-dispatch gate isif (rootlessPerUser && operatorUid > 0), sopeerUid = 0was treated as the admin/host-wide path and every per-tenant gate was skipped — a fail-open on identity loss (peer exiting betweenacceptandgetpeereid, or a kernel edge). The old comment even asserted "getpeereid only fails on non-AF_UNIX sockets which we don't accept" — not guaranteed.Fix
The listener now rejects with
403whengetpeereidfails androotless_per_useris on, instead of running the verb unauthenticated.rootless_per_useroff → nothing to gate (peerUidonly fed the audit trail) → harmless no-op, connection proceeds unchanged.getpeereidsucceeds with uid 0 → unaffected; that's still the intended admin treatment. Only a genuinegetpeereidfailure is rejected.errnocaptured beforeapplyConnectionRights(Capsicum) can clobber it, for the diagnostic log.Closes the "fail closed on identity loss" guardrail (item 3) in
docs/trust-model.md.Verification
crated), FreeBSD-only, and not exercised by automated CI — the lite workflow links onlycrate(1), notcrated, and the full FreeBSD workflow is manual (workflow_dispatch). The change is small and mechanical (add a bool, capture errno, add an early-return guard); includes verified (<cerrno>/<cstring>present),writeErrorResponseusesnvlist_sendwhich is permitted after the Capsicum rights-limit, and the logic was reviewed against the dispatch gate.writeErrorResponsesignature verified.gmake cratedcompile — needs the manual full FreeBSD build (structural CI gap: crated isn't in lite).Docs
docs/trust-model.{md,uk.md}: guardrail item 3 marked Done (1.1.19), Applies-to → 1.1.19.CHANGELOG [1.1.19]+--versionbump.Note on the structural CI gap
That a daemon-only change like this has no automated compile coverage is exactly the
crated/crate-snmpd-not-linked-in-CI gap flagged in the earlier project bug analysis. Worth a follow-up (stub-link the daemon binaries in lite) so daemon changes get at least a compile gate.https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
Generated by Claude Code