Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ RefPtr<CryptoKeyOKP> 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) {
Expand All @@ -133,8 +136,6 @@ RefPtr<CryptoKeyOKP> 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)
Expand Down
3 changes: 0 additions & 3 deletions src/jsc/bindings/webcrypto/JSJsonWebKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ template<> JsonWebKey convertDictionary<JsonWebKey>(JSGlobalObject& lexicalGloba
if (!ktyValue.isUndefined()) {
result.kty = convert<IDLDOMString>(lexicalGlobalObject, ktyValue);
RETURN_IF_EXCEPTION(throwScope, {});
} else {
throwRequiredMemberTypeError(lexicalGlobalObject, throwScope, "kty"_s, "JsonWebKey"_s, "DOMString"_s);
return {};
}
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.
JSValue nValue;
if (isNullOrUndefined)
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/webcrypto/JsonWebKey.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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<CryptoKeyUsage> key_ops;
DOMString alg;
Expand Down
69 changes: 60 additions & 9 deletions test/js/web/crypto/web-crypto.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -134,27 +169,43 @@ 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");
Comment thread
claude[bot] marked this conversation as resolved.
});

// 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(
() => null,
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<DeferredPromise>. 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);
Expand All @@ -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
Expand Down
Loading