From 3ce68f6da922665ac247b77649288ab0f9e5ae92 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 07:06:02 +0000 Subject: [PATCH 1/3] webcrypto: reject JWK with duplicate key_ops on importKey/unwrapKey RFC 7517 section 4.3 requires "Duplicate key operation values MUST NOT be present in the array", and WebCrypto importKey specifies that a key_ops member invalid per RFC 7517 is a DataError. normalizeJsonWebKey folded key_ops straight into a bitmap, which made duplicates invisible to the per-key-class importers that only compare bitmasks. normalizeJsonWebKey now detects a bit that is already set while building the bitmap and reports failure; both call sites (importKey and the JWK branch of unwrapKey) reject the promise with DataError on that result. The usages argument to importKey itself is unchanged and still permits duplicates, matching the spec and Node. --- src/jsc/bindings/webcrypto/SubtleCrypto.cpp | 33 +++++- test/js/web/crypto/web-crypto.test.ts | 124 ++++++++++++++++++++ 2 files changed, 151 insertions(+), 6 deletions(-) diff --git a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp index 0971d2aa3d1a..16cefbd57c3b 100644 --- a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp +++ b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp @@ -547,10 +547,24 @@ static void rejectWithException(Ref&& passedPromise, ExceptionC ASSERT_NOT_REACHED(); } -static void normalizeJsonWebKey(JsonWebKey& webKey) +// Returns false if key_ops contains duplicate entries. RFC 7517 §4.3 requires +// "Duplicate key operation values MUST NOT be present in the array"; callers +// reject with DataError on false. +static bool normalizeJsonWebKey(JsonWebKey& webKey) { - // Maybe we shouldn't silently bypass duplicated usages? - webKey.usages = webKey.key_ops ? toCryptoKeyUsageBitmap(webKey.key_ops.value()) : 0; + if (!webKey.key_ops) { + webKey.usages = 0; + return true; + } + CryptoKeyUsageBitmap result = 0; + for (auto usage : webKey.key_ops.value()) { + auto bit = toCryptoKeyUsageBitmap(usage); + if (result & bit) + return false; + result |= bit; + } + webKey.usages = result; + return true; } // FIXME: This returns an std::optional and takes a promise, rather than returning an @@ -580,8 +594,11 @@ static std::optional toKeyData(SubtleCrypto::KeyFormat format, SubtleCr case SubtleCrypto::KeyFormat::Jwk: return std::visit( WTF::makeVisitor( - [](JsonWebKey& webKey) -> std::optional { - normalizeJsonWebKey(webKey); + [&promise](JsonWebKey& webKey) -> std::optional { + if (!normalizeJsonWebKey(webKey)) { + promise->reject(DataError, "Duplicate key operation in JWK \"key_ops\" member"_s); + return std::nullopt; + } return KeyData { webKey }; }, [&promise](auto&) -> std::optional { @@ -1321,7 +1338,11 @@ void SubtleCrypto::unwrapKey(JSC::JSGlobalObject& state, KeyFormat format, Buffe promise->reject(Exception { ExistingExceptionError }); return; } - normalizeJsonWebKey(jwk); + if (!normalizeJsonWebKey(jwk)) { + weakThis->m_pendingPromises.remove(index); + promise->reject(DataError, "Duplicate key operation in JWK \"key_ops\" member"_s); + return; + } keyData = jwk; break; diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 1d735d47c3d3..4e59b20a8dab 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -584,3 +584,127 @@ describe("X25519 JWK import", () => { }).toEqual({ extFalse: "DataError", extTrue: "imported" }); }); }); + +// RFC 7517 §4.3: "Duplicate key operation values MUST NOT be present in the array." +// WebCrypto importKey step: if key_ops "is invalid according to the requirements of +// JSON Web Key ... then throw a DataError." +describe("JWK key_ops duplicate values", () => { + const b64u = (b: Uint8Array) => Buffer.from(b).toString("base64url"); + const outcome = (p: Promise) => + p.then( + key => (key instanceof CryptoKey ? "imported" : "other"), + e => e.name, + ); + + it("importKey rejects a JWK with duplicate key_ops entries across all key types", async () => { + const ec = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]); + const ecPub = await crypto.subtle.exportKey("jwk", ec.publicKey); + const ed = await crypto.subtle.generateKey({ name: "Ed25519" }, true, ["sign", "verify"]); + const edPub = await crypto.subtle.exportKey("jwk", (ed as CryptoKeyPair).publicKey); + const rsa = await crypto.subtle.generateKey( + { name: "RSASSA-PKCS1-v1_5", hash: "SHA-256", modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]) }, + true, + ["sign", "verify"], + ); + const rsaPub = await crypto.subtle.exportKey("jwk", rsa.publicKey); + const octAes: JsonWebKey = { kty: "oct", k: b64u(new Uint8Array(16).fill(9)), ext: true }; + const octHmac: JsonWebKey = { kty: "oct", k: b64u(new Uint8Array(32).fill(9)), ext: true }; + + expect({ + aes: await outcome( + crypto.subtle.importKey( + "jwk", + { ...octAes, key_ops: ["encrypt", "encrypt", "decrypt"] }, + { name: "AES-GCM" }, + true, + ["encrypt"], + ), + ), + hmac: await outcome( + crypto.subtle.importKey("jwk", { ...octHmac, key_ops: ["sign", "sign"] }, { name: "HMAC", hash: "SHA-256" }, true, [ + "sign", + ]), + ), + ec: await outcome( + crypto.subtle.importKey( + "jwk", + { ...ecPub, key_ops: ["verify", "verify"] }, + { name: "ECDSA", namedCurve: "P-256" }, + true, + ["verify"], + ), + ), + okp: await outcome( + crypto.subtle.importKey("jwk", { ...edPub, key_ops: ["verify", "verify"] }, { name: "Ed25519" }, true, ["verify"]), + ), + rsa: await outcome( + crypto.subtle.importKey( + "jwk", + { ...rsaPub, key_ops: ["verify", "verify"] }, + { name: "RSASSA-PKCS1-v1_5", hash: "SHA-256" }, + true, + ["verify"], + ), + ), + // Same JWKs without duplicates still import (prove the rejection is about duplicates, not shape). + aesNoDup: await outcome( + crypto.subtle.importKey("jwk", { ...octAes, key_ops: ["encrypt", "decrypt"] }, { name: "AES-GCM" }, true, [ + "encrypt", + ]), + ), + ecNoDup: await outcome( + crypto.subtle.importKey("jwk", { ...ecPub, key_ops: ["verify"] }, { name: "ECDSA", namedCurve: "P-256" }, true, [ + "verify", + ]), + ), + // The usages argument itself is allowed to contain duplicates; only JWK key_ops is constrained. + usagesDup: await outcome( + crypto.subtle.importKey("raw", new Uint8Array(16).fill(9), { name: "AES-GCM" }, true, [ + "encrypt", + "encrypt", + "decrypt", + ]), + ), + }).toEqual({ + aes: "DataError", + hmac: "DataError", + ec: "DataError", + okp: "DataError", + rsa: "DataError", + aesNoDup: "imported", + ecNoDup: "imported", + usagesDup: "imported", + }); + }); + + it("unwrapKey rejects a wrapped JWK with duplicate key_ops entries", async () => { + const keyData = new Uint8Array(32).fill(1); + const iv = new Uint8Array(12).fill(2); + const wrapper = await crypto.subtle.importKey("raw", keyData, { name: "AES-GCM" }, false, [ + "encrypt", + "decrypt", + "wrapKey", + "unwrapKey", + ]); + const wrap = async (jwk: JsonWebKey) => + crypto.subtle.encrypt({ name: "AES-GCM", iv }, wrapper, new TextEncoder().encode(JSON.stringify(jwk))); + const unwrap = (wrapped: ArrayBuffer) => + crypto.subtle.unwrapKey("jwk", wrapped, wrapper, { name: "AES-GCM", iv }, { name: "AES-GCM" }, true, ["encrypt"]); + + const dup: JsonWebKey = { + kty: "oct", + k: b64u(new Uint8Array(16).fill(9)), + ext: true, + key_ops: ["encrypt", "encrypt", "decrypt"], + }; + const noDup: JsonWebKey = { ...dup, key_ops: ["encrypt", "decrypt"] }; + + expect({ + dup: await outcome(unwrap(await wrap(dup))), + noDup: await outcome(unwrap(await wrap(noDup))), + }).toEqual({ + dup: "DataError", + noDup: "imported", + }); + }); +}); From 9c0814d55a04a094a22660f57a4137d52906074a Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 07:08:55 +0000 Subject: [PATCH 2/3] [autofix.ci] apply automated fixes --- test/js/web/crypto/web-crypto.test.ts | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 4e59b20a8dab..dd6fb9fdaaa1 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -621,9 +621,13 @@ describe("JWK key_ops duplicate values", () => { ), ), hmac: await outcome( - crypto.subtle.importKey("jwk", { ...octHmac, key_ops: ["sign", "sign"] }, { name: "HMAC", hash: "SHA-256" }, true, [ - "sign", - ]), + crypto.subtle.importKey( + "jwk", + { ...octHmac, key_ops: ["sign", "sign"] }, + { name: "HMAC", hash: "SHA-256" }, + true, + ["sign"], + ), ), ec: await outcome( crypto.subtle.importKey( @@ -635,7 +639,9 @@ describe("JWK key_ops duplicate values", () => { ), ), okp: await outcome( - crypto.subtle.importKey("jwk", { ...edPub, key_ops: ["verify", "verify"] }, { name: "Ed25519" }, true, ["verify"]), + crypto.subtle.importKey("jwk", { ...edPub, key_ops: ["verify", "verify"] }, { name: "Ed25519" }, true, [ + "verify", + ]), ), rsa: await outcome( crypto.subtle.importKey( @@ -653,9 +659,13 @@ describe("JWK key_ops duplicate values", () => { ]), ), ecNoDup: await outcome( - crypto.subtle.importKey("jwk", { ...ecPub, key_ops: ["verify"] }, { name: "ECDSA", namedCurve: "P-256" }, true, [ - "verify", - ]), + crypto.subtle.importKey( + "jwk", + { ...ecPub, key_ops: ["verify"] }, + { name: "ECDSA", namedCurve: "P-256" }, + true, + ["verify"], + ), ), // The usages argument itself is allowed to contain duplicates; only JWK key_ops is constrained. usagesDup: await outcome( From e9e7fbfac69e9a34e4bcde42ef9447e5d010a311 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 07:30:10 +0000 Subject: [PATCH 3/3] ci: retrigger