From 871dca777dafc2494c60bf44b0e7549528de5b29 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:56:04 +0000 Subject: [PATCH] node:crypto: reject RSA modulusLength that BoringSSL would round down generateKeyPair('rsa', { modulusLength: 1023 }) silently returned an 896-bit key because BoringSSL's RSA_generate_key_ex masks the request with ~127. Reject non-multiples of 128 with ERR_OUT_OF_RANGE instead of returning a weaker key than the caller asked for. --- .../node/crypto/CryptoGenRsaKeyPair.cpp | 9 +++++ .../js/node/crypto/crypto.key-objects.test.ts | 37 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp b/src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp index 648374da83b1..5e6c2848f8ff 100644 --- a/src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp +++ b/src/jsc/bindings/node/crypto/CryptoGenRsaKeyPair.cpp @@ -110,6 +110,15 @@ std::optional RsaKeyPairJobCtx::fromJS(JSC::JSGlobalObject* gl V::validateUint32(scope, globalObject, modulusLengthValue, "options.modulusLength"_s, jsUndefined(), &modulusLength); RETURN_IF_EXCEPTION(scope, std::nullopt); + // BoringSSL's RSA_generate_key_ex rounds the requested size down to a multiple of + // 128 bits (vendor/boringssl/crypto/fipsmodule/rsa/rsa_impl.cc.inc). Passing the + // value through unchecked would silently return a key up to 127 bits weaker than + // requested, so reject sizes that cannot be generated exactly. + if (modulusLength % 128 != 0) { + ERR::OUT_OF_RANGE(scope, globalObject, "options.modulusLength"_s, "a multiple of 128"_s, modulusLengthValue); + return std::nullopt; + } + JSValue publicExponentValue = optionsValue.get(globalObject, Identifier::fromString(vm, "publicExponent"_s)); RETURN_IF_EXCEPTION(scope, std::nullopt); uint32_t publicExponent = 0x10001; diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 5f433d9cf161..22b5fcae509c 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -1449,7 +1449,7 @@ describe("crypto.KeyObjects", () => { passphrase: "top secret", }); }); - // SKIPED because we round the key size to the nearest multiple of 8 like documented + // BoringSSL only generates RSA moduli that are multiples of 128 bits, so 513 is rejected. test.skip(`this tests check that generateKeyPair returns correct bit length in KeyObject's asymmetricKeyDetails.`, async () => { // This tests check that generateKeyPair returns correct bit length in // https://github.com/nodejs/node/issues/46102#issuecomment-1372153541 @@ -1472,6 +1472,41 @@ describe("crypto.KeyObjects", () => { expect(privateKey.asymmetricKeyDetails?.modulusLength).toBe(513); }); + // BoringSSL's RSA_generate_key_ex rounds the requested modulus down to a multiple of + // 128 bits. Previously Bun passed the request through unchanged, so asking for 1023 + // or 3000 bits silently returned an 896- or 2944-bit key. https://github.com/oven-sh/bun/issues/5740 + describe("generateKeyPair('rsa') never silently returns a smaller modulus than requested", () => { + const outcome = (modulusLength: number) => { + try { + const { publicKey } = generateKeyPairSync("rsa", { modulusLength }); + return publicKey.asymmetricKeyDetails?.modulusLength; + } catch (e) { + return { code: (e as NodeJS.ErrnoException).code }; + } + }; + + test("generateKeyPairSync", () => { + expect({ + 1023: outcome(1023), + 1024: outcome(1024), + 3000: outcome(3000), + }).toEqual({ + 1023: { code: "ERR_OUT_OF_RANGE" }, + 1024: 1024, + 3000: { code: "ERR_OUT_OF_RANGE" }, + }); + }); + + test("generateKeyPair validates before scheduling", () => { + expect(() => generateKeyPair("rsa", { modulusLength: 1023 }, () => {})).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: expect.stringContaining("multiple of 128"), + }), + ); + }); + }); + type TestRunInContextArg = | { fn: typeof runInContext; isIsolated: true } | { fn: typeof runInThisContext; isIsolated?: false };