fix(jwt): #1074 — algorithm const ref + sign/verify dispatch#1089
Merged
Conversation
…literal-match
`jwt.sign(payload, key, { algorithm: X, ... })` silently fell back to HS256
(using the user's PEM as an HMAC secret) when `algorithm` was anything other
than an inline string literal: const-bound identifier, ternary, spread, or
when the entire options object was a const ref. Same mirror bug on
`jwt.verify`'s `algorithm` / `algorithms: [...]` option, where ES/RS tokens
silently failed verification.
This is a real cryptographic downgrade: the JWT was HMAC-signed with the
PEM's raw bytes, the on-wire `header.alg` claimed `"HS256"`, and any
verifier that accepts either HMAC or EC/RSA quietly accepted the
downgraded token.
Fix (approach mirrors #1076's runtime-dispatch fallback for
`crypto.createHmac`):
Stdlib (perry-stdlib/src/jsonwebtoken.rs):
- js_jwt_sign_dyn(alg, payload, secret, exp, kid): reads the algorithm
name from a runtime `*const StringHeader` and dispatches to the same
`sign_common` paths the typed `js_jwt_sign_es256` / `_rs256` helpers
use. Unknown algs fall back to HS256 with a PERRY_DEBUG log.
- js_jwt_verify_dyn(alg, token, secret): mirror for verify.
- js_jwt_sign_dyn_opts(payload, secret, opts_jsvalue): case where the
entire options object is a non-extractable expression (`const opts =
{...}; jwt.sign(p, k, opts)`). Extracts `algorithm` / `expiresIn` /
`keyid` from the NaN-boxed object at runtime via
`js_object_get_field_by_name`, then defers to `js_jwt_sign_dyn`.
- js_jwt_verify_dyn_opts: mirror, including `algorithms: [...]` plural
via `js_array_get`.
Codegen (crates/perry-codegen/src/lower_call/native.rs):
- lower_jsonwebtoken_sign: when `algorithm` is non-literal, lower its
value as a `*const StringHeader` via `get_raw_string_ptr` and route to
`js_jwt_sign_dyn`. When the whole options expression is not an
inline-object (or AnonShape), route to `js_jwt_sign_dyn_opts` with
the options JSValue as the third arg.
- lower_jsonwebtoken_verify: mirror, covering both singular `algorithm`
and `algorithms: [literal]` / `algorithms: [non-literal]` / non-array
`algorithms` shapes.
The inline-literal fast path is unchanged — common case still routes
directly to the typed helpers at compile time.
Validated byte-for-byte against `node --experimental-strip-types` in
test-files/test_jwt_sign_dynamic_alg.ts (cases A inline-literal,
B const-bound ident, C const-bound opts object, D verify with const-bound
algorithm). Existing #915 and #927 jwt regressions still pass.
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.
Closes #1074.
jwt.sign(payload, key, { algorithm: X, ... })silently fell back to HS256 (using the user's PEM as an HMAC secret) whenalgorithmwas anything other than an inline string literal — const-bound identifier, ternary, spread, or a const-bound options object. Mirror bug onjwt.verify'salgorithm/algorithms: [...]. Real cryptographic downgrade: tokens were HMAC-signed with the PEM bytes whileheader.algclaimed HS256, so any verifier accepting either family quietly accepted the downgrade.Approach (mirrors #1076 / PR #1080 for
crypto.createHmac)js_jwt_sign_dyn,js_jwt_verify_dyn)The fast path (
{ algorithm: \"ES256\" }inline literal) still picks the typedjs_jwt_sign_es256/_rs256symbol at compile time. Non-literal shapes lower the alg value as*const StringHeaderand call newjs_jwt_sign_dyn(alg, payload, secret, exp, kid), which dispatches to the samesign_commonpaths at runtime.For case C (whole options is a non-extractable expression), there's a second variant
js_jwt_sign_dyn_opts(payload, secret, opts_jsvalue)that extractsalgorithm/expiresIn/keyidfrom the NaN-boxed object at runtime viajs_object_get_field_by_namebefore deferring to_dyn. Same shape for verify, includingalgorithms: [...]plural viajs_array_get.Verify side (
algorithms: [...])Yes —
lower_jsonwebtoken_verifywas updated to cover all the shapes:algorithm: \"ES256\"(literal) — fast pathalgorithm: ALG(const ref) — runtime dispatch viajs_jwt_verify_dynalgorithms: [\"ES256\"](array of literal) — fast pathalgorithms: [ALG](array of non-literal) — runtime dispatchjs_jwt_verify_dyn_optsreadsalgorithmfirst, then falls back toalgorithms[0]Deferred
algorithms: [\"ES256\", \"RS256\"]— only the first entry is honored (matches pre-fix behavior; underlying Rustjsonwebtokencrate's verify is single-algorithm anyway).algorithms(e.g. a const-bound array reference at the field-level) inside an otherwise-inline options object — falls through to the previous HS256 fallback. Rare in practice; case C handles the more common spread shape.Test plan
test-files/test_jwt_sign_dynamic_alg.ts— covers cases A (inline literal), B (const-bound ident), C (const-bound options object), D (verify with const-bound algorithm). Output matchesnode --experimental-strip-typesbyte-for-byte:(A) ES256 / (B) ES256 / (C) ES256 / (D) a.test_issue_915_jwt_sign.tsandtest_issue_927_jwt_verify_returns_object.tsstill pass.cargo test --release -p perry-codegen --test manifest_consistency— 4 passed.cargo build --release -p perry-runtime -p perry-stdlib -p perry— clean.No version bump / changelog change — maintainer handles at merge time.