diff --git a/src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp b/src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp index 1f5b17a3dc20..ae54916fd61f 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp @@ -124,6 +124,9 @@ RefPtr CryptoKeyOKP::importJwkInternal(CryptoAlgorithmIdentifier i if (!isPlatformSupportedCurve(namedCurve)) return nullptr; + if (keyData.kty != "OKP"_s) + return nullptr; + switch (namedCurve) { case NamedCurve::Ed25519: if (!keyData.d.isEmpty() && !onlyPublic) { @@ -133,8 +136,6 @@ RefPtr CryptoKeyOKP::importJwkInternal(CryptoAlgorithmIdentifier i if (usages & (CryptoKeyUsageEncrypt | CryptoKeyUsageDecrypt | CryptoKeyUsageSign | CryptoKeyUsageDeriveKey | CryptoKeyUsageDeriveBits | CryptoKeyUsageWrapKey | CryptoKeyUsageUnwrapKey)) return nullptr; } - if (keyData.kty != "OKP"_s) - return nullptr; if (keyData.crv != "Ed25519"_s) return nullptr; if (usages && !keyData.use.isEmpty() && keyData.use != "sig"_s) diff --git a/src/jsc/bindings/webcrypto/JSJsonWebKey.cpp b/src/jsc/bindings/webcrypto/JSJsonWebKey.cpp index cc703398b53a..797940f587c5 100644 --- a/src/jsc/bindings/webcrypto/JSJsonWebKey.cpp +++ b/src/jsc/bindings/webcrypto/JSJsonWebKey.cpp @@ -160,9 +160,6 @@ template<> JsonWebKey convertDictionary(JSGlobalObject& lexicalGloba if (!ktyValue.isUndefined()) { result.kty = convert(lexicalGlobalObject, ktyValue); RETURN_IF_EXCEPTION(throwScope, {}); - } else { - throwRequiredMemberTypeError(lexicalGlobalObject, throwScope, "kty"_s, "JsonWebKey"_s, "DOMString"_s); - return {}; } JSValue nValue; if (isNullOrUndefined) diff --git a/src/jsc/bindings/webcrypto/JsonWebKey.idl b/src/jsc/bindings/webcrypto/JsonWebKey.idl index 5f4ca5c4498a..875f564be85b 100644 --- a/src/jsc/bindings/webcrypto/JsonWebKey.idl +++ b/src/jsc/bindings/webcrypto/JsonWebKey.idl @@ -28,7 +28,7 @@ JSGenerateToJSObject, ] dictionary JsonWebKey { // The following fields are defined in Section 3.1 of JSON Web Key - required DOMString kty; + DOMString kty; DOMString use; sequence key_ops; DOMString alg; diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 7b59f93d3d89..f232e4e6e3d8 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -105,6 +105,41 @@ describe("Web Crypto", () => { expect(isSigValid).toBe(true); }); + // W3C WebCrypto: JsonWebKey.kty is not a required dictionary member; each + // algorithm's import key operation rejects a missing or wrong kty with DataError. + describe("importKey jwk kty validation rejects with DataError", () => { + // Well-formed 32-byte x coordinate so the kty is the only invalid member. + const x32 = Buffer.alloc(32).toString("base64url"); + const cases: Array<[string, object, object, KeyUsage[]]> = [ + ["AES-GCM", { k: "AAECAwQFBgcICQoLDA0ODw" }, { name: "AES-GCM" }, ["encrypt"]], + ["HMAC", { k: "AAECAwQFBgcICQoLDA0ODw" }, { name: "HMAC", hash: "SHA-256" }, ["sign"]], + ["RSA-OAEP", { n: "AQAB", e: "AQAB" }, { name: "RSA-OAEP", hash: "SHA-256" }, ["encrypt"]], + ["ECDSA", { crv: "P-256", x: "", y: "" }, { name: "ECDSA", namedCurve: "P-256" }, ["verify"]], + ["Ed25519", { crv: "Ed25519", x: x32 }, { name: "Ed25519" }, ["verify"]], + ["X25519", { crv: "X25519", x: x32 }, { name: "X25519" }, []], + ]; + it.each(cases)("missing kty: %s", async (_name, jwk, alg, usages) => { + const err = await crypto.subtle.importKey("jwk", jwk as JsonWebKey, alg, true, usages).then( + () => null, + e => e, + ); + expect(err).toBeInstanceOf(DOMException); + expect(err.name).toBe("DataError"); + }); + + // X25519 import key, step 2.2: if the kty field of jwk is not "OKP", throw a DataError. + it("wrong kty: X25519", async () => { + const err = await crypto.subtle + .importKey("jwk", { kty: "EC", crv: "X25519", x: x32 }, { name: "X25519" }, true, []) + .then( + () => null, + e => e, + ); + expect(err).toBeInstanceOf(DOMException); + expect(err.name).toBe("DataError"); + }); + }); + describe("unwrapKey JWK error handling", () => { // Setup: AES-GCM key that can encrypt arbitrary bytes and also unwrap keys. // We encrypt payloads that decrypt to invalid JWK data so the JWK parse path @@ -134,11 +169,27 @@ describe("Web Crypto", () => { expect(err.name).toBe("DataError"); }); - // Previously this promise never settled: the TypeError from JsonWebKey - // dictionary conversion escaped as an uncaught exception and the - // DeferredPromise was left in m_pendingPromises forever. + // An object with no recognized members (including no kty) is a valid + // JsonWebKey dictionary; the per-algorithm import key operation rejects it. it("rejects when wrapped bytes are valid JSON but not a valid JWK", async () => { const { key, iv, wrapped } = await setup(new TextEncoder().encode(JSON.stringify({ foo: "bar" }))); + const err = await crypto.subtle + .unwrapKey("jwk", wrapped, key, { name: "AES-GCM", iv }, { name: "AES-GCM" }, true, ["encrypt", "decrypt"]) + .then( + () => null, + e => e, + ); + expect(err).toBeInstanceOf(DOMException); + expect(err.name).toBe("DataError"); + }); + + // Previously this promise never settled: the exception from JsonWebKey + // dictionary conversion escaped as an uncaught exception and the + // DeferredPromise was left in m_pendingPromises forever. + it("rejects when the JWK dictionary conversion throws", async () => { + const { key, iv, wrapped } = await setup( + new TextEncoder().encode(JSON.stringify({ kty: "oct", key_ops: ["bogus"] })), + ); const err = await crypto.subtle .unwrapKey("jwk", wrapped, key, { name: "AES-GCM", iv }, { name: "AES-GCM" }, true, ["encrypt", "decrypt"]) .then( @@ -146,15 +197,15 @@ describe("Web Crypto", () => { e => e, ); expect(err).toBeInstanceOf(TypeError); - expect(err.message).toContain("kty"); + expect(err.message).toContain("enumeration"); }); it("does not leak DeferredPromise in m_pendingPromises on JWK parse errors", async () => { // Each leaked entry in m_pendingPromises holds a Ref. On - // the dictionary-conversion error path the promise was never rejected, so - // DeferredPromise never removed itself from JSDOMGlobalObject's - // guardedObjects set and the JSPromise stayed alive. Count live Promise - // cells in the JSC heap to detect the leak. + // the dictionary-conversion error path (here, an invalid key_ops enum + // value) the promise was never rejected, so DeferredPromise never removed + // itself from JSDOMGlobalObject's guardedObjects set and the JSPromise + // stayed alive. Count live Promise cells in the JSC heap to detect the leak. const fixture = /* js */ ` const { heapStats } = require("bun:jsc"); const keyData = new Uint8Array(32).fill(1); @@ -165,7 +216,7 @@ describe("Web Crypto", () => { const wrapped = await crypto.subtle.encrypt( { name: "AES-GCM", iv }, key, - new TextEncoder().encode(JSON.stringify({ foo: "bar" })), + new TextEncoder().encode(JSON.stringify({ kty: "oct", key_ops: ["bogus"] })), ); async function once() { await crypto.subtle