Skip to content

feat(gateway): transparent auth for verified tailnet peers - #6652

Open
nodomain wants to merge 1 commit into
kirodotdev:mainfrom
nodomain:feat/tailnet-transparent-auth
Open

feat(gateway): transparent auth for verified tailnet peers#6652
nodomain wants to merge 1 commit into
kirodotdev:mainfrom
nodomain:feat/tailnet-transparent-auth

Conversation

@nodomain

@nodomain nodomain commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements transparent authentication for verified tailnet peers, closing #6132.

When trust_identity is enabled and allowed_logins is non-empty, a request arriving through tailscale serve from a daemon-verified peer on the allowlist now receives a boot-bound session cookie directly, without requiring a prior token login.

Changes

src/kiro_crew/dashboard/token_auth.py — New code path in the auth middleware, inserted between the logout bypass and the token-extraction block:

  1. Detects credential-less requests (no query token, no mc_token_*/mc_refresh_* cookies)
  2. When tailnet trust is configured, resolves the forwarded peer via resolve_forwarded_peer()
  3. Checks the allowlist via login_allowed() — denies if not listed
  4. Mints a boot-bound session token (with boot claim = current_boot_id()) so the session dies on gateway restart
  5. Binds to the peer identity key via bind_token_peer() and sets the session cookie + refresh chain
  6. Subsequent requests carry the cookie and take the normal (faster) path

test/test_token_auth.py — Updated existing test_credential_less_request_never_reaches_the_daemon (now scoped to trust_identity=False) and added 4 new tests:

  • test_transparent_auth_issues_session_cookie — happy path
  • test_transparent_auth_denies_peer_not_on_allowlist — deny path
  • test_transparent_auth_skipped_when_credential_present — normal flow preserved
  • test_transparent_auth_skipped_when_no_allowlist — guard condition

Security model

  • Session is boot-bound (same as QR phone-access sessions from PR feat(dashboard): scope a phone's dashboard session to the gateway process #5763) — gateway restart invalidates it
  • Session is peer-pinned via peer_pin_key() — replay from a different node is rejected
  • Refresh chain carries require_peer=True so rotation preserves the binding
  • All conditions must be met: trust enabled + allowlist non-empty + peer verified + peer on list

Testing

326 tests pass (264 token_auth + 62 tailnet_peer), including 5 new/updated tests covering the transparent auth paths.

@nodomain
nodomain requested a review from a team as a code owner August 28, 2026 22:19
@nodomain
nodomain requested a review from hoang-phan98 August 28, 2026 22:19
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running labels Aug 28, 2026
@nodomain
nodomain force-pushed the feat/tailnet-transparent-auth branch from 9df6b25 to 60e3faa Compare August 28, 2026 22:29
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

1 similar comment
@dwu96

dwu96 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

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

When trust_identity is enabled and the request arrives from a daemon-verified
peer on the allowed_logins list, issue a boot-bound session cookie directly
without requiring a prior token login. The session is pinned to the peer
identity and dies on gateway restart (same security model as QR sessions).

Closes kirodotdev#6132
@nodomain
nodomain force-pushed the feat/tailnet-transparent-auth branch from 60e3faa to 6ac1564 Compare August 29, 2026 05:44
@dwu96

dwu96 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

1 similar comment
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 29, 2026
@bolichen97
bolichen97 enabled auto-merge August 29, 2026 23:57

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description / code mismatch

The Description presents this as a session granted to a "daemon-verified peer" arriving "through tailscale serve", but the code grants it to any caller that can reach the loopback port and name an allowlisted tailnet address in a forwarded header, and it lands that new admission path without amending the RFC that lists it as a non-goal or the spec that documents token auth as the only way a session starts.

1. Transparent auth makes a forwarded-header address the sole credential, so any local process can obtain a full dashboard session

The Description says

When trust_identity is enabled and allowed_logins is non-empty, a request arriving through tailscale serve from a daemon-verified peer on the allowlist now receives a boot-bound session cookie directly, without requiring a prior token login.

and, under Security model:

All conditions must be met: trust enabled + allowlist non-empty + peer verified + peer on list

The code does — the new block at src/kiro_crew/dashboard/token_auth.py:2447-2572 runs on a request that presents no credential at all. Its gate is src/kiro_crew/dashboard/token_auth.py:2461-2466; identity is assigned at src/kiro_crew/dashboard/token_auth.py:2509-2512 and the session cookie is set at src/kiro_crew/dashboard/token_auth.py:2534-2542. "Arriving through tailscale serve" is not a condition the code can check: nothing about the request proves it came from the proxy. What resolve_forwarded_peer() actually requires (src/kiro_crew/dashboard/tailnet.py:1052-1106) is a loopback immediate peer, exactly one X-Forwarded-For value, that value inside the tailnet ranges, and a tailscale whois that resolves it. Every one of those is satisfiable by any local process: the immediate peer is loopback for any local connection, and the header is attacker-chosen. "Peer verified" therefore means "the address the caller typed resolves in the daemon", not "the caller is that peer".

Two further divergences from the stated model:

  • The Description says the block sits "between the logout bypass and the token-extraction block". That placement is what defeats the existing invariant: main only resolves a peer once a request presents a credential, and the reason is stated in-code at src/kiro_crew/dashboard/token_auth.py:2133-2142 — "a credential-less request (static assets, probes) can never bind or satisfy a pin, so resolving identity for it would only hand an unauthenticated local caller a header-driven daemon spawn". That gate (src/kiro_crew/dashboard/token_auth.py:2129-2151) is untouched by the diff; the new block simply reaches resolve_forwarded_peer before it, so an unauthenticated caller can now spray distinct addresses on any non-bypassed /api path and force one tailscale whois subprocess per cache miss, each holding _whois_lock and a subprocess_executor() worker for the daemon timeout. The Description does not mention that this changes the credential-less contract.
  • The block re-spells trust_identity and allowed_logins inline instead of asking TailnetTrust.enforces_identity, whose documented purpose is the opposite (src/kiro_crew/dashboard/tailnet.py:927-935): "The one predicate every gate asks, so 'may this be pinned', 'may this rotate' and 'may this authenticate' cannot answer differently."

Risk — unauthenticated local auth bypass. With trust_identity on and a non-empty allowlist, any process that can open a TCP connection to the gateway's loopback port — a second user on a shared host, a malicious npm/pip postinstall running as the operator, any local service reachable through socat or ssh -R — sends one credential-less request with X-Forwarded-For: <allowlisted device's 100.x address> (readable locally from tailscale status) and receives a session cookie as that login with is_dashboard_user=True. The boot binding and peer pin do not help: the cookie is pinned to the same forged identity the caller keeps asserting, so it satisfies _extract_and_validate_token plus _check_pin on the next hop and /api/chat, /api/spawn and the rest of the agent surface follow.

Required change — do not let the forwarded-header identity be the only credential. Either keep a bearer in the loop the way the shipped phone-access flow does (docs/system-specs/features/dashboard-token-auth.md:148-152: "The initial QR URL is a claimless, five-minute enrollment bearer"), or make the Serve transport itself provable — a dedicated listener or unix socket the proxy alone connects to, or a secret only the proxy holds — before granting. If the intended trust model really is "any loopback caller that can name an allowlisted tailnet address is that user", state that plainly in the Description and in the spec rather than describing it as a verified peer arriving through tailscale serve.

2. A new admission path lands against an RFC non-goal, with no spec or RFC amendment and no audit-vocabulary entry

The Description says

src/kiro_crew/dashboard/token_auth.py — New code path in the auth middleware, inserted between the logout bypass and the token-extraction block

The code does — it adds a fifth way to obtain a dashboard session, and the RFC this feature comes from lists that as out of scope. docs/request-for-change/rfc-tailnet-dashboard-access.md:176-177, § Non-goals: "Replacing token auth. Identity trust is an additional, opt-in pin — the token path remains the only path when identity cannot be established", with goal 4 at docs/request-for-change/rfc-tailnet-dashboard-access.md:159: "Every failure mode falls back to the existing token path. Nothing this RFC adds can lock a user out or grant access on ambiguity." The PR makes identity trust an admission path rather than a pin, and amends neither the RFC nor the spec. The spec still tells the next reader that every session starts from a bearer — docs/system-specs/features/dashboard-token-auth.md:5: "An aiohttp middleware validates the token on every GATED request (query param or cookie fallback) and sets a session cookie on first use". The in-code invariant at src/kiro_crew/dashboard/token_auth.py:2133-2142 is reversed for the trust_identity + allowlist configuration, and the test that pinned it (test_credential_less_request_never_reaches_the_daemon) is rescoped to trust_identity=False rather than replaced by one that pins the new rule. Separately, the block emits a tailnet_transparent_auth SEL operation that exists nowhere else in the tree — main emits only tailnet_peer_auth (src/kiro_crew/dashboard/token_auth.py:2161, :2195) and tailnet_peer_bind (:2279, :2830) — and the spec's SEL table at docs/system-specs/features/dashboard-token-auth.md:650-657 stays silent on all three, so the audit vocabulary remains incomplete for a third tailnet operation.

Risk — the repository's same-commit spec obligation is not met for a change to the authentication contract. A reader of dashboard-token-auth.md still believes no session can begin without a bearer, the RFC still says the token path is the only path when identity cannot be established, and SEL alerting keyed on documented outcome names does not see tailnet_transparent_auth. The divergent local predicate is the drift enforces_identity exists to prevent, even though its current direction happens to be the safe one: a partially-parsed allowlist under identity_unknown with trust_identity off skips the block.

Required change — either amend docs/request-for-change/rfc-tailnet-dashboard-access.md and docs/system-specs/features/dashboard-token-auth.md in this same commit to document credential-less transparent auth — its preconditions, its threat model against a local forger of X-Forwarded-For, and the new tailnet_transparent_auth SEL rows — or drop the credential-less grant. Ask the shared tailnet_trust.enforces_identity predicate instead of re-deriving trust_identity and allowed_logins locally.

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

  • This PR is PARTIALLY_COVERED with PR #5763. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #6652: CONTINUE_DEVELOPMENT. Main already carries the session model the PR mints into, so only the credential-less admission step is genuinely new. That step is what the review blocks on. Files: src/kiro_crew/dashboard/token_auth.py.
  • This PR is PARTIALLY_COVERED with PR #6660. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #6652: CONTINUE_DEVELOPMENT. The motivating pain in Issue #6132 is now largely handled on main, which shrinks the benefit side of the PR's security trade-off. Worth settling on the issue before more code. Files: src/kiro_crew/dashboard/handlers/tailnet_mobile.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

@bolichen97

Copy link
Copy Markdown
Collaborator

@nodomain Thanks for keeping this open. Here is where it stands against current main.

Already landed: the session shape this PR mints into is on main, and more complete there than in this branch. #5763 added the boot-bound, peer-pinned phone session; #8617 bound refresh chains to the tailnet peer that opened them; #7433 made an unreadable login allowlist fail closed.

Still missing, and only this PR provides it: credential-less admission itself. Nothing on main implements it (tailnet_transparent_auth appears nowhere in src, test or docs), and Issue #6132 is still open, so this is not a candidate for closing.

Could you narrow the PR to that remaining piece and rebase? Three concrete blockers:

  1. The CHANGES_REQUESTED review still stands. resolve_forwarded_peer only needs a loopback immediate peer plus one X-Forwarded-For value that whois resolves, so the forwarded address becomes the sole credential and any local process can supply it. The block also runs before main's credential-less gate at src/kiro_crew/dashboard/token_auth.py:2139-2142, reversing that documented invariant.

  2. The rebase changes two calls. Use TailnetTrust.enforces_identity instead of the inline trust_identity and allowed_logins. Pass peer_key to generate_refresh_token(..., require_peer=True): on current main that call raises ValueError without it, and the bare except Exception here would swallow it, so the refresh chain would silently never mint.

  3. Coordinate the token-extraction seam with feat(acp): add gateway-backed ACP server #7415, which adds an X-Presigned-Token header. _has_credential here does not look at that header, so a header-only request would be read as credential-less.

Docs need a pass too: docs/request-for-change/rfc-tailnet-dashboard-access.md lists this as a non-goal, and docs/system-specs/features/dashboard-token-auth.md states the current invariant.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants