fix(verify): reject tokens with an unsupported "crit" header parameter - #1041
Open
ArockiaRajamanickam wants to merge 1 commit into
Open
fix(verify): reject tokens with an unsupported "crit" header parameter#1041ArockiaRajamanickam wants to merge 1 commit into
ArockiaRajamanickam wants to merge 1 commit into
Conversation
RFC 7515 Section 4.1.11 says that when the JOSE header carries a "crit" member listing extension header parameters, those parameters MUST be understood and processed, and that if any of them are not understood and supported by the recipient then the JWS is invalid. This library implements no "crit" extension, but jwt.verify ignored the member entirely and returned the payload as though nothing had been marked critical. The result is a cross-implementation interpretation split rather than a signature bypass: the sharpest case is RFC 7797, where b64:false with crit:["b64"] means the payload segment is not base64url encoded, so a conformant verifier reads a different payload out of the same signed token than this library does. jwt.verify now rejects any token that carries a "crit" header. Because the library supports zero crit extensions, this removes no capability it ever offered; it turns silent misinterpretation into an explicit error. Fixes auth0#1032
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.
Fixes #1032.
RFC 7515 Section 4.1.11 says that if a JWS header carries
crit, and the recipient does not understand and support every extension parameter listed there, the JWS is invalid.jsonwebtokenimplements nocrithandling. The string does not appear inverify.js,sign.js,decode.jsor the test suite, sojwt.verify()returns a payload for a token the issuer explicitly marked as requiring extensions this library does not implement.Observed on the current default branch:
The
b64case is the sharpest one. Under RFC 7797,b64:falsewithcrit:['b64']means the payload is not base64url-encoded. A conformant verifier either rejects the token or reads a different payload than this library does — so two implementations can disagree about what was signed, which is exactly whatcritexists to prevent.The change
Eight lines in
verify.js, immediately after the header is decoded and before key resolution, so a caller'sgetSecretcallback is not invoked for a token that is already invalid.Behaviour change, stated plainly
Tokens carrying any
critheader now fail verification withJsonWebTokenError: unsupported "crit" header parameter, where they previously verified. That is what the RFC requires of a recipient with no extension support, but it is a real behaviour change:crittokens through this library alone will start seeing failures. Their tokens were never interoperable with a conformant verifier, but they did round-trip here.jwt.sign()is unchanged — it will still emit acritheader if you hand it one. I left that alone deliberately: rejecting on sign is a separate decision, and this issue is about verification. Happy to add it if you want the symmetry.jwt.decode()is unchanged, since it performs no verification. There is a test asserting that.One correction to the issue's framing
The issue implies the library silently mis-decodes a genuinely RFC 7797 unencoded-payload token. It does not — I hand-built one and
jsonwebtokenfails on it earlier, in its own parsing. The real defect is narrower and simpler:critis ignored entirely, so a token asserting critical extensions is accepted as though it made no such assertion. That is still a spec violation, just not the decoding one described.Tests
New
test/header-crit.test.js, 12 cases:b64, and foralg(which a producer is not permitted to mark critical).[], a bare string, a number,null,{}.complete: truecan expose the payload.critstill verify, including ones carrying unrecognised headers that are simply not marked critical — this is the guard against over-rejecting.jwt.decode()still returns the header and payload.Reverting only
verify.jsand keeping the tests fails them; with the change they pass. Full suite: 511 passing on a clean checkout, 523 with this branch, the difference being exactly these tests. README's error list updated.I used an AI assistant while working on this. The reproduction, the RFC 7797 correction above and the decision to leave
sign()alone are mine.