[robobun]

On main (675ee5b97b): #47836 has this fix and tests the same seven falsy values. Only the three test rows for "0", {} and [] are not on main.

Problem

• crypto.publicEncrypt, privateDecrypt, privateEncrypt and publicDecrypt throw ERR_OSSL_EVP_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE when the key object has padding: null, 0, -0, false, NaN or "". padding: 0n throws a TypeError. Node v26.3.0 takes the default padding for all seven.
• The cause is rsaFunction (src/jsc/bindings/node/crypto/JSCipher.cpp:87). It skips the option only when it is undefined. Each other value goes to toInt32, which gives 0, and 0 is not a padding mode.

Fix

• rsaFunction skips the option when it is falsy (toBoolean). A truthy value goes to toInt32 as before.
• Correct because Node reads the option as key.padding || defaultPadding (cipher.js:72).
• Verified: test/js/node/crypto/crypto-rsa.test.js (10 new tests, 7 fail without the fix). Also node-crypto.test.js, crypto.key-objects.test.ts and six vendored Node tests (see Notes).

Background

• rsaFunction is the one native function behind the four calls. The default is RSA_PKCS1_OAEP_PADDING for publicEncrypt and privateDecrypt, RSA_PKCS1_PADDING for the other two.
• sign() and verify() read padding in other code, where Node refuses null. This PR does not touch it.
• No other place was a candidate: rsaFunction is the only reader of this option.

Downsides

• A call that relied on the throw for a falsy padding now runs with the default padding, as in Node.
• rsaFunction grows by 200 bytes (4458 to 4658, -O3 without LTO).

Notes

Repro. Each call below throws on Bun 1.4.3 and returns on Node v26.3.0. With this PR each returns the same result as the call without padding.

import crypto from "node:crypto";
const { publicKey, privateKey } = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
const message = Buffer.from("the same message");
const signed = crypto.privateEncrypt(privateKey, message);
const sealed = crypto.publicEncrypt(publicKey, message);
for (const padding of [null, 0, -0, false, NaN, "", 0n]) {
  crypto.privateEncrypt({ key: privateKey, padding }, message); // same bytes as `signed`
  crypto.publicDecrypt({ key: publicKey, padding }, signed); // `message`
  crypto.privateDecrypt(privateKey, crypto.publicEncrypt({ key: publicKey, padding }, message)); // `message`
  crypto.privateDecrypt({ key: privateKey, padding }, sealed); // `message`
}

Input space. A probe ran 27 padding values through the four calls on Node v26.3.0 and on this branch. Falsy values (undefined, null, 0, -0, false, NaN, "", 0n) give the default on both. Truthy values go through the integer conversion on both: true, 1 and "1" are RSA_PKCS1_PADDING, 4, 4.7, [4] and 2 ** 32 + 4 are RSA_PKCS1_OAEP_PADDING, and "0", " ", "abc", {}, [], new Number(0), -1 and Infinity are rejected as a padding mode. 1n and Symbol() throw a TypeError on both.

Two differences from Node remain. Both exist on main and come from BoringSSL, not from this read:

• The code names of the library errors differ, for example ERR_OSSL_EVP_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE on Bun and ERR_OSSL_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE on Node with OpenSSL 3.
• privateDecrypt with RSA_PKCS1_PADDING throws ERR_INVALID_ARG_VALUE on Bun. BoringSSL has no implicit rejection, and Node's PublicKeyCipher::Cipher has the same branch for that case.

Tests. The seven falsy cases fail on Bun 1.4.3 and pass on the debug build of this branch. The three truthy cases ("0", {}, []) pass on both. They hold the other side of the rule: only a falsy value means the default, so a fix written as "integer 0 means the default" would fail them.

Suites run on the debug ASAN build of this branch, all pass:

• test/js/node/crypto/crypto-rsa.test.js, also with BUN_JSC_validateExceptionChecks=1
• test/js/node/crypto/node-crypto.test.js
• test/js/node/crypto/crypto.key-objects.test.ts
• test/js/node/test/parallel/test-crypto-rsa-dsa.js
• test/js/node/test/parallel/test-crypto-sign-verify.js (it asserts that createVerify().verify({ key, padding: null }) throws ERR_INVALID_ARG_VALUE)
• test/js/node/test/parallel/test-crypto-private-decrypt-gh32240.js
• test/js/node/test/parallel/test-crypto-publicDecrypt-fails-first-time.js
• test/js/node/test/parallel/test-crypto-oaep-zero-length.js
• test/js/node/test/parallel/test-crypto-key-objects.js

Siblings. Node's lib/internal/crypto has two other reads where a falsy value means the default: the generator of createDiffieHellman (diffiehellman.js:115) and the point format of ECDH (diffiehellman.js:272). For undefined, null, 0, false, NaN and "", both give the same result on Bun 1.4.3 and on Node v26.3.0.

History. The check for undefined only dates from the native port of these calls in #19040.

How the numbers were measured. JSCipher.cpp was compiled two times, from main and from this branch, with the flags of the release build minus the LTO flags. nm -S gives the size of Bun::rsaFunction: 0x116a and 0x1232. The instruction counts come from the disassembly of the two objects, on the path from the load of the padding value to the next statement. With no padding on the key object the read costs 13 instructions, up from 3. With an integer padding it stays at 12. toBoolean is inline and calls nothing for a number, undefined, null or a boolean. The change adds no allocation and no call.

Not in this PR. rsaFunction reads padding, oaepHash and oaepLabel before it prepares the key. Node prepares the key first. The result is a different error on input that is invalid in two ways, for example publicEncrypt({ key: "not a pem", oaepHash: "nope" }, buf). This is a separate change. #45335 rewrites the key preparation in the same function.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: c19bbf93-2b39-4aef-9fbb-40189b533f4e

📥 Commits

Reviewing files that changed from the base of the PR and between a407eae and 282e4d8.

📒 Files selected for processing: 2

• src/jsc/bindings/node/crypto/JSCipher.cpp
• test/js/node/crypto/crypto-rsa.test.js

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[robobun]

Status

Reproduced on Bun 1.4.3 (release build) and on a debug build of main. Node v26.3.0 is the reference.

import crypto from "node:crypto";
const { publicKey, privateKey } = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
crypto.privateEncrypt({ key: privateKey, padding: null }, Buffer.from("the same message"));
// Bun 1.4.3: ERR_OSSL_EVP_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE
// Node v26.3.0: returns the same bytes as the call without `padding`

The same holds for publicEncrypt, privateDecrypt and publicDecrypt, and for padding: 0, -0, false, NaN, "" and 0n.

Fix: #45359

Update: main 675ee5b97b no longer reproduces this, because #47836 has the same change. The first line of the PR description says what is left.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[robobun]

^{Updated 10:41 AM PT - Oct 15th, 2026}

✅ @robobun, your commit 282e4d823aebdd55d62d5540dc8988559c213379 passed in Build #125990! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 45359

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

bun-45359 --bun

[claude]

I reviewed the updated PR and found no bugs; the second commit addresses the earlier nit by asserting the exact error code. Because this changes option handling on a node:crypto entry point, a human look is still worthwhile.

What was reviewed:

• The toBoolean gate at src/jsc/bindings/node/crypto/JSCipher.cpp:88: JSValue::toBoolean does not throw, and the toInt32 that can run user JS for {}/[] keeps its RETURN_IF_EXCEPTION, so the exception-check contract is intact.
• Sibling readers of padding (CryptoUtil.cpp getPadding used by sign/verify via getIntOption) implement Node's int32-validation contract that rejects null, so excluding them from this change matches Node.
• Test matrix covers all seven falsy values and three truthy non-numeric values across all four entry points, with byte-equality against a deterministic PKCS1 privateEncrypt.

Extended reasoning...

The change is a one-line condition flip in rsaFunction (src/jsc/bindings/node/crypto/JSCipher.cpp) so that a falsy padding option on publicEncrypt/privateDecrypt/privateEncrypt/publicDecrypt selects the default padding instead of being coerced to 0, matching Node's key.padding || defaultPadding, plus a new test.each block in test/js/node/crypto/crypto-rsa.test.js. It touches a crypto surface (RSA padding mode selection), which is why a human look is suggested even though the diff is small, self-contained, cites the Node source line, and the padding integer is still validated by BoringSSL downstream. The second commit resolved the one nit from the prior review (loose substring match on the error code), and the exception-check and sibling-site claims in the PR description were verified against the code.

[robobun]

Closing: #47836 landed this fix on main. rsaFunction now has the same key.padding || defaultPadding check, and test/js/node/crypto/crypto-rsa.test.js covers the same seven falsy values in the four calls.

The test file of this PR passes on the canary build of main (1.4.4-canary.1+675ee5b97, 32 of 32). That includes its three extra rows, which assert that "0", {} and [] are still rejected as a padding mode.