fix(jwt-svid): typ header on issued tokens + algorithm allowlist before parse#113
Closed
fix(jwt-svid): typ header on issued tokens + algorithm allowlist before parse#113
Conversation
…re parse Two JWT-SVID §3 compliance gaps in the issuance + verification paths. #46 — typ JOSE header on issued tokens. Issued JWTs lacked the typ="JWT" JOSE header, breaking spec-compliant verifiers that distinguish token kinds via the header without parsing claims. Set typ="JWT" on the protected header for both the ES256 path (NHI/agent flows) and the RS256 path (api_key SDK/human flows). #45 — algorithm allowlist BEFORE jwt.Parse. The Verifier already pinned the allowlist to {RS256, ES256} via checkAlgorithm(), but the check ran AFTER jwt.Parse. JWT-SVID §3 requires the alg peek to happen first so a token with alg=none or an HS256-confusion attempt is rejected at the JWS layer before any parse logic touches it. Moved the call earlier in verify(); the allowlist itself is unchanged. Tests: - pkg/authjwt: TestVerifyRejectsAlgNone, TestVerifyRejectsHS256 - tests/integration: TestIssuedES256TokenHasTypHeader, TestIssuedRS256TokenHasTypHeader Sibling issues: - #42 (default aud to issuer) — already fixed by PR #85. - #43 (use="JWT-SVID") — deferred. lestrrat-go/jwx v2's AlgorithmsForKey rejects keys with use != "" or "sig", which would break pkg/authjwt and every Go consumer of our JWKS. Tracked as the jwx-compatibility blocker on #43; future options include patching jwx upstream, switching our verifier to manual key resolution, or splitting internal vs. published key sets. The addToKeySet helper now takes a use parameter so the eventual fix is a one-line change at each call site. Branched off PR #93 (fix-attestation-verifier-oidc) so it can rebase cleanly the moment #93 lands on main.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements security enhancements and spec compliance for JWT-SVID. Key changes include adding the typ: "JWT" header to issued tokens, explicitly setting the key usage to "sig" in JWKS for library compatibility, and moving algorithm validation to occur before token parsing to prevent alg: none and algorithm-confusion attacks. New unit and integration tests verify these improvements. I have no feedback to provide.
10 tasks
Contributor
Author
|
Closing in favor of #103 (alg allowlist) + #104 ( #103 gates every caller-supplied JWT parse via a reusable
#104 sets Net: closing this PR forfeits no coverage; everything it added is in #103 + #104. Thanks @safayavatsal. Stacked PR #114 (jwx v4 upgrade) reparented onto |
rsharath
added a commit
that referenced
this pull request
May 5, 2026
Migrates the entire zeroid + pkg/authjwt jwx surface (211 call sites across 19 files) to jwx v4.0.1 and bumps the language target to Go 1.26. - jwx v2 is in maintenance only; v4 is the current upstream release. - v4's API (generic Import/Get accessors, struct error types, Token getters returning (value, present)) is a forcing function for the same idioms we already prefer in newer code. - Future work like #43 (use="JWT-SVID" on JWKS) needs a more flexible verifier path; v4 narrows the migration cost when that lands. - jwa.ES256 / RS256 / HS256 are now functions; every call site adds (). - jwk.FromRaw → jwk.Import[jwk.Key] (interface-typed import returns the matching concrete jwk.Key based on the raw key shape). - jwk.Fetch + jwk.WithHTTPClient removed (companion module). Replaced with manual http.Client GET + jwk.Parse, body-bounded to 1 MiB so a malicious or compromised JWKS endpoint can't exhaust process memory. - Token accessors (Subject, Issuer, Audience, JwtID, Expiration, IssuedAt) now return (value, present); call sites destructure. - Token.Get / Token.Iterate removed → jwt.Get[T] generic accessor + Token.Keys() iteration. - jws.Headers.Algorithm() / KeyID() / Type() return (value, present). - jwt.ErrTokenExpired() etc. removed → typed structs (jwt.TokenExpiredError{}, jwt.InvalidIssuerError{}, jwt.InvalidAudienceError{}) used with errors.Is. - Go directive: 1.25.8 → 1.26 in both go.mod files. - GO_VER bumped to 1.26.0 in pr-check, release, and sdk-integration workflows; GOEXPERIMENT=jsonv2 added at the workflow env level so every step inherits it (jwx v4 requires the encoding/json/v2 stack). - Dockerfile: golang:1.25.8-alpine3.22 → golang:1.26.0-alpine3.22 + ENV GOEXPERIMENT=jsonv2 on the build stage. - Makefile: export GOEXPERIMENT=jsonv2 so every target inherits it. The jwx v4 use="JWT-SVID" path was hoped to unblock issue #43, but the filter that v2 had in jwa.AlgorithmsForKey was relocated rather than removed — v4's jws/key_provider.go::keySetProvider.selectKey (lines 115–121) hardcodes the same check that rejects keys whose use is anything outside {"", "sig"}. Comment in internal/signing/jwks.go::addToKeySet now points at the new location. Unblocking #43 still needs either an upstream jwx PR or our verifier to swap WithKeySet for manual per-kid lookup; both are out of scope for this PR. - pkg/authjwt: full suite green (race, count=1). - tests/integration: full suite green against the testcontainers Postgres fixture (60+ integration tests, count=1). - go vet ./... clean on both modules. Builds on top of #113 (PR-Z0). Once #113 merges, rebase to drop the duplicate commit and this PR rebases cleanly onto main.
rsharath
added a commit
that referenced
this pull request
May 5, 2026
Migrates the entire zeroid + pkg/authjwt jwx surface (211 call sites across 19 files) to jwx v4.0.1 and bumps the language target to Go 1.26. - jwx v2 is in maintenance only; v4 is the current upstream release. - v4's API (generic Import/Get accessors, struct error types, Token getters returning (value, present)) is a forcing function for the same idioms we already prefer in newer code. - Future work like #43 (use="JWT-SVID" on JWKS) needs a more flexible verifier path; v4 narrows the migration cost when that lands. - jwa.ES256 / RS256 / HS256 are now functions; every call site adds (). - jwk.FromRaw → jwk.Import[jwk.Key] (interface-typed import returns the matching concrete jwk.Key based on the raw key shape). - jwk.Fetch + jwk.WithHTTPClient removed (companion module). Replaced with manual http.Client GET + jwk.Parse, body-bounded to 1 MiB so a malicious or compromised JWKS endpoint can't exhaust process memory. - Token accessors (Subject, Issuer, Audience, JwtID, Expiration, IssuedAt) now return (value, present); call sites destructure. - Token.Get / Token.Iterate removed → jwt.Get[T] generic accessor + Token.Keys() iteration. - jws.Headers.Algorithm() / KeyID() / Type() return (value, present). - jwt.ErrTokenExpired() etc. removed → typed structs (jwt.TokenExpiredError{}, jwt.InvalidIssuerError{}, jwt.InvalidAudienceError{}) used with errors.Is. - Go directive: 1.25.8 → 1.26 in both go.mod files. - GO_VER bumped to 1.26.0 in pr-check, release, and sdk-integration workflows; GOEXPERIMENT=jsonv2 added at the workflow env level so every step inherits it (jwx v4 requires the encoding/json/v2 stack). - Dockerfile: golang:1.25.8-alpine3.22 → golang:1.26.0-alpine3.22 + ENV GOEXPERIMENT=jsonv2 on the build stage. - Makefile: export GOEXPERIMENT=jsonv2 so every target inherits it. The jwx v4 use="JWT-SVID" path was hoped to unblock issue #43, but the filter that v2 had in jwa.AlgorithmsForKey was relocated rather than removed — v4's jws/key_provider.go::keySetProvider.selectKey (lines 115–121) hardcodes the same check that rejects keys whose use is anything outside {"", "sig"}. Comment in internal/signing/jwks.go::addToKeySet now points at the new location. Unblocking #43 still needs either an upstream jwx PR or our verifier to swap WithKeySet for manual per-kid lookup; both are out of scope for this PR. - pkg/authjwt: full suite green (race, count=1). - tests/integration: full suite green against the testcontainers Postgres fixture (60+ integration tests, count=1). - go vet ./... clean on both modules. Builds on top of #113 (PR-Z0). Once #113 merges, rebase to drop the duplicate commit and this PR rebases cleanly onto main.
rsharath
added a commit
that referenced
this pull request
May 5, 2026
Migrates the entire zeroid + pkg/authjwt jwx surface (211 call sites across 19 files) to jwx v4.0.1 and bumps the language target to Go 1.26. - jwx v2 is in maintenance only; v4 is the current upstream release. - v4's API (generic Import/Get accessors, struct error types, Token getters returning (value, present)) is a forcing function for the same idioms we already prefer in newer code. - Future work like #43 (use="JWT-SVID" on JWKS) needs a more flexible verifier path; v4 narrows the migration cost when that lands. - jwa.ES256 / RS256 / HS256 are now functions; every call site adds (). - jwk.FromRaw → jwk.Import[jwk.Key] (interface-typed import returns the matching concrete jwk.Key based on the raw key shape). - jwk.Fetch + jwk.WithHTTPClient removed (companion module). Replaced with manual http.Client GET + jwk.Parse, body-bounded to 1 MiB so a malicious or compromised JWKS endpoint can't exhaust process memory. - Token accessors (Subject, Issuer, Audience, JwtID, Expiration, IssuedAt) now return (value, present); call sites destructure. - Token.Get / Token.Iterate removed → jwt.Get[T] generic accessor + Token.Keys() iteration. - jws.Headers.Algorithm() / KeyID() / Type() return (value, present). - jwt.ErrTokenExpired() etc. removed → typed structs (jwt.TokenExpiredError{}, jwt.InvalidIssuerError{}, jwt.InvalidAudienceError{}) used with errors.Is. - Go directive: 1.25.8 → 1.26 in both go.mod files. - GO_VER bumped to 1.26.0 in pr-check, release, and sdk-integration workflows; GOEXPERIMENT=jsonv2 added at the workflow env level so every step inherits it (jwx v4 requires the encoding/json/v2 stack). - Dockerfile: golang:1.25.8-alpine3.22 → golang:1.26.0-alpine3.22 + ENV GOEXPERIMENT=jsonv2 on the build stage. - Makefile: export GOEXPERIMENT=jsonv2 so every target inherits it. The jwx v4 use="JWT-SVID" path was hoped to unblock issue #43, but the filter that v2 had in jwa.AlgorithmsForKey was relocated rather than removed — v4's jws/key_provider.go::keySetProvider.selectKey (lines 115–121) hardcodes the same check that rejects keys whose use is anything outside {"", "sig"}. Comment in internal/signing/jwks.go::addToKeySet now points at the new location. Unblocking #43 still needs either an upstream jwx PR or our verifier to swap WithKeySet for manual per-kid lookup; both are out of scope for this PR. - pkg/authjwt: full suite green (race, count=1). - tests/integration: full suite green against the testcontainers Postgres fixture (60+ integration tests, count=1). - go vet ./... clean on both modules. Builds on top of #113 (PR-Z0). Once #113 merges, rebase to drop the duplicate commit and this PR rebases cleanly onto main.
rsharath
added a commit
that referenced
this pull request
May 5, 2026
Migrates the entire zeroid + pkg/authjwt jwx surface (211 call sites across 19 files) to jwx v4.0.1 and bumps the language target to Go 1.26. - jwx v2 is in maintenance only; v4 is the current upstream release. - v4's API (generic Import/Get accessors, struct error types, Token getters returning (value, present)) is a forcing function for the same idioms we already prefer in newer code. - Future work like #43 (use="JWT-SVID" on JWKS) needs a more flexible verifier path; v4 narrows the migration cost when that lands. - jwa.ES256 / RS256 / HS256 are now functions; every call site adds (). - jwk.FromRaw → jwk.Import[jwk.Key] (interface-typed import returns the matching concrete jwk.Key based on the raw key shape). - jwk.Fetch + jwk.WithHTTPClient removed (companion module). Replaced with manual http.Client GET + jwk.Parse, body-bounded to 1 MiB so a malicious or compromised JWKS endpoint can't exhaust process memory. - Token accessors (Subject, Issuer, Audience, JwtID, Expiration, IssuedAt) now return (value, present); call sites destructure. - Token.Get / Token.Iterate removed → jwt.Get[T] generic accessor + Token.Keys() iteration. - jws.Headers.Algorithm() / KeyID() / Type() return (value, present). - jwt.ErrTokenExpired() etc. removed → typed structs (jwt.TokenExpiredError{}, jwt.InvalidIssuerError{}, jwt.InvalidAudienceError{}) used with errors.Is. - Go directive: 1.25.8 → 1.26 in both go.mod files. - GO_VER bumped to 1.26.0 in pr-check, release, and sdk-integration workflows; GOEXPERIMENT=jsonv2 added at the workflow env level so every step inherits it (jwx v4 requires the encoding/json/v2 stack). - Dockerfile: golang:1.25.8-alpine3.22 → golang:1.26.0-alpine3.22 + ENV GOEXPERIMENT=jsonv2 on the build stage. - Makefile: export GOEXPERIMENT=jsonv2 so every target inherits it. The jwx v4 use="JWT-SVID" path was hoped to unblock issue #43, but the filter that v2 had in jwa.AlgorithmsForKey was relocated rather than removed — v4's jws/key_provider.go::keySetProvider.selectKey (lines 115–121) hardcodes the same check that rejects keys whose use is anything outside {"", "sig"}. Comment in internal/signing/jwks.go::addToKeySet now points at the new location. Unblocking #43 still needs either an upstream jwx PR or our verifier to swap WithKeySet for manual per-kid lookup; both are out of scope for this PR. - pkg/authjwt: full suite green (race, count=1). - tests/integration: full suite green against the testcontainers Postgres fixture (60+ integration tests, count=1). - go vet ./... clean on both modules. Builds on top of #113 (PR-Z0). Once #113 merges, rebase to drop the duplicate commit and this PR rebases cleanly onto main.
rsharath
added a commit
that referenced
this pull request
May 5, 2026
Migrates the entire zeroid + pkg/authjwt jwx surface (211 call sites across 19 files) to jwx v4.0.1 and bumps the language target to Go 1.26. - jwx v2 is in maintenance only; v4 is the current upstream release. - v4's API (generic Import/Get accessors, struct error types, Token getters returning (value, present)) is a forcing function for the same idioms we already prefer in newer code. - Future work like #43 (use="JWT-SVID" on JWKS) needs a more flexible verifier path; v4 narrows the migration cost when that lands. - jwa.ES256 / RS256 / HS256 are now functions; every call site adds (). - jwk.FromRaw → jwk.Import[jwk.Key] (interface-typed import returns the matching concrete jwk.Key based on the raw key shape). - jwk.Fetch + jwk.WithHTTPClient removed (companion module). Replaced with manual http.Client GET + jwk.Parse, body-bounded to 1 MiB so a malicious or compromised JWKS endpoint can't exhaust process memory. - Token accessors (Subject, Issuer, Audience, JwtID, Expiration, IssuedAt) now return (value, present); call sites destructure. - Token.Get / Token.Iterate removed → jwt.Get[T] generic accessor + Token.Keys() iteration. - jws.Headers.Algorithm() / KeyID() / Type() return (value, present). - jwt.ErrTokenExpired() etc. removed → typed structs (jwt.TokenExpiredError{}, jwt.InvalidIssuerError{}, jwt.InvalidAudienceError{}) used with errors.Is. - Go directive: 1.25.8 → 1.26 in both go.mod files. - GO_VER bumped to 1.26.0 in pr-check, release, and sdk-integration workflows; GOEXPERIMENT=jsonv2 added at the workflow env level so every step inherits it (jwx v4 requires the encoding/json/v2 stack). - Dockerfile: golang:1.25.8-alpine3.22 → golang:1.26.0-alpine3.22 + ENV GOEXPERIMENT=jsonv2 on the build stage. - Makefile: export GOEXPERIMENT=jsonv2 so every target inherits it. The jwx v4 use="JWT-SVID" path was hoped to unblock issue #43, but the filter that v2 had in jwa.AlgorithmsForKey was relocated rather than removed — v4's jws/key_provider.go::keySetProvider.selectKey (lines 115–121) hardcodes the same check that rejects keys whose use is anything outside {"", "sig"}. Comment in internal/signing/jwks.go::addToKeySet now points at the new location. Unblocking #43 still needs either an upstream jwx PR or our verifier to swap WithKeySet for manual per-kid lookup; both are out of scope for this PR. - pkg/authjwt: full suite green (race, count=1). - tests/integration: full suite green against the testcontainers Postgres fixture (60+ integration tests, count=1). - go vet ./... clean on both modules. Builds on top of #113 (PR-Z0). Once #113 merges, rebase to drop the duplicate commit and this PR rebases cleanly onto main.
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
1 participant
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.
Two JWT-SVID §3 compliance gaps in the issuance + verification paths.
#46 — typ JOSE header on issued tokens
Issued JWTs lacked the
typ="JWT"JOSE header, breaking spec-compliant verifiers that distinguish token kinds via the header without parsing claims. Settyp="JWT"on the protected header for both the ES256 path (NHI/agent flows) and the RS256 path (api_key SDK/human flows).#45 — algorithm allowlist BEFORE jwt.Parse
The Verifier already pinned the allowlist to
{RS256, ES256}viacheckAlgorithm(), but the check ran AFTERjwt.Parse. JWT-SVID §3 requires the alg peek to happen first so a token withalg=noneor an HS256-confusion attempt is rejected at the JWS layer before any parse logic touches it. Moved the call earlier inverify(); the allowlist itself is unchanged.Tests
pkg/authjwt:TestVerifyRejectsAlgNone,TestVerifyRejectsHS256tests/integration:TestIssuedES256TokenHasTypHeader,TestIssuedRS256TokenHasTypHeaderSibling issues — status
audto issuer) — already fixed by PR fix: populate aud claim on every issued token (JWT-SVID section3) #85; not part of this PR.use="JWT-SVID"on JWKS) — deferred. lestrrat-go/jwx v2'sAlgorithmsForKeyrejects keys withuse != "" or "sig", which would break our ownpkg/authjwtverifier and every Go consumer of our JWKS. Tracked as a jwx-compatibility blocker on JWT-SVID: JWKS use parameter is "sig" instead of "jwt-svid" #43; theaddToKeySethelper now takes auseparameter so the eventual fix is a one-line change at each call site once we upgrade jwx (next PR).Test plan
go test ./...(full integration suite) — greengo test ./...inpkg/authjwt/— greengo vet ./...— cleanmain(post replace attestation trust-promotion stub with pluggable verifiers (OIDC) #93 merge); replays cleanly as a single commit.