Skip to content

Commit 7a82a65

Browse files
jose: skip malformed keys in Jwt.verify multi-key import
Follow-up to the security hardening: Jwt.verify imported every compatible candidate key and failed the whole verification if any single one rejected, so one malformed key in an otherwise-valid JWK Set could deny service to tokens signed by the good keys. Import each candidate independently and skip the unusable ones, mirroring Jws.verify; still fail closed when none import.
1 parent 2d2c533 commit 7a82a65

2 files changed

Lines changed: 26 additions & 5 deletions

File tree

packages/effect/src/unstable/jose/Jwt.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,19 @@ export const verify = Effect.fnUntraced(function*(
215215
.filter((jwk) => hint.kid === undefined || jwk.kid === undefined || jwk.kid === hint.kid)
216216
if (candidates.length === 0) return yield* new JwtError({ reason: "UnknownKey" })
217217

218-
const publicKeys = yield* Effect.forEach(candidates, (jwk) =>
219-
Effect.tryPromise({
220-
try: () => crypto.subtle.importKey("jwk", jwk as JsonWebKey, Jwa.importParameters(hint.alg), false, ["verify"]),
221-
catch: () => new JwtError({ reason: "UnknownKey" })
222-
}))
218+
// Import each candidate independently, skipping any key whose material is
219+
// malformed rather than failing the whole verification — one bad key in an
220+
// otherwise-valid JWK Set must not deny service to tokens signed by the
221+
// good keys.
222+
const imported = yield* Effect.forEach(
223+
candidates,
224+
(jwk) =>
225+
Effect.tryPromise(() =>
226+
crypto.subtle.importKey("jwk", jwk as JsonWebKey, Jwa.importParameters(hint.alg), false, ["verify"])
227+
).pipe(Effect.catch(() => Effect.succeed(null as CryptoKey | null)))
228+
)
229+
const publicKeys = imported.filter((key): key is CryptoKey => key !== null)
230+
if (publicKeys.length === 0) return yield* new JwtError({ reason: "UnknownKey" })
223231

224232
const result = yield* Jws.verify({ publicKeys, payload: ClaimsFromJson })(flattened).pipe(
225233
Effect.mapError((error) =>

packages/effect/test/unstable/jose/Security.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,19 @@ describe("JOSE security remediations", () => {
4343
assert.strictEqual(verified.sub, "sub")
4444
}).pipe(Effect.runPromise))
4545

46+
it("verifies against a JWK Set containing a malformed key alongside the good one", () =>
47+
Effect.gen(function*() {
48+
const { privateJwk, publicJwk } = yield* Jwt.generateSigningKey()
49+
const token = yield* Jwt.sign({ privateJwk, payload: claims })
50+
// a compatible (ES256) but structurally broken key must be skipped, not fatal
51+
const brokenKey = { ...publicJwk, x: "!!!not-base64!!!", kid: undefined }
52+
const verified = yield* Jwt.verify(token, {
53+
jwks: { keys: [brokenKey as typeof publicJwk, publicJwk] },
54+
algorithms: ["ES256"]
55+
})
56+
assert.strictEqual(verified.sub, "sub")
57+
}).pipe(Effect.runPromise))
58+
4659
it("enforces the typ header when types is supplied", () =>
4760
Effect.gen(function*() {
4861
const { privateJwk, publicJwk } = yield* Jwt.generateSigningKey()

0 commit comments

Comments
 (0)