Slice 2: native effect-session API + capability negotiation (pkgops) - #74
Merged
Conversation
…t refusal Groundwork for the native effect session. Three coupled changes to the broker C client, all behavior-preserving for the existing sink: - Link libjansson (anticonf configure + Makevars.in; SystemRequirements) for strict in-C extraction on the coming effect-receipt path. runix already Imports janssonr, which links the same system library on Unix, so no new system requirement is added here. - Extract rab_transport(): one framed request/response over the socket on a malloc buffer, never touching an R object. C_rab_broker_call keeps copying the response into a RAWSXP for non-secret payloads; the effect session will parse-and-wipe the same buffer in C. One transport, two consumers. - C_rab_broker_call refuses any request body carrying a top-level `effect` member (new status RAB_ST_EFFECT_REFUSED -> runix_effect_via_generic_path), so a receipt-minting open_intent is serviceable only via the effect session. 335 existing checks green; 5 new checks pin the refusal (fires before connect, top-level only, non-JSON passes through).
The security-critical core of the apt-effect issuer. A PID-bound EXTPTRSXP handle runs the open_intent(+effect) -> write_outcome state machine in C: - Custody: the single-use effect receipt and outcome binding are extracted from the broker response with linked Jansson, in C, into wipeable heap and never become R objects. explicit_bzero on consumption and in the finalizer. R sees only the handle, correlation id, and status. A test proves neither minted token appears in the serialized handle. - Handle refuses reuse (wrong state), a fork (owner-PID mismatch), and a restore-from-disk (serialized external pointer loses its address). The finalizer only wipes -- a dropped handle leaves the intent open (fail-closed). - The verb is a runix-owned closed enum mapped in C to the immutable pkexec entrypoint path; no path ever crosses from R. (The map's entrypoint column is used by the commit path, next slice.) - Extraction is strict, mirroring the R adapter: dup-key rejection, exact open_ok_effect key set, documented value formats, the two tokens required distinct. Split into effect_session.c behind a shared rab_internal.h; unix_socket.c's transport helpers are un-static'd to back it (behavior-preserving). Adds a serve_seq test broker (one bound path, many connections) so the session's open+write_outcome round-trip is exercised without a fresh forked server per call -- which the fork-based harness does not tolerate at scale. 376 checks green (36 new), no root or dpkg needed.
…ypoint
Completes the C session core. commit posix_spawns (no shell) pkexec + this
verb's immutable entrypoint, delivers the commit request on the child's stdin
and explicit_bzero's it the instant it is sent, and reads the strict result
frame from the child's stdout under a wall-clock deadline (SIGKILL on overrun).
- The commit request is exactly the entrypoint's accepted member set
{effect_receipt, correlation_id, plan_schema, packages[], lock_timeout};
built with Jansson in wipeable heap. The receipt is single-use and wiped
whether delivered or not.
- effect_issued is helper-authoritative: FALSE only when the effect provably
did not run (spawn failed, or pkexec denied with exit 126/127 before exec),
NA only when genuinely unknown (child ran, no valid result), else the helper's
own boolean. State -> result_known or effect_unknown accordingly.
- The verb -> entrypoint path map is C-owned and immutable; no path crosses
from R. The fake-entrypoint seam is compile-time only (-DRUNIX_TESTING via
configure), ABSENT from the production build -- no runtime env var can
redirect the production pkexec target.
Pipe writes use write() (rab_write_all is send()/socket-only); SIGPIPE is
ignored across the child interaction. Commit tests (a fake entrypoint script
driving all four outcome scenarios: ok / effect_unknown / unauthorized /
spawn_failed, plus receipt-delivered-then-wiped and the verb-map path) run
under the testing build (57 checks); production skips them and ships 376 green.
Address the eight findings from the C3 core review: 1. Peer uid is pinned to root (0) in the shipped build and is no longer an R argument; only a -DRUNIX_TESTING build reads RUNIX_TEST_PEER_UID. 2. The receipt is delivered only with SIGPIPE ignored; if that guard fails the write is refused and the effect left UNKNOWN, never signal-killing R. 3. Every posix_spawn_file_actions_* return is checked; a bad dup2/close aborts before spawn (no child -> effect provably not run). 4. The result parser rejects a detail > 128 bytes and an exit code that contradicts its status (exit 0 iff ok/no_op) as malformed -> UNKNOWN. 5. The entrypoint's bounded request grammar (count <= 256, Debian-name pattern, per-verb arity, duplicate refusal, 64 KiB cap) is enforced in C before the single-use receipt is spent. 6. Pipe/socket writers bail on zero forward progress; the delivery result is recorded, not discarded. 7. All inherited fds >= 3 are closed in the child (addclosefrom_np, glibc 2.34+), so nothing R holds open leaks to pkexec/the helper. 8. A timeout SIGKILLs pkexec but classifies the outcome UNKNOWN, documented: the privileged work runs in a separate polkit scope. Tests: production build pins uid 0 (a non-root fake broker is refused untrusted); the -DRUNIX_TESTING build exercises the full open/commit flow plus every new failure path (input bounds, detail overflow, exit/status mismatch, timeout). Production 352/352, testing 416/416.
… guards Address the three follow-up blockers from the second core review: 1. A commit result is trusted only when io.delivered is set. An undelivered receipt whose child still returns a valid, cid-matching result (a guessed or replayed id) is now classified effect_unknown, not "ok". Covered by a compile-time-only RUNIX_TEST_FORCE_UNDELIVERED seam. 2. FD hygiene no longer degrades to inheritance without addclosefrom_np: es_cloexec_from() marks every inherited fd >= 3 close-on-exec in the parent (via /proc/self/fd, else the RLIMIT_NOFILE range). A leak-sentinel test (R's own non-CLOEXEC connection fd) proves the child never inherits it, both via the closefrom primitive and via the forced sweep fallback; the fallback also compiles clean under -DRUNIX_NO_CLOSEFROM_NP. 3. Every native string input is rejected if its byte length differs from its C-string length (embedded NUL) before validation/serialization, in rab_arg_string and the commit package loop. Ordinary R construction refuses embedded NULs (rawToChar/parser/intToUtf8/mkCharLenCE all error or strip), so this guards a non-standard C caller; it mirrors the entrypoint's own str_clean check. Production 352/352, -DRUNIX_TESTING 423/423, all -Wall -Wextra clean.
Second re-review: the parent-side CLOEXEC sweep could not hold the "nothing leaks" invariant (a concurrent thread can open an fd between the sweep and posix_spawn, and it mutated the caller's own descriptors), and its fcntl failures were unchecked. Resolution (the reviewer's own option): use only the ATOMIC in-child addclosefrom_np, and refuse the commit fail-closed where it is unavailable. - Remove es_cloexec_from() and the /proc/RLIMIT sweep entirely; the commit path closes inherited fds only via posix_spawn_file_actions_addclosefrom_np (in-child, race-free), whose return is already checked. - effect_session_commit() refuses fail-closed (a clear error, before any state change or handle work) when the primitive is absent, so it never spawns with an unbounded fd set. es_run_commit keeps a belt-and-suspenders refuse for any direct caller. - New effect_session_commit_supported() (registered + .Call shim) reports whether commit is available, so a caller/the coming effect-capability gate discovers the refusal without minting a receipt. - Tests: assert commit_supported() TRUE as a precondition for the commit suite; a guarded no-primitive check asserts the refusal (runs in a -DRUNIX_NO_CLOSEFROM_NP build). Drop the now-gone sweep-fallback test; keep the leak-sentinel test on the addclosefrom path. Verified: production 352/352, -DRUNIX_TESTING 422/422, all four configs -Wall -Wextra clean, and a -DRUNIX_NO_CLOSEFROM_NP build reports commit_supported=FALSE and refuses commit with the platform error.
The exported slice-2 surface in runix, over the native C shims (C1-C3):
- effect_capability(socket_path, plan_schema): the real effect-receipt
negotiation on top of broker_available's peer auth. Sends the broker's
{"type":"capabilities"} query and confirms it advertises the effect-receipt
extension AND accepts the required plan schema before any intent is opened.
Fail-closed -> runix_capability_unavailable on absent extension / wrong
schema / unreachable, nothing minted. Pins the broker peer to root; an
internal .effect_capability() takes an expected uid so tests drive an
unprivileged fake broker (mirrors .broker_audit_sink).
- The three-call session API: effect_session_open() returns an opaque,
PID-bound handle object carrying only the external pointer and the
non-secret correlation id (no receipt/binding/path in R); on failure it
raises the typed broker taxonomy. effect_session_commit() returns the raw
commit result verbatim -- runix does NOT map the 12-status vocabulary, the
issuer does -- and refuses fail-closed (runix_capability_unavailable) on a
platform without the atomic fd-close primitive. effect_session_write_outcome()
validates the record shape and returns the raw status. print methods show
state/cid, never a secret.
- Document the effect-session condition taxonomy (runix_effect_conditions):
which subclasses runix raises (capability/broker) vs the issuer
(helper_bad_result, preview_failed, verification_failed, unauthorized,
approval_required), and the retryability rule. No class registry; the
subclass string is the taxonomy.
Production 383/383, -DRUNIX_TESTING 469/469.
…ation Two API-level blockers from the PR #74 review: 1. Numeric arguments were coerced with as.integer() before validation, so a fractional value silently truncated (plan_schema = 1.5 -> 1, wrong schema; same for session deadlines / lock_timeout / deadline_ms). New .scalar_int() validates a single, finite, non-NA, integer-VALUED number (optionally bounded) and errors rather than truncate; every numeric argument that reaches a .Call now goes through it (effect_capability + the session shims). 2. The effect-receipt extension version was accepted by .broker_is_count() (any positive integer), so a v1 client would accept effect_receipt: 2. Negotiation now requires the exact version this client implements (.RUNIX_EFFECT_RECEIPT_VERSION = 1); a different advertised version fails closed to runix_capability_unavailable, carrying the offered version. Tests: fractional plan_schema/connect_ms/lock_timeout/deadline_ms rejected (open + commit, before the receipt is spent); a broker advertising effect_receipt: 2 refused. Production 390/390, -DRUNIX_TESTING 479/479.
Contributor
Author
|
Both C4 blockers fixed in 3119a70. 1. Silent integer truncation. New
2. Extension version accepted loosely. Negotiation now requires the exact version this client implements — Tests added: fractional |
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
1 participant
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.
Slice 2 of the pkgops apt-mutation arc (per the approved plan,
docs/pkgops-implementation-plan.md§3): the native effect-session that lets an unprivileged issuer mint a single-use broker receipt, commit an apt change through the immutablepkexecentrypoint, and close the durable outcome — with the receipt and binding held in wipeable C heap and never becoming R objects.Built and reviewed in checkpoints (all on this branch):
libjansson+ generic-path effect refusal.C_rab_broker_callnow refuses any body carrying a top-leveleffectmember (runix_effect_via_generic_path), so a receipt-minting intent is serviceable only through the effect session.open/write_outcome/state): a PID-boundEXTPTRSXPover theopen_intent(+effect)→write_outcomestate machine; receipt/binding extracted with in-C Jansson intoexplicit_bzero-wiped heap; a forked / unserialized handle is refused; the finalizer only wipes (dropped handle → intent left open, fail-closed).effect_session_commit:posix_spawn(no shell) ofpkexec+ the verb's immutable entrypoint; receipt delivered on the child's stdin and wiped the instant it is sent; strict result read under a wall-clock deadline. The verb→path map lives in C; no path ever crosses from R. The fake-entrypoint test seam is compile-time only (-DRUNIX_TESTING), absent from the shipped build.SIGPIPEsafely ignored, else the effect is left UNKNOWN; everyposix_spawn_file_actions_*return checked; the bounded request grammar (count/name/arity/dup/size) enforced before the receipt is spent; result rejected ifdetail > 128or the exit code contradicts the status; a timeout classifies effect-UNKNOWN (the privileged work runs in a separate polkit scope); a commit result is trusted only when the receipt was actually delivered; embedded-NUL rejection on native string inputs; fd hygiene is atomic-only (addclosefrom_np) with the commit path refused fail-closed where the primitive is unavailable (effect_session_commit_supported()reports it).effect_capability()(real effect-receipt + plan-schema negotiation, fail-closed torunix_capability_unavailable); the three-call session APIeffect_session_open/effect_session_commit/effect_session_write_outcome(opaque PID-bound handle, no secrets/paths in R, issuer maps the 12-status commit vocabulary — not runix); and the documented condition taxonomy (runix_effect_conditions).Custody & safety invariants
RAWSXP/CHARSXP; they live in wipeable C heap and areexplicit_bzero'd on consumption and in the finalizer.pkexectarget in C.pkexectarget or lower the broker peer-uid bar; the test seams are compile-time (-DRUNIX_TESTING) and absent from the shipped build.Build
Links
libjanssonvia an anticonfconfigure(newSystemRequirements), for strict in-C extraction.SystemRequirementsand CI/.debmetadata updated accordingly.Tests
-DRUNIX_TESTINGbuild (fake broker + fake entrypoint, no root, no dpkg): 469/469, covering the full open/commit/write_outcome flow, every result-status and effect-unknown path, the input-bounds/detail/exit-consistency/timeout/delivery-gate cases, fd-hygiene (leak sentinel), and the exported API + capability negotiation.-Wall -Wextraclean; the shipped build carries no test seam.Note: full session/commit coverage requires a
-DRUNIX_TESTINGCI leg (production pins uid 0 and cannot stand up a root broker without root), and the no-closefromfail-closed refusal was hand-verified via a-DRUNIX_NO_CLOSEFROM_NPbuild — a small CI leg for it would be a useful follow-up.Bumps to 0.0.1.12.