Skip to content

node:crypto: import OKP JWKs like Node 26, in one raw-key arm shared with AKP #32910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
Closed

node:crypto: import OKP JWKs like Node 26, in one raw-key arm shared with AKP #32910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd53fd7
node:crypto: import OKP JWKs like Node 26, in one raw-key arm shared …
robobun Oct 2, 2026
5a165e3
node:crypto: shorten two comments in the raw-key JWK arm
robobun Oct 3, 2026
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
150 changes: 50 additions & 100 deletions src/jsc/bindings/node/crypto/KeyObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const unsigned char> 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 {};
}
Comment on lines 1297 to 1304

@claude claude Bot Oct 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Apps that use @ libp2p/crypto below 5.1.18 on Bun lose Ed25519 key handling after merge, which works on the base branch. Those versions call createPrivateKey with a JWK carrying a valid d and x: '' to derive the public key; KeyObject.cpp:1300 now rejects any x whose decoded length differs from the derived key, so the call throws ERR_CRYPTO_INVALID_JWK "Invalid JWK OKP key". This follows from the stated purpose (Node 26 parity), and the description names the package. Fix: the call is the maintainers' — if the breakage is accepted, label the PR breaking and release-note it; otherwise the derived-key check must stay while a documented, Node-matching lenience for a placeholder x is agreed upstream first.

Why this was flagged

The trigger is a private OKP JWK whose x is an empty string or otherwise not the public key of d, passed to createPrivateKey, createPublicKey, sign, verify or any other node:crypto entry point that imports a JWK. @ libp2p/crypto 4.1.9 and 5.1.7 through 5.1.17 build exactly this JWK ({ crv: 'Ed25519', x: '', d, kty: 'OKP' }) inside derivePublicKey and then export the key to read x. In the new shared arm KeyObject.cpp:1263 decodes x unconditionally, KeyObject.cpp:1298-1301 derives the public key from d and compares it with CRYPTO_memcmp, and the size mismatch (32 versus 0) throws ERR_CRYPTO_INVALID_JWK "Invalid JWK OKP key" at KeyObject.cpp:1302. On the base branch the old OKP arm read only d for private consumers and never looked at the content of x, so the same call succeeded and returned the key. No safeguard mitigates this: the PR description lists it as a downside but there is no release note or breaking label visible in the change. Node 26.3.0 is claimed to reject the same input, so the behaviour is the stated purpose.

Verification: In src/jsc/bindings/node/crypto/KeyObject.cpp the merged OKP arm now always decodes x (line ~1263), then for a string d builds the key from d and compares (lines ~1296-1304). For x: '', expected.size() is 0 vs derivedPub.size() 32, so createPrivateKey throws ERR_CRYPTO_INVALID_JWK "Invalid JWK OKP key". On base x was never decoded or compared for a private consumer, so the same input succeeded.

@robobun robobun Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct, and the decision belongs to a maintainer, not to me. The break is the stated purpose: the same call throws on Node v26.3.0. I checked the versions myself: @libp2p/crypto 4.1.9, 5.1.7, 5.1.12, 5.1.13 and 5.1.17 pass x: '' next to d, and 5.1.18 does not.

I added the breaking label and put the choice first in the Notes of the PR body. The one alternative is a Bun-only exception that treats an empty x next to a string d as absent. Node has no such exception, so this PR does not add one. I leave this thread open for a maintainer to decide.

@robobun robobun Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is settled on main by #47567. An x that does not belong to d is rejected at every entry point, an empty x included, as on Node. I checked createPrivateKey and createPublicKey with x: '' on a build of main at cee9dd9: both throw ERR_CRYPTO_INVALID_JWK. This PR is closed as covered by #47567.

}

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<WTF::StringView> dView = GCOwnedDataScope<WTF::StringView>(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 {};
}

Expand Down
16 changes: 16 additions & 0 deletions test/js/node/crypto/crypto-pqc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
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"],
Expand Down
110 changes: 110 additions & 0 deletions test/js/node/crypto/crypto.key-objects.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>;
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<string, (key: any, callback: () => 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"),
Expand Down
Loading