Repository navigation
fix(verifier): bound how long RemoteKeys serves a stale key when the JWKS endpoint is down - #12
Closed
terraboops wants to merge 1 commit into
Closed
terraboops wants to merge 1 commit into
terraboops wants to merge 1 commit into
Conversation
… endpoint is down
RemoteKeys served a cached key indefinitely whenever a refresh failed ("stale
but usable beats failing closed"), so a rotated or revoked key stayed trusted
for as long as the JWKS endpoint was unreachable — the cache never gave up.
Add MaxStale, an absolute staleness ceiling (default 1h, and it must not exceed
the signer's key overlap window). Within TTL a set is served freely; past TTL it
is served only until it reaches MaxStale, after which the key is refused even
though the endpoint is still down. A successful refresh resets the clock.
Regression test drives an httptest JWKS endpoint, takes it down, and asserts the
stale key still serves within the ceiling but is refused (ErrNoKey) past it, and
serves again after recovery; it fails without the ceiling (the past-ceiling key
is still served).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
terraboops
force-pushed
the
fix/remotekeys-staleness-ceiling
branch
from
September 27, 2026 08:52
a45fbf2 to
30956ed
Compare
terraboops
added a commit
that referenced
this pull request
Sep 28, 2026
* fix(verifier): bound how long RemoteKeys serves a stale key while the endpoint is down
RemoteKeys served a cached key indefinitely whenever a refresh failed ("stale
but usable beats failing closed"), so a rotated or revoked key stayed trusted
for as long as the JWKS endpoint was unreachable — the cache never gave up.
Add MaxStale, an absolute staleness ceiling (default 1h, and it must not exceed
the signer's key overlap window). Within TTL a set is served freely; past TTL it
is served only until it reaches MaxStale, after which the key is refused even
though the endpoint is still down. A successful refresh resets the clock.
Regression test drives an httptest JWKS endpoint, takes it down, and asserts the
stale key still serves within the ceiling but is refused (ErrNoKey) past it, and
serves again after recovery; it fails without the ceiling (the past-ceiling key
is still served).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* test(fuzz): fuzz the claims, key-store, route-file, and JWKS parsers
Adds no-panic fuzz targets for the untrusted-input parsers beyond the existing
token-parser fuzzer:
- FuzzClaimsUnmarshal — Claims.UnmarshalJSON (custom Extra collection) + re-marshal
- FuzzUnmarshalEntries — the shared key store a peer replica controls
- FuzzRouteFileParse — the rendered route file the data plane mounts
- FuzzJWKSToPublic — a JWKS a backend fetches from a signer, per-key jwkToPublic
Each asserts only that no input panics. Ran 20-60s per target (millions of
execs, hundreds of interesting inputs); all clean, no crashers. Seeds cover the
shapes that trip hand-written parsers (wrong types, null, truncated, non-base64,
int64 extremes).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* ci: run the parser fuzzers on every change
Wire the claims, key-store, route-file, and JWKS fuzz targets into the ci `test`
job next to the token-parser fuzzer, 20s each, so a parser that starts panicking
on hostile input is caught in CI rather than in production.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* fix(tsnetid): refuse a request whose source is not a tailnet address
Identity is the WireGuard peer read via WhoIs on r.RemoteAddr, so the whole
model assumes the connection arrives directly from a tailnet peer. If something
terminates the connection in front of this node — an L7 proxy, or Tailscale
Funnel — r.RemoteAddr is that fronting layer, not the caller, and the request
would be attributed to it.
WhoIs already fails closed for an address that is not a proven peer, but make
the fronting case explicit and early: reject a source outside Tailscale's
address space (100.64.0.0/10 and fd7a:115c:a1e0::/48) before calling WhoIs,
with an error that names the likely cause.
This does not (and cannot) catch a fronting layer that is itself a tailnet
node; the design's rule that the proxy must be its own node, not sit behind
one, still stands (SECURITY.md). Test covers the address classification;
without the guard the helper does not exist.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* ci(secrets): exclude tsnetid from the tailnet-address check
tsnetid now defines the CGNAT range for the source-address guard and uses
address literals in its tests, so the "no tailnet addresses" grep flags its own
legitimate code. Exclude the tsnetid package from the scan; the check still
guards examples, manifests and docs where a real node address does not belong.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* feat(gateway): report Accepted status on each attached HTTPRoute
The controller logged a rejected-route count but never wrote status back to the
HTTPRoute, so an author could not tell from `kubectl describe httproute`
whether their route was accepted or why it was refused — the existing
routeStatusCondition helper was unused.
After translating a Gateway's routes, set this controller's parent status on
every attached route: Accepted=True when it serves, or Accepted=False with the
translation error's reason (e.g. RefNotPermitted, UnsupportedValue) otherwise.
Status writes are idempotent (unchanged conditions keep their transition time
and skip the update) and never fail the reconcile.
Regression test drives the real Reconcile through the fake client and asserts a
good route gets Accepted=True and a cross-namespace-backendRef route gets
Accepted=False/RefNotPermitted; it fails without the status wiring.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
* fix(gateway): evaluate allowedRoutes per matching listener
The allowedRoutes namespace check was per-Gateway: attachedTo admitted a route
if *any* listener's allowedRoutes permitted its namespace, while hostnamesFor
intersected the route's hostnames against *all* listeners. On a Gateway with a
From=All listener and a From=Same listener owning different hostnames, a route
from another namespace — admitted only by the From=All listener — could still
serve the From=Same listener's hostname, borrowing a name that listener would
never have admitted it for.
Evaluate the namespace policy per listener: admittingListeners returns the
listeners whose allowedRoutes admit the route's namespace, attachedTo requires
at least one, and hostnamesFor restricts the route's servable hostnames to
exactly those listeners. A route on a From=All listener still serves that
listener's hostname; it can no longer reach a stricter listener's.
Regression test builds a mixed-listener Gateway and asserts a foreign-namespace
route may serve the open listener's hostname but not the strict listener's,
while a same-namespace route may serve the strict one; it fails without the
change (the foreign route serves the strict hostname).
Closes adversarial finding B.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Superseded: this content landed on main via #17 (untangling the audit-fix stack). The RemoteKeys staleness ceiling is on main now. |
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
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.
What
RemoteKeysserved a cached key indefinitely whenever a refresh failed (the "stale but usable beats failing closed" branch). So if the signer's JWKS endpoint stayed down, a rotated or revoked key remained trusted forever — the cache never gave up.Fix
New
MaxStalefield — an absolute staleness ceiling (default 1h; must not exceed the signer's key overlap window). WithinTTLa set is served freely; pastTTLit is served only until it reachesMaxStale, after which the key is refused even though the endpoint is still down.fetchedAtonly advances on a successful refresh, so a successful fetch resets the clock.Test (fails without the fix)
verifier.TestRemoteKeysStalenessCeilinguses anhttptestJWKS endpoint, takes it down, and asserts: stale key still serves within the ceiling (t+30m), is refused withErrNoKeypast it (t+2h), and serves again after recovery. Reverting only the ceiling logic makes it fail exactly at "a cached key past MaxStale was still served".Scope
Availability↔freshness trade-off, now bounded. Default preserves today's behavior for the first hour; deployments should set
MaxStale≤ their key overlap window. Full suite green.No merge — review only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SDfMmeRXGsanqzDDSjL1uU