close
Skip to content

feat: pin tunnel sessions to daemon-verified tailnet peer (#1762) - #2411

Merged
iamwhatever merged 1 commit into
mainfrom
fix/tailnet-identity-pin-1762
Aug 10, 2026
Merged

feat: pin tunnel sessions to daemon-verified tailnet peer (#1762)#2411
iamwhatever merged 1 commit into
mainfrom
fix/tailnet-identity-pin-1762

Conversation

@bolichen97

Copy link
Copy Markdown
Collaborator

Summary

Implements RFC rfc-tailnet-dashboard-access Phase 3 (§2–§3.1). Behind every documented tunnel (cloudflared, ngrok, tailscale serve) the session IP pin binds from request.remote, which is the tunnel's loopback address — so one pin is satisfied by anyone behind the same tunnel for up to the 20h session TTL, and the SEL audit trail records 127.0.0.1 as the caller. Phase 1 (PR #1761) made that state visible; this PR repairs it for the one provider whose local daemon can verify a peer.

What changes

  • dashboard/tailnet.pyresolve_forwarded_peer() (RFC §2, all conditions fail-closed): peer resolves only when the immediate peer is loopback, trust_identity is on with a non-empty allowed_logins, X-Forwarded-For carries exactly one address (multi-value = reject, never first/last) inside the tailnet ranges, and tailscale whois --json (vetted absolute CLI paths, hard timeout, bounded TTL cache, dedicated subprocess executor) resolves it. Tailscale-User-Login is corroboration only — a disagreement is a rejection. Transient daemon failures (spawn error/timeout) cache ~2s so a startup blip clears fast; definitive answers keep the 30s TTL. POSIX-only per RFC OQ4 — Windows degrades to the token path.
  • dashboard/token_auth.py — peer-keyed session pin (RFC §3/§3.1): ip:<addr> byte-for-byte today's behaviour for every non-Tailscale path; ts:node:<login>|<node> (default) / ts:login:<login> for a verified peer. The | separator is forbidden inside components (identity charset allowlist), so keys cannot collide. An ACL-tagged node (tagged-devices login) is always node-scoped — login scope would collapse the pin across the whole tagged fleet. allowed_logins is enforced at the auth decision (the early deny also covers the /api/auth/refresh middleware bypass), the pin check also runs on the internal cookie-auth branches, and mismatch denials name device identity / unverified identity rather than a misleading "IP mismatch". Resolution is gated on the request presenting a credential, so an unauthenticated local caller can never drive daemon spawns with headers. SEL: caller attribution to the resolved login + one tailnet_peer_bind row per session.
  • handlers/auth_refresh.py: a rotated access token re-binds to the verified peer key, so rotation cannot launder a node pin into an unbound token.
  • server.py: an enterprise ceiling pinning capabilities.tailnet_origin off also forces identity trust off at startup (audited governance decision) — a stored trust_identity: true cannot keep whois calls alive under a policy that forbids the integration.
  • config/loader.py: trust_identity / allowed_logins / pin_scope with narrowing-only load validation: trust with an empty allowlist is refused (never silently permissive); an unrecognised pin_scope falls back to node.
  • What stays closed (RFC §5): is_direct_local_request() unchanged — a verified peer still receives read_only: true on the messaging config surfaces (regression-tested).
  • Docs: guide gains the opt-in config snippet and honest per-provider pin semantics; RFC status updated; token-auth spec updated.

Adversarial review (pre-push)

The issue required adversarial review before landing. A dual model-pinned reviewer fleet (GPT + Opus lanes mirroring the repo's CI reviewers) ran against the working tree with the explicit attack list (XFF injection, multi-value XFF, header/whois disagreement, daemon-absent fallback, timeout fail-closed, tagged-node login-scope collapse, allowlist bypass). All blocking findings were fixed before this PR: internal-path pin bypass, refresh-rotation pin laundering, governance force-off gap, shared-executor starvation + unauthenticated daemon spawn, 30s negative-cache lockout with misattributed reason — plus advisories (key-separator ambiguity, identity charset allowlist, SEL bind row, _log_auth attribution).

Tested

  • test/test_tailnet_peer.py (new): full RFC §2 resolution matrix, daemon failure modes at the subprocess seam, cache TTL/boundedness, pin-key shapes + tagged override + collision-resistance, config load validation, RFC §5 read-only regression, refresh re-bind.
  • test/test_token_auth.py: middleware integration — identity bind, node/login-scope replay semantics, allowlist deny, credential gate, internal mixed-path pin enforcement, unverified-identity denial reason, byte-for-byte non-Tailscale behaviour (whois never called, posture SHARED preserved).
  • Local gates green: isort, flake8, mypy (856 files), targeted pytest (1,800+ tests across all touched surfaces), docs-lint, brand gate.

Closes #1762

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 304fa9e15cae87a30531d095c5e391a9108541fc — this comment is updated in place on each push.

Review details

No findings that meet the bar. The PR is a large but carefully-constructed identity-pinning feature: peer resolution is fail-closed at every RFC condition, the blocking tailscale whois call is offloaded onto subprocess_executor() (no event-loop block), the allowlist is deny-by-default, internal paths gain pin enforcement (a strengthening, not a removed guard), and the credential-presence gate prevents unauthenticated header-driven daemon spawns. The bind_ipbind_peer rename preserves the address-pin path byte-for-byte behind compat wrappers. No AUTOSDE rule violation and no reachable residual-class defect on the changed lines.

No findings.

[OPUS-REVIEWED] 304fa9e

Verdict parsed from the review's SHA-scoped output markers for commit 304fa9e15cae87a30531d095c5e391a9108541fc.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 304fa9e15cae87a30531d095c5e391a9108541fc: <one-sentence reason>

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 304fa9e15cae87a30531d095c5e391a9108541fc — updated in place on each push; does not block merge.

Design-Verdict: PASS

A real, RFC-specified defect (tunnel-shared session pin, misattributed audit) fixed at its root with fail-closed identity, opt-in narrowing-only config, and fully reversible in-memory state.

Watch

  • Identity-trust settings (including allowed_logins) are read once at startup, so removing a compromised login does not take effect until a gateway restart — sessions live up to 20h. Documented in the field metadata, but worth confirming this revocation latency is acceptable before recommending pin_scope: "login" to users.

Note for the human reviewer: the CHANGELOG hunks in the 37bb79fb...HEAD range are base drift (the branch parents at 1e401987, before docs PR #2413), not changes this PR makes — the actual delta contains no CHANGELOG edits.

[DESIGN-REVIEWED] 304fa9e

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ human override accepted

Human judgment by @bolichen97 overrides the GPT 5.6 finding for 304fa9e15cae87a30531d095c5e391a9108541fc; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 304fa9e15cae87a30531d095c5e391a9108541fc: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@bolichen97
bolichen97 force-pushed the fix/tailnet-identity-pin-1762 branch from d1eaae5 to a8e1ca0 Compare August 9, 2026 22:06
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

🤖 Kiro Crew Auto-Pipeline — response to the review round on d1eaae5 (new head: a8e1ca0).

GPT 5.6 blocking findings

1. "Tailnet allowlist fails open when peer verification fails" — rebutted (by design, per RFC).
Two independent reasons this is not an exploitable fail-open:

  • Candidacy is attacker-controlled for the local adversary. A local process that can reach the loopback socket chooses whether to send X-Forwarded-For at all. Any deny keyed on an unverified header is bypassed by simply omitting the header — the request then isn't a tailnet candidate and takes the token path anyway. The allowlist can only have teeth when the daemon has verified the peer, which is exactly when it's enforced.
  • For the remote adversary, the window is nearly self-contradictory. A real tailnet request only arrives through tailscale serve, and serve is tailscaled — a stopped daemon means no proxied request arrives at all. The reachable failure is a whois blip during daemon startup, which is negative-cached for only ~2s. Meanwhile the RFC is explicit that daemon unavailability must degrade to the token path, never deny (rfc-tailnet-dashboard-access §2 "Daemon absent or down → … A stopped tailscaled degrades access, it does not deny it"; Goal 4 "Nothing this RFC adds can lock a user out"). Denying unresolved candidates would convert every daemon restart into a lockout of all legitimate tailnet users — the exact failure mode the design forbids. The token remains the primary credential, unchanged from baseline; the allowlist is a narrowing on top of it. The Opus lane independently falsified the same concern ("not the fail-open class").

2. "Refresh rotation transfers the node pin to the requester" — rebutted as in-scope behaviour; hardening tracked in #2417.
Pre-Phase-3 baseline: a rotated access token was never bound at all — usable from any address, by anyone holding it. This PR binds it to the daemon-verified, allowlisted peer that performed the rotation, which is a strict tightening (Opus lane: "strict tightening, not a removed guard"). The residual — a refresh cookie stolen from allowed node A and replayed from allowed node B — is the refresh cookie's documented bearer-credential property (docs/guides/remote-and-mobile.md, the "kirocrew logout does not end that" note), not something this PR introduces or widens; note the thief must themselves be a daemon-verified allowlisted peer, and the rotation now leaves an SEL trail naming their login. Binding refresh chains to their opening peer changes the persisted chain format and the refresh availability contract (RFC OQ2 territory), so it is split into follow-up #2417 rather than bundled into an already-large auth change.

3. "Request-triggered execution trusts user-writable CLI locations" — accepted and fixed in a8e1ca0.
Correct: Homebrew chowns /opt/homebrew/bin (sometimes /usr/local/bin) to the console user, so the module's "need root to write" claim did not hold there, and identity resolution made the spawn request-triggered. _cli_path now refuses any candidate whose binary or parent directory is writable by the gateway user (POSIX, non-root); a refused Homebrew install degrades exactly like a missing binary and the documented dashboard.url fallback still works. Test added (TestCliPathTrust).

Design Review suggestions (both applied in a8e1ca0)

  • Trust construction + governance force-off extracted into one shared tailnet.governed_tailnet_trust() used by both startup surfaces — no more dual-site drift.
  • trust_identity / pin_scope config descriptions now state the restart-to-apply behaviour explicitly (the values are read into TailnetTrust once at startup).

CI failures on d1eaae5 (fixed in a8e1ca0)

  • spawn-audit: allowlist entry follows the _run_json_run_json_detail rename (comment widened to cover the whois argv).
  • Windows shard: peer-resolution tests now pin IS_POSIX=True so the matrix runs identically there; the explicit Windows-degrade test still asserts the production gate; os.geteuid guarded for Windows.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@bolichen97
bolichen97 force-pushed the fix/tailnet-identity-pin-1762 branch from a8e1ca0 to 19e2713 Compare August 9, 2026 22:16
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

🤖 Kiro Crew Auto-Pipeline — response to the GPT round on a8e1ca0 (new head: 19e2713).

"Missing session bindings are accepted after restart" — accepted and fixed in 19e2713. Legitimate: the binding map is in-memory by design (RFC: "regenerated on restart"), so after a restart a surviving cookie was unbound and the pin protected nothing for the rest of that cookie's life. Now the first request that carries a verified peer identity re-claims the pin for an unbound cookie (SEL tailnet_peer_bind row, "first-use re-pin"), and any other device is denied from then on. Deliberately scoped to resolved peers: blanket-rejecting unbound cookies would log every user out on every restart (including all non-tailnet users), and re-pinning ip: keys would change app-token semantics that predate this PR. Regression tests added for both the re-pin and the preserved no-peer semantics.

The other two findings are unchanged from the previous round and were rebutted with evidence in the comment above (#2411 (comment)): the allowlist-on-daemon-failure behaviour is the RFC's explicit degrade-not-deny design (candidacy is attacker-controlled for local forgers, and serve dying is the daemon dying for remote ones), and refresh rotation binding to the presenting verified allowlisted peer is a strict tightening over the never-bound baseline, with full chain binding tracked in #2417 because it changes the persisted chain format and the refresh availability contract (RFC OQ2).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
Behind every documented tunnel (cloudflared, ngrok, tailscale serve) the
session IP pin binds to the tunnel's loopback address, so one pin is
satisfied by anyone behind the same tunnel and the audit trail records
127.0.0.1. Implements RFC rfc-tailnet-dashboard-access Phase 3 (§2–§3.1):

- dashboard/tailnet.py: resolve_forwarded_peer() — a peer resolves only
  when the immediate peer is loopback, trust_identity is on with a
  non-empty allowed_logins, X-Forwarded-For carries exactly one address
  inside the tailnet ranges, and tailscale whois (vetted CLI paths, hard
  timeout, bounded TTL cache on the dedicated subprocess executor)
  confirms it; a Tailscale-User-Login header is corroboration only and a
  disagreement rejects. Transient daemon failures cache ~2s so a startup
  blip clears fast. POSIX-only per RFC OQ4; Windows degrades to the token
  path.
- dashboard/token_auth.py: the session pin generalises to a peer key —
  ip:<addr> byte-for-byte today, ts:node:<login>|<node> /
  ts:login:<login> for a verified peer ('|' is forbidden inside
  components, so keys cannot collide; ACL-tagged nodes are always
  node-scoped). allowed_logins enforced at the auth decision, including
  the /api/auth/refresh bypass; the peer-pin check also runs on the
  internal cookie-auth branches; pin-mismatch denials name device
  identity or an unverified identity, never a misleading 'IP mismatch';
  resolution is gated on the request presenting a credential so
  unauthenticated requests never reach the daemon; SEL audit attributes
  the resolved login and records the identity bind.
- handlers/auth_refresh.py: a rotated access token re-binds to the
  verified peer key so rotation cannot launder a node pin.
- server.py: an enterprise ceiling pinning capabilities.tailnet_origin
  off also forces identity trust off at startup (audited).
- config/loader.py: trust_identity/allowed_logins/pin_scope with
  narrowing-only validation (trust with an empty allowlist is refused;
  an unrecognised pin_scope falls back to node).
- is_direct_local_request() is unchanged: a verified peer still gets
  read_only on the config surfaces (RFC §5, regression-tested).
- Docs: guide opt-in instructions, RFC status, token-auth spec.

Adversarially reviewed pre-push by a dual model-pinned fleet; all
blocking findings and advisories addressed.

Closes #1762

Post-review CI round: spawn-audit allowlist follows the _run_json_detail
rename; peer-resolution tests pin IS_POSIX so the Windows shard runs the
same matrix; _cli_path refuses gateway-user-writable CLI locations
(Homebrew prefixes are not root-owned); the trust construction and
governance force-off live in one shared tailnet.governed_tailnet_trust()
helper for both startup surfaces; config metadata notes the
restart-to-apply behaviour. Refresh-chain peer binding tracked as a
follow-up (#2417).

Restart hardening: the in-memory binding map empties on restart, so the
first request that carries a VERIFIED peer identity re-claims the pin
for an unbound surviving cookie (SEL-audited); unbound cookies without a
verified peer keep today's semantics.

The re-pin lives in the shared pin check so the internal cookie-auth
branches get it too, and the CLI trust check refuses group/world-writable
paths for everyone and requires root ownership when running as root.
@bolichen97
bolichen97 force-pushed the fix/tailnet-identity-pin-1762 branch from 19e2713 to 304fa9e Compare August 9, 2026 22:25
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 9, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

🤖 Kiro Crew Auto-Pipeline — response to the GPT round on 19e2713 (new head: 304fa9e).

"Restarted cookies remain unbound on mixed internal routes" — accepted and fixed. The first-use re-pin now lives inside the shared _check_pin, so the main flow and both internal cookie-auth branches claim the pin identically; regression test added for the /api/spawn-style path.

"Root bypass defeats the planted-binary defense" — accepted and fixed. The trust check no longer has a root exemption: group/world-writable binaries or directories are refused for everyone, and a root-run gateway additionally requires root ownership of both (for root the W_OK probe says nothing, so ownership is the meaningful signal). Test added.

Refresh rotation (third round of the same finding) — the rebuttal stands as posted twice above with RFC citations: rotation binding to the presenting, daemon-verified, allowlisted peer is a strict tightening over the never-bound baseline (independently confirmed by the Opus lane), the stolen-refresh-cookie residual is the refresh cookie's pre-existing documented bearer property, and full chain binding is tracked in #2417 because it changes the persisted chain format and the refresh availability contract (RFC OQ2). Requiring "the existing session pin to match before rotating" is not implementable at this endpoint: the refresh handler never sees the old access token (the refresh cookie is the only credential presented), which is exactly why the chain-level binding in #2417 is the right vehicle.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

🤖 Kiro Crew Auto-Pipeline — final response on the remaining GPT finding (304fa9e, fourth round of the same class).

The proposed fix — "deny unbound node-scoped cookies and refresh rotation until the original peer key can be verified" — is impossible by construction and forbidden by the design:

  1. There is no original peer key to verify. The binding map is in-memory and regenerated on restart, by RFC design ("no persisted-format change beyond the peer-key string, which lives in the in-memory session pin state and is regenerated on restart"). After a restart the original key does not exist anywhere.
  2. "Deny until verifiable" = mass logout on every restart for every cookie holder (tailnet and non-tailnet alike), violating RFC Goal 4 ("Nothing this RFC adds can lock a user out") — and for refresh, it would end every 30-day session on restart.
  3. First-verified-claimant is strictly tighter than both alternatives that exist. Baseline: the cookie stays unbound forever and ANY device uses it silently. This PR: only a daemon-verified, allowlisted peer can claim it, exactly once, with a SEL tailnet_peer_bind row naming the claimant; every other device is denied thereafter. The stolen-credential-from-an-allowed-peer residual is the documented pre-existing bearer property of these cookies, and the persistent fix (chain/pin persistence) is tracked in Bind refresh chains to the tailnet peer identity that opened them #2417 as it changes the persisted format and the availability contract (RFC OQ2).

Cross-lane evidence: the Opus CI lane reviewed the same heads and found no blocking issues, explicitly falsifying this class ("strict tightening, not a removed guard"); the Design lane returned PASS. Overriding per the process below; the override and this rationale remain on the record for the human merger.

@bolichen97

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 304fa9e: first-verified-claimant re-pin is strictly tighter than the never-bound baseline and the only alternative (deny unbound cookies/rotation) is a restart mass-logout the RFC forbids; persistent chain binding is tracked in #2417.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 304fa9e15cae87a30531d095c5e391a9108541fc.

first-verified-claimant re-pin is strictly tighter than the never-bound baseline and the only alternative (deny unbound cookies/rotation) is a restart mass-logout the RFC forbids; persistent chain binding is tracked in #2417.

This decision applies only to this commit. A new push requires a new judgment.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 9, 2026
@iamwhatever
iamwhatever merged commit 642739c into main Aug 10, 2026
53 of 54 checks passed
@iamwhatever
iamwhatever deleted the fix/tailnet-identity-pin-1762 branch August 10, 2026 04:53
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 10, 2026
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…#1762) (kirodotdev#2411)

Behind every documented tunnel (cloudflared, ngrok, tailscale serve) the
session IP pin binds to the tunnel's loopback address, so one pin is
satisfied by anyone behind the same tunnel and the audit trail records
127.0.0.1. Implements RFC rfc-tailnet-dashboard-access Phase 3 (§2–§3.1):

- dashboard/tailnet.py: resolve_forwarded_peer() — a peer resolves only
  when the immediate peer is loopback, trust_identity is on with a
  non-empty allowed_logins, X-Forwarded-For carries exactly one address
  inside the tailnet ranges, and tailscale whois (vetted CLI paths, hard
  timeout, bounded TTL cache on the dedicated subprocess executor)
  confirms it; a Tailscale-User-Login header is corroboration only and a
  disagreement rejects. Transient daemon failures cache ~2s so a startup
  blip clears fast. POSIX-only per RFC OQ4; Windows degrades to the token
  path.
- dashboard/token_auth.py: the session pin generalises to a peer key —
  ip:<addr> byte-for-byte today, ts:node:<login>|<node> /
  ts:login:<login> for a verified peer ('|' is forbidden inside
  components, so keys cannot collide; ACL-tagged nodes are always
  node-scoped). allowed_logins enforced at the auth decision, including
  the /api/auth/refresh bypass; the peer-pin check also runs on the
  internal cookie-auth branches; pin-mismatch denials name device
  identity or an unverified identity, never a misleading 'IP mismatch';
  resolution is gated on the request presenting a credential so
  unauthenticated requests never reach the daemon; SEL audit attributes
  the resolved login and records the identity bind.
- handlers/auth_refresh.py: a rotated access token re-binds to the
  verified peer key so rotation cannot launder a node pin.
- server.py: an enterprise ceiling pinning capabilities.tailnet_origin
  off also forces identity trust off at startup (audited).
- config/loader.py: trust_identity/allowed_logins/pin_scope with
  narrowing-only validation (trust with an empty allowlist is refused;
  an unrecognised pin_scope falls back to node).
- is_direct_local_request() is unchanged: a verified peer still gets
  read_only on the config surfaces (RFC §5, regression-tested).
- Docs: guide opt-in instructions, RFC status, token-auth spec.

Adversarially reviewed pre-push by a dual model-pinned fleet; all
blocking findings and advisories addressed.

Closes kirodotdev#1762

Post-review CI round: spawn-audit allowlist follows the _run_json_detail
rename; peer-resolution tests pin IS_POSIX so the Windows shard runs the
same matrix; _cli_path refuses gateway-user-writable CLI locations
(Homebrew prefixes are not root-owned); the trust construction and
governance force-off live in one shared tailnet.governed_tailnet_trust()
helper for both startup surfaces; config metadata notes the
restart-to-apply behaviour. Refresh-chain peer binding tracked as a
follow-up (kirodotdev#2417).

Restart hardening: the in-memory binding map empties on restart, so the
first request that carries a VERIFIED peer identity re-claims the pin
for an unbound surviving cookie (SEL-audited); unbound cookies without a
verified peer keep today's semantics.

The re-pin lives in the shared pin check so the internal cookie-auth
branches get it too, and the CLI trust check refuses group/world-writable
paths for everyone and requires root ownership when running as root.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session pin is inert behind every documented tunnel (cloudflared / ngrok / tailscale serve)

2 participants