Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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