node:crypto: support SHA-3 digests in sign and verify

[robobun]

Repro

crypto.getHashes() advertises sha3-256 and createHash()/createHmac() compute it correctly, but every signing entry point throws, for both RSA and EC keys.

const msg = Buffer.from("sha3 probe");
const rsa = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
const ec = crypto.generateKeyPairSync("ec", { namedCurve: "P-256" });

crypto.getHashes().includes("sha3-256");                  // true
crypto.createHash("sha3-256").update(msg).digest("hex");  // works

crypto.sign("sha3-256", msg, rsa.privateKey);             // ERR_OSSL_UNKNOWN_ALGORITHM_TYPE
crypto.createSign("sha3-256").update(msg).sign(rsa.privateKey); // TypeError: Failed to create signature
crypto.sign("sha3-256", msg, ec.privateKey);              // ERR_OSSL_INVALID_DIGEST_TYPE

Node signs and verifies sha3-* for RSA and EC, so code that feature-detects a digest via getHashes() before signing (the jose/jsonwebtoken pattern) is correct on Node and fails at runtime on Bun.

Cause

Our BoringSSL fork ships EVP_sha3_* (added in #29323) but never wired them into the signing paths:

• kPKCS1SigPrefixes in crypto/fipsmodule/rsa/rsa.cc.inc has no SHA-3 DigestInfo entries, so RSA_sign() bails with RSA_R_UNKNOWN_ALGORITHM_TYPE.
• pkey_ec_ctrl() in crypto/evp/p_ec.cc rejects any digest outside SHA-1/SHA-2 with EVP_R_INVALID_DIGEST_TYPE — even though pkey_ec_sign()/pkey_ec_verify() hand the digest straight to ECDSA_sign()/ECDSA_verify() and never read dctx->md.

RSA-PSS was already fine: it routes through RSA_sign_pss_mgf1(), which takes the EVP_MD directly and needs no DigestInfo.

Separately, Sign.prototype.sign() threw a bare TypeError when the signing operation itself failed, so err.code was undefined where the one-shot crypto.sign() reports ERR_OSSL_*. The three sibling failure sites in the same function already use throwCryptoError().

Fix

patches/boringssl/sha3-sign.patch (the build system's mechanism for source patches to a github-archive dep):

• Add the four id-sha3-* DigestInfo prefixes. The OIDs come from the NIST CSOR arc 2.16.840.1.101.3.4.2.{7,8,9,10}, with the NULL parameters RFC 8017 specifies for PKCS#1 v1.5. The exact bytes were read back out of OpenSSL's own output rather than transcribed: sign with Node, raw-modexp the signature with the public key, and read the EM block.
• Allow NID_sha3_{224,256,384,512} through the ECDSA digest check.

JSSign.cpp: route the signInto() failure through throwCryptoError(..., ERR_get_error(), ...), matching the one-shot path and CryptoErrorList::capture(). createCryptoError() falls back to a plain Error with the original message when OpenSSL queued nothing, so this is never worse than the TypeError it replaces.

Verification

Signatures are interoperable with OpenSSL, not merely self-consistent:

• Known-answer vectors. BoringSSL already ships wycheproof vectors for exactly this. All 2334 rsa_signature_*_sha3_* and ecdsa_*_sha3_* cases pass, matching Node 1:1 — including every negative vector (wrong OID, BER-encoded padding, tampered signatures), so malformed signatures are still rejected.
• Byte identity. RSA PKCS#1 v1.5 is deterministic; with a fixed key Bun now produces byte-identical signatures to Node for all four digests, and each runtime verifies the other's RSA and ECDSA output.

The regression test pins the Node-produced signatures rather than round-tripping, so a wrong DigestInfo encoding would fail it.

Fail-before / pass-after, per half

build	result
released Bun (neither fix)	21 of the new tests fail
BoringSSL patch reverted, JSSign.cpp fix kept	the 20 SHA-3 tests fail
src/ reverted, BoringSSL patch kept	the error-code test fails
both applied	905 pass, 0 fail across test/js/node/crypto/

Note the patch lives in patches/ + scripts/build/, so a src/-only revert does not exercise it; the middle row is the fail-before for that half.

Unchanged: test/js/web/crypto/ (40 pass). The three test/js/node/tls/ failures on this branch reproduce on released Bun or are a 5s default-timeout on a debug+ASAN test that spawns three TLS children (passes at 30s).

Scope: shake128/shake256 and BLAKE2 still throw for signing, as they do on Node for RSA. The RSA-SHA3-* signature-algorithm aliases are still missing, but getHashes() does not advertise them either, so there is no contradiction there.

The patch can be dropped once the fork carries the change and BORINGSSL_COMMIT moves.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 20ba795e-e077-4205-abcd-25dc66be2927

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and c0aa5f2.

📒 Files selected for processing (4)

• patches/boringssl/sha3-sign.patch
• scripts/build/deps/boringssl.ts
• src/jsc/bindings/node/crypto/JSSign.cpp
• test/js/node/crypto/crypto.test.ts

---

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

[robobun]

^{Updated 5:06 PM PT - Jul 6th, 2026}

❌ @robobun, your commit c0aa5f2 has some failures in Build #69216 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33518

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

bun-33518 --bun

[github-actions]

Found 1 issue this PR may fix:

1. getHashes function does not report all available hashes #20592 - Reports that getHashes() is missing SHA-3 variants (sha3-224, sha3-256, sha3-384, sha3-512, RSA-SHA3-*) and that libraries like @sap/xssec feature-detect via getHashes() before signing — exactly the pattern this PR fixes by wiring SHA-3 digests into BoringSSL's signing paths.

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #20592

🤖 Generated with Claude Code

[robobun]

Found 1 issue this PR may fix: #20592

Checked, and I don't think it does — Fixes #20592 would close it prematurely.

#20592 asks for getHashes() to return a complete list. It's still missing the RSA-* signature-algorithm aliases (RSA-SHA3-256, RSA-MD5, …), blake2b512/blake2s256, and shake128/shake256.

This PR is the inverse problem: the digests getHashes() already advertises threw from every sign/verify entry point. It doesn't touch getHashes(), and the list is identical before and after:

md4 md5 ripemd160 sha1 sha224 sha256 sha384 sha512 sha512-224 sha512-256 sha3-224 sha3-256 sha3-384 sha3-512

So the two are adjacent but disjoint: #20592 is "the list is too short", this is "the advertised entries don't work for signing". Leaving it open.

For what it's worth, the RSA-SHA3-* aliases are the one place the two overlap — Node accepts them as signing algorithm names and Bun doesn't. I left them out here because Bun's getHashes() doesn't advertise them either, so there's no contradiction to fix, and the alias table is already being touched by #32818.

[claude]

I didn't find any issues — the DigestInfo prefixes and tests look correct — but since this patches BoringSSL's RSA/ECDSA signing tables it's worth a human sanity-check before merging.

Extended reasoning...

Overview

This PR wires SHA-3 digests into crypto.sign()/crypto.verify() by:

• Adding a vendored-source patch (patches/boringssl/sha3-sign.patch) that (a) appends four id-sha3-* DigestInfo prefixes to kPKCS1SigPrefixes in crypto/fipsmodule/rsa/rsa.cc.inc, and (b) whitelists NID_sha3_{224,256,384,512} in pkey_ec_ctrl()'s digest check in crypto/evp/p_ec.cc.
• Registering the patch via the existing patches: field on the boringssl Dependency in scripts/build/deps/boringssl.ts (same mechanism already used by highway, libuv, lshpack, tinycc, etc.).
• A one-line change in src/jsc/bindings/node/crypto/JSSign.cpp swapping throwTypeError for throwCryptoError(..., ERR_get_error(), ...) on the signInto() failure path, matching the three sibling failure sites in the same function.
• ~120 lines of new tests in test/js/node/crypto/crypto.test.ts that pin byte-exact Node/OpenSSL-produced RSA PKCS#1 v1.5 signatures, verify PSS/ECDSA signatures from OpenSSL, round-trip ECDSA, and assert tampered/wrong-digest signatures are rejected.

The referenced fixture files (rsa_private_2048.pem, rsa_public_2048.pem, ec_p256_private.pem, ec_p256_public.pem) already exist in test/js/node/crypto/fixtures/.

Security risks

The change is additive on both BoringSSL paths — it does not relax any existing check for SHA-1/SHA-2 signatures. The DigestInfo bytes match RFC 8017 §9.2 / Appendix B.1 for the NIST CSOR OIDs 2.16.840.1.101.3.4.2.{7,8,9,10} with NULL parameters (I spot-checked the DER: 06 09 60 86 48 01 65 03 04 02 08 decodes to id-sha3-256, and the outer/inner SEQUENCE and OCTET STRING lengths line up with each digest size). The ECDSA change only widens a digest whitelist; ECDSA_sign/ECDSA_verify operate on the raw digest bytes and don't care which hash produced them. The negative tests confirm tampered signatures are still rejected.

That said, this is a source-level patch to a crypto library's signature encoding/verification tables. A transcription error here would produce signatures that fail to interoperate with OpenSSL (mitigated by the pinned known-answer tests) or, in the worst case, could weaken verification. This is squarely security-sensitive.

Level of scrutiny

High. Patching crypto/fipsmodule/rsa/rsa.cc.inc and crypto/evp/p_ec.cc in vendored BoringSSL is exactly the category the approval guidelines call out for human review. The PR is well-researched (bytes derived by raw-modexp'ing an OpenSSL signature, wycheproof vectors run, fail-before/pass-after matrix documented) and the diff is small and focused, so review should be quick — but it should be a human.

Other factors

• No CODEOWNERS cover the touched paths.
• The patches: mechanism is well-established across ~10 other deps; the build-script change is a 3-line addition following that convention.
• The JSSign.cpp change is a strict improvement in error fidelity and mirrors the neighboring throwCryptoError calls in the same function; ncrypto::ClearErrorOnReturn at the top of jsSignProtoFuncSign ensures the OpenSSL error queue starts clean, so ERR_get_error() here reads the actual signing failure.
• No prior human reviews or outstanding comments on the timeline; the bug hunter found nothing.

[robobun]

Agreed on the framing, and thanks for independently decoding the DER. Since the ask is a human sanity-check of the two BoringSSL tables, here are the two checks that don't require trusting anything in this PR.

1. Re-derive the DigestInfo prefixes from OpenSSL, no transcription involved. RSA PKCS#1 v1.5 is deterministic, so raw-modexp a Node signature with the public key and read the EM block back out. This is how the bytes in the patch were produced, not from a spec table:

// node digestinfo.js
const crypto = require("crypto");
const { privateKey, publicKey } = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
const jwk = publicKey.export({ format: "jwk" });
const b64u = s => BigInt("0x" + Buffer.from(s, "base64url").toString("hex"));
const n = b64u(jwk.n), e = b64u(jwk.e);
const modpow = (b, x, m) => { let r = 1n; b %= m; while (x > 0n) { if (x & 1n) r = r * b % m; b = b * b % m; x >>= 1n; } return r; };

for (const alg of ["sha3-224", "sha3-256", "sha3-384", "sha3-512"]) {
  const sig = crypto.sign(alg, Buffer.from("x"), privateKey);
  const em = Buffer.from(modpow(BigInt("0x" + sig.toString("hex")), e, n).toString(16).padStart(512, "0"), "hex");
  let i = 2; // EM = 0x00 || 0x01 || 0xFF* || 0x00 || DigestInfo
  while (em[i] === 0xff) i++;
  const di = em.subarray(i + 1);
  const digest = crypto.createHash(alg).update("x").digest();
  console.log(alg, [...di.subarray(0, di.length - digest.length)].map(b => "0x" + b.toString(16).padStart(2, "0")).join(", "));
}

Output is byte-for-byte the four bytes[19] arrays in the patch.

2. Re-run the known-answer vectors. BoringSSL already vendors them, so no new fixtures are needed:

$ ls vendor/boringssl/third_party/wycheproof_testvectors | grep sha3_
ecdsa_secp256r1_sha3_256_test.json    rsa_signature_2048_sha3_256_test.json
ecdsa_secp256r1_sha3_512_test.json    rsa_signature_2048_sha3_512_test.json
ecdsa_secp521r1_sha3_512_test.json    rsa_signature_3072_sha3_256_test.json
...

Feeding every valid/invalid case through crypto.verify() gives 2334 pass / 0 fail on this branch, matching Node 1:1. The invalid cases are the interesting half: wrong OID, BER-encoded padding, truncated and tampered signatures. They all still get rejected, which is the property a bad prefix table would break.

On ERR_get_error(): slightly stronger than you described. ClearErrorOnReturn's constructor calls ERR_clear_error() (ncrypto.cpp:49), not just its destructor, and it's the first statement of jsSignProtoFuncSign. So the queue is provably empty on entry and ERR_get_error() reads exactly what signInto() pushed. get rather than peek also matters downstream: it pops the error before createCryptoError() runs CryptoErrorList::capture(), so the error doesn't get duplicated into opensslErrorStack. Observed shape now:

name:    Error                                  (was TypeError)
code:    ERR_OSSL_DIGEST_TOO_BIG_FOR_RSA_KEY    (was undefined)
message: error:04000076:RSA routines:OPENSSL_internal:DIGEST_TOO_BIG_FOR_RSA_KEY
opensslErrorStack: undefined                    (no duplicate)

Node reports ERR_OSSL_RSA_DIGEST_TOO_BIG_FOR_RSA_KEY for the same failure. The RSA_ prefix difference is BoringSSL's reason string, and it already affects the one-shot crypto.sign() path today, so the test asserts the two entry points agree with each other rather than hardcoding Node's string.

[claude]

I didn't find any issues — the DigestInfo bytes and test coverage check out — but patching BoringSSL's RSA/ECDSA signing tables is the kind of change I'd rather a human sign off on.

Extended reasoning...

Overview

This PR wires SHA-3 digests into BoringSSL's signing paths via a source patch (patches/boringssl/sha3-sign.patch) applied through the existing patches mechanism in scripts/build/deps/boringssl.ts. The patch adds four DigestInfo prefix entries to kPKCS1SigPrefixes in rsa.cc.inc and widens the ECDSA digest whitelist in p_ec.cc. A one-line change in JSSign.cpp swaps a bare throwTypeError for throwCryptoError(..., ERR_get_error(), ...) to match the sibling failure sites in the same function. Tests pin Node/OpenSSL-produced signatures (deterministic PKCS#1 v1.5) plus verify-only PSS/ECDSA vectors and negative cases.

Security risks

The change is purely additive — new table rows and a widened whitelist — and doesn't touch any existing SHA-1/SHA-2 entries or the padding/verification logic itself. I independently checked the DER: the OID arc 2.16.840.1.101.3.4.2.{7,8,9,10} encodes to 60 86 48 01 65 03 04 02 {07,08,09,0a}, the outer/inner SEQUENCE lengths (0x2d/0x31/0x41/0x51 and 0x0d) and OCTET STRING lengths (0x1c/0x20/0x30/0x40) all agree with RFC 8017 §9.2 note 1, and the digest-length constants match. The author's wycheproof run (2334 vectors including negative cases) and byte-identity check against Node are the right validations. That said, the kPKCS1SigPrefixes table drives both RSA_sign and RSA_verify's constant-time comparison, so a transcription error here would be a real interop/security concern — hand-entered crypto constants are exactly where a second pair of human eyes is warranted.

Level of scrutiny

High. This is a source patch to the vendored crypto library's core signing/verification tables. The JSSign.cpp and build-script changes are trivial and I'd approve them on their own; the BoringSSL patch is small and looks correct, but "patches vendored BoringSSL" is a category I'm not comfortable auto-approving regardless of how good the verification story is.

Other factors

The patches field on Dependency is an established mechanism (scripts/build/source.ts:426) already used by several other deps, so the delivery vehicle is conventional. Test fixtures referenced by the new tests exist on disk. No CODEOWNERS entries cover these paths. No bugs were found by the bug-hunting pass. The PR thread already lays out reproducible self-checks (modexp derivation, wycheproof), which should make human review quick.

[robobun]

CI status: green on the diff, red only on macOS artifact-download infra

The diff is passing everywhere a binary actually ran. The only red is the macOS artifact-download step, not a test failure, and it is not caused by this change.

Two builds, same story:

• Build #69141: every test lane green (debian/ubuntu/alpine x64 + aarch64 + x64-baseline + x64-asan, windows 2019 x64 + x64-baseline, windows 11 aarch64). Only darwin 26 aarch64 - test-bun red.
• Build #69216 (retrigger): same, except one of the two darwin 26 aarch64 - test-bun shards passed this time and the other failed.

The failure is an artifact download timing out, with zero tests run:

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

The darwin-aarch64-build-bun step that produces that artifact passed on both builds, so the BoringSSL patch compiles and links fine on macOS aarch64. And in #69216 a sibling darwin 26 aarch64 shard downloaded the same artifact and ran the full suite, crypto tests included, green. The red shard simply couldn't pull the binary within 120s. The darwin 14 lanes never started (agents stuck in scheduled).

I've used my one retrigger. Rather than spam more empty commits, flagging for a maintainer: the change is ready and the remaining red is a macOS S3/agent hiccup unrelated to the diff. A re-run of that one lane should clear it.

Summary of what's in the PR, unchanged:

• BoringSSL patch: SHA-3 DigestInfo prefixes for RSA PKCS#1 v1.5 + SHA-3 NIDs through the ECDSA digest check.
• JSSign.cpp: createSign().sign() now surfaces the OpenSSL error via throwCryptoError instead of a bare code-less TypeError.
• Verified against BoringSSL's own vendored wycheproof vectors (2334 pass, negatives included) and byte-identical to Node's OpenSSL output.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.