diff --git a/src/jsc/bindings/node/crypto/KeyObject.cpp b/src/jsc/bindings/node/crypto/KeyObject.cpp index d3c513910d90..ace77ca300be 100644 --- a/src/jsc/bindings/node/crypto/KeyObject.cpp +++ b/src/jsc/bindings/node/crypto/KeyObject.cpp @@ -1210,154 +1210,104 @@ __attribute__((minsize)) KeyObject KeyObject::getKeyObjectHandleFromJwk(JSGlobal : CryptoKeyType::Private; switch (kty) { - case Kty::Akp: { - // "AKP" covers the ML-DSA and ML-KEM parameter sets. The parameter set - // is named by "alg" (matched case-sensitively, e.g. "ML-DSA-44"), the - // public key lives in "pub", and the private key is the seed in "priv". + case Kty::Akp: + case Kty::Okp: { + // OKP and AKP are both raw-key JWKs: a name, a public member and an optional private member. + const bool isAkp = kty == Kty::Akp; + const ASCIILiteral invalidKey = isAkp ? "Invalid JWK AKP key"_s : "Invalid JWK OKP key"_s; + VM& vm = globalObject->vm(); - JSValue algValue = jwk->get(globalObject, Identifier::fromString(vm, "alg"_s)); + JSValue nameValue = jwk->get(globalObject, Identifier::fromString(vm, isAkp ? "alg"_s : "crv"_s)); RETURN_IF_EXCEPTION(scope, {}); - JSValue pubValue = jwk->get(globalObject, Identifier::fromString(vm, "pub"_s)); + JSValue pubValue = jwk->get(globalObject, Identifier::fromString(vm, isAkp ? "pub"_s : "x"_s)); RETURN_IF_EXCEPTION(scope, {}); - JSValue privValue = jwk->get(globalObject, Identifier::fromString(vm, "priv"_s)); + JSValue privValue = jwk->get(globalObject, Identifier::fromString(vm, isAkp ? "priv"_s : "d"_s)); RETURN_IF_EXCEPTION(scope, {}); int nid = 0; - if (algValue.isString()) { - WTF::String algString = algValue.toWTFString(globalObject); + if (nameValue.isString()) { + WTF::String name = nameValue.toWTFString(globalObject); RETURN_IF_EXCEPTION(scope, {}); - int candidate = pqcKeyTypeToNid(algString.convertToASCIILowercase()); - // Only the canonical upper-case spelling is accepted. - if (candidate && WTF::String(pqcNidToKeyTypeName(candidate)).convertToASCIIUppercase() == algString) - nid = candidate; + if (isAkp) { + int candidate = pqcKeyTypeToNid(name.convertToASCIILowercase()); + // Only the canonical upper-case spelling is accepted. + if (candidate && WTF::String(pqcNidToKeyTypeName(candidate)).convertToASCIIUppercase() == name) + nid = candidate; + } else if (name == "Ed25519"_s) { + nid = EVP_PKEY_ED25519; + } else if (name == "Ed448"_s) { + nid = EVP_PKEY_ED448; + } else if (name == "X25519"_s) { + nid = EVP_PKEY_X25519; + } else if (name == "X448"_s) { + nid = EVP_PKEY_X448; + } } if (!nid) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Unsupported JWK AKP \"alg\""_s); + ERR::CRYPTO_INVALID_JWK(scope, globalObject, isAkp ? "Unsupported JWK AKP \"alg\""_s : invalidKey); return {}; } if (!pubValue.isString() || (!privValue.isUndefined() && !privValue.isString())) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK AKP key"_s); + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } - // The JWK itself decides whether private key material is present. + // The JWK itself, not the caller's mode, decides whether it is a private key. CryptoKeyType jwkType = privValue.isString() ? CryptoKeyType::Private : CryptoKeyType::Public; - if (keyType == CryptoKeyType::Private && jwkType == CryptoKeyType::Public) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "JWK does not contain private key material"_s); - return {}; - } // pubValue / privValue were already read and type-checked above; decode // them directly so each JWK property is observed exactly once. auto pubView = asString(pubValue)->view(globalObject); RETURN_IF_EXCEPTION(scope, {}); - auto* pubBuf = decodeJwkString(globalObject, scope, pubView, "key.pub"_s); + auto* pubBuf = decodeJwkString(globalObject, scope, pubView, isAkp ? "key.pub"_s : "key.x"_s); RETURN_IF_EXCEPTION(scope, {}); JSArrayBufferView* privBuf = nullptr; if (jwkType == CryptoKeyType::Private) { auto privView = asString(privValue)->view(globalObject); RETURN_IF_EXCEPTION(scope, {}); - privBuf = decodeJwkString(globalObject, scope, privView, "key.priv"_s); + privBuf = decodeJwkString(globalObject, scope, privView, isAkp ? "key.priv"_s : "key.d"_s); RETURN_IF_EXCEPTION(scope, {}); } MarkPopErrorOnReturn markPopError; - ncrypto::EVPKeyPointer key = jwkType == CryptoKeyType::Private - ? newFromPrivateSeed(nid, privBuf->span()) - : newFromRawPublic(nid, pubBuf->span()); + // A wrong length fails here: BoringSSL checks it for every raw key type. + auto keySpan = jwkType == CryptoKeyType::Private ? privBuf->span() : pubBuf->span(); + ncrypto::EVPKeyPointer key; + if (isAkp) { + key = jwkType == CryptoKeyType::Private + ? newFromPrivateSeed(nid, keySpan) + : newFromRawPublic(nid, keySpan); + } else { + ncrypto::Buffer keyBuf { .data = keySpan.data(), .len = keySpan.size() }; + key = jwkType == CryptoKeyType::Private + ? EVPKeyPointer::NewRawPrivate(nid, keyBuf) + : EVPKeyPointer::NewRawPublic(nid, keyBuf); + } + JSC::ensureStillAliveHere(privBuf); if (!key) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK AKP key"_s); + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } - // "pub" must agree with the public key derived from the seed. + // The public member must be the public key derived from the private one. if (jwkType == CryptoKeyType::Private) { auto derivedPub = key.rawPublicKey(); auto expected = pubBuf->span(); if (!derivedPub || derivedPub.size() != expected.size() || CRYPTO_memcmp(derivedPub.get(), expected.data(), expected.size()) != 0) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK AKP key"_s); + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } } - JSC::ensureStillAliveHere(pubBuf); - return create(keyType, WTF::move(key)); - } - case Kty::Okp: { - auto crvView = getJwkStringView(globalObject, scope, jwk, "crv"_s, "key.crv"_s); - RETURN_IF_EXCEPTION(scope, {}); - - int nid; - if (crvView == "Ed25519"_s) { - nid = EVP_PKEY_ED25519; - } else if (crvView == "Ed448"_s) { - nid = EVP_PKEY_ED448; - } else if (crvView == "X25519"_s) { - nid = EVP_PKEY_X25519; - } else if (crvView == "X448"_s) { - nid = EVP_PKEY_X448; - } else { - // validateOneOf - ERR::INVALID_ARG_VALUE(scope, globalObject, "key.crv"_s, crvView.owner, "must be one of: 'Ed25519', 'Ed448', 'X25519', 'X448'"_s); - return {}; - } - - auto xView = getJwkStringView(globalObject, scope, jwk, "x"_s, "key.x"_s); - RETURN_IF_EXCEPTION(scope, {}); - - GCOwnedDataScope dView = GCOwnedDataScope(nullptr, WTF::nullStringView()); - - if (keyType != CryptoKeyType::Public) { - dView = getJwkStringView(globalObject, scope, jwk, "d"_s, "key.d"_s); - RETURN_IF_EXCEPTION(scope, {}); - } - - auto dataView = keyType == CryptoKeyType::Public ? xView : dView; - - auto* dataBuf = decodeJwkString(globalObject, scope, dataView, "key.x"_s); - RETURN_IF_EXCEPTION(scope, {}); - auto bufSpan = dataBuf->span(); - - switch (nid) { - case EVP_PKEY_ED25519: - case EVP_PKEY_X25519: - if (bufSpan.size() != 32) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject); - return {}; - } - break; - case EVP_PKEY_ED448: - if (bufSpan.size() != 57) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject); - return {}; - } - break; - case EVP_PKEY_X448: - if (bufSpan.size() != 56) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject); - return {}; - } - break; - } - - MarkPopErrorOnReturn markPopError; - - auto buf = ncrypto::Buffer { - .data = bufSpan.data(), - .len = bufSpan.size(), - }; - auto key = keyType == CryptoKeyType::Public - ? EVPKeyPointer::NewRawPublic(nid, buf) - : EVPKeyPointer::NewRawPrivate(nid, buf); - - if (!key) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject); + // Node rejects this only in createPrivateKey. Here every private-key consumer does. + if (keyType == CryptoKeyType::Private && jwkType == CryptoKeyType::Public) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, "JWK does not contain private key material"_s); return {}; } diff --git a/test/js/node/crypto/crypto-pqc.test.ts b/test/js/node/crypto/crypto-pqc.test.ts index de76c2d9ac22..65d8228ac93d 100644 --- a/test/js/node/crypto/crypto-pqc.test.ts +++ b/test/js/node/crypto/crypto-pqc.test.ts @@ -93,6 +93,22 @@ describe("ML-KEM", () => { }); }); +// Node validates the JWK before a private-key consumer asks whether it holds +// private key material, so for a public-only JWK a malformed "pub" wins. +describe("public-only AKP JWK given to a private-key consumer", () => { + test.each(["ml-dsa-44", "ml-kem-768"] as const)("%s", type => { + const jwk = generateKeyPairSync(type as any).publicKey.export({ format: "jwk" }) as Record; + const malformed = { ...jwk, pub: Buffer.from(jwk.pub, "base64url").subarray(1).toString("base64url") }; + const invalidKey = expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "Invalid JWK AKP key" }); + + expect(() => createPrivateKey({ key: jwk, format: "jwk" })).toThrow( + expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "JWK does not contain private key material" }), + ); + expect(() => createPrivateKey({ key: malformed, format: "jwk" })).toThrow(invalidKey); + expect(() => sign(undefined, Buffer.from("data"), { key: malformed, format: "jwk" })).toThrow(invalidKey); + }); +}); + describe("encrypted PKCS#8", () => { for (const [name, type] of [ ["ml_dsa_44_private_encrypted.pem", "ml-dsa-44"], diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 03d475833d0d..d8697d4613bb 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -490,6 +490,116 @@ describe("crypto.KeyObjects", () => { }); }); + // Node's ImportJWKEdKey: an OKP JWK is a private key when "d" is a string, + // whichever API receives it, and then "x" must be the public key of "d". + describe.each(["ed25519", "x25519"] as const)("%s JWK import", keyType => { + const pem = readFile(path.join(import.meta.dir, "fixtures", `${keyType}_private.pem`), "ascii"); + const jwk = createPrivateKey(pem).export({ format: "jwk" }) as Record; + const { d: _, ...publicJwk } = jwk; + const otherX = generateKeyPairSync(keyType as any).publicKey.export({ format: "jwk" }).x; + const data = Buffer.from("data"); + const signature = Buffer.alloc(64); + const invalidJwk = expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "Invalid JWK OKP key" }); + + // Every node:crypto call that imports a key. + const callForms: Record void) => unknown> = { + "createPrivateKey": key => createPrivateKey(key), + "createPublicKey": key => createPublicKey(key), + "sign": key => sign(null, data, key), + "sign with a callback": (key, callback) => sign(null, data, key, callback), + "verify": key => verify(null, data, key, signature), + "verify with a callback": (key, callback) => verify(null, data, key, signature, callback), + "createSign().sign": key => createSign("sha256").update(data).sign(key), + "createVerify().verify": key => createVerify("sha256").update(data).verify(key, signature), + "publicEncrypt": key => publicEncrypt(key, data), + "publicDecrypt": key => publicDecrypt(key, data), + "privateEncrypt": key => privateEncrypt(key, data), + "privateDecrypt": key => privateDecrypt(key, data), + }; + const forms = Object.keys(callForms); + + const bytes = (length: number) => Buffer.alloc(length, 1).toString("base64url"); + const without = (member: string) => { + const { [member]: _, ...rest } = jwk; + return rest; + }; + describe.each([ + ["the x of another key", { ...jwk, x: otherX }], + ["an empty x", { ...jwk, x: "" }], + ["an 8-byte x", { ...jwk, x: bytes(8) }], + ["a 33-byte x", { ...jwk, x: bytes(33) }], + ["no x", without("x")], + ["a number for x", { ...jwk, x: 1 }], + ["null for d", { ...jwk, d: null }], + ["a number for d", { ...jwk, d: 1 }], + ["an empty d", { ...jwk, d: "" }], + ["an 8-byte d", { ...jwk, d: bytes(8) }], + ["no crv", without("crv")], + ["a number for crv", { ...jwk, crv: 1 }], + ["an unknown crv", { ...jwk, crv: "Ed25518" }], + ])("a JWK with %s", (_, key) => { + test.each(forms)("is rejected by %s", form => { + let callbacks = 0; + expect(() => callForms[form]({ key, format: "jwk" }, () => callbacks++)).toThrow(invalidJwk); + expect(callbacks).toBe(0); + }); + }); + + test.each(forms)("%s reads kty, crv, x and d once each, before it validates them", form => { + for (const target of [jwk, { ...jwk, crv: "Ed25518" }]) { + const reads: PropertyKey[] = []; + const key = new Proxy(target, { + get(target, name, receiver) { + reads.push(name); + return Reflect.get(target, name, receiver); + }, + }); + try { + callForms[form]({ key, format: "jwk" }, () => {}); + } catch {} + expect(reads).toEqual(["kty", "crv", "x", "d"]); + } + }); + + test("x is read once, so a getter cannot pass the check with one value and import another", () => { + let reads = 0; + const key = { + ...jwk, + get x() { + return reads++ === 0 ? otherX : jwk.x; + }, + }; + expect(() => createPublicKey({ key, format: "jwk" })).toThrow(invalidJwk); + expect(reads).toBe(1); + }); + + test("createPrivateKey reports a public-only JWK after it validates the JWK", () => { + expect(() => createPrivateKey({ key: publicJwk, format: "jwk" })).toThrow( + expect.objectContaining({ + code: "ERR_CRYPTO_INVALID_JWK", + message: "JWK does not contain private key material", + }), + ); + expect(() => createPrivateKey({ key: { ...publicJwk, x: bytes(8) }, format: "jwk" })).toThrow(invalidJwk); + }); + + test("x is compared after base64 decoding, so padded standard-alphabet base64 works", () => { + const standard = (x: string) => Buffer.from(x, "base64url").toString("base64"); + expect(standard(jwk.x)).not.toBe(jwk.x); + const key = { ...jwk, x: standard(jwk.x) }; + expect(createPrivateKey({ key, format: "jwk" }).export({ format: "jwk" })).toEqual(jwk); + expect(createPublicKey({ key, format: "jwk" }).export({ format: "jwk" })).toEqual(publicJwk); + expect(() => createPrivateKey({ key: { ...jwk, x: standard(otherX) }, format: "jwk" })).toThrow(invalidJwk); + }); + + test("a consistent private JWK is still a public key for createPublicKey", () => { + const key = createPublicKey({ key: jwk, format: "jwk" }); + expect(key.type).toBe("public"); + expect(key.export({ format: "jwk" })).toEqual(publicJwk); + expect(key.equals(createPublicKey({ key: publicJwk, format: "jwk" }))).toBe(true); + }); + }); + [ { private: readFile(path.join(import.meta.dir, "fixtures", "ec_p256_private.pem"), "ascii"),