feat(mosaic): add authenticated external lease broker #836
Assignees
jason.woltje
Clear assignees
jason.woltje
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#836
Reference in New Issue
Block a user
Delete Branch "feat/828-lease-broker"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes #828
Summary
Verification
Merge gate
Do not merge until coordinator-owned exact-head Opus security review and independent exact-head review complete.
OPUS SECREV (mandatory, distinct principal)
Verdict: ✅ SECREV GO — reviewed exact head
b118b65f111e0411c08f8e3bd61d4b2e4d97940f— 0 blocking security findings.Base =
mainauthoritative tipd801d6c4; clean merge-base confirmed; reviewed =git diff main...HEAD+ full new files. Reviewed-SHA equals the head to be merged (exact-head discipline).Independence attestation (Gate 16 — by lane/process)
Single Gitea account (
jason.woltje) fans out to three distinct principals by lane:ms-cr-wi1(pi, gpt-5.6-sol)terra(separate lane)Author ≠ reviewer holds by lane/process separation. This verdict was earned from the product code (
packages/mosaic/framework/tools/lease-broker/daemon.py, 490 LOC, stdlib-only, self-contained), not from the author's red-first tests. The 10 red-first commits are sound process but were treated as non-evidence.Sole product surface
daemon.pyis the only shipped product code: zero local imports (stdlib only);state_store_unittest.pyandlease-broker.acceptance.spec.tsare test-only and not imported by the daemon. The onlyMath.randomoccurrence in the diff is a TS test that poisonsMath.randomto prove the crypto path never uses it.The six mandated properties — proven in product code
SO_PEERCRED identity is authoritative.
(pid,uid,gid)read from the kernel viagetsockopt(SOL_SOCKET, SO_PEERCRED, 12)/struct.unpack("3i", …)(daemon.py:445-446). No path trusts a client-asserted principal:register_anchorrefuses anysession_idfield even when null/empty (:329-330); authenticated actions bind authority to kernelpeer_pidviaverified_ancestry(peer_pid, anchor_pid, anchor_starttime)(:283) — the client-namedsession_idis only a lookup key, never authority. Spoof-resistant. HOLDS.Broker-minted
session_id, server-authoritative, unguessable. Minted only server-side withsecrets.token_hex(32)= 256-bit OS CSPRNG (:338). Client cannot supply it (:329-330). Zero client influence on the value. HOLDS.Crypto single-use tokens.
secrets.token_hex(32)CSPRNG issuance (:363) — notMath.random, not model output. Consumption is atomic: single-threaded serial accept loop; consume re-authenticates then requiressession_id+runtime_generation+consumed is Falsematch beforedel tokens[token](:371-378); a replay finds nothing →TOKEN_REPLAY. Durability is atomic (temp-file → fsync →os.replace→ dir-fsync, :221-259); on any post-rename uncertainty the store poisons and the daemon terminates rather than acking (fail-closed). Capacity bounded at 256 before mutation (:361). Generation bump revokes prior-incarnation tokens (:289, :351). No wall-clock TTL exists, and none is claimed — "expiry" is expressed as generation-scoped revocation + single-use; acceptable for WI-1 scope. HOLDS.Frame-drain safety. One frame per connection (structurally impossible to desync — connection is single-shot).
read_framecaps atMAX_FRAME(64 KiB), enforces a 1 s connection deadline, and requires exactly one trailing\nwith no embedded newline (data.count(b"\n") != 1, :402) — blocks multi-object injection. Oversized frames enter a bounded drain that reads into a throwaway (no accumulation) until EOF/deadline, then fails closedMALFORMED_REQUEST(:393-401). Partial/unterminated/malformed/non-dict JSON all fail closed. No unbounded buffering; slowloris bounded by the 1 s deadline (and same-UID trust). HOLDS.Fail-closed on uncertain durable state. Post-rename directory-fsync failure sets
poisoned=Trueand raisesStateCommitUncertainwithout rolling memory back (:245-253);handle()re-raises it (not caught asBrokerFailureinserve), so the daemon terminates via the top-level handler (:488-490) — no reply, owned socket unlinked. Every subsequent entry is denied atif self.store.poisoned(:311-312). On restart,validate_staterefuses to load impossible/inconsistent/oversized/duplicate-anchor/wrong-generation/consumed persisted state (:78-125) → startup refusal. The uncertain branch demonstrably denies; there is no fail-open lease. HOLDS.Lease lifecycle. No TOCTOU:
verified_ancestrywalksppidto the(pid,starttime)anchor, then re-reads every walked PID's starttime and raisesPID_STARTTIME_RACEon any change (:157-161); anchor starttime mismatch (recycled PID) →ANCESTRY_MISMATCH(:149-151). No double-grant:register_anchoris idempotent per(anchor_pid,anchor_starttime)(:336) andvalidate_staterejects duplicate anchors (:105-107). Revocation authoritative: higher generation replaces the incarnation and deletes prior tokens before commit (:287-289, :349-351). Peer-death reclaim: a dead/recycled anchor can no longer authenticate (PID gone →PID_UNAVAILABLE; PID reused → starttime mismatch), so its session/tokens become inert and unreachable by any other principal. Sibling substitution rejected (a process outside the anchor subtree never reachesanchor_pidin its chain →while…else: return False, :144-155). HOLDS.Also confirmed
secure_parentrequires the parent dir to be exactly mode0o700(:164-167, applied to both socket and state paths); socket set to0600after bind inside the already-0700dir (:421); refuses to start if the socket path already exists or is a symlink (SOCKET_ALREADY_EXISTS, :415-416); on shutdown unlinks only the inode it created (dev+ino match, :422/:467-469).O_NOFOLLOW | O_CLOEXEC, must be a regular file at0600, bounded byMAX_STATE(4 MiB) (:176-198).READYand symbolic error codes (STATE_INTEGRITY,ANCESTRY_MISMATCH, …); session IDs and tokens are never logged (grep-confirmed).type(value) is intguards (is_non_negative_integer, :40-41).Non-blocking observations (informational only — NOT change requests)
MAX_STATE(4 MiB) and same-UID trust. Harmless/inert. Candidate for a future hygiene pass.OSErrorand a post-SO_PEERCREDstruct.errorpropagate to daemon termination (fail-closed / availability tradeoff), not a reply — safe under the stated trust model.None are security defects; all are documented scope or safe fail-closed behavior within the same-UID trust boundary.
Disclosed residual (acknowledged, deliberately NOT flagged)
The harness-side tail-preserving middle-drop of the injected block (planner-sol ruling
b7bbb6ea, A-v5-1 replaced) and the T-C same-UID socket unlink/counterfeit residual (server-side branch-protection backstop = WI-7) are ratified residuals disclosed inlease-broker-security.mdandlease-broker-operations.md. Per mission scope these are not new findings.Process note:
tea/pr-review.sh approveis broken for this surface; this durable comment is the formal SECREV approval record. Reviewed headb118b65f. Merge authority remains with the coordinator (Mos) after both the independent code-review gate (terra) and this SECREV gate are green. This reviewer performed no code edits, commits, pushes, or merges.INDEPENDENT CODE REVIEW (terra, LANE-attested author≠reviewer)
Verdict: REQUEST CHANGES
Reviewed exact head:
b118b65f111e0411c08f8e3bd61d4b2e4d97940f(reviewed-SHA must equal merge SHA). Author lane was gpt-5.6-sol (Pi); reviewer is terra/gpt-5.6-terra in a separate session. The shared Gitea account does not attest identity; author≠reviewer is LANE/PROCESS-attested.Blocking finding
packages/mosaic/framework/tools/lease-broker/daemon.py:435-447— one slow/silent same-UID client serially blocks the complete broker accept loop for its one-second deadline.serve()accepts a connection and calls blockingread_frame()before accepting another. An attacker can queue/continuously replenish silent Unix-socket connections, delaying legitimate register/auth/mint/consume operations by one second per queued connection. I reproduced four silent peer connections delaying a validregister_anchorreply by 4.02 seconds. This contradicts the acceptance test's intended “cannot prevent the next valid registration” property and is a same-UID availability/security concern for this trust boundary. Handle connections concurrently or use a selector/event loop with independent per-connection deadlines and a bounded concurrency/resource policy; add a multi-silent-client regression test proving a valid request is not serialized behind queued stalls.Verification performed
git diff d801d6c4...HEADplus every new file.SO_PEERCRED+/procstarttime, refusal ofregister_anchor.session_id, ancestry re-read, generation/token revocation,secrets.token_hex, 0700/0600 modes, bounded/drained frames, and fail-closed persistence.d61c5441before implementationdeb11df7; later remediation RED/GREEN pairs are present. No changed existing test and no--no-verifyin this branch history.35/35; Python state tests10/10; package suite71 files / 1,369 tests; rootpnpm test43/43Turbo tasks; root typecheck/lint/format;git diff --check; PR CI green (Woodpecker PR pipeline 1908).The separate mandatory Opus-SECREV remains required and is not replaced by this review.
OPUS RE-SECREV — head
b072a7bd(concurrency re-verified)Verdict: ✅ SECREV GO — reviewed exact head
b072a7bd429715c82a086f6c36705b10a17c7103— 0 blocking security findings.This is a genuine re-verification, not a fast-path re-confirm. The concurrency model changed (serial-accept → bounded concurrent thread-pool accept), which is precisely the assumption my prior SECREV (
b118b65f, comment 18071) relied on for properties 2/3/6. Those assumptions were discarded and re-proven from scratch against the new product code. The sol fix's own green tests were treated as process, not evidence. Base =maind801d6c4; reviewed =git diff main...HEAD+ full files. Reviewed-SHA == merge head (exact-head).The actual synchronization primitive (read, not assumed)
The fix (daemon.py, commit
b072a7bd) introduces:broker_lock = threading.Lock()— a single global mutex;slots = threading.BoundedSemaphore(16)+ThreadPoolExecutor(max_workers=16),MAX_IN_FLIGHT_CONNECTIONS=16— bounded concurrency;slots.acquire(timeout=0.1)→server.accept()(serversettimeout(0.1)) →executor.submit(process_connection, …); each worker releases its slot in afinally.Decisive structural property:
broker.handle(...)has exactly one call site (daemon.py:434), and it is entirely insidewith broker_lock:(grep-confirmed, single occurrence). Every mutation of shared state (store.value,sessions,tokens,poisoned,commit()) lives only insidebroker.handle, hence only inside the mutex. The slow, attacker-influenced I/O —getsockopt(SO_PEERCRED),read_frameand its oversized-frame drain — runs outside the lock (daemon.py:424-426, before line 433). So the coarse global lock reduces every critical section to the exact serial semantics my prior review verified, while only lock-free, no-shared-state I/O is concurrent.Corroborating:
git diff b118b65f..b072a7bd -- daemon.pyconfines all changes to imports + one const +handle_connection+serve. TheBroker,StateStore,validate_state, andread_frameinternals are byte-unchanged from the prior-verified head — so their internal logic carries no regression; the only new question is whether concurrency breaks their atomicity, answered below.RIDER A — props 2/3/6 re-proven under REAL concurrency
Property 3 — single-use tokens, atomic consume, no double-spend/replay. Two peers consuming the same token: both read their own frames concurrently (no shared state), then contend on
broker_lock. The winner runsconsume_token—tokens().get→ replay checks (session_id/generation/consumed is False) →del tokens[t]→commit()— all under one uninterrupted lock hold (daemon.py:371-378 inside handle()). The loser then acquires the lock and finds the token gone →TOKEN_REPLAY. The check→delete→persist sequence is indivisible w.r.t. any other broker op; no interleaving can double-spend. Mint↔consume and mint↔mint likewise serialize. HOLDS under concurrency.Property 2 — broker-minted session_id, no issuance race.
register_anchorrunssession_for_anchor(idempotency read) and thesessions()[secrets.token_hex(32)] = …insert atomically under the lock, so the "existing is None?"→insert is not a TOCTOU. Distinct peers carry distinct SO_PEERCREDpeer_pid→ distinct(anchor_pid,starttime)keys → distinct sessions; the same peer connecting twice concurrently serializes to one idempotent session (no duplicate, no lost update). IDs are 256-bit OS-CSPRNG (secrets, thread-safe) → no collision/reuse under interleaving. HOLDS under concurrency.Property 6 — TOCTOU/double-grant/revocation/peer-death under concurrency. The full grant sequence (
authenticate→verified_ancestry/proc double-read→mint→commit) executes under one lock hold, so no other broker op can interleave between grant and use — no broker-internal TOCTOU is opened by concurrency (the inherent broker↔kernel /proc race is unchanged and still mitigated by SO_PEERCRED + the second starttime pass). No double-grant: anchor registration is idempotent per(pid,starttime)andvalidate_staterejects duplicate anchors; concurrent same-anchor registers serialize to one session. Revocation authoritative: a generation bump deletes prior tokens under the lock, so a concurrent consume either runs first (consumes) or after (seesTOKEN_REPLAY) — no window. Peer-death reclaim: SO_PEERCRED PID captured at connect;verified_ancestryre-reads /proc under the lock → dead anchor ⇒PID_UNAVAILABLE/starttime-mismatchANCESTRY_MISMATCH; the dead session's tokens are unreachable by any other principal. HOLDS under concurrency.RIDER B — bounded concurrency is NOT a slot-exhaustion DoS; reap is fail-closed
A slot is held from just-before-
acceptuntil the worker'sfinally: slots.release(). The worker bodyhandle_connectionis bounded by a per-connectiondeadline = monotonic()+1.0s:read_frameshrinks eachrecvtimeout todeadline(settimeout(remaining);remaining<=0→MALFORMED_REQUEST). A silent peer'srecvraisessocket.timeout(⊂OSError) within ≤1s →except OSError: return→ connection closed, no reply, no lease/token minted, slot released. The deadline reap is fail-closed: a past-deadline peer is dropped, never granted. A silent peer therefore holds a slot for ≤1s, not indefinitely — this is the exact regression fixed vs. the serial-accept permanent block (regression testff33cf31). A same-UID attacker cycling silent connections can induce only bounded latency (slots recycle every ≤1s; forward progress guaranteed), never permanent starvation, and only within the same-UID trust boundary. Slot accounting is balanced on every path (acquire→release on accept-timeout / accept-OSError / submit-failure in the loop, else in the workerfinally);BoundedSemaphorewould raise on over-release and none occurs (single release per acquire). Crucially, the attacker-controlled slow path (read_frame/drain) is outsidebroker_lock, so a slow/oversized-frame client cannot hold the mutex to serialize-starve legitimate callers — only fast in-memory+single-fsync work runs under the lock. RIDER B satisfied.Unchanged surface re-confirmed not regressed by the refactor
getsockopt(SO_PEERCRED,12)/unpack("3i")(daemon.py:424-425); no client-asserted principal trusted (register_anchorrefusessession_id; authority =peer_pidancestry). ✓secrets.token_hex(32)for both session_id and token; the onlyMath.randomin the diff is a TS test that poisons it to prove the crypto path never uses it. ✓0o700(secure_parent), socket0600after bind, pre-existing/symlink socket refused, shutdown unlinks only the created inode (dev+ino). ✓PID_STARTTIME_RACE), generation-bump revoke, frame-drain fail-closed (one frame/conn, exactly-one-\n, bounded throwaway drain), fail-closed persistence (post-rename uncertainty → poison-no-rollback → daemon terminates;validate_staterefuses impossible/oversized/duplicate/consumed state on load). All byte-unchanged and intact. ✓StateCommitUncertainpoison is set insidecommit()whilebroker_lockis still held; the next thread to acquire the lock seespoisoned=Trueand is denied — no thread can grant after poison.process_connectionrecords the first fatal intofatal_errors(underfatal_lock); the accept loop raises it, andfinallydoesexecutor.shutdown(wait=True)(draining in-flight workers, all now denied) before unlinking. Fail-closed preserved. ✓READYand symbolic error codes; session IDs/tokens never logged. ✓Non-blocking observations (informational — NOT change requests)
MAX_STATE(4 MiB). Same-UID trust; inert.Disclosed residual (acknowledged, deliberately NOT flagged)
Harness-side tail-preserving middle-drop (planner ruling
b7bbb6ea, A-v5-1 replaced) and the T-C same-UID socket unlink/counterfeit residual (server-side branch-protection backstop = WI-7) are ratified residuals disclosed in the security/operations docs. Per scope, not new findings.Independence (Gate 16 — by lane/process): author
ms-cr-wi1(pi) ≠ code-reviewterra≠ this Opus RE-SECREV — three distinct principals on one Gitea account, separated by lane/process. Earned from product code, not the author's tests.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal RE-SECREV approval record. Reviewed headb072a7bd. Merge authority remains with the coordinator (Mos) after both gates green. No code edits, commits, pushes, or merges performed by this reviewer.INDEPENDENT CODE RE-REVIEW (terra) — head
b072a7bdVerdict: APPROVE
Reviewed exact head:
b072a7bd429715c82a086f6c36705b10a17c7103against authoritativemainbased801d6c4. 0 blocking findings. Author/reviewer separation remains LANE/PROCESS-attested: sol/Pi authored; terra reviewed in a distinct session (the shared Gitea account is not identity evidence).Prior blocker and RIDER B
serve()now dispatches accepted peers to a bounded worker pool, while each connection has its own one-second deadline. The broker-state lock preserves atomic state transitions.Re-verified
SO_PEERCREDwith/procstarttime and ancestry re-read, refusal of callersession_id, generation revocation,secrets.token_hexsingle-use tokens, 0700/0600 posture, safe oversized-frame drain, and fail-closed durable state.ff33cf31precedes implementation commitb072a7bd; the implementation commit did not alter the new regression test. No--no-verify; no test was edited to pass.37/37; Python state suite10/10; rootpnpm test43/43Turbo tasks (Mosaic suite71 files / 1,371 tests); root typecheck, lint, format check, andgit diff --check; PR CI Woodpecker pipeline 1909 green.This is the independent code/correctness review only. The separately mandated Opus-SECREV is still required before merge.