fix(pod): send the token-mint secret over the pod's unix socket, never TCP - #9051
fix(pod): send the token-mint secret over the pod's unix socket, never TCP#9051javenciu wants to merge 2 commits into
Conversation
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All evidence gathered. Composing the review. First-Principles-Verdict: CONCERNS Round 2's rebind attacker can already read the secret it supposedly captures — the peer-pid verifier defends a boundary the codebase doesn't have. Not justified as shipped
What this change shipsIntent: stop the pod's
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 3e40189 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsThe discovery pass produced no candidates, and my independent verification confirms the design holds: No findings. [OPUS-REVIEWED] 3e40189 |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Both premises I probed check out in the base tree: pods are POSIX-only by design ( The one design risk that survives falsification is version skew: Design-Verdict: CONCERNS Sound root-cause fix — credential moved onto a peer-verified transport — but the structural no-fallback hard-fails against pod gateways built before this PR, with a misdirecting error. Watch
Suggestions
[DESIGN-REVIEWED] 3e40189 |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsFINDING -- src/kiro_crew/dashboard/handlers/core.py:2525 -- function-local |
a617b3b to
0242dd9
Compare
0242dd9 to
e4bb192
Compare
…r TCP (kirodotdev#8552) mint_token attested pod ownership and then opened a SEPARATE loopback TCP connection carrying X-Local-Secret: a pod exiting inside that window frees the port for any local user to bind, and loopback TCP has no peer-credential API to tell the squatter from the gateway. Route the mint over the pod's private AF_UNIX socket -- unix_socket_urlopen has no TCP handler, so no fallback is structural rather than a flag. Server side, admit kernel-verified unix peers at /api/token/local: the endpoint is token_auth-bypassed and gated on is_loopback(request.remote), which 403s AF_UNIX requests (empty remote) -- the transport that is strictly harder to reach than loopback TCP. Admission is deny-by-default via check_peer_is_self: MISMATCH and UNVERIFIABLE both refuse, and the X-Local-Secret check is unchanged on both transports. health()/probe stay on TCP deliberately: they carry no credential, and port_owner() already reports who replied.
…ng the secret (kirodotdev#8552) The unix-socket transport removed the TCP rebind window, but the socket FILE has one of its own: its directory is owner-writable, so a same-UID process can unlink the path and bind its own listener there -- and the mint request would hand that listener the pod's .local_secret. The kernel can prove who answered. unix_socket_urlopen grows a verify_peer callback that runs on the connected socket BEFORE any HTTP bytes; _attested_gateway_verifier builds it from the freshness-proven recorded gateway pid vs the kernel's peer credentials (SO_PEERCRED on Linux, LOCAL_PEERPID on macOS). Deny-by-default: a mismatched peer, an unreadable peer, and an unprovable pid record all refuse before the request line goes out. Both send sites are covered (token mint and pod api), and the stub-gateway fixture now attests itself, so every pod-api test exercises the real kernel check end to end.
e4bb192 to
3e40189
Compare
|
Rebased onto main Clean rebase — no conflicts, despite Gates run locally on the changed files only: Please review the rebased branch. Note that a maintainer push makes the maintainer the last pusher, so under this repo's last-push rule a second approver is needed before merge. The two open review items (the GPT lane's test-cleanup finding at |
Problem / Motivation
mint_tokenreads the pod's.local_secretand sends it in anX-Local-Secretheader over loopback TCP tohttp://127.0.0.1:<port>/api/token/local. Ownership is attested (OWNER_PODpositive proof) and then a separate TCP connection carries the secret — a pod exiting inside that window frees the port for any local user to bind, and loopback TCP has no peer-credential API, so nothing on the wire can tell the squatter from the pod's gateway. #8218 closed exactly this class for the token-bearing request;mint_tokenis the residual its review disposition called out, and it could not ride #8218 because the server refuses the better transport:api_token_localgates onis_loopback(request.remote), and anAF_UNIXrequest has an emptyrequest.remote, so the endpoint answers 403 to the transport that is strictly harder to reach than loopback TCP.Why it matters
The captured value is the pod's per-gateway-start
.local_secret. The impact is bounded exactly as the issue bounds it (capture-now / replay-before-any-gateway-restart, and #8218's socket transport means a fabricated token is only ever handed to the pod's own socket) — but pods carry ~2h TTLs and are torn down routinely, so the port-release window is a normal lifecycle event, not an anomaly. A secret-bearing request should not depend on winning a race against ordinary pod churn.What changed (motivation → approach → change)
All three steps of the issue's proposal, with the admission taken at the stronger of the two options the issue offered:
mint_tokennow sends the mint over the pod's privateAF_UNIXsocket viaunix_socket_urlopen(req, timeout=5, socket_path=pod_socket_path(cfg, name, port)). That opener has no TCP handler, so "no fallback" is structural rather than a flag: a missing, stale, or refusing socket raises instead of handing the header to whatever answered — the error message says explicitly that the call is not retried on127.0.0.1:<port>and why. The URL keeps the loopback host so the gateway's Host validation sees exactly what it saw on TCP; the socket path is derived, never caller-supplied. TheOWNER_PODpre-check stays (the issue's own suggestion): it costs one process lookup and buys refusal messages that name why the pod cannot answer, instead of a bare connection error.api_token_localadmits kernel-verified unix peers via a new_unix_peer_is_selfhelper —request_is_unix_socket(request)ANDcheck_peer_is_self(sock) is PeerCredResult.MATCH. Because this endpoint istoken_auth-bypassed, admission is deny-by-default:MISMATCH(another principal reached our socket — precisely when the0700directory gate has failed and refusing matters most) andUNVERIFIABLE(no platform mechanism, failed syscall) are both refused, so a platform without peer credentials never silently widens the gate. This admits a transport, never a caller: theX-Local-Secretcheck downstream is unchanged on both transports. Function-local imports mirror the sibling admission inhandlers/updates.py.health()/ probe stay on TCP — they carry no credential, andport_owner()already reports who replied. The docker guide's local-bootstrap paragraph now describes the two admitted transports.Tests
Server half (
test/test_dashboard_handlers_core_coverage.py,TestLocalToken):test_unix_peer_match_with_valid_secret_issues_a_token— kernel MATCH + valid secret mints (403 → 200 before the fix; the benign path is admitted, not just attacks refused).test_unix_peer_still_needs_the_secret— admitted transport, wrong secret → 403invalid secret(notloopback only), pinning that admission never weakens the secret check and that the request genuinely passed the transport gate.test_unix_peer_uid_mismatch_is_refused_even_with_the_secret— kernel says another principal: refused with the audit log recordingnon-loopback.test_unix_peer_unverifiable_is_refused_even_with_the_secret— no peer-credential mechanism: refused (deny-by-default pin).Client half (
test/test_pod.py):test_mint_token_sends_the_secret_only_over_the_pod_socket— asserts the TCP opener list stays empty, the unix opener gotpod_socket_path(...), and theX-Local-Secretheader rode the socket.test_mint_token_reads_secret_and_postsre-pointed at the socket transport;test_mint_token_refuses_a_foreign_port_holderpasses unchanged on both sides (control: the pre-check's refusal behavior is untouched).Fails-before at unfixed
fd0fb5dbb(committed tests copied onto a pristine worktree): server 2 failed / 2 passed — admission 403-not-200 and the needs-secret refusal citing the transport (loopback only) instead of the secret; the two refusal pins pass both sides by design (deny-by-default is not new behavior). Client 2 failed / 4 passed — the secret rode TCP and no socket call existed. After the fix: 4/4 and 6/6. Full targeted files 502 passed / 1 skipped; seam neighbors (test_pod_api.py,test_socketsec*.py) 135 passed / 2 skipped; mypy clean over 1302 files; flake8 / isort / black-baseline / docs-lint all pass.Manual verification
N/A — unit coverage sufficient: the two new seams are unit-pinned with the kernel verdict and transports faked, and both underlying primitives carry their own pre-existing real-socket suites (
test_pod_api.pybinds a liveAF_UNIXserver againstunix_socket_urlopen;test_socketsec*.pycoverscheck_peer_is_selfincluding real-socket SO_PEERCRED). A live gateway round-trip was not run in this environment (no running pod); CI exercises the composed path.Related Issues
Fixes #8552
Pattern harvest
Rule candidate: review-prompt
Pattern: "attest-then-send across separate connections is a TOCTOU on the transport — a credential-bearing request must ride a connection whose peer is verified at send time (unix socket + peer creds), not a port whose owner was checked moments earlier". Flag any request that sends a secret over plain loopback TCP when a peer-verified unix transport exists for the same server.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement
Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).
Round 2: connect-time peer verification (review finding)
Review flagged a residual window on the socket itself: the unix socket
path lives in an owner-writable directory, so a same-UID process can
unlink it and bind its own listener there — and the mint request would
hand that listener the pod's
.local_secret. A fresh pid record cannotsee this; the record proves the gateway is alive, not that it is the
process answering the file.
Commit 2 closes it with the kernel's own answer:
unix_socket_urlopengrows a
verify_peercallback that runs on the connected socket beforeany HTTP bytes, and
_attested_gateway_verifierbuilds that check fromthe freshness-proven recorded gateway pid vs the kernel's peer
credentials (
SO_PEERCREDon Linux,LOCAL_PEERPIDon macOS).Deny-by-default: a mismatched peer, an unreadable peer, and an
unprovable pid record all refuse before the request line goes out. Both
send sites are covered (token mint and pod api — a rebind between the
two sends would otherwise capture a live credential), and the
stub-gateway fixture now attests itself, so every pod-api test drives
the real kernel check end to end.
New tests pin the window shut from both sides: a live in-process
listener answering the pod's derived socket path with a mismatched
attested pid observes ZERO bytes (the refusal precedes the request
line), and the same listener attested as this process completes the
mint through the real peer-credential read — no monkeypatch on the
kernel path.