Skip to content

fix(security): bind UCAN JWT verification to the iss key, not the kid header#588

Merged
mikera merged 1 commit into
developfrom
security/ucan-jwt-issuer-binding
Jun 22, 2026
Merged

fix(security): bind UCAN JWT verification to the iss key, not the kid header#588
mikera merged 1 commit into
developfrom
security/ucan-jwt-issuer-binding

Conversation

@mikera

@mikera mikera commented Jun 22, 2026

Copy link
Copy Markdown
Member

Vulnerability

UCAN.fromJWT verified the EdDSA signature against the public key named in the attacker-controlled kid header, then trusted the iss claim without checking the two agree. An attacker could sign a token with their own key, place that key in kid, and claim any iss (e.g. a trusted root) — an issuer-spoofing authorization bypass.

The forged token passed validateJWT / parseTransportUCANs, and via UCANValidator.capabilitiesFor(..., issuer=root) yielded capabilities attributed to the root. Reachable from transport input (e.g. DlfsMcpTools).

Fix

Derive the verification key from the iss DID (did:key encodes the issuer's public key) and verify against it. The kid header is no longer trusted for key selection. Legitimately signed tokens are unaffected (kid already equals the iss key). The CAD3 path (UCAN.verifySignature / validate) was already correct.

Audit of other JWT verification paths

Path Verdict
UCAN.fromJWT was vulnerable → fixed
PeerAuth.verifySelfIssued safe — binds sub == did:key(kid) (covered by testSelfIssuedJWTKidMismatch, testBadlySignedJWTWithCorrectAudience)
PeerAuth.verifyPeerSigned safe (verifies against trusted peer key)
OAuthService (JWKS RS256) safe (kid selects among the provider's trusted keys; iss/aud validated)
JWKSKeys safe (parser only)

Added a SECURITY WARNING to the kid-trusting JWT.verifyEdDSA() / verifyPublic(jwt) to prevent reintroduction.

Adversarial tests

UCANTest now forges iss=ROOT signed by a rogue key and asserts rejection at fromJWT, validateJWT, the transport boundary (parseTransportUCANscapabilitiesFor), and inside a proof chain; plus a regression that genuine tokens still verify. These four were confirmed to fail against the pre-fix kid-based verification (the forged token was accepted as iss=ROOT granting can:"*"), then pass after the fix.

Testing

  • convex-core: 2131 pass
  • convex-peer (PeerAuthTest): 20 pass
  • convex-dlfs: 68 pass

🤖 Generated with Claude Code

… header

UCAN.fromJWT verified the EdDSA signature against the public key named in the
attacker-controlled `kid` header, then trusted the `iss` claim without checking
the two agree. An attacker could sign a token with their own key, place that key
in `kid`, and claim any `iss` (e.g. a trusted root) — an issuer-spoofing
authorization bypass. The forged token passed validateJWT / parseTransportUCANs,
and via UCANValidator.capabilitiesFor(..., issuer=root) yielded capabilities
attributed to the root. Reachable from transport input (e.g. DlfsMcpTools).

Fix: derive the verification key from the `iss` DID (did:key encodes the issuer
key) and verify against it; the `kid` header is no longer trusted for key
selection. Legitimately signed tokens are unaffected (kid == iss key already).
The CAD3 path (UCAN.verifySignature / validate) was already correct.

Audit of the other JWT verification paths:
- PeerAuth.verifySelfIssued: safe — binds sub to the signing key (sub == did:key(kid))
- PeerAuth.verifyPeerSigned / OAuthService (JWKS RS256) / JWKSKeys: safe
Added a SECURITY WARNING to the kid-trusting JWT.verifyEdDSA()/verifyPublic(jwt).

Adversarial tests (UCANTest): forge iss=ROOT signed by a rogue key and assert
rejection at fromJWT, validateJWT, the transport boundary, and in a proof chain;
plus a regression that genuine tokens still verify. Confirmed all four fail
against the pre-fix kid-based verification.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mikera mikera merged commit b58503b into develop Jun 22, 2026
2 checks passed
@mikera mikera deleted the security/ucan-jwt-issuer-binding branch June 22, 2026 13:26
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.

1 participant