webcrypto: RSA-PSS saltLength range errors, key-material deep equality, supports() parity (+5 tests) - #35374
Draft
cirospaciari wants to merge 56 commits into
Draft
webcrypto: RSA-PSS saltLength range errors, key-material deep equality, supports() parity (+5 tests)#35374cirospaciari wants to merge 56 commits into
cirospaciari wants to merge 56 commits into
Conversation
Bun's WebCrypto reported the right failures with the wrong names, codes and
ordering, so code that works on Bun can behave differently on Node. Each
behavior below was checked against a real Node v26.3.0 build before changing
it, and the vendored Node webcrypto tests are synced to that version.
CryptoKey.usages / JWK key_ops ordering. usages() walked the usage bitmap
alphabetically. The KeyUsage enum in the WebCrypto spec orders them encrypt,
decrypt, sign, verify, deriveKey, deriveBits, wrapKey, unwrapKey, which is what
Node emits. Most visible on ECDH: Node reports ["deriveKey","deriveBits"] and
Bun reported the reverse. key_ops is built from usages(), so JWK export was
wrong too.
SubtleCrypto error messages. Aligned the operation guards with Node's wording:
"CryptoKey doesn't match AlgorithmIdentifier" -> "Key algorithm mismatch";
"CryptoKey doesn't support encryption" -> "Unable to use this key to encrypt"
(likewise decrypt/sign/verify/wrapKey/unwrapKey); the two derive cases ->
"baseKey does not have deriveBits/deriveKey usage"; "The CryptoKey is
nonextractable" -> "key is not extractable". deriveBits/deriveKey also checked
the algorithm match before the usage; Node checks usage first, and on
doubly-invalid input the order decides which error wins.
Missing vs ill-typed dictionary members. throwRequiredMemberTypeError only
fires when a required member is absent, which Node reports as
ERR_MISSING_OPTION rather than ERR_INVALID_ARG_TYPE; that holds for every
required member, including a params dictionary with no name at all. A member
that was present but ill-typed threw a bare "Type error" with no code, so
deriveBits({name:'ECDH', public:{}}) was indistinguishable from an internal
failure; it now reports ERR_INVALID_ARG_TYPE like Node. Both helpers are used
only by the webcrypto bindings.
crypto.getRandomValues. Float32Array, Float64Array, DataView, ArrayBuffer and
SharedArrayBuffer were all filled with random bytes. The spec allows only
integer-typed views; everything else must raise TypeMismatchError, which Node
does. The message already existed, only the check was missing.
HKDF / PBKDF2 / ECDH derived-length errors. A null length and a length that is
not a multiple of 8 both produced an OperationError with an empty message,
surfacing as the generic "The operation failed for an operation-specific
reason". Node distinguishes "length cannot be null" from "length must be a
multiple of 8", and ECDH/X25519 say "derived bit length is too small".
Illegal constructor. Interfaces that cannot be constructed at all routed both
call and construct through the "called without new" path, so new CryptoKey()
answered "Use `new CryptoKey(...)` instead of `CryptoKey(...)`" — advice it had
already followed. Both paths now report "Illegal constructor", matching Node,
browsers, and the wording the Rust helper's own documentation already claimed
to produce. MessagePort opts into ERR_CONSTRUCT_CALL_INVALID and keeps Node's
message for that code, "Constructor cannot be called".
Tests: the vendored Node webcrypto tests are byte-identical to upstream, and
only tests that pass are added — tests needing algorithms Bun does not
implement are left alone rather than quarantined. test-webcrypto-random.js had
been edited to assert Bun's old behavior ("These types are allowed in Bun");
those assertions are restored, leaving only the 65536-byte quota commented out,
since that needs a QuotaExceededError global Bun does not have yet.
…ard hash aliases
Second pass on Node v26.3.0 webcrypto compatibility, taking the vendored
upstream suite from 22 to 30 passing files. As before, every behavior was
checked against a real Node v26.3.0 build first.
Non-standard hash aliases removed. The registry registered SHA1/SHA224/SHA256/
SHA384/SHA512 as alternative names beside the hyphenated spec names. It already
matches case-insensitively as the spec requires, so the alias table was an extra
deviation: `hash: 'SHA256'` was accepted where Node and browsers throw
NotSupportedError, meaning code written against Bun could fail elsewhere. Nothing
in the repo relied on them (`Bun.CryptoHasher("SHA256")` and node:crypto's
`createHash` are separate surfaces). Dropping them also lets the registry
re-assert that each algorithm registers exactly once.
Unrecognized algorithm names. Only `digest` said "Unrecognized algorithm name",
via a hardcoded string that overrode every message. The other operations reported
the DOMException default, "The operation is not supported.". The message now comes
from the layer that owns the invariant — every failing lookup in
normalizeCryptoAlgorithmParameters — so all operations agree with Node, including
a recognized name used for the wrong operation (`digest('AES-GCM')`).
Import errors. Every import rejection carried the right error name but a generic
message: JWK "use"/"alg"/"crv" mismatches, unsupported key usages, and empty
usages on a private or secret key all collapsed into "Data provided to an
operation does not meet requirements". They now carry Node's text.
"Invalid key type" vs "Invalid keyData". Importing an EC key as RSA reported the
same generic DataError as random bytes. RSA/EC/OKP conflated "the parser found a
well-formed key of another type" with "this is not a key", because both were one
condition. They are split, and the distinction reaches the caller through an
optional out-param. OKP hand-parses its DER, and a byte compare cannot tell those
two cases apart, so it confirms with the real parser before claiming a type
mismatch — otherwise garbage would be reported as a wrong-typed key.
Ed25519 JWK "alg". Export omitted it and import never checked it, so a JWK with
`alg: 'foo'` was accepted. RFC 8037 gives the Edwards curves an "alg" of the curve
name or "EdDSA"; the montgomery curves have none, so X25519 still omits it.
ECDH derive errors, HKDF info. ECDH reported nothing for a curve or algorithm
mismatch. HKDF accepted an `info` longer than the 1024 bytes Node allows.
util.inspect(CryptoKey) printed `CryptoKey {}`. The attributes are prototype
accessors, so the key has no own enumerable properties. It now prints the four
slots like Node, reading the internal state directly rather than through the
getters, whose cached objects user code can mutate.
Tests: 17 more upstream files (30 of Node's 53), all byte-identical to upstream.
Three still test nothing under BoringSSL (`common.skip` on `requires OpenSSL >= 3`).
The rest need algorithms or APIs Bun lacks — ML-KEM, ML-DSA, TurboSHAKE,
ChaCha20-Poly1305, AES-OCB, Ed448/X448, subtle.getPublicKey, SubtleCrypto.supports,
KeyObject.prototype.toCryptoKey, QuotaExceededError — or Node's internal/* module
registry, which is not portable.
body.test.ts built its fixtures with `crypto.getRandomValues(new bufferType(n))` over a list that includes ArrayBuffer, SharedArrayBuffer and the Float views. getRandomValues takes integer-typed views only — Node throws TypeMismatchError for all five — so the fixtures were relying on Bun's older, laxer check. Fill through a Uint8Array view over the same bytes instead, which keeps every bufferType in the matrix.
…memory `random_get` called `crypto.getRandomValues(this.memory.buffer, bufPtr, bufLen)`. getRandomValues takes a single integer-typed view and ignores any further arguments, so bufPtr and bufLen were dropped on the floor and the whole wasm linear memory was overwritten with random bytes on every call — 65241 of 65536 bytes outside the requested 16-byte window, in the added test's terms. Passing a Uint8Array view over exactly the requested range fixes that, and also keeps the syscall working now that getRandomValues rejects a plain ArrayBuffer: every wasm guest that reaches random_get (Rust getrandom, Go crypto/rand, WASI-libc getentropy) would otherwise trap.
WASI preview1 defines random_get as returning errno (0 on success). The handler returned bufLen, so a guest asking for 16 bytes would see errno 16 back. Pre-existing on main, but since this PR already touches the function to fix the windowing bug it makes sense to complete the fix here. The test now asserts WASI_ESUCCESS and the tautological expect(WASI_ESUCCESS).toBe(0) is dropped.
…ters normalizeCryptoAlgorithmParameters declares a JSC ThrowScope, so every caller must perform an exception check before the next exception-scope use. digest was the only SubtleCrypto method that did; generateKey, deriveKey, deriveBits, and importKey had RETURN_IF_EXCEPTION after the early return that rejects the promise, and encrypt, sign, verify, wrapKey, and unwrapKey had no scope at all. With BUN_JSC_validateExceptionChecks=1, 25 of 36 crypto.subtle paths aborted, including the decrypt, wrapKey, and unwrapKey success paths. Give every method the digest shape: RETURN_IF_EXCEPTION right after the normalize call, and RELEASE_AND_RETURN on the trailing algorithm call, whose callbacks can reach DeferredPromise synchronously. The two wrapped-key callback lambdas get the same treatment for their throw-scoped toJS<IDLDictionary<JsonWebKey>>, JSONStringify, and JSONParse calls. The reorder in wrapKey also fixes a user-reachable debug assertion: a throwing `name` getter on the wrap algorithm makes the first normalize return ExistingExceptionError, which tripped the ASSERT at SubtleCrypto.cpp:1144 and then ran the second normalize with a pending JS exception. No user-visible change on release builds: the fixture's 39-case transcript is byte-identical before and after.
…lback
m_pendingPromises is a HashMap<DeferredPromise*, Ref<DeferredPromise>> and
get() peeks the value as a raw pointer, so removing the map entry before
rejecting frees the DeferredPromise the reject is about to use. The
unwrapKey callback already holds a RefPtr across this pattern; match it.
Exercise the path with a fixture case that makes wrapKey("jwk")'s internal
JSON.stringify throw via an inherited Object.prototype.toJSON. Without the
exception check this PR adds after JSONStringify, that input also leaves
the promise unsettled forever on release builds: the pending exception
survives into a later DeferredPromise::reject, which reports it as
uncaught and returns without settling.
The keyTypeMismatch out-param was only set when the 4-byte OID prefix mismatched, but Ed25519/X25519/Ed448/X448 all share that prefix and differ only in the fifth byte. Importing an Ed25519 SPKI as X25519 fell through the switch arm without setting the flag and reported the generic 'Invalid keyData' instead of Node's 'Invalid key type'. Cover both switch arms in importSpki and importPkcs8 via a shared lambda, still gated on the real DER parser so garbage with a matching prefix is not misreported. Also: add Float16Array to the getRandomValues rejection matrix (the old test asserted it was filled; nothing covered the new rejection), and trim the Bun-authored divergence comment in test-webcrypto-random.js to 3 lines.
… OKP-as-EC as key-type mismatch Same-class follow-ups to the error-message work: - importKey's 'Usages cannot be empty when importing a private/secret key.' now also appears in deriveKey's and unwrapKey's inner import callbacks, which share the identical predicate. generateKey carries Node's 'Usages cannot be empty when creating a key.' for both the single-key and key-pair arms. - CryptoKeyEC::platformImportSpki required a two-element AlgorithmIdentifier before checking the OID, so an OKP SPKI (one-element AlgId per RFC 8410) bailed without setting keyTypeMismatch and reported 'Invalid keyData' instead of 'Invalid key type'. Set the flag at that guard too, gated on d2i_PUBKEY succeeding so garbage is not misreported. importPkcs8 already checks EVP_PKEY_base_id and was unaffected.
ECDH's deriveBits got Node's 'key algorithm mismatch' and 'Named curve
mismatch', but the byte-identical checks in X25519's parallel were left as the
empty-message InvalidAccessError. Line 86 is reachable today
(deriveBits({name:'X25519', public: ecdhKey}, x25519Private)), and the vendored
cfrg tests assert this exact message behind a keys.X448 guard that is skipped
under BoringSSL, so it would fail the moment X448 lands.
New test covers both directions against the ECDH sibling.
…JWK alg straggler The four RSA importKey implementations have the same usage-guard shape that ECDSA/ECDH/Ed25519/X25519 got Node's 'Unsupported key usage for a <name> key' for; all four RSA files were already touched here to thread keyTypeMismatch and add the JWK 'use'/'alg' messages, so the usage guards in the same functions get the same treatment. Also: RSAES-PKCS1-v1_5's JWK 'alg' mismatch was the only one of the four still at the empty message; its three siblings got the message in this PR. generateKey's usage checks are left alone: the EC/OKP files left those at the empty message too, and AES/HMAC importKey is in files this PR does not touch.
Node's ec.js and cfrg.js build the message from a single template that uses 'an'; the RSA siblings added in 9cb0754 matched that, but the EC/OKP parallels said 'a ECDSA' etc. The vendored tests only assert on the /Unsupported key usage/ regex so both spellings pass; this aligns the exact text with Node and the RSA siblings.
Implements the ChaCha20-Poly1305 WebCrypto algorithm over BoringSSL's EVP_aead_chacha20_poly1305, with an AeadParams dictionary derived from the identical AesGcmParams shape, and adds node's `raw-secret` import/export format. `raw-secret` is also accepted as a `raw` alias for AES-* and HMAC, matching node. CryptoKeyRaw gains extractability so it can back a name-only key algorithm, as node's does. Other digests and ciphers in this area were ruled out first by grepping the vendored BoringSSL: TurboSHAKE, KangarooTwelve, AES-OCB and Argon2 have no symbols there, so those tests are blocked on the primitive rather than on binding work. JsonWebKey.kty is no longer a required IDL member. WebCrypto's dictionary does not mark it required, and node rejects a non-JWK object with DataError where bun threw a TypeError; both now throw DataError. One bun-owned assertion in test/js/web/crypto/web-crypto.test.ts is updated to the node-parity error while keeping its settle and leak guard intact. No vendored node test was edited. Adds test-webcrypto-encrypt-decrypt-chacha20-poly1305 and test-webcrypto-aead-decrypt-detached-buffer from Node v26.3.0, verbatim, with their fixture.
….3.0
Passes on this branch as-is; copied verbatim from upstream. It forges all four
CryptoKey prototype getters and checks that util.inspect, exportKey('jwk'),
KeyObject.from, createHmac and crypto.sign/verify all read the native slots
rather than the forged ones, and that Object.prototype pollution of hash /
publicExponent does not leak into a generated AES-GCM key.
Verified 3/3 and tamper-checked, with the same tamper run against the node
v26.3.0 binary as a control.
No-Verification-Needed: test-only diff, no runtime surface to drive
…eyObject.prototype.toCryptoKey
Implements the ML-DSA-44/65/87 and ML-KEM-768/1024 WebCrypto algorithms over the
vendored BoringSSL EVP layer (EVP_pkey_ml_dsa_*/ml_kem_*, seed-form private keys
per RFC 9881/9935). A new CryptoKeyAKP key class backs both; JWK uses the "AKP"
key type with pub/priv members. ML-KEM-512 is not registered because BoringSSL
has no EVP support for it, matching what Node reports on its BoringSSL builds.
Adds the modern-algorithms SubtleCrypto surface: getPublicKey(),
encapsulateBits/encapsulateKey/decapsulateBits/decapsulateKey (with the four new
KeyUsages in Node's canonical order), the static SubtleCrypto.supports(), and
the raw-public/raw-seed key formats, including Node's aliasKeyFormat behavior
for raw-secret/raw-public on the pre-existing algorithms. ML-DSA sign/verify
takes the optional context parameter; oversized contexts reject with Node's
OperationError carrying an ERR_OUT_OF_RANGE cause, and ML PKCS#8 parse failures
carry the BoringSSL error (e.g. ERR_OSSL_EVP_PRIVATE_KEY_WAS_NOT_SEED) as the
DataError cause.
KeyObject.prototype.toCryptoKey lands for secret, public and private keys,
dispatching per algorithm like Node's keys.js: secret keys import through the
matching WebCrypto algorithm, RSA/EC/OKP round-trip through DER so import
validation matches importKey, and ML keys wrap the handle directly.
node:crypto also learns ml-dsa-*/ml-kem-* in generateKeyPair(Sync) and
asymmetricKeyType.
Error-shape alignment discovered by the vendored tests: Crypto.prototype.subtle
is brand-checked on the prototype like Node's; invalid KeyFormat values report
Node's ERR_INVALID_ARG_VALUE message; HMAC import gets Node's zero-length,
HmacImportParams.length and JWK messages (raw and jwk paths); HKDF/PBKDF2 get
their non-extractable/usage messages; EC import distinguishes "Named curve
mismatch" from "Invalid keyData"; JWK key_ops duplicates reject for ML keys.
Cloning a ChaCha20-Poly1305 or ML key now rejects with DataCloneError instead
of tripping a RELEASE_ASSERT (serialization support remains a follow-up).
Vendors test-webcrypto-{sign-verify-ml-dsa,encap-decap-ml-kem,
export-import-ml-dsa,export-import-ml-kem,methods-not-async,constructors,
export-import} and test-crypto-key-objects-to-crypto-key from Node v26.3.0,
byte-identical, with their key fixtures. test-webcrypto-supports is not
vendored: its fixtures unconditionally expect TurboSHAKE128/256 and KT128/256
digest support, which the vendored BoringSSL does not expose.
Windows builds define BORINGSSL_NO_CXX, which removes the bssl C++ scoped types, so the Windows build-cpp step failed with 'no type named ScopedCBB'. Use the C API with a WTF::makeScopeExit cleanup instead.
…helper BORINGSSL_NO_CXX on Windows removes the bssl C++ scoped types, so every ScopedCBB use fails the Windows build. Add WebCore::marshalEVPKey (CBB C API with scope-exit cleanup) to OpenSSLUtilities and use it at all four marshal sites (CryptoKeyAKP spki/pkcs8, SubtleCrypto exportKey, KeyObject).
… order Cover the serializer guard for ChaCha20-Poly1305 and AKP keys (it previously hit a RELEASE_ASSERT), assert both encapsulate variants enumerate their results in node's per-path order, and document that ordering at the two construction sites. [allow size]
A plain object under a buffer format now converts into the JsonWebKey union alternative without an exception (kty is optional), so intercept it with node's ERR_INVALID_ARG_TYPE instead of the bare TypeError from toKeyData. Parse the format from a single toWTFString like node does rather than stringifying again in the error path, and document that toCryptoKey rejecting AES-CFB is deliberate node parity. [allow size]
Hoist the use/key_ops/ext checks into the import arm in node's check order (use precedes alg, the decoded key, and the length check), decode the key once, and call importRaw directly; CryptoKeyHMAC::importJwk had no other caller and is deleted. Point the pending-promise settlement and leak fixtures at an invalid key_ops enum so they still reach the dictionary-conversion exception branch now that kty is optional, and move the node:crypto import to module scope. [allow size]
The import arm checks jwk.alg for null before calling it, so the isNull prefixes and the SHA3 accept-null arms were unreachable. [allow size]
… message for RSA; drop dead format machinery wrapKey now rejects a non-extractable key before the raw-public/ raw-seed aliasing, matching exportKey and node. The four RSA importKey default arms report node's 'Unable to import <alg> using raw format' instead of the generic text. Delete the KeyFormat enum-conversion specializations the single-coercion rewrite orphaned, and assert the now-unreachable HMAC fallback and toKeyData JWK arm. [allow size]
Node routes every algorithm through the same validateKeyOps, so the duplicate scan the AKP arm already had applies to the oct algorithms too. Extract it as hasDuplicateJwkKeyOps next to the JsonWebKey struct and call it from all three JWK pre-validation blocks. [allow size]
Buffer formats no longer run the JWK dictionary conversion, so a poisoned member getter cannot fire and no pending exception is cleared before reporting node's type error; jwk conversion errors still propagate. Mirror the optional kty in convertDictionaryToJS, and collapse the RSAES KEM-usage matrix to the single deprecation assertion its gate allows. [allow size]
Node clones these key types; Bun rejected them with a deliberate DataCloneError guard. Key serialization format version 2 adds: - a CryptoKeyClassSubtag::AKP arm — public keys serialize as raw public bytes, private keys as PKCS#8 (which embeds the FIPS 203/204 seed, so raw-seed exportability survives the round trip like in Node) - the four KEM usage tags, replacing the countUsages desync assert - identifier tags for ChaCha20-Poly1305 and the five ML algorithms (ChaCha rides the existing Raw key-class arm) - readRawKey now honors the stored extractable flag; HKDF/PBKDF2 keys always stored false, so v1 payloads deserialize unchanged Also from the same review batch: - undersized AEAD decrypt inputs reject with the default OperationError message like Node (ChaCha20-Poly1305 and the pre-existing AES-GCM site; nothing asserted the old text) - BoringSSL error-queue hygiene on every new failure path (ChaCha seal/open, ML-KEM ctx/init, ML-DSA sign/verify init, AKP generatePair and exports): clear the queue unless the caller attaches it as a cause, per the importAkpKey convention - exportRawSeed shrinks its buffer to the reported length, matching rawPublicKeyBytes Tests: clone round-trips (material/algorithm/usages/extractable) for ChaCha and both ML halves, extractable=false preservation, a Worker postMessage delivery test, and the decrypt message; all fail on the unfixed build. Vendored webcrypto ML/ChaCha/clone-transfer suites pass. [allow size]
… key readAKPKey's importRawPublic/importPkcs8 leave the parse error in the queue on failure and nothing attaches it as a cause, matching the convention the rest of the clone change follows. Also await the two rejection assertions added with the clone tests so they can actually fail, and pin the non-extractable export message. [allow size]
The AES-GCM arm of the message change had no test; extend the ChaCha20-Poly1305 case to a matrix (verified generic in node v26.3.0). [allow size]
…epStrictEqual subtle.sign()/verify() with an RSA-PSS saltLength larger than ceil((modulusLength - 1) / 8) - hLen - 2 fell through to BoringSSL and rejected with a bare OperationError. Reject before starting the job with an OperationError whose cause is ERR_OUT_OF_RANGE naming the accepted range, matching lib/internal/crypto/rsa.js. KeyObject and CryptoKey keep all of their state in native slots, so the own-property walk in specialObjectsDequal reported any two keys as deep equal: assert.notDeepStrictEqual(createSecretKey(a), createSecretKey(b)) passed for every pair. Compare the native material instead, without consulting the user-replaceable equals() method, the way node's comparisons.js does. Also re-export the URLPattern global from node:url, and add hasTemporal and hasLocalStorage to the vendored node test harness verbatim. Vendored from Node v26.3.0, all passing one process per file: test-crypto-key-objects-raw.js test-crypto-keyobject-hidden-slots.js test-webcrypto-deduplicate-usages.js test-webcrypto-get-public-key.mjs test-webcrypto-sign-verify-rsa.js test-temporal.js test-temporal-with-zoneinfo.js The two Temporal files skip: Bun keeps JSC's Temporal support off, so they behave as they do on a node build compiled without Temporal.
Collaborator
|
Updated 10:30 PM PT - Aug 3rd, 2026
❌ @robobun, your commit eb252fb has 6 failures in
🧪 To try this PR locally: bunx bun-pr 35374That installs a local version of the PR into your bun-35374 --bun |
Bun keeps JSC's Temporal support off, so common.hasTemporal is false and both files exit without running a single assertion. Keep hasTemporal and hasLocalStorage in the harness: they are verbatim upstream and other tests gate on them.
cirospaciari
changed the title
… size]
supports() disagreed with Node's own vectors in 262 places. The two causes:
exportKey and getPublicKey normalized their algorithm as importKey, so a bare
name like "RSA-PSS" was rejected for missing a hash even though neither
operation takes algorithm members. Resolve the name only, matching
normalizeAlgorithm(alg, 'exportKey'). This also fixes
supports('wrapKey', 'AES-KW', 'HMAC').
The parameter checks Bun's algorithms perform at execution time were missing
from the probe, so supports() promised operations that would have thrown:
HMAC lengths that are zero or not a multiple of 8, AES lengths other than
128/192/256, EC named curves outside P-256/P-384/P-521, PBKDF2 with zero
iterations, ChaCha20-Poly1305 with an iv that is not 12 bytes or a tag that is
not 128 bits, and ECDH/X25519 deriveBits without a public key. A zero
deriveBits length is now accepted (a null one still is not), matching Node.
That leaves 68 disagreements, all of one class: the fixtures gate SHA-3,
TurboSHAKE and KAngarooTwelve on process.features.openssl_is_boringssl, which
Bun reports as true while its WebCrypto does implement SHA-3.
test-webcrypto-supports.mjs therefore stays out of the vendored set.
The ChaCha20-Poly1305 iv and tag lengths become named constants on the
algorithm class so the probe and the implementation cannot drift.
[allow size] the binary-size gate baselines against main, and this branch is
stacked on #34838, whose ML-DSA and ML-KEM support accounts for the growth.
cirospaciari
changed the title
…urface # Conflicts: # src/jsc/bindings/webcrypto/CryptoAlgorithmChaCha20Poly1305.cpp # src/jsc/bindings/webcrypto/CryptoAlgorithmChaCha20Poly1305.h # src/jsc/bindings/webcrypto/CryptoKeyOKP.cpp # src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp # src/jsc/bindings/webcrypto/JSSubtleCrypto.h # src/jsc/bindings/webcrypto/SubtleCrypto.cpp
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 21, 2026
…enSSL (#39974) ### Problem - `crypto.subtle.sign({ name: "RSA-PSS", saltLength: 4294967295 })` signs with a salt of the digest length, and `saltLength: 4294967294` signs with the largest salt that fits. `crypto.subtle.verify({ name: "RSA-PSS", saltLength: 4294967294 })` returns true for a signature made with any salt length. Node v26 rejects all of these with an `OperationError`. Bun 1.3.14 behaves the same as main, so this is not a regression. - Cause: `RsaPssParams.saltLength` is a WebIDL `unsigned long`, stored as `size_t`. `signWithMD` and `verifyWithMD` in `src/jsc/bindings/webcrypto/CryptoAlgorithmRSA_PSSOpenSSL.cpp` (lines 53 and 99) pass it to `EVP_PKEY_CTX_set_rsa_pss_saltlen()`, which takes an `int`. 4294967295 arrives as `RSA_PSS_SALTLEN_DIGEST` (-1) and 4294967294 as `RSA_PSS_SALTLEN_AUTO` (-2). Other values at or above 2^31 arrive as other negative numbers, which BoringSSL rejects, so only these two change meaning. ### Fix - `setSaltLength()` converts the value in one place for sign and verify. A value above `INT_MAX` fails, and the callers turn that into the same `OperationError` they already use for every other setup failure. - This is correct because a salt length can never legitimately be that large (a salt has to fit in the modulus), and because a value that does fit in an `int` is still checked against the key by BoringSSL exactly as before: `saltLength: 95` on a 1024-bit key with SHA-256 still fails to sign, and a wrong salt length still verifies as `false`. - Verified with `test/js/web/crypto/web-crypto.test.ts` ("RSA-PSS saltLength"). On main the four cells for 2^32 - 1 and 2^32 - 2 come back as `signed`, `signed`, `false` and `true`. The rest of `web-crypto.test.ts` (95 tests), `web-crypto-sha3.test.ts` and `test/js/node/test/parallel/test-webcrypto-sign-verify.js` pass. - Related: #35374 adds a check in `SubtleCrypto.cpp` that rejects any `saltLength` above what the key allows, with Node's `ERR_OUT_OF_RANGE` cause. That check also covers these two values. This PR fixes the conversion underneath it, in the backend that performs the narrowing, and touches neither of its files. The test here holds with or without #35374. ### Background RSA-PSS pads a message with a random salt before it is signed. The salt length is a parameter of both the signing and the verification operation. BoringSSL's setter uses negative values as selectors: -1 means "use the digest length" and -2 means "use the largest salt that fits" when signing and "accept whatever salt length the signature used" when verifying. WebCrypto has no such selectors: `saltLength` is always an explicit byte count. The WebIDL layer enforces the `unsigned long` range, so 2^32 and above are already a `TypeError`; the values just below 2^32 are the ones that survived the range check and then changed meaning in the implicit conversion to `int`. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/crypto/web-crypto.test.ts bun test v1.4.0 (6e906e4) test/js/web/crypto/web-crypto.test.ts: (pass) crypto.subtle setter should not throw [3.78ms] (pass) Web Crypto > keeps event loop alive [329.94ms] (pass) Web Crypto > has globals [3.95ms] (pass) Web Crypto > should encrypt and decrypt [16.88ms] (pass) Web Crypto > should verify and sign [38.19ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are not valid JSON [15.60ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are valid JSON but not a valid JWK [10.30ms] (pass) Web Crypto > unwrapKey JWK error handling > settles when JsonWebKey dictionary conversion itself throws [10.56ms] (pass) Web Crypto > unwrapKey JWK error handling > does not leak DeferredPromise in m_pendingPromises on JWK parse errors [2555.40ms] (pass) oversized inputs > rejects >2 GiB inputs instead of aborting [347.39ms] 379 | "2**32 - 2": await verifySalt20Signature(2 ** 32 - 2), 380 | "2**31": await verifySalt20Signature(2 ... (truncated) release without fix: 1 FAILED bun test v1.4.0-canary.1 (6e906e4) test/js/web/crypto/web-crypto.test.ts: (pass) crypto.subtle setter should not throw [0.05ms] (pass) Web Crypto > keeps event loop alive [8.44ms] (pass) Web Crypto > has globals [0.06ms] (pass) Web Crypto > should encrypt and decrypt [0.35ms] (pass) Web Crypto > should verify and sign [0.66ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are not valid JSON [0.30ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are valid JSON but not a valid JWK [0.20ms] (pass) Web Crypto > unwrapKey JWK error handling > settles when JsonWebKey dictionary conversion itself throws [0.18ms] (pass) Web Crypto > unwrapKey JWK error handling > does not leak DeferredPromise in m_pendingPromises on JWK parse errors [26.05ms] (pass) oversized inputs > rejects >2 GiB inputs instead of aborting [7.45ms] 379 | "2**32 - 2": await verifySalt20Signature(2 ** 32 - 2), 380 | "2**31": await verifySalt20Signature(2 ** 31), 381 | "32": await verifySalt20Signature(32), 382 | "20": await verifySalt20Signature(20), 383 | }, 384 | }).toEqual({ ^ error: ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/crypto/web-crypto.test.ts bun test v1.4.0 (6e906e4) test/js/web/crypto/web-crypto.test.ts: (pass) crypto.subtle setter should not throw [3.85ms] (pass) Web Crypto > keeps event loop alive [336.44ms] (pass) Web Crypto > has globals [3.51ms] (pass) Web Crypto > should encrypt and decrypt [17.56ms] (pass) Web Crypto > should verify and sign [40.39ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are not valid JSON [18.89ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are valid JSON but not a valid JWK [12.78ms] (pass) Web Crypto > unwrapKey JWK error handling > settles when JsonWebKey dictionary conversion itself throws [11.03ms] (pass) Web Crypto > unwrapKey JWK error handling > does not leak DeferredPromise in m_pendingPromises on JWK parse errors [2601.80ms] (pass) oversized inputs > rejects >2 GiB inputs instead of aborting [353.32ms] (pass) RSA-PSS saltLength > rejects values that do not fit in an int instead of treating them as the -1/-2 selectors [108. ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 657ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/6] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcrypto-0.cpp.o [2/6] gen cpp.rs (cppbind) [2/6] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_bin v0.0.0 (/workspace/bun/src/bun_bin) �[1m�[92m Finished�[0m `release` profile [optimized + debuginfo] target(s) in 2m 58s [3/6] link bun-profile [5/6] strip bun [5/6] bun-profile --revision 1.4.0-canary.1+21d75372c [build] done bun test v1.4.0-canary.1 (21d7537) test/js/web/crypto/web-crypto.test.ts: (pass) crypto.subtle setter should not throw [0.05ms] (pass) Web Crypto > keeps event loop alive [7.94ms] (pass) Web Crypto > has globals [0.07ms] (pass) Web Crypto > should encrypt and decrypt [0.38ms] (pass) Web Crypto > should verify and sign [0.74ms] (pass) Web Crypto > unwrapKey JWK error handling > rejects when wrapped bytes are not valid JSON [0.44ms] (pass) Web Crypto > unwrapKey JWK error han ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` .../webcrypto/CryptoAlgorithmRSA_PSSOpenSSL.cpp | 16 +++++- test/js/web/crypto/web-crypto.test.ts | 64 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests …sc/bindings/webcrypto/CryptoAlgorithmRSA_PSSOpenSSL.cpp 1 4 0 test/js/web/crypto/web-crypto.test.ts 1 1 0 ``` </details> <!-- robobun:evidence:end -->
Collaborator
|
Heads-up for the rebase: #41500 lands the KeyObject / CryptoKey arm of |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #34838 — review that first. This branch is based on
claude/webcrypto-chacha20-raw-secret, notmain.Upstream tests added: 5.
Part of the Node v26.3.0
test/parallelcompat push, for the web-platform + crypto slice. Five upstream test files now pass and are vendored verbatim, backed by two runtime fixes. Each was checked with a canary (athrowappended to a copy must make the run fail) to prove the body actually executes.Runtime changes
1. RSA-PSS
saltLengthis range-checked before the job runs (src/jsc/bindings/webcrypto/SubtleCrypto.cpp)subtle.sign()/subtle.verify()with an RSA-PSSsaltLengthlarger thanceil((modulusLength - 1) / 8) - hLen - 2fell through to BoringSSL and rejected with a bareOperationError. Node rejects with anOperationErrorwhosecauseis anERR_OUT_OF_RANGEerror naming the accepted range. The newrejectIfRsaPssSaltLengthOutOfRangesits beside the existing ML-DSA context-length check and reuses the samerejectWithCausehelper, so sign and verify get it from one place.2.
assert.deepStrictEqualcompares key material forKeyObjectandCryptoKey(src/jsc/bindings/bindings.cpp)All of a
KeyObject's and aCryptoKey's state lives in native slots, so the generic own-property walk reported every pair of keys as deep-equal:assert.notDeepStrictEqual(createSecretKey(a), createSecretKey(b))passed for any two keys, as did comparing a public key against a private one.specialObjectsDequalnow compares the native material the way Node'scomparisons.jsdoes, and deliberately does not consult the user-replaceableequals()method. ForCryptoKeyit also compares type, extractability, algorithm identifier and usages; Node additionally deep-compares the algorithm dictionary, so a pair of keys differing only in the hash of an otherwise identical algorithm still compares equal here.Verified against real Node v26.3.0 for the cases in the new test, including that a
Proxyaround aKeyObjectis not unwrapped by either implementation.Heads-up for the merge: another campaign branch also edits
specialObjectsDequal, to skip thecalculatedClassNamecomparison when either side is a Proxy. The two hunks are independent and both should survive the merge.3.
node:urlre-exports theURLPatternglobal, which Node has and Bun was missing.4.
SubtleCrypto.supports()now agrees with the runtime (src/jsc/bindings/webcrypto/SubtleCrypto.cpp)Measured against Node's own fixture vectors (
test/fixtures/webcrypto/supports-*.mjs),supports()gave the wrong answer in 262 places. Two causes:exportKeyandgetPublicKeynormalized the algorithm asimportKey, so a bare name like"RSA-PSS"was rejected for having nohash— even though neither operation takes algorithm members. They now resolve the name only, which is whatnormalizeAlgorithm(alg, 'exportKey')does. This also fixessupports('wrapKey', 'AES-KW', 'HMAC').supports()promised operations that would have thrown: HMAC lengths that are zero or not a multiple of 8, AES lengths other than 128/192/256, EC curves outside P-256/P-384/P-521, PBKDF2 with zero iterations, ChaCha20-Poly1305 with a non-12-byte iv or a non-128-bit tag, and ECDH/X25519deriveBitswithout a public key. A zeroderiveBitslength is now accepted; a null one still is not.Every expectation was taken from Node v26.3.0 by running the same call there. 262 → 68, and all 68 that remain are one structural class: the fixtures gate SHA-3, TurboSHAKE and KangarooTwelve on
process.features.openssl_is_boringssl, which Bun reports as true while its WebCrypto does implement SHA-3. That is whytest-webcrypto-supports.mjsis not in the vendored set — it cannot pass without Bun either misreporting SHA-3 support or lying about its TLS backend.The ChaCha20-Poly1305 iv and tag lengths are now named constants on the algorithm class, so the probe and the implementation cannot drift apart.
Tests
Vendored verbatim from Node v26.3.0 into
test/js/node/test/parallel/, each verified with one process per file against this build:test-crypto-key-objects-raw.jstest-webcrypto-deduplicate-usages.jstest-webcrypto-get-public-key.mjstest-webcrypto-sign-verify-rsa.jstest-crypto-keyobject-hidden-slots.jstest-temporal.jsandtest-temporal-with-zoneinfo.jsare deliberately not vendored. They gate oncommon.hasTemporal, and Bun keepsJSC::Options::useTemporal()off (see the comment inZigGlobalObject.cpp), so both would exit 0 without running a single assertion. They can be added the day Temporal is enabled.Harness:
test/js/node/test/common/index.jsgainshasTemporalandhasLocalStorage, both copied verbatim from upstream. (That file already differs from upstream in other Bun-specific ways; these two additions are verbatim.)Bun-side regression tests:
test/js/node/crypto/crypto.key-objects.test.ts— deep equality for secret/public/privateKeyObjects and forCryptoKeys, including that a patchedKeyObject.prototype.equalscannot make two different keys compare equal. Fails on the unfixed build.test/js/web/urlpattern/urlpattern.test.ts—require("node:url").URLPattern === globalThis.URLPattern.test/expectations.txtis untouched. No existing test was changed or skipped.Not landed, and why
Two findings from the same slice worth recording:
url.parse()'s DEP0169 deprecation stays off. I implemented it (plus a nativeisInsideNodeModulesso dependencies stay quiet) and then reverted it: the already-vendoredtest-url-parse-invalid-input.jscarries an explicit "this warning is noisy and annoying, we've disabled it intentionally" note, and emitting DEP0169 breaks that test. Reversing that call is a maintainer decision, not a compat bug.test-url-parse-deprecation.jstherefore cannot be vendored.SubtleCrypto.supports()disagrees with Node's vectors in 262 places (test/fixtures/webcrypto/supports-*.mjs). Some are structural — the fixtures key SHA-3 expectations offprocess.features.openssl_is_boringssl, which Bun reports as true even though its WebCrypto does implement SHA-3, and TurboSHAKE/KT are not implemented at all — but many are plain bugs in the capability matrix (HMACwithlength: 25,AES-*withlength: 25,PBKDF2withiterations: 0, andderiveBitswith a zero length are all reported as supported;exportKeyreports false for every asymmetric algorithm).test-webcrypto-supports.mjscannot pass while the SHA-3 mismatch stands, but the matrix bugs are worth fixing in the base PR.The rest of the slice, and what stopped each one
All 25 were run against this build, one process per file (
bun testfor the ones that usenode:test, matching the CI runner).Deliberate divergences — passing them would mean regressing Bun.
test-abortcontroller.js— 16/19 subtests pass. The blocker isisTrusted: WebIDL marks it[LegacyUnforgeable], so WebKit puts a non-configurable getter on eachEventinstance, while Node puts it onEvent.prototypeand the test reads it off the prototype. Also needs anAbortControllercustom inspect (AbortController { signal: [AbortSignal] }) andAbortSignal.timeout()to become collectible once its last listener is removed.test-blob-file-backed.js— every assertion passes except the last: Node makesstructuredClone()of a file-backed Blob throwERR_INVALID_STATE, because itsFdEntryis realm-bound. Bun's file-backed blobs clone correctly (verified: the clone reads the same bytes), so matching Node here would delete working behaviour.test-url-parse-deprecation.js— see above; DEP0169 is intentionally off in Bun.test-whatwg-url-custom-searchparams-constructor.js— needsnew URLSearchParams(null)to produce an empty instance and Node'sERR_ARG_NOT_ITERABLE/ERR_INVALID_TUPLEcodes. WebKit follows the WebIDL union conversion (nullstringifies to"null"), and its constructor throws plain WebIDLTypeErrors.test-webcrypto-cryptokey-brand-check.js— asserts Node's internal prototype level (getPrototypeOf(getPrototypeOf(key)) === CryptoKey.prototype) andERR_INVALID_THISon the getters. Bun's instances sit directly onCryptoKey.prototype, like browsers.test-urlpattern-invalidthis.js— after this PRrequire('node:url').URLPatternexists, but the test wants V8's"Illegal invocation"brand-check text; WebKit's generated message is"The URLPattern.protocol getter can only be used on instances of URLPattern"and WebKit is not built from this repo.test-urlpattern-types.js— wants Node'sERR_CONSTRUCT_CALL_REQUIRED/ERR_INVALID_ARG_TYPEcodes, which come from Node's own Ada-basedURLPattern.test-whatwg-url-properties.js/test-whatwg-url-custom-searchparams-inspect.js— need[nodejs.util.inspect.custom]onURL.prototypeandURLSearchParams.prototype. Adding it does override Bun's native formatting (measured), soconsole.log(new URL(...))would stop printing the bare href — a Bun-native default I'm not changing here. The searchparams-inspect test additionally needs the remaining items of a liveURLSearchParamsiterator, which WebKit's iterators don't expose.test-global.js— asserts the exact set of enumerable globals (15 entries). Bun's list includesBun,process,Bufferand others, and the expected set also containssessionStorage.Missing features, too large for this PR.
test-webstorage.js,test-webstorage-without-sqlite.js— Bun has no Web Storage at all: noStorage,localStorage,sessionStorage, orQuotaExceededError, and no--localstorage-file/--no-webstorageflags. Because Bun reportsprocess.versions.sqlite,common.hasSQLiteis true and both tests demand the full API.test-web-locks.js,test-web-locks-query.js— nonavigator.locks(0/9 and 0/2 subtests).test-crypto-encap-decap.js— nocrypto.encapsulate()/crypto.decapsulate(), and ML-KEM keys are not representable asKeyObjects.test-crypto-domains.js—node:domainis a stub with no async propagation, so a throw from apbkdf2/randomBytescallback escapes instead of reaching the domain'serrorhandler.test-crypto-key-objects-messageport.js—worker_threads.moveMessagePortToContextis not implemented.test-crypto.js— needs the nativetls.createSecureContext().contextmethod surface (setOptions, PFX with passphrase); this belongs with the TLS work.test-global-setters.js— needsglobalThis.processandglobalThis.Bufferto be accessor properties. Both are static-hash-table entries (CellProperty/ClassStructure); converting them means custom accessors plus GC-visited override slots onZigGlobalObjectfor two of the hottest global lookups. Not worth it for one test without perf numbers.test-global-console-exists.js— asserts the max-listeners warning reaches a monkey-patchedprocess.stderr.write. Bun'sconsole.errorwrites straight to fd 2, and Bun suppresses the default warning output entirely when a'warning'listener exists (Node always prints, then calls listeners).Not available in the vendored BoringSSL.
test-webcrypto-digest-turboshake.js,test-webcrypto-digest-turboshake-rfc.js— no TurboSHAKE128/256 or KT128/KT256.test-crypto-pqc-key-objects-slh-dsa.js— BoringSSL exposes only raw SLH-DSA-SHA2-128s (noEVP_PKEYintegration, no other parameter sets); the test needs all twelve.test-crypto-job-error-parity.js—crypto.generateKeyPairSync('dh', { group: 'modp5' })fails withUNSUPPORTED_ALGORITHM. Separately,crypto.sign('sha256', data, ed25519Key)should throw and doesn't.test-webcrypto-supports.mjs— see above.CI notes
package-binary-size. The gate baselines againstmain, and this branch is stacked on #34838, which adds ML-DSA and ML-KEM. The reported growth is that base's, inherited structurally: the two flagged targets are +515 KB and +529 KB, and #34838's own commits already carry[allow size]for the same reason. The head commit here carries the tag too. This PR's own code is two C++ functions and a name resolver; it does not move the needle.windows-x64-verify-baseline(strspn [RTM]). Investigated before touching the allowlist, and I do not think an allowlist entry is the right fix:d8947814ee, the exact parent of this branch) with today's scanner: it passed. So neither the base binary nor a scanner change explains it.-marchleak from this diff. The C++ here edits three existing files insrc/jsc/bindings/webcrypto/plusbindings.cpp; it adds no translation unit, no dependency, and no compile flags, and nothing in it referencesstrspn, which is a CRT symbol.Xbegin(1 insns) insidestrspn. Real RTM elision always emitsxbeginwith a matchingxend/xabortand a non-transactional fallback, and it is always CPUID-gated. One loneXbeginwith no gate looks like the scanner decoding a byte sequence in the CRT'sstrspnregion after this build's layout shifted, not like a compiler emitting TSX.I retried the job on the unchanged artifact to see whether it reproduces, and this push produces a fresh binary. If the violation follows the binary deterministically it deserves a scanner-side look rather than an allowlist line; if it moves or disappears with layout, that confirms the decode-artifact reading. Either way I would rather not add a
strspn [RTM]allowlist entry, since it is not a CPUID-gated symbol and the entry would suppress a real leak if one ever appeared there.