node:crypto: validate EC JWK private scalar matches public point in createPrivateKey

[robobun]

What does this PR do?

crypto.createPrivateKey({format: "jwk"}) currently accepts an EC private JWK whose d does not correspond to its (x, y), producing a KeyObject whose derived public key cannot verify its own signatures:

const crypto = require("node:crypto");
const A = crypto.generateKeyPairSync("ec", { namedCurve: "prime256v1" }).privateKey.export({ format: "jwk" });
const B = crypto.generateKeyPairSync("ec", { namedCurve: "prime256v1" }).privateKey.export({ format: "jwk" });
const k = crypto.createPrivateKey({ key: { ...A, d: B.d }, format: "jwk" });
const m = Buffer.from("msg");
crypto.verify("sha256", m, crypto.createPublicKey(k), crypto.sign("sha256", m, k));
// Node v26:  throws ERR_CRYPTO_INVALID_JWK at createPrivateKey
// Bun:       returns false (key accepted, self-verify fails)

A truncated d is similarly accepted and re-exports as a different scalar. Bun's own crypto.subtle.importKey("jwk", ...) already rejects both inputs in the same process, so the two JWK importers disagreed with each other.

The cause is that KeyObject::getKeyObjectHandleFromJwk sets the public point via EC_KEY_set_public_key_affine_coordinates (which validates the point is on the curve) and the private scalar via EC_KEY_set_private_key (which does not cross-check against the public point), then wraps the result without ever calling EC_KEY_check_key. Node performs that check after setting the private key, as does Bun's WebCrypto CryptoKeyEC::platformImportJWKPrivate.

This PR calls ec.checkKey() (the existing ECKeyPointer wrapper around EC_KEY_check_key) after setPrivateKey and throws ERR_CRYPTO_INVALID_JWK on failure, matching Node. It also adds the missing RETURN_IF_EXCEPTION after base64-decoding d.

Sibling of #32910, which makes the analogous fix for OKP private JWKs in the same function.

How did you verify your code works?

New tests in test/js/node/crypto/crypto.key-objects.test.ts cover mismatched d, truncated d, and leading-zero-padded d (which Node accepts, since the scalar value is unchanged) for P-256/P-384/P-521. They fail on current main (USE_SYSTEM_BUN=1 bun test) and pass with bun bd test; the rest of the file (88 tests) is unaffected. Also verified the test expectations match Node v26.3.0 for all three curves.

[coderabbitai]

Walkthrough

This change adds a validation step to EC JWK private key import in KeyObject.cpp, checking for exceptions after decoding the private scalar and calling checkKey() to validate consistency, throwing CRYPTO_INVALID_JWK on failure. New tests verify this behavior across multiple curves.

Changes

EC JWK Import Validation

Layer / File(s)	Summary
EC private key consistency validation src/jsc/bindings/node/crypto/KeyObject.cpp, test/js/node/crypto/crypto.key-objects.test.ts	EC JWK private key import now checks for pending exceptions after decoding d and calls ec.checkKey() after setting the private key, throwing ERR_CRYPTO_INVALID_JWK on failure; new tests cover successful import, mismatched d, truncated d, and zero-padded d across prime256v1, secp384r1, and secp521r1 curves.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and specifically summarizes the main change to EC JWK private-key validation in createPrivateKey.
Description check	✅ Passed	The description includes both required sections and provides detailed implementation and verification information.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 7:05 AM PT - Jul 8th, 2026}

❌ @robobun, your commit cc6043a has 1 failures in Build #70487 (All Failures):

• test/js/sql/postgres-binary-array-bounds.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 33752

That installs a local version of the PR into your bun-33752 executable, so you can run:

bun-33752 --bun

[claude]

LGTM — small, well-tested tightening of EC JWK validation to match Node.

Extended reasoning...

Overview

This PR adds a single ec.checkKey() call (wrapping BoringSSL's EC_KEY_check_key) after setting the private scalar in KeyObject::getKeyObjectHandleFromJwk, so that an EC JWK whose d does not correspond to its (x, y) is rejected with ERR_CRYPTO_INVALID_JWK at import time — matching Node.js and Bun's own WebCrypto importer. It also adds a missing RETURN_IF_EXCEPTION after base64-decoding d, mirroring the sibling x/y decode sites just above. New tests in crypto.key-objects.test.ts cover mismatched, truncated, and leading-zero-padded d across P-256/P-384/P-521, plus a positive self-verify control.

Security risks

None introduced. This change only tightens input validation (fails closed) using the standard BoringSSL key-consistency check; it cannot cause previously-rejected keys to be accepted. Rejecting inconsistent JWKs is a correctness/safety improvement — the prior behavior silently produced a KeyObject that couldn't verify its own signatures.

Level of scrutiny

While this is crypto code, the change is mechanical: a 4-line addition calling an existing, already-used ncrypto helper on the exact error path pattern used by the surrounding code, and it directly mirrors both Node's crypto_ec.cc and Bun's CryptoKeyEC::platformImportJWKPrivate. It is also the sibling of the already-merged #32910 (same fix for OKP JWKs in the same function). Low scrutiny is appropriate.

Other factors

No CODEOWNERS entry covers this path, no outstanding human review comments, and the multi-agent bug hunter found nothing. The added RETURN_IF_EXCEPTION is an unambiguous fix for a missing exception check. Test coverage is thorough (positive control, two rejection cases, one acceptance-of-equivalent-encoding case, three curves).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/crypto/crypto.key-objects.test.ts`:
- Around line 613-646: The curve matrix in the createPrivateKey EC JWK rejection
suite is manually expanded with a for loop around test(), but this test file
should use parameterized helpers instead. Refactor the "createPrivateKey rejects
inconsistent EC JWK" block to use describe.each() or test.each() for the
namedCurve cases, keeping the existing assertions and symbols like
generateKeyPairSync, createPrivateKey, createPublicKey, and verify unchanged
inside the shared test body.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a5c98878-638c-4abf-83eb-5c050b64f33d

📥 Commits

Reviewing files that changed from the base of the PR and between 332f744 and cc6043a.

📒 Files selected for processing (2)

• src/jsc/bindings/node/crypto/KeyObject.cpp
• test/js/node/crypto/crypto.key-objects.test.ts

[claude]

No issues found — the change is small, strictly tightens validation, and mirrors both Node's crypto_ec.cc and Bun's own JSECDHPrototype.cpp checkKey() usage, but key-import validation in node:crypto is worth a human sign-off.

Extended reasoning...

Overview

The PR adds an EC_KEY_check_key call (via the existing ECKeyPointer::checkKey() wrapper) after setting the private scalar in the EC-JWK branch of KeyObject::getKeyObjectHandleFromJwk, so a JWK whose d doesn't correspond to its (x, y) now throws ERR_CRYPTO_INVALID_JWK instead of producing a self-inconsistent KeyObject. It also adds the missing RETURN_IF_EXCEPTION after base64-decoding d, matching every other decodeJwkString call site in the same function. New tests cover mismatched, truncated, and leading-zero-padded d across P-256/P-384/P-521, with a positive control that a valid key still imports and self-verifies.

Security risks

This is a hardening change: it makes key import strictly more restrictive (fail-closed) and cannot cause previously-rejected keys to be accepted. The only behavioral risk would be rejecting a key that Node accepts, but the PR cites Node's identical check and includes the padded-d case to guard against over-rejection. MarkPopErrorOnReturn is already active in this scope, so the extra OpenSSL error-queue entry from a failed EC_KEY_check_key is cleaned up.

Level of scrutiny

This is production crypto key-parsing code in src/jsc/bindings/node/crypto/. Even though the diff is five native lines using an existing wrapper in the same pattern as JSECDHPrototype.cpp:98, key-import validation is exactly the surface where a human reviewer should confirm the Node-parity claim and the padded-scalar acceptance behavior.

Other factors

No CODEOWNERS cover this path. No prior reviews or outstanding comments. The bug-hunting pass found nothing. The added RETURN_IF_EXCEPTION is independently correct per the file's own conventions. Tests are well-structured (positive + three negative variants per curve, exact error code asserted). The PR is a sibling of #32910 (same fix for OKP), which suggests the pattern has already been accepted once.

[robobun]

The diff is ready. The two CI runs so far each hit a single unrelated infra flake:

• #70477: test/js/sql/sql-prepare-false.test.ts timed out 4x on darwin-x64 (all other lanes green; crypto.key-objects.test.ts not in that shard)
• #70487: test/js/sql/postgres-binary-array-bounds.test.ts and postgres-invalid-message-length.test.ts failed on one windows-x64 shard with ERR_POSTGRES_CONNECTION_REFUSED (Postgres service down on that box; all linux-x64-asan shards and the other Windows shards green)

Neither touches crypto; both are SQL infra on a single lane each.