webcrypto: reject JWK with duplicate key_ops on importKey/unwrapKey

src/jsc/bindings/webcrypto/SubtleCrypto.cpp

@@ -547,10 +547,24 @@ static void rejectWithException(Ref<DeferredPromise>&& 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<KeyData> and takes a promise, rather than returning an
@@ -580,8 +594,11 @@ static std::optional<KeyData> toKeyData(SubtleCrypto::KeyFormat format, SubtleCr
     case SubtleCrypto::KeyFormat::Jwk:
         return std::visit(
             WTF::makeVisitor(
-                [](JsonWebKey& webKey) -> std::optional<KeyData> {
-                    normalizeJsonWebKey(webKey);
+                [&promise](JsonWebKey& webKey) -> std::optional<KeyData> {
+                    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<KeyData> {
@@ -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;

test/js/web/crypto/web-crypto.test.ts

@@ -584,3 +584,137 @@ 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<CryptoKey>) =>
+    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",
+    });
+  });
+});