From 2f317b035128e23c2138c46ed97a5b1d71099d4b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:33 +0000 Subject: [PATCH 01/19] node:crypto: size cipher output with BoringSSL's sized calls cipher.update() and cipher.final() ask BoringSSL for the most the call can write and tell it how much room the output has. update() reads the input's address and length once. The result's ArrayBuffer is as long as the result, like Node's. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 24 ++++++ src/jsc/bindings/ncrypto.h | 7 ++ .../node/crypto/JSCipherPrototype.cpp | 79 ++++++++++++------- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index 12d88beb9e5a..2e9781208792 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -2897,6 +2897,30 @@ bool CipherCtxPointer::update(const Buffer& in, return EVP_CipherFinal_ex(ctx_.get(), out, out_len) == 1; } +size_t CipherCtxPointer::maxUpdateSize(size_t in_len) const +{ + if (!ctx_) return 0; + return EVP_CIPHER_CTX_max_next_update(ctx_.get(), in_len); +} + +size_t CipherCtxPointer::maxFinalSize() const +{ + if (!ctx_) return 0; + return EVP_CIPHER_CTX_max_final(ctx_.get()); +} + +bool CipherCtxPointer::update(const Buffer& in, std::span out, size_t* out_len) +{ + if (!ctx_) return false; + return EVP_CipherUpdate_ex(ctx_.get(), out.data(), out_len, out.size(), in.data, in.len) == 1; +} + +bool CipherCtxPointer::finalize(std::span out, size_t* out_len) +{ + if (!ctx_) return false; + return EVP_CipherFinal_ex2(ctx_.get(), out.data(), out_len, out.size()) == 1; +} + bool CipherCtxPointer::getAeadTag(size_t len, unsigned char* out) { if (!ctx_) return false; diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index 5cc80cd1adc0..12ec1f9ea51b 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -641,6 +641,13 @@ class CipherCtxPointer final { unsigned char* out, int* out_len, bool finalize = false); + + // Not upstream: BoringSSL's calls that are told how much room `out` has, and how much they need at most. + size_t maxUpdateSize(size_t in_len) const; + size_t maxFinalSize() const; + bool update(const Buffer& in, std::span out, size_t* out_len); + bool finalize(std::span out, size_t* out_len); + bool getAeadTag(size_t len, unsigned char* out); private: diff --git a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp index 2a7b1a95e022..c2a5200c7465 100644 --- a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp +++ b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp @@ -38,6 +38,14 @@ void JSCipherPrototype::finishCreation(JSC::VM& vm) Bun::putToStringTagWithoutTransition(vm, this, info()); } +// The sizes BoringSSL gives up front are upper bounds. The result's ArrayBuffer is as long as what was written, like node's. +static RefPtr trimToWritten(RefPtr&& buffer, size_t written) +{ + if (written == buffer->byteLength()) + return WTF::move(buffer); + return ArrayBuffer::tryCreate(buffer->span().first(written)); +} + JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) { auto& vm = lexicalGlobalObject->vm(); @@ -56,10 +64,13 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj JSArrayBufferView* dataView = getArrayBufferOrView(lexicalGlobalObject, scope, dataValue, "data"_s, encodingValue); RETURN_IF_EXCEPTION(scope, {}); + // Read once: the length of a length-tracking view is not fixed. + const std::span data = dataView->span(); + ClearErrorOnReturn clearErrorOnReturn; - if (dataView->byteLength() > INT_MAX) { - return ERR::OUT_OF_RANGE(scope, lexicalGlobalObject, "data is too big"_s, 0, INT_MAX, jsNumber(dataView->byteLength())); + if (data.size() > INT_MAX) { + return ERR::OUT_OF_RANGE(scope, lexicalGlobalObject, "data is too big"_s, 0, INT_MAX, jsNumber(data.size())); } if (!cipher->m_ctx) { @@ -67,7 +78,7 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj return {}; } - if (cipher->m_ctx.isCcmMode() && !cipher->checkCCMMessageLength(dataView->byteLength())) { + if (cipher->m_ctx.isCcmMode() && !cipher->checkCCMMessageLength(data.size())) { // return undefined // https://github.com/nodejs/node/blob/6b4255434226491449b7d925038008439e5586b2/src/crypto/crypto_cipher.cc#L742 return JSValue::encode(jsUndefined()); @@ -79,48 +90,55 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj } const int32_t blockSize = cipher->m_ctx.getBlockSize(); - if (dataView->byteLength() + blockSize > INT_MAX) { + if (data.size() + blockSize > INT_MAX) { throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); return {}; } - int32_t bufLen = dataView->byteLength() + blockSize; ncrypto::Buffer buf { - .data = reinterpret_cast(dataView->vector()), - .len = dataView->byteLength(), + .data = data.data(), + .len = data.size(), }; - if (cipher->m_kind == CipherKind::Cipher && cipher->m_ctx.isWrapMode() && !cipher->m_ctx.update(buf, nullptr, &bufLen)) { - throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); - return {}; + size_t maxOutLen = cipher->m_ctx.maxUpdateSize(data.size()); + if (cipher->m_kind == CipherKind::Cipher && cipher->m_ctx.isWrapMode()) { + int32_t wrappedLen = 0; + if (!cipher->m_ctx.update(buf, nullptr, &wrappedLen)) { + throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); + return {}; + } + maxOutLen = wrappedLen; } - RefPtr outBuf = JSC::ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + bufLen, 1); + RefPtr outBuf = JSC::ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + maxOutLen, 1); if (!outBuf) { throwOutOfMemoryError(lexicalGlobalObject, scope); return {}; } - const size_t heldBack = cipher->takeHeldBackBlock(outBuf->mutableSpan()); + size_t outLen = cipher->takeHeldBackBlock(outBuf->mutableSpan()); - buf = { - .data = reinterpret_cast(dataView->vector()), - .len = dataView->byteLength(), - }; - - bool res = cipher->m_ctx.update(buf, static_cast(outBuf->data()) + heldBack, &bufLen); - ASSERT(heldBack + bufLen <= outBuf->byteLength()); + size_t written = 0; + bool res = cipher->m_ctx.update(buf, outBuf->mutableSpan().subspan(outLen), &written); + ensureStillAliveHere(dataView); + outLen += written; if (!res && cipher->m_kind == CipherKind::Decipher && cipher->m_ctx.isCcmMode()) { cipher->m_pendingAuthFailed = true; RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), 0))); } - if (res != 1) { + if (!res) { throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); return {}; } - RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, heldBack + bufLen))); + outBuf = trimToWritten(WTF::move(outBuf), outLen); + if (!outBuf) { + throwOutOfMemoryError(lexicalGlobalObject, scope); + return {}; + } + + RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, outLen))); } JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) @@ -149,13 +167,12 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje throwCryptoError(globalObject, scope, ERR_peek_error(), isAuthMode ? "Unsupported state or unable to authenticate data" : "Unsupported state"); }; - int32_t outLen = cipher->m_ctx.getBlockSize(); - RefPtr outBuf = ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + outLen, 1); + RefPtr outBuf = ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + cipher->m_ctx.maxFinalSize(), 1); if (!outBuf) { throwOutOfMemoryError(lexicalGlobalObject, scope); return {}; } - const size_t heldBack = cipher->takeHeldBackBlock(outBuf->mutableSpan()); + size_t outLen = cipher->takeHeldBackBlock(outBuf->mutableSpan()); if (cipher->m_kind == CipherKind::Decipher && Cipher::FromCtx(cipher->m_ctx).isSupportedAuthenticatedMode()) { cipher->maybePassAuthTagToOpenSSL(); @@ -169,10 +186,10 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje bool ok; if (cipher->m_kind == CipherKind::Decipher && cipher->m_ctx.isCcmMode()) { ok = !cipher->m_pendingAuthFailed; - outLen = 0; } else { - ok = cipher->m_ctx.update({}, static_cast(outBuf->data()) + heldBack, &outLen, true); - ASSERT(heldBack + outLen <= outBuf->byteLength()); + size_t written = 0; + ok = cipher->m_ctx.finalize(outBuf->mutableSpan().subspan(outLen), &written); + outLen += written; if (ok && cipher->m_kind == CipherKind::Cipher && cipher->isAuthenticatedMode()) { ASSERT(cipher->m_authTagLen.has_value()); @@ -188,7 +205,13 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje return {}; } - RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, heldBack + outLen))); + outBuf = trimToWritten(WTF::move(outBuf), outLen); + if (!outBuf) { + throwOutOfMemoryError(lexicalGlobalObject, scope); + return {}; + } + + RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, outLen))); } JSC_DEFINE_HOST_FUNCTION(jsCipherSetAutoPadding, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) From 5513cb74d0b0fee82700b6476bf818ee59548b34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:37 +0000 Subject: [PATCH 02/19] node:crypto: compute DH secrets with DH_compute_key_padded It writes the secret at its full length, so computeSecret() no longer moves the bytes and writes the leading zeros back. The result is the same. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index 2e9781208792..569f624c145e 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -1495,17 +1495,8 @@ DataPointer DHPointer::computeSecret(const BignumPointer& peer) const auto dp = DataPointer::Alloc(size()); if (!dp) return {}; - int size = DH_compute_key(static_cast(dp.get()), peer.get(), dh_.get()); - if (size < 0) return {}; - - // The size of the computed key can be smaller than the size of the DH key. - // We want to make sure that the key is correctly padded. - if (static_cast(size) < dp.size()) { - const size_t padding = dp.size() - size; - uint8_t* data = static_cast(dp.get()); - memmove(data + padding, data, size); - memset(data, 0, padding); - } + // Not upstream, which calls DH_compute_key and puts back the leading zeros that it strips. + if (DH_compute_key_padded(static_cast(dp.get()), peer.get(), dh_.get()) < 0) return {}; return dp; } From 47abf9a3f5f77650ba2fbe1a7d94a5da7a354152 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:41 +0000 Subject: [PATCH 03/19] crypto.getRandomValues: fill from RAND_bytes, convert the argument like Web IDL The view is filled with RAND_bytes whatever its length, like Node. Views of 256 bytes or less were filled from the cache that randomUUID() draws from. The argument is converted to an ArrayBufferView before its element type is looked at. What is not a view is a TypeError (ERR_INVALID_ARG_TYPE), and so is a view on a SharedArrayBuffer or on a resizable ArrayBuffer. node:wasi's random_get fills an array of its own and copies it into the instance's memory, which can be shared. Co-authored-by: dylanc --- src/js/node/wasi.ts | 6 +- src/runtime/webcore/Crypto.rs | 57 +++++++++++-------- test/js/bun/wasm/wasi.test.js | 9 ++- .../test/parallel/test-webcrypto-random.js | 5 +- test/js/web/crypto/web-crypto.test.ts | 37 +++++++++++- 5 files changed, 79 insertions(+), 35 deletions(-) diff --git a/src/js/node/wasi.ts b/src/js/node/wasi.ts index bd2cab39e6ab..51670f6a210b 100644 --- a/src/js/node/wasi.ts +++ b/src/js/node/wasi.ts @@ -1663,10 +1663,8 @@ class WASI { }, random_get: (bufPtr: number, bufLen: number) => { this.refreshMemory(); - // getRandomValues takes one integer-typed view and ignores any further - // arguments, so a bare `buffer, bufPtr, bufLen` randomized all of linear - // memory rather than the requested window. - crypto.getRandomValues(new Uint8Array(this.memory.buffer, bufPtr, bufLen)); + // getRandomValues refuses a view on shared memory, so it fills an array of its own. + new Uint8Array(this.memory.buffer, bufPtr, bufLen).set(crypto.getRandomValues(new Uint8Array(bufLen))); return WASI_ESUCCESS; }, sched_yield() { diff --git a/src/runtime/webcore/Crypto.rs b/src/runtime/webcore/Crypto.rs index b6cb48f5c0d4..77ecd6a8f665 100644 --- a/src/runtime/webcore/Crypto.rs +++ b/src/runtime/webcore/Crypto.rs @@ -16,12 +16,38 @@ impl Crypto { array: JsLocal<'a>, ) -> bindgen::Result<'a, JsLocal<'a>> { let global = cx.global(); - // https://w3c.github.io/webcrypto/#Crypto-method-getRandomValues accepts only - // integer-typed views. This is an allow-list: DataView, ArrayBuffer and SharedArrayBuffer - // all pass `as_array_buffer` but must still raise TypeMismatchError. - let array_buffer = array.to_js().as_array_buffer(global).filter(|_| { + let js_type = array.to_js().js_type(); + // Web IDL's conversion to an ArrayBufferView that is neither [AllowShared] nor [AllowResizable]. + let view = array + .to_js() + .as_array_buffer(global) + .filter(|_| js_type != JSType::ArrayBuffer); + let refused = match &view { + None => Some("is not an ArrayBufferView"), + Some(view) if view.shared => { + Some("is a view on a SharedArrayBuffer, which is not allowed") + } + Some(view) if view.resizable => { + Some("is backed by a resizable ArrayBuffer, which is not allowed") + } + Some(_) => None, + }; + if let Some(refused) = refused { + return Err(global + .err( + bun_jsc::ErrorCode::INVALID_ARG_TYPE, + format_args!( + "Failed to execute 'getRandomValues' on 'Crypto': 1st argument {refused}." + ), + ) + .throw() + .into()); + } + + // https://w3c.github.io/webcrypto/#Crypto-method-getRandomValues accepts only integer-typed views. + let array_buffer = view.filter(|_| { matches!( - array.to_js().js_type(), + js_type, JSType::Int8Array | JSType::Uint8Array | JSType::Uint8ClampedArray @@ -42,7 +68,7 @@ impl Crypto { .into()); }; - random_data(global, array_buffer.byte_slice_mut()); + bun_boringssl_sys::rand_bytes(array_buffer.byte_slice_mut()); Ok(array) } @@ -85,25 +111,6 @@ pub(crate) fn timing_safe_equal<'a>( Ok(bun_boringssl::c::constant_time_eq(buf1, buf2)) } -fn random_data(global: &JSGlobalObject, slice: &mut [u8]) { - const ENTROPY_CACHE_FAST_PATH_MAX: usize = bun_jsc::RareData::EntropyCache::SIZE / 8; - match slice.len() { - 0 => {} - // 512 bytes or less we reuse from the same cache as UUID generation. - 1..=ENTROPY_CACHE_FAST_PATH_MAX => { - let src = global - .bun_vm() - .as_mut() - .rare_data() - .entropy_slice(slice.len()); - slice[..src.len()].copy_from_slice(src); - } - _ => { - bun_boringssl_sys::rand_bytes(slice); - } - } -} - fn encoding_from_label<'a>(heap: &JsHeap<'a>, label: &[u8]) -> bindgen::Result<'a, Encoding> { Encoding::from(label).ok_or_else(|| { heap.error_with_code( diff --git a/test/js/bun/wasm/wasi.test.js b/test/js/bun/wasm/wasi.test.js index 8d1bd1c02bab..ebabec8f838f 100644 --- a/test/js/bun/wasm/wasi.test.js +++ b/test/js/bun/wasm/wasi.test.js @@ -47,9 +47,14 @@ it("fd_fdstat_set_rights only narrows the rights of a descriptor", () => { expect(wasi.FD_MAP.get(0).rights).toEqual({ base: WASI_RIGHT_FD_READ, inheriting: BigInt(0) }); }); -it("random_get fills only the requested window", () => { +it.each([ + ["memory", () => new WebAssembly.Memory({ initial: 1 })], + ["shared memory", () => new WebAssembly.Memory({ initial: 1, maximum: 1, shared: true })], + // Not every build of JavaScriptCore backs shared memory with a SharedArrayBuffer. + ["a SharedArrayBuffer", () => ({ buffer: new SharedArrayBuffer(65536) })], +])("random_get fills only the requested window of %s", (_, memory) => { const wasi = new WASI({}); - wasi.setMemory(new WebAssembly.Memory({ initial: 1 })); + wasi.setMemory(memory()); const WASI_ESUCCESS = 0; const bufPtr = 1024; diff --git a/test/js/node/test/parallel/test-webcrypto-random.js b/test/js/node/test/parallel/test-webcrypto-random.js index 6712dbad2fca..b32eb50d953a 100644 --- a/test/js/node/test/parallel/test-webcrypto-random.js +++ b/test/js/node/test/parallel/test-webcrypto-random.js @@ -9,8 +9,11 @@ const { Buffer } = require('buffer'); const assert = require('assert'); const { crypto } = globalThis; +for (const value of [undefined, null, '', 1, {}, [], new ArrayBuffer(1)]) { + assert.throws(() => crypto.getRandomValues(value), TypeError); +} + [ - undefined, null, '', 1, {}, [], new Float32Array(1), new Float64Array(1), new DataView(new ArrayBuffer(1)), diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index b3a2018319b5..3cf116bcd290 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1475,9 +1475,6 @@ describe("crypto.getRandomValues argument types", () => { ["Float32Array", () => new Float32Array(4)], ["Float64Array", () => new Float64Array(4)], ["DataView", () => new DataView(new ArrayBuffer(4))], - ["ArrayBuffer", () => new ArrayBuffer(4)], - ["SharedArrayBuffer", () => new SharedArrayBuffer(4)], - ["plain object", () => ({})], ] as const; for (const [name, make] of rejected) { @@ -1490,6 +1487,40 @@ describe("crypto.getRandomValues argument types", () => { ); }); } + + // Web IDL's conversion of the argument to an ArrayBufferView comes first. + const notConverted = [ + ["ArrayBuffer", () => new ArrayBuffer(4), "is not an ArrayBufferView"], + ["SharedArrayBuffer", () => new SharedArrayBuffer(4), "is not an ArrayBufferView"], + ["plain object", () => ({}), "is not an ArrayBufferView"], + [ + "view on a SharedArrayBuffer", + () => new Uint8Array(new SharedArrayBuffer(4)), + "is a view on a SharedArrayBuffer, which is not allowed", + ], + [ + "view on a resizable ArrayBuffer", + () => new Uint8Array(new ArrayBuffer(4, { maxByteLength: 8 })), + "is backed by a resizable ArrayBuffer, which is not allowed", + ], + [ + "Float32Array on a SharedArrayBuffer", + () => new Float32Array(new SharedArrayBuffer(4)), + "is a view on a SharedArrayBuffer, which is not allowed", + ], + ] as const; + + for (const [name, make, why] of notConverted) { + it(`rejects ${name} with TypeError`, () => { + expect(() => crypto.getRandomValues(make() as any)).toThrow( + expect.objectContaining({ + name: "TypeError", + code: "ERR_INVALID_ARG_TYPE", + message: `Failed to execute 'getRandomValues' on 'Crypto': 1st argument ${why}.`, + }), + ); + }); + } }); describe("exception scope discipline", () => { From 1dd39b4626d7e19b6514500f35b92c2b3d513560 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:45 +0000 Subject: [PATCH 04/19] node:crypto: add mgf1Hash to publicEncrypt() and privateDecrypt() Node added the option in v26.8.0. It names the digest of RSA-OAEP's MGF1, which is oaepHash unless it is given. Bun ignored it. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 9 ++++- src/jsc/bindings/ncrypto.h | 2 + src/jsc/bindings/node/crypto/JSCipher.cpp | 20 +++++++++- test/js/node/crypto/crypto-rsa.test.js | 48 ++++++++++++++++++++++- 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index 569f624c145e..f046af7c8ba1 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -3307,6 +3307,13 @@ bool EVPKeyCtxPointer::setRsaOaepMd(const Digest& md) return EVP_PKEY_CTX_set_rsa_oaep_md(ctx_.get(), md_ptr) > 0; } +bool EVPKeyCtxPointer::setRsaMgf1Md(const Digest& md) +{ + if (!md || !ctx_) return false; + const EVP_MD* md_ptr = md; + return EVP_PKEY_CTX_set_rsa_mgf1_md(ctx_.get(), md_ptr) > 0; +} + bool EVPKeyCtxPointer::setRsaPadding(int padding) { return setRsaPadding(ctx_.get(), padding, std::nullopt); @@ -3435,7 +3442,7 @@ DataPointer CipherImpl(const EVPKeyPointer& key, { if (!key) return {}; EVPKeyCtxPointer ctx = key.newCtx(); - if (!ctx || init(ctx.get()) <= 0 || !ctx.setRsaPadding(params.padding) || (params.digest != nullptr && !ctx.setRsaOaepMd(params.digest))) { + if (!ctx || init(ctx.get()) <= 0 || !ctx.setRsaPadding(params.padding) || (params.digest != nullptr && !ctx.setRsaOaepMd(params.digest)) || (params.mgf1_digest != nullptr && !ctx.setRsaMgf1Md(params.mgf1_digest))) { return {}; } diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index 12ec1f9ea51b..d98be1c5db28 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -321,6 +321,7 @@ class Cipher final { struct CipherParams { int padding; Digest digest; + Digest mgf1_digest; const Buffer label; }; @@ -680,6 +681,7 @@ class EVPKeyCtxPointer final { bool setEcParameters(int curve, int encoding); bool setRsaOaepMd(const Digest& md); + bool setRsaMgf1Md(const Digest& md); bool setRsaPadding(int padding); bool setRsaKeygenPubExp(BignumPointer&& e); bool setRsaKeygenBits(int bits); diff --git a/src/jsc/bindings/node/crypto/JSCipher.cpp b/src/jsc/bindings/node/crypto/JSCipher.cpp index f4ffbf9e24f5..f8ecbe6f66e1 100644 --- a/src/jsc/bindings/node/crypto/JSCipher.cpp +++ b/src/jsc/bindings/node/crypto/JSCipher.cpp @@ -76,9 +76,10 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca JSValue optionsValue = callFrame->argument(0); JSValue bufferValue = callFrame->argument(1); - // Like Node, validate the cipher options (padding, oaepHash, oaepLabel) before any key + // Like Node, validate the cipher options (padding, oaepHash, mgf1Hash, oaepLabel) before any key // parsing so an invalid digest is reported even when the key material itself is invalid. ncrypto::Digest digest; + ncrypto::Digest mgf1Digest; int32_t padding = defaultPadding; std::optional> oaepLabel; JSValue encodingValue = jsUndefined(); @@ -106,6 +107,22 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca } } + JSValue mgf1HashValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "mgf1Hash"_s)); + RETURN_IF_EXCEPTION(scope, {}); + if (!mgf1HashValue.isUndefined()) { + V::validateString(scope, lexicalGlobalObject, mgf1HashValue, "options.mgf1Hash"_s); + RETURN_IF_EXCEPTION(scope, {}); + JSString* mgf1HashString = mgf1HashValue.toString(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + GCOwnedDataScope mgf1HashView = mgf1HashString->view(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + mgf1Digest = ncrypto::Digest::FromName(mgf1HashView); + if (!mgf1Digest) { + ERR::OSSL_EVP_INVALID_DIGEST(scope, lexicalGlobalObject); + return {}; + } + } + encodingValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "encoding"_s)); RETURN_IF_EXCEPTION(scope, {}); @@ -195,6 +212,7 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca ncrypto::Cipher::CipherParams cipherParams { .padding = padding, .digest = digest, + .mgf1_digest = mgf1Digest, .label = labelBuf, }; diff --git a/test/js/node/crypto/crypto-rsa.test.js b/test/js/node/crypto/crypto-rsa.test.js index 3d360b096e25..45b9842cbedc 100644 --- a/test/js/node/crypto/crypto-rsa.test.js +++ b/test/js/node/crypto/crypto-rsa.test.js @@ -320,6 +320,41 @@ test(`RSA with hash mismatch`, () => { }).toThrow(expect.objectContaining(decryptError)); }); +test("RSA-OAEP with an mgf1Hash other than oaepHash", () => { + const input = Buffer.from("Hello Node.js"); + const decodingError = expect.objectContaining({ code: "ERR_OSSL_RSA_OAEP_DECODING_ERROR" }); + const roundTrip = (encrypt, decrypt = encrypt) => + crypto.privateDecrypt({ key: rsaKeyPem, ...decrypt }, crypto.publicEncrypt({ key: rsaPubPem, ...encrypt }, input)); + + for (const [oaepHash, mgf1Hash] of [ + ["sha256", "sha1"], + ["sha512", "sha1"], + ["sha384", "sha256"], + ["sha1", "sha256"], + ]) { + expect(roundTrip({ oaepHash, mgf1Hash })).toEqual(input); + } + + // Without mgf1Hash, MGF1 uses oaepHash, which is sha1 by default. + expect(roundTrip({ oaepHash: "sha256" }, { oaepHash: "sha256", mgf1Hash: "sha256" })).toEqual(input); + expect(roundTrip({}, { mgf1Hash: "sha1" })).toEqual(input); + expect(() => roundTrip({ oaepHash: "sha256", mgf1Hash: "sha1" }, { oaepHash: "sha256" })).toThrow(decodingError); + expect(() => roundTrip({ oaepHash: "sha256" }, { oaepHash: "sha256", mgf1Hash: "sha1" })).toThrow(decodingError); + + // publicEncrypt({ key: rsaPubPem, oaepHash: "sha256", mgf1Hash: "sha1" }, input) in Node.js v26.12.1. + const fromNode = Buffer.from( + "C582UzGmT/stl/cSxOtvfMXt2jOR4L/XUxXRN2PngRLMqrfbJCdqyLv8Z4dLLs0ajL9QL8NIgJYOaxbmstzCIk" + + "9GNfN30b83Aun3/TxMCB4bfdFRKPwRCy2ObGWLyo6ictk90VmdReBinQDYiJhBA116pczFakKkKQTIjBO2fKxK" + + "b3ZRXbdmflrtjj4MtHTLjoLvsV9oCkWk+R5HX86yS91Kpcz+8lZ15wIbOIt775XY4NR8qXZAYJRggtLSxYdToc" + + "jpYSBNkxBhgKXvWUstqYx7TDhyx0KsdmOgorT+9dZ8rCkIrDKGXBsZeoF5+hxGYthmvKH1u41Gkk+0SI/ZaQ==", + "base64", + ); + expect(crypto.privateDecrypt({ key: rsaKeyPem, oaepHash: "sha256", mgf1Hash: "sha1" }, fromNode)).toEqual(input); + expect(() => crypto.privateDecrypt({ key: rsaKeyPem, oaepHash: "sha1", mgf1Hash: "sha256" }, fromNode)).toThrow( + decodingError, + ); +}); + test("RSA-OAEP test vectors", () => { const { decryptionTests } = JSON.parse(fixtures.readSync("rsa-oaep-test-vectors.js", "utf8")); @@ -351,7 +386,7 @@ test("RSA-OAEP test vectors", () => { } }); -describe("Invalid oaepHash and oaepLabel options", () => { +describe("Invalid oaepHash, mgf1Hash and oaepLabel options", () => { const testCases = [ { fn: crypto.publicEncrypt, name: "publicEncrypt", key: rsaPubPem }, { fn: crypto.privateDecrypt, name: "privateDecrypt", key: rsaKeyPem }, @@ -390,6 +425,17 @@ describe("Invalid oaepHash and oaepLabel options", () => { }); }); + test(`${name} with invalid mgf1Hash`, () => { + expect(() => fn({ key, mgf1Hash: "Hello world" }, Buffer.alloc(10))).toThrow( + expect.objectContaining({ code: "ERR_OSSL_EVP_INVALID_DIGEST" }), + ); + for (const mgf1Hash of [0, false, null, Symbol(), () => {}]) { + expect(() => fn({ key, mgf1Hash }, Buffer.alloc(10))).toThrow( + expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" }), + ); + } + }); + test(`${name} with invalid oaepLabel`, () => { [0, false, null, Symbol(), () => {}, {}].forEach(oaepLabel => { expect(() => { From da1758f15f083cdb938a35750af31165532a3d5f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:49 +0000 Subject: [PATCH 05/19] x509.checkIP(): a string with a NUL is not an IP address It throws ERR_INVALID_ARG_VALUE, like Node. Co-authored-by: dylanc --- src/jsc/bindings/JSX509Certificate.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/JSX509Certificate.cpp b/src/jsc/bindings/JSX509Certificate.cpp index 7b5b644d59eb..07de22c7e62a 100644 --- a/src/jsc/bindings/JSX509Certificate.cpp +++ b/src/jsc/bindings/JSX509Certificate.cpp @@ -581,9 +581,10 @@ bool JSX509Certificate::checkIP(JSGlobalObject* globalObject, WTF::StringView ip VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - // utf8() asserts on failure. A string too long to convert isn't an IP address. + // utf8() asserts on failure. A string too long to convert isn't an IP address, and neither is one with a NUL, + // where the C string that checkIp() takes would end. auto ipUtf8 = ip.tryGetUTF8(); - auto result = ipUtf8 ? view().checkIp(ipUtf8->legacyCStringPointer(), 0) : ncrypto::X509View::CheckMatch::INVALID_NAME; + auto result = ipUtf8 && !ip.contains(u'\0') ? view().checkIp(ipUtf8->legacyCStringPointer(), 0) : ncrypto::X509View::CheckMatch::INVALID_NAME; return handleMatchResult(globalObject, "Invalid IP address"_s, scope, result); } From 70256170ce732798ecfc28f33a402f36b0439f45 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:54:49 +0000 Subject: [PATCH 06/19] crypto.getCipherInfo(): validate keyLength and ivLength as uint32 Like Node, which also names them options.keyLength and options.ivLength. A negative keyLength returned undefined, and a negative ivLength was returned as the IV length of a GCM cipher. Co-authored-by: dylanc --- .../node/crypto/node_crypto_binding.cpp | 18 +++++++++--------- test/js/node/crypto/node-crypto.test.js | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/jsc/bindings/node/crypto/node_crypto_binding.cpp b/src/jsc/bindings/node/crypto/node_crypto_binding.cpp index 11be82165c88..08926e6df0cc 100644 --- a/src/jsc/bindings/node/crypto/node_crypto_binding.cpp +++ b/src/jsc/bindings/node/crypto/node_crypto_binding.cpp @@ -203,8 +203,8 @@ JSC_DEFINE_HOST_FUNCTION(jsGetCipherInfo, (JSC::JSGlobalObject * lexicalGlobalOb JSValue options = callFrame->argument(1); - std::optional keyLengthOpt = std::nullopt; - std::optional ivLengthOpt = std::nullopt; + std::optional keyLengthOpt = std::nullopt; + std::optional ivLengthOpt = std::nullopt; if (!options.isUndefined()) { V::validateObject(scope, lexicalGlobalObject, options, "options"_s); RETURN_IF_EXCEPTION(scope, {}); @@ -215,14 +215,14 @@ JSC_DEFINE_HOST_FUNCTION(jsGetCipherInfo, (JSC::JSGlobalObject * lexicalGlobalOb JSValue ivLengthValue = optionsObj->get(lexicalGlobalObject, Identifier::fromString(vm, "ivLength"_s)); RETURN_IF_EXCEPTION(scope, {}); if (!keyLengthValue.isUndefined()) { - int32_t length = 0; - V::validateInt32(scope, lexicalGlobalObject, keyLengthValue, "keyLength"_s, jsUndefined(), jsUndefined(), &length); + uint32_t length = 0; + V::validateUint32(scope, lexicalGlobalObject, keyLengthValue, "options.keyLength"_s, jsUndefined(), &length); RETURN_IF_EXCEPTION(scope, {}); keyLengthOpt = length; } if (!ivLengthValue.isUndefined()) { - int32_t length = 0; - V::validateInt32(scope, lexicalGlobalObject, ivLengthValue, "ivLength"_s, jsUndefined(), jsUndefined(), &length); + uint32_t length = 0; + V::validateUint32(scope, lexicalGlobalObject, ivLengthValue, "options.ivLength"_s, jsUndefined(), &length); RETURN_IF_EXCEPTION(scope, {}); ivLengthOpt = length; } @@ -244,8 +244,8 @@ JSC_DEFINE_HOST_FUNCTION(jsGetCipherInfo, (JSC::JSGlobalObject * lexicalGlobalOb return JSValue::encode(jsUndefined()); } - int32_t keyLength = cipher.getKeyLength(); - int32_t ivLength = cipher.getIvLength(); + uint32_t keyLength = cipher.getKeyLength(); + uint32_t ivLength = cipher.getIvLength(); int32_t blockSize = cipher.getBlockSize(); ASCIILiteral modeLabel = cipher.getModeLabel(); String name = cipher.getName(); @@ -265,7 +265,7 @@ JSC_DEFINE_HOST_FUNCTION(jsGetCipherInfo, (JSC::JSGlobalObject * lexicalGlobalOb } if (ivLengthOpt.has_value()) { - int32_t length = ivLengthOpt.value(); + uint32_t length = ivLengthOpt.value(); if (cipher.isCcmMode()) { if (length < 7 || length > 13) { return JSValue::encode(jsUndefined()); diff --git a/test/js/node/crypto/node-crypto.test.js b/test/js/node/crypto/node-crypto.test.js index 2ce43a06f5fc..a8547bffb91d 100644 --- a/test/js/node/crypto/node-crypto.test.js +++ b/test/js/node/crypto/node-crypto.test.js @@ -908,6 +908,20 @@ describe("integer arguments whose JSValue is a double", () => { }); }); +it("getCipherInfo() takes keyLength and ivLength as unsigned 32-bit integers", () => { + for (const option of ["keyLength", "ivLength"]) { + for (const length of [-1, 2 ** 32]) { + expect(() => crypto.getCipherInfo("aes-128-gcm", { [option]: length })).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: expect.stringContaining(`"options.${option}" is out of range. It must be >= 0 && <= 4294967295.`), + }), + ); + } + } + expect(crypto.getCipherInfo("aes-128-gcm", { ivLength: 2 ** 32 - 1 }).ivLength).toBe(2 ** 32 - 1); +}); + describe("ECDH", () => { it("should have correct method names", () => { const ecdh = crypto.createECDH("prime256v1"); From ed7258b49eeddc83429f4274923539191c85930a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:56:32 +0000 Subject: [PATCH 07/19] RSA JWK import: n must be p times q, and oth is rejected Like Node, createPrivateKey() and subtle.importKey("jwk") compare n with the product of p and q (ncrypto's Rsa::checkPrimeProduct). BoringSSL has no multi-prime RSA, so a JWK with oth is invalid. In node:crypto the JWK decides whether it is a private key, as in Node's ImportJWKRsaKey, so createPublicKey() and the functions that take a key make the same checks. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 14 ++++++++ src/jsc/bindings/ncrypto.h | 1 + src/jsc/bindings/node/crypto/KeyObject.cpp | 23 ++++++++++-- src/jsc/bindings/webcrypto/CryptoKeyRSA.cpp | 31 +++------------- .../webcrypto/CryptoKeyRSAOpenSSL.cpp | 4 +++ .../js/node/crypto/crypto.key-objects.test.ts | 20 +++++++++++ test/js/web/crypto/web-crypto.test.ts | 35 +++++++++++++++++++ 7 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index f046af7c8ba1..20fe8fccefa1 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -3503,6 +3503,20 @@ const Rsa::PrivateKey Rsa::getPrivateKey() const return key; } +// Upstream also multiplies by the primes of getOtherPrimeInfos(). BoringSSL has no multi-prime RSA. +bool Rsa::checkPrimeProduct() const +{ + const auto pub = getPublicKey(); + const auto priv = getPrivateKey(); + if (pub.n == nullptr || priv.p == nullptr || priv.q == nullptr) return false; + auto product = BignumPointer::New(); + BignumCtxPointer ctx(BN_CTX_new()); + if (!product || !ctx || BN_mul(product.get(), priv.p, priv.q, ctx.get()) != 1) { + return false; + } + return BN_cmp(product.get(), pub.n) == 0; +} + const std::optional Rsa::getPssParams() const { if (rsa_ == nullptr) return std::nullopt; diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index d98be1c5db28..41b0ef9af806 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -402,6 +402,7 @@ class Rsa final { const PublicKey getPublicKey() const; const PrivateKey getPrivateKey() const; + bool checkPrimeProduct() const; const std::optional getPssParams() const; bool setPublicKey(BignumPointer&& n, BignumPointer&& e); diff --git a/src/jsc/bindings/node/crypto/KeyObject.cpp b/src/jsc/bindings/node/crypto/KeyObject.cpp index 7ff29eb41935..5ed98606e7c0 100644 --- a/src/jsc/bindings/node/crypto/KeyObject.cpp +++ b/src/jsc/bindings/node/crypto/KeyObject.cpp @@ -1392,6 +1392,17 @@ __attribute__((minsize)) KeyObject KeyObject::getKeyObjectHandleFromJwk(JSGlobal RETURN_IF_EXCEPTION(scope, {}); auto eView = getJwkStringView(globalObject, scope, jwk, "e"_s, "key.e"_s); RETURN_IF_EXCEPTION(scope, {}); + VM& vm = globalObject->vm(); + JSValue dValue = jwk->get(globalObject, Identifier::fromString(vm, "d"_s)); + RETURN_IF_EXCEPTION(scope, {}); + JSValue othValue = jwk->get(globalObject, Identifier::fromString(vm, "oth"_s)); + RETURN_IF_EXCEPTION(scope, {}); + + // BoringSSL has no multi-prime RSA. + if (!othValue.isUndefined()) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK RSA key"_s); + return {}; + } auto* nBuf = decodeJwkString(globalObject, scope, nView, "key.n"_s); RETURN_IF_EXCEPTION(scope, {}); @@ -1408,8 +1419,13 @@ __attribute__((minsize)) KeyObject KeyObject::getKeyObjectHandleFromJwk(JSGlobal return {}; } - if (keyType == CryptoKeyType::Private) { - auto* dBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "d"_s, "key.d"_s); + // The JWK itself decides whether private key material is present, as in Node's ImportJWKRsaKey. + if (keyType == CryptoKeyType::Private || !dValue.isUndefined()) { + V::validateString(scope, globalObject, dValue, "key.d"_s); + RETURN_IF_EXCEPTION(scope, {}); + auto dView = asString(dValue)->view(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + auto* dBuf = decodeJwkString(globalObject, scope, dView, "key.d"_s); RETURN_IF_EXCEPTION(scope, {}); auto* pBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "p"_s, "key.p"_s); RETURN_IF_EXCEPTION(scope, {}); @@ -1428,7 +1444,8 @@ __attribute__((minsize)) KeyObject KeyObject::getKeyObjectHandleFromJwk(JSGlobal jwkBufToBn(pBuf), jwkBufToBn(dpBuf), jwkBufToBn(dqBuf), - jwkBufToBn(qiBuf))) { + jwkBufToBn(qiBuf)) + || !rsaView.checkPrimeProduct()) { ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK RSA key"_s); return {}; } diff --git a/src/jsc/bindings/webcrypto/CryptoKeyRSA.cpp b/src/jsc/bindings/webcrypto/CryptoKeyRSA.cpp index 6ad09a83b4f8..a97805343dbf 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyRSA.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyRSA.cpp @@ -43,6 +43,9 @@ RefPtr CryptoKeyRSA::importJwk(CryptoAlgorithmIdentifier algorithm return nullptr; if (keyData.ext && !keyData.ext.value() && extractable) return nullptr; + // BoringSSL has no multi-prime RSA. + if (keyData.oth) + return nullptr; if (keyData.n.isNull() || keyData.e.isNull()) return nullptr; @@ -100,33 +103,7 @@ RefPtr CryptoKeyRSA::importJwk(CryptoAlgorithmIdentifier algorithm secondPrimeInfo.factorCRTExponent = WTF::move(*secondFactorCRTExponent); secondPrimeInfo.factorCRTCoefficient = WTF::move(*secondFactorCRTCoefficient); - if (!keyData.oth) { - auto privateKeyComponents = CryptoKeyRSAComponents::createPrivateWithAdditionalData(WTF::move(*modulus), WTF::move(*exponent), WTF::move(*privateExponent), WTF::move(firstPrimeInfo), WTF::move(secondPrimeInfo), {}); - // Notice: CryptoAlgorithmIdentifier::SHA_1 is just a placeholder. It should not have any effect if hash is std::nullopt. - return CryptoKeyRSA::create(algorithm, hash.value_or(CryptoAlgorithmIdentifier::SHA_1), !!hash, *privateKeyComponents, extractable, usages); - } - - Vector otherPrimeInfos; - for (const auto& value : keyData.oth.value()) { - auto primeFactor = base64URLDecode(value.r); - if (!primeFactor) - return nullptr; - auto factorCRTExponent = base64URLDecode(value.d); - if (!factorCRTExponent) - return nullptr; - auto factorCRTCoefficient = base64URLDecode(value.t); - if (!factorCRTCoefficient) - return nullptr; - - CryptoKeyRSAComponents::PrimeInfo info; - info.primeFactor = WTF::move(*primeFactor); - info.factorCRTExponent = WTF::move(*factorCRTExponent); - info.factorCRTCoefficient = WTF::move(*factorCRTCoefficient); - - otherPrimeInfos.append(WTF::move(info)); - } - - auto privateKeyComponents = CryptoKeyRSAComponents::createPrivateWithAdditionalData(WTF::move(*modulus), WTF::move(*exponent), WTF::move(*privateExponent), WTF::move(firstPrimeInfo), WTF::move(secondPrimeInfo), WTF::move(otherPrimeInfos)); + auto privateKeyComponents = CryptoKeyRSAComponents::createPrivateWithAdditionalData(WTF::move(*modulus), WTF::move(*exponent), WTF::move(*privateExponent), WTF::move(firstPrimeInfo), WTF::move(secondPrimeInfo), {}); // Notice: CryptoAlgorithmIdentifier::SHA_1 is just a placeholder. It should not have any effect if hash is std::nullopt. return CryptoKeyRSA::create(algorithm, hash.value_or(CryptoAlgorithmIdentifier::SHA_1), !!hash, *privateKeyComponents, extractable, usages); } diff --git a/src/jsc/bindings/webcrypto/CryptoKeyRSAOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyRSAOpenSSL.cpp index 66fff89c8b9c..c5e2bb835865 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyRSAOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyRSAOpenSSL.cpp @@ -32,6 +32,7 @@ #include "CryptoKeyPair.h" #include "CryptoKeyRSAComponents.h" #include "OpenSSLUtilities.h" +#include "ncrypto.h" #include #include #include @@ -118,6 +119,9 @@ RefPtr CryptoKeyRSA::create(CryptoAlgorithmIdentifier identifier, p.release(); q.release(); + if (!ncrypto::Rsa(rsa.get()).checkPrimeProduct()) + return nullptr; + // We set dmp1, dmpq1, and iqmp member of the RSA struct if the keyData has corresponding data. // dmp1 -- d mod (p - 1) diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 36250b67a630..70f811da805a 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -647,6 +647,26 @@ describe("crypto.KeyObjects", () => { ); }); + test("keys from a private RSA jwk need the n that is p times q, and no oth", () => { + const jwk = createPrivateKey(privatePem).export({ format: "jwk" }); + const { kty, n, e } = jwk; + expect(createPrivateKey({ key: jwk, format: "jwk" }).export({ format: "jwk" })).toEqual(jwk); + expect(createPublicKey({ key: jwk, format: "jwk" }).export({ format: "jwk" })).toEqual({ kty, n, e }); + + const changed = (encoded?: string) => { + const bytes = Buffer.from(encoded!, "base64url"); + bytes[0] ^= 1; + return bytes.toString("base64url"); + }; + const invalid = expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "Invalid JWK RSA key" }); + for (const wrong of [{ n: changed(n) }, { p: changed(jwk.p) }, { q: changed(jwk.q) }, { p: "" }, { oth: [] }]) { + for (const create of [createPrivateKey, createPublicKey]) { + expect(() => create({ key: { ...jwk, ...wrong }, format: "jwk" })).toThrow(invalid); + } + } + expect(() => createPublicKey({ key: { kty, n, e, oth: [] }, format: "jwk" })).toThrow(invalid); + }); + test("private encrypted should work", async () => { // Reading an encrypted key without a passphrase should fail. expect(() => createPrivateKey(privateEncryptedPem)).toThrow(); diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 3cf116bcd290..7ea22aab3e58 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1632,6 +1632,41 @@ describe("JWK import validation", () => { }); }); + it("RSA private keys need the n that is p times q, and no oth", async () => { + const algorithm = { + name: "RSASSA-PKCS1-v1_5", + modulusLength: 1024, + publicExponent: new Uint8Array([1, 0, 1]), + hash: "SHA-256", + }; + const { privateKey } = await crypto.subtle.generateKey(algorithm, true, ["sign", "verify"]); + const jwk = await crypto.subtle.exportKey("jwk", privateKey); + const { kty, n, e } = jwk; + const importJwk = (members: JsonWebKey) => + crypto.subtle.importKey("jwk", { ...jwk, ...members }, algorithm, true, ["sign"]); + const changed = (encoded?: string) => { + const bytes = Buffer.from(encoded!, "base64url"); + bytes[0] ^= 1; + return bytes.toString("base64url"); + }; + + expect({ + unchanged: await rejection(importJwk({})), + n: await rejection(importJwk({ n: changed(n) })), + p: await rejection(importJwk({ p: changed(jwk.p) })), + q: await rejection(importJwk({ q: changed(jwk.q) })), + oth: await rejection(importJwk({ oth: [] })), + publicOth: await rejection(crypto.subtle.importKey("jwk", { kty, n, e, oth: [] }, algorithm, true, ["verify"])), + }).toEqual({ + unchanged: "resolved", + n: "DataError", + p: "DataError", + q: "DataError", + oth: "DataError", + publicOth: "DataError", + }); + }); + it("rejects a repeated key_ops entry for every key type", async () => { const pair = async (algorithm: any, usages: KeyUsage[]) => crypto.subtle.exportKey( From 4cae008af3b3061ee8747c5d2c8a18f6022cecd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:56:32 +0000 Subject: [PATCH 08/19] webcrypto: deriveBits length range, EC pkcs8 export, SyntaxError as a DOMException - subtle.deriveBits() takes its length as an [EnforceRange] unsigned long, like Node. NaN derived 0 bits and 2 ** 32 + 64 derived 64. - An EC private key imported from a PKCS#8 structure without the optional public key is exported with it. - The SyntaxError for bad or empty keyUsages, from SubtleCrypto and from keyObject.toCryptoKey(), is a DOMException. Co-authored-by: dylanc --- src/jsc/bindings/node/crypto/KeyObject.cpp | 6 ++ .../bindings/webcrypto/CryptoKeyECOpenSSL.cpp | 2 + src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp | 2 +- src/jsc/bindings/webcrypto/SubtleCrypto.cpp | 9 ++- test/js/web/crypto/web-crypto.test.ts | 55 +++++++++++++++++++ 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/node/crypto/KeyObject.cpp b/src/jsc/bindings/node/crypto/KeyObject.cpp index 5ed98606e7c0..55a8549b34a9 100644 --- a/src/jsc/bindings/node/crypto/KeyObject.cpp +++ b/src/jsc/bindings/node/crypto/KeyObject.cpp @@ -26,6 +26,7 @@ #include "JSDOMConvertStrings.h" #include "JSDOMConvertUnion.h" #include "JSDOMConvertEnumeration.h" +#include "JSDOMException.h" #include "JSDOMExceptionHandling.h" #include "OpenSSLUtilities.h" #include "CryptoGenKeyPair.h" @@ -967,6 +968,11 @@ JSValue KeyObject::toCryptoKey(JSGlobalObject* lexicalGlobalObject, ThrowScope& auto usagesBitmap = WebCore::SubtleCrypto::toCryptoKeyUsageBitmap(keyUsages); auto throwDOMException = [&](WebCore::ExceptionCode code, const String& message) -> JSValue { + // createDOMException() turns this code into JavaScript's own SyntaxError. + if (code == WebCore::SyntaxError) { + throwException(lexicalGlobalObject, scope, WebCore::toJSNewlyCreated(lexicalGlobalObject, globalObject, WebCore::DOMException::create(code, message))); + return {}; + } WebCore::propagateException(*lexicalGlobalObject, scope, WebCore::Exception { code, message }); return {}; }; diff --git a/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp index 35eff39b93da..2c29a4888a5f 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp @@ -397,6 +397,8 @@ RefPtr CryptoKeyEC::platformImportPkcs8(CryptoAlgorithmIdentifier i } EC_KEY_set_asn1_flag(ecKey, OPENSSL_EC_NAMED_CURVE); + // BoringSSL sets this on a key that came without its optional public key, and would export it without one. + EC_KEY_set_enc_flags(ecKey, EC_KEY_get_enc_flags(ecKey) & ~EC_PKEY_NO_PUBKEY); return adoptRef(new CryptoKeyEC(identifier, curve, CryptoKeyType::Private, WTF::move(pkey), extractable, usages)); } diff --git a/src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp b/src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp index 6d61bc9ad873..e87be6666d10 100644 --- a/src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp +++ b/src/jsc/bindings/webcrypto/JSSubtleCrypto.cpp @@ -464,7 +464,7 @@ static inline JSC::EncodedJSValue jsSubtleCryptoPrototypeFunction_deriveBitsBody auto baseKey = convert>(*lexicalGlobalObject, argument1.value(), [](JSC::JSGlobalObject& lexicalGlobalObject, JSC::ThrowScope& scope) { throwArgumentTypeError(lexicalGlobalObject, scope, 1, "baseKey"_s, "SubtleCrypto"_s, "deriveBits"_s, "CryptoKey"_s); }); RETURN_IF_EXCEPTION(throwScope, {}); EnsureStillAliveScope argument2 = callFrame->argument(2); - auto length = convert>(*lexicalGlobalObject, argument2.value()); + auto length = convert>>(*lexicalGlobalObject, argument2.value()); RETURN_IF_EXCEPTION(throwScope, {}); RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS>(*lexicalGlobalObject, *castedThis->globalObject(), throwScope, [&]() -> decltype(auto) { return impl.deriveBits(*uncheckedDowncast(lexicalGlobalObject), WTF::move(algorithm), *baseKey, WTF::move(length), WTF::move(promise)); }))); } diff --git a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp index 0fe653761b49..f6ad1febe3de 100644 --- a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp +++ b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp @@ -58,6 +58,7 @@ #include "CryptoKeyRSA.h" #include "BunString.h" #include "ErrorCode.h" +#include "JSDOMException.h" #include "JSDOMExceptionHandling.h" #include "VectorSizeLimit.h" #include "node/crypto/CryptoUtil.h" @@ -551,6 +552,11 @@ CryptoKeyUsageBitmap SubtleCrypto::toCryptoKeyUsageBitmap(const Vector&& passedPromise, ExceptionCode ec, const String& msg) { + // createDOMException() turns this code into JavaScript's own SyntaxError. + if (ec == SyntaxError) { + passedPromise->reject>(DOMException::create(ec, msg.isEmpty() ? "A required parameter was missing or out-of-range"_s : msg)); + return; + } if (!msg.isEmpty()) { passedPromise->reject(ec, msg); return; @@ -559,9 +565,6 @@ static void rejectWithException(Ref&& passedPromise, ExceptionC case NotSupportedError: passedPromise->reject(ec, "The algorithm is not supported"_s); return; - case SyntaxError: - passedPromise->reject(ec, "A required parameter was missing or out-of-range"_s); - return; case InvalidStateError: passedPromise->reject(ec, "The requested operation is not valid for the current state of the provided key"_s); return; diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 7ea22aab3e58..279c0ae8cfcb 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1192,6 +1192,27 @@ describe("SubtleCrypto.deriveBits length", () => { }); }); + it("takes the length as an [EnforceRange] unsigned long", async () => { + const key = await crypto.subtle.importKey("raw", enc.encode("secret"), "HKDF", false, ["deriveBits"]); + const hkdf = { name: "HKDF", hash: "SHA-256", salt: enc.encode("salt"), info: enc.encode("info") }; + const results: Record = {}; + for (const length of [16, 16.9, "16", NaN, -1, 2 ** 32, 2 ** 32 + 16, Infinity, "x", {}]) { + results[Bun.inspect(length)] = await probe(crypto.subtle.deriveBits(hkdf, key, length as number)); + } + expect(results).toEqual({ + "16": "f6d2", + "16.9": "f6d2", + '"16"': "f6d2", + NaN: "TypeError", + "-1": "TypeError", + "4294967296": "TypeError", + "4294967312": "TypeError", + Infinity: "TypeError", + '"x"': "TypeError", + "{}": "TypeError", + }); + }); + // deriveKey(derivedKeyType: HKDF) has no inherent key length, so the entire shared // secret must become the imported HKDF key; a derivedKeyType with a concrete length // (AES-GCM 256) must still flow that length through. @@ -1303,6 +1324,24 @@ describe("OKP spki/pkcs8 cross-curve import", () => { }); }); +it.each([ + ["ECDSA", "sign"], + ["ECDH", "deriveBits"], +] as const)("%s: a pkcs8 key without its optional public key is exported with it", async (name, usage) => { + const algorithm = { name, namedCurve: "P-256" }; + const { privateKey } = await crypto.subtle.generateKey(algorithm, true, [usage]); + const { d } = await crypto.subtle.exportKey("jwk", privateKey); + // A PrivateKeyInfo whose ECPrivateKey (RFC 5915) is only the version and the private key. + const withoutPublicKey = Buffer.concat([ + Buffer.from("3041020100301306072a8648ce3d020106082a8648ce3d030107042730250201010420", "hex"), + Buffer.from(d!, "base64url"), + ]); + const imported = await crypto.subtle.importKey("pkcs8", withoutPublicKey, algorithm, true, [usage]); + expect(Buffer.from(await crypto.subtle.exportKey("pkcs8", imported))).toEqual( + Buffer.from(await crypto.subtle.exportKey("pkcs8", privateKey)), + ); +}); + // importKey's empty-usages guard got Node's message; the same predicate in // generateKey, deriveKey's inner import and unwrapKey's inner import still // carried the empty-message SyntaxError. @@ -1339,6 +1378,22 @@ describe("empty usages on a private or secret key", () => { unwrapKey: "SyntaxError: Usages cannot be empty when importing a secret key.", }); }); + + it("the SyntaxError is a DOMException", async () => { + const errors = await Promise.all( + [ + crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, false, []), + crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, false, ["sign"]), + crypto.subtle.importKey("raw", new Uint8Array(16), "AES-GCM", false, ["sign"]), + (async () => createSecretKey(new Uint8Array(16)).toCryptoKey("AES-GCM", false, []))(), + (async () => createSecretKey(new Uint8Array(16)).toCryptoKey("AES-GCM", false, ["sign"]))(), + ].map(promise => promise.catch(e => e)), + ); + for (const error of errors) { + expect(error).toBeInstanceOf(DOMException); + expect({ name: error.name, code: error.code }).toEqual({ name: "SyntaxError", code: DOMException.SYNTAX_ERR }); + } + }); }); // X25519 deriveBits is a line-for-line parallel of ECDH's; ECDH got Node's From 17ee452c061f894990598fdb5c1e7605942d1a80 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:16 +0000 Subject: [PATCH 09/19] node:crypto: call BoringSSL's sized cipher functions directly update() and final() call EVP_CipherUpdate_ex and EVP_CipherFinal_ex2 themselves, so ncrypto.cpp needs no wrappers that upstream does not have. final() writes at most two blocks, into an array on the stack. The branch of update() for key-wrap ciphers goes: BoringSSL has none. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 24 -------- src/jsc/bindings/ncrypto.h | 7 --- .../node/crypto/JSCipherPrototype.cpp | 57 +++++-------------- 3 files changed, 13 insertions(+), 75 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index 20fe8fccefa1..cbe3f7abe7f3 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -2888,30 +2888,6 @@ bool CipherCtxPointer::update(const Buffer& in, return EVP_CipherFinal_ex(ctx_.get(), out, out_len) == 1; } -size_t CipherCtxPointer::maxUpdateSize(size_t in_len) const -{ - if (!ctx_) return 0; - return EVP_CIPHER_CTX_max_next_update(ctx_.get(), in_len); -} - -size_t CipherCtxPointer::maxFinalSize() const -{ - if (!ctx_) return 0; - return EVP_CIPHER_CTX_max_final(ctx_.get()); -} - -bool CipherCtxPointer::update(const Buffer& in, std::span out, size_t* out_len) -{ - if (!ctx_) return false; - return EVP_CipherUpdate_ex(ctx_.get(), out.data(), out_len, out.size(), in.data, in.len) == 1; -} - -bool CipherCtxPointer::finalize(std::span out, size_t* out_len) -{ - if (!ctx_) return false; - return EVP_CipherFinal_ex2(ctx_.get(), out.data(), out_len, out.size()) == 1; -} - bool CipherCtxPointer::getAeadTag(size_t len, unsigned char* out) { if (!ctx_) return false; diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index 41b0ef9af806..3d15fd5b8abe 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -643,13 +643,6 @@ class CipherCtxPointer final { unsigned char* out, int* out_len, bool finalize = false); - - // Not upstream: BoringSSL's calls that are told how much room `out` has, and how much they need at most. - size_t maxUpdateSize(size_t in_len) const; - size_t maxFinalSize() const; - bool update(const Buffer& in, std::span out, size_t* out_len); - bool finalize(std::span out, size_t* out_len); - bool getAeadTag(size_t len, unsigned char* out); private: diff --git a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp index c2a5200c7465..e70a9b6d043c 100644 --- a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp +++ b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp @@ -38,14 +38,6 @@ void JSCipherPrototype::finishCreation(JSC::VM& vm) Bun::putToStringTagWithoutTransition(vm, this, info()); } -// The sizes BoringSSL gives up front are upper bounds. The result's ArrayBuffer is as long as what was written, like node's. -static RefPtr trimToWritten(RefPtr&& buffer, size_t written) -{ - if (written == buffer->byteLength()) - return WTF::move(buffer); - return ArrayBuffer::tryCreate(buffer->span().first(written)); -} - JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) { auto& vm = lexicalGlobalObject->vm(); @@ -89,36 +81,21 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj ASSERT_UNUSED(passed, passed); } - const int32_t blockSize = cipher->m_ctx.getBlockSize(); - if (data.size() + blockSize > INT_MAX) { + if (data.size() + cipher->m_ctx.getBlockSize() > INT_MAX) { throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); return {}; } - ncrypto::Buffer buf { - .data = data.data(), - .len = data.size(), - }; - - size_t maxOutLen = cipher->m_ctx.maxUpdateSize(data.size()); - if (cipher->m_kind == CipherKind::Cipher && cipher->m_ctx.isWrapMode()) { - int32_t wrappedLen = 0; - if (!cipher->m_ctx.update(buf, nullptr, &wrappedLen)) { - throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state"); - return {}; - } - maxOutLen = wrappedLen; - } - - RefPtr outBuf = JSC::ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + maxOutLen, 1); + RefPtr outBuf = JSC::ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + EVP_CIPHER_CTX_max_next_update(cipher->m_ctx, data.size()), 1); if (!outBuf) { throwOutOfMemoryError(lexicalGlobalObject, scope); return {}; } size_t outLen = cipher->takeHeldBackBlock(outBuf->mutableSpan()); + auto out = outBuf->mutableSpan().subspan(outLen); size_t written = 0; - bool res = cipher->m_ctx.update(buf, outBuf->mutableSpan().subspan(outLen), &written); + bool res = EVP_CipherUpdate_ex(cipher->m_ctx, out.data(), &written, out.size(), data.data(), data.size()) == 1; ensureStillAliveHere(dataView); outLen += written; @@ -132,20 +109,21 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj return {}; } - outBuf = trimToWritten(WTF::move(outBuf), outLen); + // EVP_CIPHER_CTX_max_next_update() is an upper bound. The result's ArrayBuffer is as long as what was written, like Node's. + if (outLen != outBuf->byteLength()) + outBuf = ArrayBuffer::tryCreate(outBuf->span().first(outLen)); if (!outBuf) { throwOutOfMemoryError(lexicalGlobalObject, scope); return {}; } - RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, outLen))); + RELEASE_AND_RETURN(scope, JSValue::encode(createBuffer(lexicalGlobalObject, outBuf.releaseNonNull()))); } JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) { auto& vm = lexicalGlobalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - auto* globalObject = defaultGlobalObject(lexicalGlobalObject); ClearErrorOnReturn clearErrorOnReturn; @@ -167,12 +145,9 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje throwCryptoError(globalObject, scope, ERR_peek_error(), isAuthMode ? "Unsupported state or unable to authenticate data" : "Unsupported state"); }; - RefPtr outBuf = ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + cipher->m_ctx.maxFinalSize(), 1); - if (!outBuf) { - throwOutOfMemoryError(lexicalGlobalObject, scope); - return {}; - } - size_t outLen = cipher->takeHeldBackBlock(outBuf->mutableSpan()); + // Room for the held-back block and the last one. + uint8_t out[2 * EVP_MAX_BLOCK_LENGTH]; + size_t outLen = cipher->takeHeldBackBlock(out); if (cipher->m_kind == CipherKind::Decipher && Cipher::FromCtx(cipher->m_ctx).isSupportedAuthenticatedMode()) { cipher->maybePassAuthTagToOpenSSL(); @@ -188,7 +163,7 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje ok = !cipher->m_pendingAuthFailed; } else { size_t written = 0; - ok = cipher->m_ctx.finalize(outBuf->mutableSpan().subspan(outLen), &written); + ok = EVP_CipherFinal_ex2(cipher->m_ctx, out + outLen, &written, sizeof(out) - outLen) == 1; outLen += written; if (ok && cipher->m_kind == CipherKind::Cipher && cipher->isAuthenticatedMode()) { @@ -205,13 +180,7 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherFinal, (JSC::JSGlobalObject * lexicalGlobalObje return {}; } - outBuf = trimToWritten(WTF::move(outBuf), outLen); - if (!outBuf) { - throwOutOfMemoryError(lexicalGlobalObject, scope); - return {}; - } - - RELEASE_AND_RETURN(scope, JSValue::encode(JSUint8Array::create(lexicalGlobalObject, globalObject->JSBufferSubclassStructure(), WTF::move(outBuf), 0, outLen))); + RELEASE_AND_RETURN(scope, JSValue::encode(createBuffer(lexicalGlobalObject, std::span { out }.first(outLen)))); } JSC_DEFINE_HOST_FUNCTION(jsCipherSetAutoPadding, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) From 236086855263a2876afccf2977cf33aa7f68cba5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:19 +0000 Subject: [PATCH 10/19] webcrypto: the SyntaxError DOMException has a stack The default arm of createDOMException() is what gives a DOMException its stack, line, column and sourceURL. It moves into createDOMExceptionObject(), which SubtleCrypto and toCryptoKey() now call for their SyntaxError. Co-authored-by: dylanc --- src/jsc/bindings/JSDOMExceptionHandling.cpp | 28 ++++++++++++--------- src/jsc/bindings/JSDOMExceptionHandling.h | 2 ++ src/jsc/bindings/node/crypto/KeyObject.cpp | 3 +-- src/jsc/bindings/webcrypto/SubtleCrypto.cpp | 5 ++-- test/js/web/crypto/web-crypto.test.ts | 1 + 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/jsc/bindings/JSDOMExceptionHandling.cpp b/src/jsc/bindings/JSDOMExceptionHandling.cpp index 5f7c437fb326..9448b7b859db 100644 --- a/src/jsc/bindings/JSDOMExceptionHandling.cpp +++ b/src/jsc/bindings/JSDOMExceptionHandling.cpp @@ -106,6 +106,20 @@ String retrieveErrorMessage(JSGlobalObject& lexicalGlobalObject, VM& vm, JSValue return errorMessage; } +JSValue createDOMExceptionObject(JSGlobalObject* lexicalGlobalObject, ExceptionCode ec, const String& message) +{ + // FIXME: All callers to createDOMException need to pass in the correct global object. + // For now, we're going to assume the lexicalGlobalObject. Which is wrong in cases like this: + // frames[0].document.createElement(null, null); // throws an exception which should have the subframe's prototypes. + // https://bugs.webkit.org/show_bug.cgi?id=222229 + JSDOMGlobalObject* globalObject = deprecatedGlobalObjectForPrototype(lexicalGlobalObject); + JSValue errorObject = toJS(lexicalGlobalObject, globalObject, DOMException::create(ec, message)); + + ASSERT(errorObject); + addErrorInfo(lexicalGlobalObject, asObject(errorObject), true); + return errorObject; +} + JSValue createDOMException(JSGlobalObject* lexicalGlobalObject, ExceptionCode ec, const String& message, const String& extra) { auto& vm = JSC::getVM(lexicalGlobalObject); @@ -168,18 +182,8 @@ JSValue createDOMException(JSGlobalObject* lexicalGlobalObject, ExceptionCode ec case ExceptionCode::WORKER_INVALID_EXEC_ARGV: return Bun::createError(lexicalGlobalObject, Bun::ErrorCode::ERR_WORKER_INVALID_EXEC_ARGV, message); - default: { - // FIXME: All callers to createDOMException need to pass in the correct global object. - // For now, we're going to assume the lexicalGlobalObject. Which is wrong in cases like this: - // frames[0].document.createElement(null, null); // throws an exception which should have the subframe's prototypes. - // https://bugs.webkit.org/show_bug.cgi?id=222229 - JSDOMGlobalObject* globalObject = deprecatedGlobalObjectForPrototype(lexicalGlobalObject); - JSValue errorObject = toJS(lexicalGlobalObject, globalObject, DOMException::create(ec, message)); - - ASSERT(errorObject); - addErrorInfo(lexicalGlobalObject, asObject(errorObject), true); - return errorObject; - } + default: + return createDOMExceptionObject(lexicalGlobalObject, ec, message); } return {}; } diff --git a/src/jsc/bindings/JSDOMExceptionHandling.h b/src/jsc/bindings/JSDOMExceptionHandling.h index c6c7bf156a3d..7731e598a340 100644 --- a/src/jsc/bindings/JSDOMExceptionHandling.h +++ b/src/jsc/bindings/JSDOMExceptionHandling.h @@ -63,6 +63,8 @@ WEBCORE_EXPORT void reportException(JSC::JSGlobalObject*, JSC::Exception*, Cache JSC::JSValue createDOMException(JSC::JSGlobalObject&, Exception&&); JSC::JSValue createDOMException(JSC::JSGlobalObject*, ExceptionCode, const String& = emptyString(), const String& extra = String()); +// Always a DOMException: createDOMException() turns ExceptionCode::SyntaxError into JavaScript's own SyntaxError. +JSC::JSValue createDOMExceptionObject(JSC::JSGlobalObject*, ExceptionCode, const String& message); // Convert a DOM implementation exception into a JavaScript exception in the execution lexicalGlobalObject. WEBCORE_EXPORT void propagateExceptionSlowPath(JSC::JSGlobalObject&, JSC::ThrowScope&, Exception&&); diff --git a/src/jsc/bindings/node/crypto/KeyObject.cpp b/src/jsc/bindings/node/crypto/KeyObject.cpp index 55a8549b34a9..47c4db2c3ad1 100644 --- a/src/jsc/bindings/node/crypto/KeyObject.cpp +++ b/src/jsc/bindings/node/crypto/KeyObject.cpp @@ -26,7 +26,6 @@ #include "JSDOMConvertStrings.h" #include "JSDOMConvertUnion.h" #include "JSDOMConvertEnumeration.h" -#include "JSDOMException.h" #include "JSDOMExceptionHandling.h" #include "OpenSSLUtilities.h" #include "CryptoGenKeyPair.h" @@ -970,7 +969,7 @@ JSValue KeyObject::toCryptoKey(JSGlobalObject* lexicalGlobalObject, ThrowScope& auto throwDOMException = [&](WebCore::ExceptionCode code, const String& message) -> JSValue { // createDOMException() turns this code into JavaScript's own SyntaxError. if (code == WebCore::SyntaxError) { - throwException(lexicalGlobalObject, scope, WebCore::toJSNewlyCreated(lexicalGlobalObject, globalObject, WebCore::DOMException::create(code, message))); + throwException(lexicalGlobalObject, scope, WebCore::createDOMExceptionObject(lexicalGlobalObject, code, message)); return {}; } WebCore::propagateException(*lexicalGlobalObject, scope, WebCore::Exception { code, message }); diff --git a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp index f6ad1febe3de..318aec31784c 100644 --- a/src/jsc/bindings/webcrypto/SubtleCrypto.cpp +++ b/src/jsc/bindings/webcrypto/SubtleCrypto.cpp @@ -58,7 +58,6 @@ #include "CryptoKeyRSA.h" #include "BunString.h" #include "ErrorCode.h" -#include "JSDOMException.h" #include "JSDOMExceptionHandling.h" #include "VectorSizeLimit.h" #include "node/crypto/CryptoUtil.h" @@ -554,7 +553,9 @@ static void rejectWithException(Ref&& passedPromise, ExceptionC { // createDOMException() turns this code into JavaScript's own SyntaxError. if (ec == SyntaxError) { - passedPromise->reject>(DOMException::create(ec, msg.isEmpty() ? "A required parameter was missing or out-of-range"_s : msg)); + passedPromise->rejectWithCallback([&](JSDOMGlobalObject& globalObject) { + return createDOMExceptionObject(&globalObject, ec, msg.isEmpty() ? "A required parameter was missing or out-of-range"_s : msg); + }); return; } if (!msg.isEmpty()) { diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 279c0ae8cfcb..dffc21630d17 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1392,6 +1392,7 @@ describe("empty usages on a private or secret key", () => { for (const error of errors) { expect(error).toBeInstanceOf(DOMException); expect({ name: error.name, code: error.code }).toEqual({ name: "SyntaxError", code: DOMException.SYNTAX_ERR }); + expect(error.stack).toBeString(); } }); }); From dd1f833e73d0573e4a900c1ff80fc19706c20400 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:23 +0000 Subject: [PATCH 11/19] webcrypto: add the EC public key when exporting pkcs8, on a copy Clearing the flag on the imported key also changed what KeyObject.from(key).export() gives, which Node leaves as the key came. exportKey("pkcs8") now encodes a copy without the flag, as ncrypto's Ec::ExportPrivatePkcs8 does. Co-authored-by: dylanc --- src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp | 14 +++++++++++--- test/js/web/crypto/web-crypto.test.ts | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp b/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp index 2c29a4888a5f..8fb781d90f6f 100644 --- a/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp +++ b/src/jsc/bindings/webcrypto/CryptoKeyECOpenSSL.cpp @@ -397,8 +397,6 @@ RefPtr CryptoKeyEC::platformImportPkcs8(CryptoAlgorithmIdentifier i } EC_KEY_set_asn1_flag(ecKey, OPENSSL_EC_NAMED_CURVE); - // BoringSSL sets this on a key that came without its optional public key, and would export it without one. - EC_KEY_set_enc_flags(ecKey, EC_KEY_get_enc_flags(ecKey) & ~EC_PKEY_NO_PUBKEY); return adoptRef(new CryptoKeyEC(identifier, curve, CryptoKeyType::Private, WTF::move(pkey), extractable, usages)); } @@ -471,7 +469,17 @@ Vector CryptoKeyEC::platformExportPkcs8() const if (type() != CryptoKeyType::Private) return {}; - auto p8inf = PKCS8PrivKeyInfoPtr(EVP_PKEY2PKCS8(platformKey())); + // A key that came without its optional public key has a flag that leaves it out again. As in ncrypto's + // Ec::ExportPrivatePkcs8, a copy without the flag is what is encoded. + auto ecKey = ECKeyPtr(EC_KEY_dup(EVP_PKEY_get0_EC_KEY(platformKey()))); + if (!ecKey) + return {}; + EC_KEY_set_enc_flags(ecKey.get(), EC_KEY_get_enc_flags(ecKey.get()) & ~EC_PKEY_NO_PUBKEY); + auto pkey = EvpPKeyPtr(EVP_PKEY_new()); + if (!pkey || EVP_PKEY_set1_EC_KEY(pkey.get(), ecKey.get()) != 1) + return {}; + + auto p8inf = PKCS8PrivKeyInfoPtr(EVP_PKEY2PKCS8(pkey.get())); if (!p8inf) return {}; diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index dffc21630d17..997eaafbb5b4 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1,7 +1,7 @@ import { spawnSync } from "bun"; import { describe, expect, it } from "bun:test"; import { bunEnv, bunExe } from "harness"; -import { createCipheriv, createSecretKey, type webcrypto } from "node:crypto"; +import { createCipheriv, createSecretKey, KeyObject, type webcrypto } from "node:crypto"; type AlgorithmIdentifier = webcrypto.AlgorithmIdentifier; type HmacImportParams = webcrypto.HmacImportParams; @@ -1340,6 +1340,8 @@ it.each([ expect(Buffer.from(await crypto.subtle.exportKey("pkcs8", imported))).toEqual( Buffer.from(await crypto.subtle.exportKey("pkcs8", privateKey)), ); + // The key itself stays as it came. + expect(KeyObject.from(imported).export({ format: "der", type: "pkcs8" })).toEqual(withoutPublicKey); }); // importKey's empty-usages guard got Node's message; the same predicate in From 50183b4dc47968851eaf0033201040502a5e7d44 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:27 +0000 Subject: [PATCH 12/19] node:crypto: read an RSA JWK the way Node's ImportJWKRsaKey does Every member is read once, and a member that is missing or not a string is ERR_CRYPTO_INVALID_JWK like every other failure, whichever function was given the JWK. It was ERR_INVALID_ARG_TYPE. A public JWK where a private key is wanted gets the message the OKP arm already has. checkPrimeProduct() returns false without multiplying for a modulus that BoringSSL refuses to use, or a factor that is longer than the modulus. Co-authored-by: dylanc --- src/jsc/bindings/ncrypto.cpp | 5 +- src/jsc/bindings/ncrypto.h | 1 + src/jsc/bindings/node/crypto/KeyObject.cpp | 93 +++++++++++-------- .../js/node/crypto/crypto.key-objects.test.ts | 28 ++++-- test/js/web/crypto/web-crypto.test.ts | 11 +-- 5 files changed, 80 insertions(+), 58 deletions(-) diff --git a/src/jsc/bindings/ncrypto.cpp b/src/jsc/bindings/ncrypto.cpp index cbe3f7abe7f3..d3f30afc1cd8 100644 --- a/src/jsc/bindings/ncrypto.cpp +++ b/src/jsc/bindings/ncrypto.cpp @@ -3479,12 +3479,15 @@ const Rsa::PrivateKey Rsa::getPrivateKey() const return key; } -// Upstream also multiplies by the primes of getOtherPrimeInfos(). BoringSSL has no multi-prime RSA. +// Not upstream: there is no loop over getOtherPrimeInfos(), which is empty with BoringSSL, and nothing is multiplied +// for a modulus that BoringSSL refuses to use or a factor that is longer than the modulus. bool Rsa::checkPrimeProduct() const { const auto pub = getPublicKey(); const auto priv = getPrivateKey(); if (pub.n == nullptr || priv.p == nullptr || priv.q == nullptr) return false; + const unsigned bits = BN_num_bits(pub.n); + if (bits > OPENSSL_RSA_MAX_MODULUS_BITS || BN_num_bits(priv.p) > bits || BN_num_bits(priv.q) > bits) return false; auto product = BignumPointer::New(); BignumCtxPointer ctx(BN_CTX_new()); if (!product || !ctx || BN_mul(product.get(), priv.p, priv.q, ctx.get()) != 1) { diff --git a/src/jsc/bindings/ncrypto.h b/src/jsc/bindings/ncrypto.h index 3d15fd5b8abe..8ed9d29bca66 100644 --- a/src/jsc/bindings/ncrypto.h +++ b/src/jsc/bindings/ncrypto.h @@ -402,6 +402,7 @@ class Rsa final { const PublicKey getPublicKey() const; const PrivateKey getPrivateKey() const; + // Check that n is the product of all private-key prime factors. bool checkPrimeProduct() const; const std::optional getPssParams() const; diff --git a/src/jsc/bindings/node/crypto/KeyObject.cpp b/src/jsc/bindings/node/crypto/KeyObject.cpp index 47c4db2c3ad1..b878979d0116 100644 --- a/src/jsc/bindings/node/crypto/KeyObject.cpp +++ b/src/jsc/bindings/node/crypto/KeyObject.cpp @@ -1195,20 +1195,18 @@ GCOwnedDataScope getJwkStringView(JSGlobalObject* globalObject, return strView; } -JSArrayBufferView* getDecodedJwkStringBuf(JSGlobalObject* globalObject, ThrowScope& scope, JSObject* jwk, ASCIILiteral propName, ASCIILiteral keyName) +inline BignumPointer jwkBufToBn(JSArrayBufferView* buf) { - auto strView = getJwkStringView(globalObject, scope, jwk, propName, keyName); - RETURN_IF_EXCEPTION(scope, {}); - - auto* dataBuf = decodeJwkString(globalObject, scope, strView, keyName); - RETURN_IF_EXCEPTION(scope, {}); - - return dataBuf; + return BignumPointer(reinterpret_cast(buf->vector()), buf->byteLength()); } -inline BignumPointer jwkBufToBn(JSArrayBufferView* buf) +static BignumPointer jwkStringToBn(JSGlobalObject* globalObject, ThrowScope& scope, JSValue string, ASCIILiteral keyName) { - return BignumPointer(reinterpret_cast(buf->vector()), buf->byteLength()); + auto strView = asString(string)->view(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + auto* dataBuf = decodeJwkString(globalObject, scope, strView, keyName); + RETURN_IF_EXCEPTION(scope, {}); + return jwkBufToBn(dataBuf); } static bool hasRawPublicKey(const EVPKeyPointer& key, std::span expected) @@ -1393,65 +1391,78 @@ __attribute__((minsize)) KeyObject KeyObject::getKeyObjectHandleFromJwk(JSGlobal return create(keyType, WTF::move(key)); } case Kty::Rsa: { - auto nView = getJwkStringView(globalObject, scope, jwk, "n"_s, "key.n"_s); + // Node's ImportJWKRsaKey: https://github.com/nodejs/node/blob/v26.12.1/src/crypto/crypto_rsa.cc#L372 + const ASCIILiteral invalidKey = "Invalid JWK RSA key"_s; + VM& vm = globalObject->vm(); + JSValue nValue = jwk->get(globalObject, Identifier::fromString(vm, "n"_s)); RETURN_IF_EXCEPTION(scope, {}); - auto eView = getJwkStringView(globalObject, scope, jwk, "e"_s, "key.e"_s); + JSValue eValue = jwk->get(globalObject, Identifier::fromString(vm, "e"_s)); RETURN_IF_EXCEPTION(scope, {}); - VM& vm = globalObject->vm(); JSValue dValue = jwk->get(globalObject, Identifier::fromString(vm, "d"_s)); RETURN_IF_EXCEPTION(scope, {}); JSValue othValue = jwk->get(globalObject, Identifier::fromString(vm, "oth"_s)); RETURN_IF_EXCEPTION(scope, {}); - // BoringSSL has no multi-prime RSA. - if (!othValue.isUndefined()) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK RSA key"_s); + // BoringSSL has no multi-prime RSA, so no "oth" is valid. + if (!nValue.isString() || !eValue.isString() || (!dValue.isUndefined() && !dValue.isString()) || !othValue.isUndefined()) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } - auto* nBuf = decodeJwkString(globalObject, scope, nView, "key.n"_s); - RETURN_IF_EXCEPTION(scope, {}); - auto* eBuf = decodeJwkString(globalObject, scope, eView, "key.e"_s); - RETURN_IF_EXCEPTION(scope, {}); + // The JWK itself decides whether private key material is present. + const bool isPrivate = dValue.isString(); + if (keyType == CryptoKeyType::Private && !isPrivate) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, "JWK does not contain private key material"_s); + return {}; + } MarkPopErrorOnReturn markPopError; + BignumPointer n = jwkStringToBn(globalObject, scope, nValue, "key.n"_s); + RETURN_IF_EXCEPTION(scope, {}); + BignumPointer e = jwkStringToBn(globalObject, scope, eValue, "key.e"_s); + RETURN_IF_EXCEPTION(scope, {}); + RSAPointer rsa(RSA_new()); Rsa rsaView(rsa.get()); - if (!rsaView.setPublicKey(jwkBufToBn(nBuf), jwkBufToBn(eBuf))) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK RSA key"_s); + if (!rsaView.setPublicKey(WTF::move(n), WTF::move(e))) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } - // The JWK itself decides whether private key material is present, as in Node's ImportJWKRsaKey. - if (keyType == CryptoKeyType::Private || !dValue.isUndefined()) { - V::validateString(scope, globalObject, dValue, "key.d"_s); + if (isPrivate) { + JSValue pValue = jwk->get(globalObject, Identifier::fromString(vm, "p"_s)); RETURN_IF_EXCEPTION(scope, {}); - auto dView = asString(dValue)->view(globalObject); + JSValue qValue = jwk->get(globalObject, Identifier::fromString(vm, "q"_s)); RETURN_IF_EXCEPTION(scope, {}); - auto* dBuf = decodeJwkString(globalObject, scope, dView, "key.d"_s); + JSValue dpValue = jwk->get(globalObject, Identifier::fromString(vm, "dp"_s)); + RETURN_IF_EXCEPTION(scope, {}); + JSValue dqValue = jwk->get(globalObject, Identifier::fromString(vm, "dq"_s)); + RETURN_IF_EXCEPTION(scope, {}); + JSValue qiValue = jwk->get(globalObject, Identifier::fromString(vm, "qi"_s)); + RETURN_IF_EXCEPTION(scope, {}); + + if (!pValue.isString() || !qValue.isString() || !dpValue.isString() || !dqValue.isString() || !qiValue.isString()) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); + return {}; + } + + BignumPointer d = jwkStringToBn(globalObject, scope, dValue, "key.d"_s); RETURN_IF_EXCEPTION(scope, {}); - auto* pBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "p"_s, "key.p"_s); + BignumPointer q = jwkStringToBn(globalObject, scope, qValue, "key.q"_s); RETURN_IF_EXCEPTION(scope, {}); - auto* qBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "q"_s, "key.q"_s); + BignumPointer p = jwkStringToBn(globalObject, scope, pValue, "key.p"_s); RETURN_IF_EXCEPTION(scope, {}); - auto* dpBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "dp"_s, "key.dp"_s); + BignumPointer dp = jwkStringToBn(globalObject, scope, dpValue, "key.dp"_s); RETURN_IF_EXCEPTION(scope, {}); - auto* dqBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "dq"_s, "key.dq"_s); + BignumPointer dq = jwkStringToBn(globalObject, scope, dqValue, "key.dq"_s); RETURN_IF_EXCEPTION(scope, {}); - auto* qiBuf = getDecodedJwkStringBuf(globalObject, scope, jwk, "qi"_s, "key.qi"_s); + BignumPointer qi = jwkStringToBn(globalObject, scope, qiValue, "key.qi"_s); RETURN_IF_EXCEPTION(scope, {}); - if (!rsaView.setPrivateKey( - jwkBufToBn(dBuf), - jwkBufToBn(qBuf), - jwkBufToBn(pBuf), - jwkBufToBn(dpBuf), - jwkBufToBn(dqBuf), - jwkBufToBn(qiBuf)) - || !rsaView.checkPrimeProduct()) { - ERR::CRYPTO_INVALID_JWK(scope, globalObject, "Invalid JWK RSA key"_s); + if (!rsaView.setPrivateKey(WTF::move(d), WTF::move(q), WTF::move(p), WTF::move(dp), WTF::move(dq), WTF::move(qi)) || !rsaView.checkPrimeProduct()) { + ERR::CRYPTO_INVALID_JWK(scope, globalObject, invalidKey); return {}; } } diff --git a/test/js/node/crypto/crypto.key-objects.test.ts b/test/js/node/crypto/crypto.key-objects.test.ts index 70f811da805a..f853fb66cf6e 100644 --- a/test/js/node/crypto/crypto.key-objects.test.ts +++ b/test/js/node/crypto/crypto.key-objects.test.ts @@ -14,6 +14,7 @@ import { generateKeyPair, generateKeyPairSync, generateKeySync, + type JsonWebKey, type JsonWebKeyInput, KeyObject, privateDecrypt, @@ -647,24 +648,35 @@ describe("crypto.KeyObjects", () => { ); }); - test("keys from a private RSA jwk need the n that is p times q, and no oth", () => { + test("an RSA jwk needs strings for its members, the n that is p times q, and no oth", () => { const jwk = createPrivateKey(privatePem).export({ format: "jwk" }); const { kty, n, e } = jwk; expect(createPrivateKey({ key: jwk, format: "jwk" }).export({ format: "jwk" })).toEqual(jwk); expect(createPublicKey({ key: jwk, format: "jwk" }).export({ format: "jwk" })).toEqual({ kty, n, e }); - const changed = (encoded?: string) => { - const bytes = Buffer.from(encoded!, "base64url"); - bytes[0] ^= 1; - return bytes.toString("base64url"); - }; const invalid = expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "Invalid JWK RSA key" }); - for (const wrong of [{ n: changed(n) }, { p: changed(jwk.p) }, { q: changed(jwk.q) }, { p: "" }, { oth: [] }]) { + const wrongMembers = [ + { n: jwk.d }, + { p: jwk.q }, + { q: jwk.p }, + { p: "" }, + { oth: [] }, + { n: undefined }, + { e: 5 }, + { d: null }, + { p: undefined }, + { qi: 5 }, + { p: undefined, q: undefined, dp: undefined, dq: undefined, qi: undefined }, + ]; + for (const wrong of wrongMembers) { for (const create of [createPrivateKey, createPublicKey]) { - expect(() => create({ key: { ...jwk, ...wrong }, format: "jwk" })).toThrow(invalid); + expect(() => create({ key: { ...jwk, ...wrong } as JsonWebKey, format: "jwk" })).toThrow(invalid); } } expect(() => createPublicKey({ key: { kty, n, e, oth: [] }, format: "jwk" })).toThrow(invalid); + expect(() => createPrivateKey({ key: { kty, n, e }, format: "jwk" })).toThrow( + expect.objectContaining({ code: "ERR_CRYPTO_INVALID_JWK", message: "JWK does not contain private key material" }), + ); }); test("private encrypted should work", async () => { diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 997eaafbb5b4..253109bd6667 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1702,17 +1702,12 @@ describe("JWK import validation", () => { const { kty, n, e } = jwk; const importJwk = (members: JsonWebKey) => crypto.subtle.importKey("jwk", { ...jwk, ...members }, algorithm, true, ["sign"]); - const changed = (encoded?: string) => { - const bytes = Buffer.from(encoded!, "base64url"); - bytes[0] ^= 1; - return bytes.toString("base64url"); - }; expect({ unchanged: await rejection(importJwk({})), - n: await rejection(importJwk({ n: changed(n) })), - p: await rejection(importJwk({ p: changed(jwk.p) })), - q: await rejection(importJwk({ q: changed(jwk.q) })), + n: await rejection(importJwk({ n: jwk.d })), + p: await rejection(importJwk({ p: jwk.q })), + q: await rejection(importJwk({ q: jwk.p })), oth: await rejection(importJwk({ oth: [] })), publicOth: await rejection(crypto.subtle.importKey("jwk", { kty, n, e, oth: [] }, algorithm, true, ["verify"])), }).toEqual({ From 71b37d3bbea4c18a1f282a25345bc200da9b7161 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:31 +0000 Subject: [PATCH 13/19] node:crypto: the OAEP options under another padding, and Node's names for them With a padding other than OAEP, publicEncrypt() and privateDecrypt() ignore oaepHash, mgf1Hash and oaepLabel, as they do in Node. OpenSSL ignores them there and BoringSSL refuses them. A wrong type is reported as key.oaepHash, key.mgf1Hash or key.oaepLabel, which are Node's names. oaepHash and mgf1Hash are read by one function. Node's own test of mgf1Hash is added as it is, with the two helpers of test/common/crypto.js it needs. Co-authored-by: dylanc --- src/jsc/bindings/node/crypto/JSCipher.cpp | 58 ++++--- test/js/node/crypto/crypto-rsa.test.js | 46 +++--- test/js/node/test/common/crypto.js | 8 + .../parallel/test-crypto-rsa-oaep-mgf1.js | 149 ++++++++++++++++++ 4 files changed, 212 insertions(+), 49 deletions(-) create mode 100644 test/js/node/test/parallel/test-crypto-rsa-oaep-mgf1.js diff --git a/src/jsc/bindings/node/crypto/JSCipher.cpp b/src/jsc/bindings/node/crypto/JSCipher.cpp index f8ecbe6f66e1..2c160a4bed6b 100644 --- a/src/jsc/bindings/node/crypto/JSCipher.cpp +++ b/src/jsc/bindings/node/crypto/JSCipher.cpp @@ -66,6 +66,22 @@ enum class CipherOperation { recover, }; +// Reads an option that names a digest. `digest` stays empty if the option is undefined. +static void getDigestOption(JSGlobalObject* globalObject, ThrowScope& scope, JSObject* options, ASCIILiteral name, ASCIILiteral argName, ncrypto::Digest& digest) +{ + JSValue value = options->get(globalObject, Identifier::fromString(globalObject->vm(), name)); + RETURN_IF_EXCEPTION(scope, ); + if (value.isUndefined()) + return; + V::validateString(scope, globalObject, value, argName); + RETURN_IF_EXCEPTION(scope, ); + GCOwnedDataScope view = asString(value)->view(globalObject); + RETURN_IF_EXCEPTION(scope, ); + digest = ncrypto::Digest::FromName(view); + if (!digest) + ERR::OSSL_EVP_INVALID_DIGEST(scope, globalObject); +} + JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, KeyType keyType, CipherOperation operation, int32_t defaultPadding) { VM& vm = lexicalGlobalObject->vm(); @@ -76,7 +92,7 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca JSValue optionsValue = callFrame->argument(0); JSValue bufferValue = callFrame->argument(1); - // Like Node, validate the cipher options (padding, oaepHash, mgf1Hash, oaepLabel) before any key + // The cipher options (padding, oaepHash, mgf1Hash, oaepLabel) are validated before any key // parsing so an invalid digest is reported even when the key material itself is invalid. ncrypto::Digest digest; ncrypto::Digest mgf1Digest; @@ -91,37 +107,10 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca RETURN_IF_EXCEPTION(scope, {}); } - JSValue oaepHashValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "oaepHash"_s)); + getDigestOption(lexicalGlobalObject, scope, options, "oaepHash"_s, "key.oaepHash"_s, digest); RETURN_IF_EXCEPTION(scope, {}); - if (!oaepHashValue.isUndefined()) { - V::validateString(scope, lexicalGlobalObject, oaepHashValue, "options.oaepHash"_s); - RETURN_IF_EXCEPTION(scope, {}); - JSString* oaepHashString = oaepHashValue.toString(lexicalGlobalObject); - RETURN_IF_EXCEPTION(scope, {}); - GCOwnedDataScope oaepHashView = oaepHashString->view(lexicalGlobalObject); - RETURN_IF_EXCEPTION(scope, {}); - digest = ncrypto::Digest::FromName(oaepHashView); - if (!digest) { - ERR::OSSL_EVP_INVALID_DIGEST(scope, lexicalGlobalObject); - return {}; - } - } - - JSValue mgf1HashValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "mgf1Hash"_s)); + getDigestOption(lexicalGlobalObject, scope, options, "mgf1Hash"_s, "key.mgf1Hash"_s, mgf1Digest); RETURN_IF_EXCEPTION(scope, {}); - if (!mgf1HashValue.isUndefined()) { - V::validateString(scope, lexicalGlobalObject, mgf1HashValue, "options.mgf1Hash"_s); - RETURN_IF_EXCEPTION(scope, {}); - JSString* mgf1HashString = mgf1HashValue.toString(lexicalGlobalObject); - RETURN_IF_EXCEPTION(scope, {}); - GCOwnedDataScope mgf1HashView = mgf1HashString->view(lexicalGlobalObject); - RETURN_IF_EXCEPTION(scope, {}); - mgf1Digest = ncrypto::Digest::FromName(mgf1HashView); - if (!mgf1Digest) { - ERR::OSSL_EVP_INVALID_DIGEST(scope, lexicalGlobalObject); - return {}; - } - } encodingValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "encoding"_s)); RETURN_IF_EXCEPTION(scope, {}); @@ -131,7 +120,7 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca if (!oaepLabelValue.isUndefined()) { // Key parsing below runs user JS (option getters, warning events) that could // detach a borrowed label buffer, so copy the bytes instead of holding a view. - auto view = getArrayBufferOrView2(lexicalGlobalObject, scope, oaepLabelValue, "options.oaepLabel"_s, encodingValue); + auto view = getArrayBufferOrView2(lexicalGlobalObject, scope, oaepLabelValue, "key.oaepLabel"_s, encodingValue); RETURN_IF_EXCEPTION(scope, {}); oaepLabel = copyArgumentBytes(lexicalGlobalObject, scope, std::span { view->data(), view->size() }, "oaepLabel"_s); if (!oaepLabel) return {}; @@ -201,6 +190,13 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca return {}; } + // Under another padding, OpenSSL's encryption and decryption ignore the OAEP options. BoringSSL refuses them. + if (padding != RSA_PKCS1_OAEP_PADDING && (operation == CipherOperation::encrypt || operation == CipherOperation::decrypt)) { + digest = nullptr; + mgf1Digest = nullptr; + oaepLabel.reset(); + } + ncrypto::Buffer labelBuf = {}; if (oaepLabel) { labelBuf = { diff --git a/test/js/node/crypto/crypto-rsa.test.js b/test/js/node/crypto/crypto-rsa.test.js index 45b9842cbedc..818dd047172c 100644 --- a/test/js/node/crypto/crypto-rsa.test.js +++ b/test/js/node/crypto/crypto-rsa.test.js @@ -323,23 +323,6 @@ test(`RSA with hash mismatch`, () => { test("RSA-OAEP with an mgf1Hash other than oaepHash", () => { const input = Buffer.from("Hello Node.js"); const decodingError = expect.objectContaining({ code: "ERR_OSSL_RSA_OAEP_DECODING_ERROR" }); - const roundTrip = (encrypt, decrypt = encrypt) => - crypto.privateDecrypt({ key: rsaKeyPem, ...decrypt }, crypto.publicEncrypt({ key: rsaPubPem, ...encrypt }, input)); - - for (const [oaepHash, mgf1Hash] of [ - ["sha256", "sha1"], - ["sha512", "sha1"], - ["sha384", "sha256"], - ["sha1", "sha256"], - ]) { - expect(roundTrip({ oaepHash, mgf1Hash })).toEqual(input); - } - - // Without mgf1Hash, MGF1 uses oaepHash, which is sha1 by default. - expect(roundTrip({ oaepHash: "sha256" }, { oaepHash: "sha256", mgf1Hash: "sha256" })).toEqual(input); - expect(roundTrip({}, { mgf1Hash: "sha1" })).toEqual(input); - expect(() => roundTrip({ oaepHash: "sha256", mgf1Hash: "sha1" }, { oaepHash: "sha256" })).toThrow(decodingError); - expect(() => roundTrip({ oaepHash: "sha256" }, { oaepHash: "sha256", mgf1Hash: "sha1" })).toThrow(decodingError); // publicEncrypt({ key: rsaPubPem, oaepHash: "sha256", mgf1Hash: "sha1" }, input) in Node.js v26.12.1. const fromNode = Buffer.from( @@ -355,6 +338,30 @@ test("RSA-OAEP with an mgf1Hash other than oaepHash", () => { ); }); +test("the OAEP options under another padding", () => { + const options = { oaepHash: "sha256", mgf1Hash: "sha256", oaepLabel: Buffer.from("label") }; + const block = Buffer.alloc(rsaKeySize / 8, 1); + for (const [name, value] of Object.entries(options)) { + // Encryption and decryption ignore them. + const noPadding = { padding: constants.RSA_NO_PADDING, [name]: value }; + const encrypted = crypto.publicEncrypt({ key: rsaPubPem, ...noPadding }, block); + expect(crypto.privateDecrypt({ key: rsaKeyPem, ...noPadding }, encrypted)).toEqual(block); + expect( + crypto.publicEncrypt( + { key: rsaPubPem, padding: constants.RSA_PKCS1_PADDING, [name]: value }, + block.subarray(0, 32), + ), + ).toHaveLength(block.length); + + expect(() => crypto.privateEncrypt({ key: rsaKeyPem, [name]: value }, block.subarray(0, 32))).toThrow( + expect.objectContaining({ code: expect.stringMatching(/^ERR_OSSL_/) }), + ); + expect(() => crypto.publicDecrypt({ key: rsaPubPem, [name]: value }, block)).toThrow( + expect.objectContaining({ code: expect.stringMatching(/^ERR_OSSL_/) }), + ); + } +}); + test("RSA-OAEP test vectors", () => { const { decryptionTests } = JSON.parse(fixtures.readSync("rsa-oaep-test-vectors.js", "utf8")); @@ -431,7 +438,10 @@ describe("Invalid oaepHash, mgf1Hash and oaepLabel options", () => { ); for (const mgf1Hash of [0, false, null, Symbol(), () => {}]) { expect(() => fn({ key, mgf1Hash }, Buffer.alloc(10))).toThrow( - expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" }), + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: expect.stringContaining('The "key.mgf1Hash" property must be of type string.'), + }), ); } }); diff --git a/test/js/node/test/common/crypto.js b/test/js/node/test/common/crypto.js index 988878b374d9..d23075135189 100644 --- a/test/js/node/test/common/crypto.js +++ b/test/js/node/test/common/crypto.js @@ -118,6 +118,12 @@ const hasOpenSSL = (major = 0, minor = 0, patch = 0) => { return OPENSSL_VERSION_NUMBER >= opensslVersionNumber(major, minor, patch); }; +const isBoringSSL = process.features.openssl_is_boringssl; + +const hasFIPS = (major = 0, minor = 0, patch = 0) => { + return crypto.getFips() === 1 && hasOpenSSL(major, minor, patch); +}; + let opensslCli = null; module.exports = { @@ -134,6 +140,8 @@ module.exports = { sec1Exp, sec1EncExp, hasOpenSSL, + hasFIPS, + isBoringSSL, get hasOpenSSL3() { return hasOpenSSL(3); }, diff --git a/test/js/node/test/parallel/test-crypto-rsa-oaep-mgf1.js b/test/js/node/test/parallel/test-crypto-rsa-oaep-mgf1.js new file mode 100644 index 000000000000..7ba7f19f6b5c --- /dev/null +++ b/test/js/node/test/parallel/test-crypto-rsa-oaep-mgf1.js @@ -0,0 +1,149 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Tests the `mgf1Hash` option of crypto.publicEncrypt() and +// crypto.privateDecrypt(), which allows the MGF1 digest of RSA-OAEP padding to +// differ from the OAEP message digest (`oaepHash`). This is required for +// interoperability with profiles such as XML Encryption's `rsa-oaep-mgf1p`, +// where the OAEP digest may be changed but MGF1 is fixed to SHA-1. + +const assert = require('assert'); +const crypto = require('crypto'); +const fixtures = require('../common/fixtures'); +const { hasFIPS } = require('../common/crypto'); + +const constants = crypto.constants; + +const publicKey = fixtures.readKey('rsa_public.pem', 'ascii'); +const privateKey = fixtures.readKey('rsa_private.pem', 'ascii'); + +const input = Buffer.from('the quick brown fox jumps over the lazy dog'); + +// A round-trip with mismatched OAEP and MGF1 digests must succeed when both +// sides agree on the digests. +{ + const encrypted = crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash: 'sha1', + }, input); + + const decrypted = crypto.privateDecrypt({ + key: privateKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash: 'sha1', + }, encrypted); + + assert.deepStrictEqual(decrypted, input); +} + +// mgf1Hash actually affects the padding: a ciphertext produced with +// oaepHash=sha256 and mgf1Hash=sha1 must NOT decrypt when MGF1 defaults to the +// OAEP digest (sha256), which is the pre-existing behavior. +{ + const encrypted = crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash: 'sha1', + }, input); + + assert.throws(() => { + crypto.privateDecrypt({ + key: privateKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + // No mgf1Hash: MGF1 follows oaepHash (sha256) and must fail to unpad. + }, encrypted); + }, { + code: hasFIPS(3, 5) ? 'ERR_OSSL_EVP_PROVIDER_ASYM_CIPHER_FAILURE' : + 'ERR_OSSL_RSA_OAEP_DECODING_ERROR' + }); +} + +// Backward compatibility: omitting mgf1Hash on both sides keeps MGF1 == oaepHash +// (the historical behavior), so this round-trips, and setting mgf1Hash equal to +// oaepHash is equivalent to omitting it. +{ + const encrypted = crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + }, input); + + const decrypted = crypto.privateDecrypt({ + key: privateKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash: 'sha256', + }, encrypted); + + assert.deepStrictEqual(decrypted, input); +} + +// The default oaepHash is sha1, so mgf1Hash defaults to sha1 as well. A +// ciphertext encrypted with all defaults must decrypt with an explicit +// mgf1Hash: 'sha1'. +{ + const encrypted = crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + }, input); + + const decrypted = crypto.privateDecrypt({ + key: privateKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + mgf1Hash: 'sha1', + }, encrypted); + + assert.deepStrictEqual(decrypted, input); +} + +// A few other digest combinations round-trip. +for (const [oaepHash, mgf1Hash] of [ + ['sha512', 'sha1'], + ['sha384', 'sha256'], + ['sha1', 'sha256'], +]) { + const encrypted = crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash, + mgf1Hash, + }, input); + + const decrypted = crypto.privateDecrypt({ + key: privateKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash, + mgf1Hash, + }, encrypted); + + assert.deepStrictEqual(decrypted, input); +} + +// mgf1Hash must be a string. +for (const mgf1Hash of [1, true, {}, [], null]) { + assert.throws(() => { + crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash, + }, input); + }, { code: 'ERR_INVALID_ARG_TYPE' }); +} + +// An unknown mgf1Hash digest name is rejected. +assert.throws(() => { + crypto.publicEncrypt({ + key: publicKey, + padding: constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: 'sha256', + mgf1Hash: 'not-a-real-digest', + }, input); +}, { code: 'ERR_OSSL_EVP_INVALID_DIGEST' }); From ac818f019456da7f51c5a297f083cc14cf1a8321 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:35 +0000 Subject: [PATCH 14/19] crypto.getRandomValues: ERR_MISSING_ARGS with no argument Node tells no argument from undefined. body.test.ts filled a view on a SharedArrayBuffer with getRandomValues(), which now refuses one, so its helper copies from an array of its own. Co-authored-by: dylanc --- src/js/node/wasi.ts | 2 +- src/jsc/rare_data.rs | 2 +- src/runtime/webcore/Crypto.bind.ts | 4 ++-- src/runtime/webcore/Crypto.rs | 30 ++++++++++++++++----------- test/js/web/crypto/web-crypto.test.ts | 15 ++++++++++++-- test/js/web/fetch/body.test.ts | 6 +++--- test/js/web/web-globals.test.js | 2 +- 7 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/js/node/wasi.ts b/src/js/node/wasi.ts index 51670f6a210b..8ee33b770fb9 100644 --- a/src/js/node/wasi.ts +++ b/src/js/node/wasi.ts @@ -1663,7 +1663,7 @@ class WASI { }, random_get: (bufPtr: number, bufLen: number) => { this.refreshMemory(); - // getRandomValues refuses a view on shared memory, so it fills an array of its own. + // getRandomValues refuses a view on shared memory, so fill a fresh array and copy it in. new Uint8Array(this.memory.buffer, bufPtr, bufLen).set(crypto.getRandomValues(new Uint8Array(bufLen))); return WASI_ESUCCESS; }, diff --git a/src/jsc/rare_data.rs b/src/jsc/rare_data.rs index 44fac1d2cc04..f103a4d4a7b0 100644 --- a/src/jsc/rare_data.rs +++ b/src/jsc/rare_data.rs @@ -127,7 +127,7 @@ impl Default for EntropyCache { } impl EntropyCache { pub(crate) const BUFFERED_UUIDS_COUNT: usize = 16; - pub const SIZE: usize = Self::BUFFERED_UUIDS_COUNT * 128; + pub(crate) const SIZE: usize = Self::BUFFERED_UUIDS_COUNT * 128; pub(crate) fn fill(&mut self) { bun_boringssl::rand_bytes(&mut self.cache); diff --git a/src/runtime/webcore/Crypto.bind.ts b/src/runtime/webcore/Crypto.bind.ts index 2acca475b9a2..d21480299420 100644 --- a/src/runtime/webcore/Crypto.bind.ts +++ b/src/runtime/webcore/Crypto.bind.ts @@ -58,8 +58,8 @@ export const Crypto = b.class({ rustPath: "crate::webcore::crypto::Crypto", construct: "illegal", proto: { - /** What is not an integer typed array throws WebCrypto's `TypeMismatchError`, which no type here throws. */ - getRandomValues: { args: { array: b.any }, returns: b.any, capability: "js", throws: true }, + /** A view that is not an integer typed array throws WebCrypto's `TypeMismatchError`, which no type here throws. */ + getRandomValues: { args: { array: b.any.ifPassed }, returns: b.any, capability: "js", throws: true, length: 1 }, randomUUID: { returns: b.string }, timingSafeEqual: equal, }, diff --git a/src/runtime/webcore/Crypto.rs b/src/runtime/webcore/Crypto.rs index 77ecd6a8f665..53bc4b96535b 100644 --- a/src/runtime/webcore/Crypto.rs +++ b/src/runtime/webcore/Crypto.rs @@ -13,15 +13,25 @@ impl Crypto { pub(crate) fn get_random_values<'a>( &self, cx: &mut JsScope<'a>, - array: JsLocal<'a>, + array: Option>, ) -> bindgen::Result<'a, JsLocal<'a>> { let global = cx.global(); - let js_type = array.to_js().js_type(); + let Some(array) = array else { + return Err(global + .err( + bun_jsc::ErrorCode::MISSING_ARGS, + format_args!( + "Failed to execute 'getRandomValues' on 'Crypto': 1 argument required, but only 0 present." + ), + ) + .throw() + .into()); + }; // Web IDL's conversion to an ArrayBufferView that is neither [AllowShared] nor [AllowResizable]. let view = array .to_js() .as_array_buffer(global) - .filter(|_| js_type != JSType::ArrayBuffer); + .filter(|view| view.typed_array_type != JSType::ArrayBuffer); let refused = match &view { None => Some("is not an ArrayBufferView"), Some(view) if view.shared => { @@ -34,20 +44,16 @@ impl Crypto { }; if let Some(refused) = refused { return Err(global - .err( - bun_jsc::ErrorCode::INVALID_ARG_TYPE, - format_args!( - "Failed to execute 'getRandomValues' on 'Crypto': 1st argument {refused}." - ), - ) - .throw() + .throw_invalid_arguments(format_args!( + "Failed to execute 'getRandomValues' on 'Crypto': 1st argument {refused}." + )) .into()); } // https://w3c.github.io/webcrypto/#Crypto-method-getRandomValues accepts only integer-typed views. - let array_buffer = view.filter(|_| { + let array_buffer = view.filter(|view| { matches!( - js_type, + view.typed_array_type, JSType::Int8Array | JSType::Uint8Array | JSType::Uint8ClampedArray diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 253109bd6667..3adc4e73bdf7 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1506,8 +1506,8 @@ describe("CryptoKey.usages ordering", () => { }); }); -// getRandomValues takes integer-typed views only; every other ArrayBufferView-ish -// argument raises TypeMismatchError rather than being filled. +// getRandomValues takes integer-typed views only; every other ArrayBufferView +// raises TypeMismatchError rather than being filled. describe("crypto.getRandomValues argument types", () => { const integerViews = [ ["Int8Array", () => new Int8Array(4)], @@ -1568,6 +1568,17 @@ describe("crypto.getRandomValues argument types", () => { ], ] as const; + it("needs an argument", () => { + // @ts-expect-error + expect(() => crypto.getRandomValues()).toThrow( + expect.objectContaining({ + name: "TypeError", + code: "ERR_MISSING_ARGS", + message: "Failed to execute 'getRandomValues' on 'Crypto': 1 argument required, but only 0 present.", + }), + ); + }); + for (const [name, make, why] of notConverted) { it(`rejects ${name} with TypeError`, () => { expect(() => crypto.getRandomValues(make() as any)).toThrow( diff --git a/test/js/web/fetch/body.test.ts b/test/js/web/fetch/body.test.ts index f859a062efd4..34a4f79a5471 100644 --- a/test/js/web/fetch/body.test.ts +++ b/test/js/web/fetch/body.test.ts @@ -41,15 +41,15 @@ const bufferTypes = [ Float64Array, ]; -// getRandomValues takes integer-typed views only, so fill through a Uint8Array -// view to cover the ArrayBuffer/SharedArrayBuffer/Float* cases too. +// getRandomValues takes integer-typed views on unshared memory only, so copy from a Uint8Array +// to cover the ArrayBuffer/SharedArrayBuffer/Float* cases too. function randomFilled(bufferType: (typeof bufferTypes)[number], length: number) { const buffer = new (bufferType as any)(length); const bytes = buffer instanceof ArrayBuffer || buffer instanceof SharedArrayBuffer ? new Uint8Array(buffer) : new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength); - crypto.getRandomValues(bytes); + bytes.set(crypto.getRandomValues(new Uint8Array(bytes.length))); return buffer; } diff --git a/test/js/web/web-globals.test.js b/test/js/web/web-globals.test.js index d83ba557f3f4..296c7125971a 100644 --- a/test/js/web/web-globals.test.js +++ b/test/js/web/web-globals.test.js @@ -200,7 +200,7 @@ it("crypto.getRandomValues", () => { expect(!!crypto.getRandomValues(new Uint8Array(8192)).find(a => a > 0)).toBe(true); { - // any additional input into getRandomValues() makes it unbuffered + // additional arguments are ignored var array = crypto.getRandomValues(foo, "unbuffered"); expect(array).toBe(foo); expect(array.reduce((sum, a) => (sum += a === 0), 0) != foo.length).toBe(true); From 6ba4781cf978b48fabd705dc0267d709ddbf97b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:36:36 +0000 Subject: [PATCH 15/19] webcrypto: [EnforceRange] in SubtleCrypto.idl, and getCipherInfo's whole message in its test Co-authored-by: dylanc --- src/jsc/bindings/webcrypto/SubtleCrypto.idl | 2 +- test/js/node/crypto/node-crypto.test.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/webcrypto/SubtleCrypto.idl b/src/jsc/bindings/webcrypto/SubtleCrypto.idl index 029f3b940300..160351dcb9ee 100644 --- a/src/jsc/bindings/webcrypto/SubtleCrypto.idl +++ b/src/jsc/bindings/webcrypto/SubtleCrypto.idl @@ -40,7 +40,7 @@ typedef (object or DOMString) AlgorithmIdentifier; [CallWith=CurrentGlobalObject] Promise digest(AlgorithmIdentifier algorithm, BufferSource data); [CallWith=CurrentGlobalObject] Promise generateKey(AlgorithmIdentifier algorithm, boolean extractable, sequence keyUsages); [CallWith=CurrentGlobalObject] Promise deriveKey(AlgorithmIdentifier algorithm, CryptoKey baseKey, AlgorithmIdentifier derivedKeyType, boolean extractable, sequence keyUsages); - [CallWith=CurrentGlobalObject] Promise deriveBits(AlgorithmIdentifier algorithm, CryptoKey baseKey, optional unsigned long? length = null); + [CallWith=CurrentGlobalObject] Promise deriveBits(AlgorithmIdentifier algorithm, CryptoKey baseKey, optional [EnforceRange] unsigned long? length = null); [CallWith=CurrentGlobalObject] Promise importKey(KeyFormat format, (BufferSource or JsonWebKey) keyData, AlgorithmIdentifier algorithm, boolean extractable, sequence keyUsages); Promise exportKey(KeyFormat format, CryptoKey key); [CallWith=CurrentGlobalObject] Promise wrapKey(KeyFormat format, CryptoKey key, CryptoKey wrappingKey, AlgorithmIdentifier wrapAlgorithm); diff --git a/test/js/node/crypto/node-crypto.test.js b/test/js/node/crypto/node-crypto.test.js index a8547bffb91d..d5eae9b6b6bd 100644 --- a/test/js/node/crypto/node-crypto.test.js +++ b/test/js/node/crypto/node-crypto.test.js @@ -914,12 +914,13 @@ it("getCipherInfo() takes keyLength and ivLength as unsigned 32-bit integers", ( expect(() => crypto.getCipherInfo("aes-128-gcm", { [option]: length })).toThrow( expect.objectContaining({ code: "ERR_OUT_OF_RANGE", - message: expect.stringContaining(`"options.${option}" is out of range. It must be >= 0 && <= 4294967295.`), + message: `The value of "options.${option}" is out of range. It must be >= 0 && <= 4294967295. Received ${length}`, }), ); } } expect(crypto.getCipherInfo("aes-128-gcm", { ivLength: 2 ** 32 - 1 }).ivLength).toBe(2 ** 32 - 1); + expect(crypto.getCipherInfo("aes-128-gcm", { keyLength: 2 ** 32 - 1 })).toBeUndefined(); }); describe("ECDH", () => { From dc54501a78c1320d52c60c3a536d9268093a6f40 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:51:52 +0000 Subject: [PATCH 16/19] node:wasi: random_get fills 64 KiB at a time The array it fills and copies from was as long as the guest's request. It is now at most 64 KiB and is used as often as it takes. Co-authored-by: dylanc --- src/js/node/wasi.ts | 8 ++++++-- test/js/bun/wasm/wasi.test.js | 18 +++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/js/node/wasi.ts b/src/js/node/wasi.ts index 8ee33b770fb9..28baf2a2538b 100644 --- a/src/js/node/wasi.ts +++ b/src/js/node/wasi.ts @@ -1663,8 +1663,12 @@ class WASI { }, random_get: (bufPtr: number, bufLen: number) => { this.refreshMemory(); - // getRandomValues refuses a view on shared memory, so fill a fresh array and copy it in. - new Uint8Array(this.memory.buffer, bufPtr, bufLen).set(crypto.getRandomValues(new Uint8Array(bufLen))); + // getRandomValues refuses a view on shared memory, so fill an array of our own and copy it in. + const dest = new Uint8Array(this.memory.buffer, bufPtr, bufLen); + const chunk = new Uint8Array(Math.min(dest.length, 65536)); + for (let offset = 0; offset < dest.length; offset += chunk.length) { + dest.set(crypto.getRandomValues(chunk.subarray(0, dest.length - offset)), offset); + } return WASI_ESUCCESS; }, sched_yield() { diff --git a/test/js/bun/wasm/wasi.test.js b/test/js/bun/wasm/wasi.test.js index ebabec8f838f..9c2274b20442 100644 --- a/test/js/bun/wasm/wasi.test.js +++ b/test/js/bun/wasm/wasi.test.js @@ -48,17 +48,17 @@ it("fd_fdstat_set_rights only narrows the rights of a descriptor", () => { }); it.each([ - ["memory", () => new WebAssembly.Memory({ initial: 1 })], - ["shared memory", () => new WebAssembly.Memory({ initial: 1, maximum: 1, shared: true })], + ["memory", () => new WebAssembly.Memory({ initial: 1 }), 16], + ["shared memory", () => new WebAssembly.Memory({ initial: 1, maximum: 1, shared: true }), 16], // Not every build of JavaScriptCore backs shared memory with a SharedArrayBuffer. - ["a SharedArrayBuffer", () => ({ buffer: new SharedArrayBuffer(65536) })], -])("random_get fills only the requested window of %s", (_, memory) => { + ["a SharedArrayBuffer", () => ({ buffer: new SharedArrayBuffer(65536) }), 16], + ["memory, 64 KiB at a time", () => new WebAssembly.Memory({ initial: 3 }), 2 * 65536 + 16], +])("random_get fills only the requested window of %s", (_, memory, bufLen) => { const wasi = new WASI({}); wasi.setMemory(memory()); const WASI_ESUCCESS = 0; const bufPtr = 1024; - const bufLen = 16; const before = new Uint8Array(wasi.memory.buffer.slice(0)); expect(wasi.wasiImport.random_get(bufPtr, bufLen)).toBe(WASI_ESUCCESS); @@ -73,8 +73,12 @@ it.each([ } expect(changedOutside).toBe(0); - // ...and the window itself is filled (all-zero is a 1-in-2^128 false failure). - expect(after.subarray(bufPtr, bufPtr + bufLen).some(b => b !== 0)).toBe(true); + // ...and the window itself is filled (16 zeros in a row are a 1-in-2^128 false failure). + let zeroRuns = 0; + for (let i = bufPtr; i < bufPtr + bufLen; i += 16) { + if (after.subarray(i, i + 16).every(b => b === 0)) zeroRuns++; + } + expect(zeroRuns).toBe(0); }); it("path_open reports the host errno to the guest when the open fails", () => { From e04456fa4ef2980f2d0d8c5c2fd369d24b17b757 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 02:41:15 +0000 Subject: [PATCH 17/19] crypto.getRandomValues: count the arguments with bindgen's requiredArguments The schema used .ifPassed and the body threw ERR_MISSING_ARGS itself. .ifPassed is for a function where f() and f(undefined) both work, and requiredArguments is for one where f() is an error. The code is the same. The message is bindgen's: The "array" argument must be specified. Co-authored-by: dylanc --- src/runtime/webcore/Crypto.bind.ts | 2 +- src/runtime/webcore/Crypto.rs | 13 +------------ test/js/web/crypto/web-crypto.test.ts | 2 +- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/runtime/webcore/Crypto.bind.ts b/src/runtime/webcore/Crypto.bind.ts index d21480299420..3e69b4c9b0b3 100644 --- a/src/runtime/webcore/Crypto.bind.ts +++ b/src/runtime/webcore/Crypto.bind.ts @@ -59,7 +59,7 @@ export const Crypto = b.class({ construct: "illegal", proto: { /** A view that is not an integer typed array throws WebCrypto's `TypeMismatchError`, which no type here throws. */ - getRandomValues: { args: { array: b.any.ifPassed }, returns: b.any, capability: "js", throws: true, length: 1 }, + getRandomValues: { args: { array: b.any }, returns: b.any, capability: "js", throws: true, requiredArguments: 1 }, randomUUID: { returns: b.string }, timingSafeEqual: equal, }, diff --git a/src/runtime/webcore/Crypto.rs b/src/runtime/webcore/Crypto.rs index 53bc4b96535b..de23844d8b17 100644 --- a/src/runtime/webcore/Crypto.rs +++ b/src/runtime/webcore/Crypto.rs @@ -13,20 +13,9 @@ impl Crypto { pub(crate) fn get_random_values<'a>( &self, cx: &mut JsScope<'a>, - array: Option>, + array: JsLocal<'a>, ) -> bindgen::Result<'a, JsLocal<'a>> { let global = cx.global(); - let Some(array) = array else { - return Err(global - .err( - bun_jsc::ErrorCode::MISSING_ARGS, - format_args!( - "Failed to execute 'getRandomValues' on 'Crypto': 1 argument required, but only 0 present." - ), - ) - .throw() - .into()); - }; // Web IDL's conversion to an ArrayBufferView that is neither [AllowShared] nor [AllowResizable]. let view = array .to_js() diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 3adc4e73bdf7..719aa787ba6b 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1574,7 +1574,7 @@ describe("crypto.getRandomValues argument types", () => { expect.objectContaining({ name: "TypeError", code: "ERR_MISSING_ARGS", - message: "Failed to execute 'getRandomValues' on 'Crypto': 1 argument required, but only 0 present.", + message: 'The "array" argument must be specified', }), ); }); From f2af886520e9b1869a1d3a5e8ccfba132db1e324 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:14:51 +0000 Subject: [PATCH 18/19] crypto.getRandomValues: keep filling a view on shared or resizable memory The specification refuses one, and so does Node since v26.11.0, but the call is synchronous and randomFillSync() fills one in both. It worked before, so it still does. node:wasi's random_get, its test and the helper of body.test.ts go back to what they were. Co-authored-by: dylanc --- src/js/node/wasi.ts | 10 ++++------ src/runtime/webcore/Crypto.rs | 16 +++------------ test/js/bun/wasm/wasi.test.js | 19 +++++------------- test/js/web/crypto/web-crypto.test.ts | 28 ++++++++------------------- test/js/web/fetch/body.test.ts | 6 +++--- 5 files changed, 23 insertions(+), 56 deletions(-) diff --git a/src/js/node/wasi.ts b/src/js/node/wasi.ts index 28baf2a2538b..bd2cab39e6ab 100644 --- a/src/js/node/wasi.ts +++ b/src/js/node/wasi.ts @@ -1663,12 +1663,10 @@ class WASI { }, random_get: (bufPtr: number, bufLen: number) => { this.refreshMemory(); - // getRandomValues refuses a view on shared memory, so fill an array of our own and copy it in. - const dest = new Uint8Array(this.memory.buffer, bufPtr, bufLen); - const chunk = new Uint8Array(Math.min(dest.length, 65536)); - for (let offset = 0; offset < dest.length; offset += chunk.length) { - dest.set(crypto.getRandomValues(chunk.subarray(0, dest.length - offset)), offset); - } + // getRandomValues takes one integer-typed view and ignores any further + // arguments, so a bare `buffer, bufPtr, bufLen` randomized all of linear + // memory rather than the requested window. + crypto.getRandomValues(new Uint8Array(this.memory.buffer, bufPtr, bufLen)); return WASI_ESUCCESS; }, sched_yield() { diff --git a/src/runtime/webcore/Crypto.rs b/src/runtime/webcore/Crypto.rs index de23844d8b17..a38fd62a87af 100644 --- a/src/runtime/webcore/Crypto.rs +++ b/src/runtime/webcore/Crypto.rs @@ -16,25 +16,15 @@ impl Crypto { array: JsLocal<'a>, ) -> bindgen::Result<'a, JsLocal<'a>> { let global = cx.global(); - // Web IDL's conversion to an ArrayBufferView that is neither [AllowShared] nor [AllowResizable]. + // Web IDL's conversion to an ArrayBufferView, except that a view on shared or resizable memory is taken. let view = array .to_js() .as_array_buffer(global) .filter(|view| view.typed_array_type != JSType::ArrayBuffer); - let refused = match &view { - None => Some("is not an ArrayBufferView"), - Some(view) if view.shared => { - Some("is a view on a SharedArrayBuffer, which is not allowed") - } - Some(view) if view.resizable => { - Some("is backed by a resizable ArrayBuffer, which is not allowed") - } - Some(_) => None, - }; - if let Some(refused) = refused { + if view.is_none() { return Err(global .throw_invalid_arguments(format_args!( - "Failed to execute 'getRandomValues' on 'Crypto': 1st argument {refused}." + "Failed to execute 'getRandomValues' on 'Crypto': 1st argument is not an ArrayBufferView." )) .into()); } diff --git a/test/js/bun/wasm/wasi.test.js b/test/js/bun/wasm/wasi.test.js index 9c2274b20442..8d1bd1c02bab 100644 --- a/test/js/bun/wasm/wasi.test.js +++ b/test/js/bun/wasm/wasi.test.js @@ -47,18 +47,13 @@ it("fd_fdstat_set_rights only narrows the rights of a descriptor", () => { expect(wasi.FD_MAP.get(0).rights).toEqual({ base: WASI_RIGHT_FD_READ, inheriting: BigInt(0) }); }); -it.each([ - ["memory", () => new WebAssembly.Memory({ initial: 1 }), 16], - ["shared memory", () => new WebAssembly.Memory({ initial: 1, maximum: 1, shared: true }), 16], - // Not every build of JavaScriptCore backs shared memory with a SharedArrayBuffer. - ["a SharedArrayBuffer", () => ({ buffer: new SharedArrayBuffer(65536) }), 16], - ["memory, 64 KiB at a time", () => new WebAssembly.Memory({ initial: 3 }), 2 * 65536 + 16], -])("random_get fills only the requested window of %s", (_, memory, bufLen) => { +it("random_get fills only the requested window", () => { const wasi = new WASI({}); - wasi.setMemory(memory()); + wasi.setMemory(new WebAssembly.Memory({ initial: 1 })); const WASI_ESUCCESS = 0; const bufPtr = 1024; + const bufLen = 16; const before = new Uint8Array(wasi.memory.buffer.slice(0)); expect(wasi.wasiImport.random_get(bufPtr, bufLen)).toBe(WASI_ESUCCESS); @@ -73,12 +68,8 @@ it.each([ } expect(changedOutside).toBe(0); - // ...and the window itself is filled (16 zeros in a row are a 1-in-2^128 false failure). - let zeroRuns = 0; - for (let i = bufPtr; i < bufPtr + bufLen; i += 16) { - if (after.subarray(i, i + 16).every(b => b === 0)) zeroRuns++; - } - expect(zeroRuns).toBe(0); + // ...and the window itself is filled (all-zero is a 1-in-2^128 false failure). + expect(after.subarray(bufPtr, bufPtr + bufLen).some(b => b !== 0)).toBe(true); }); it("path_open reports the host errno to the guest when the open fails", () => { diff --git a/test/js/web/crypto/web-crypto.test.ts b/test/js/web/crypto/web-crypto.test.ts index 719aa787ba6b..c469528f324f 100644 --- a/test/js/web/crypto/web-crypto.test.ts +++ b/test/js/web/crypto/web-crypto.test.ts @@ -1519,6 +1519,9 @@ describe("crypto.getRandomValues argument types", () => { ["Uint32Array", () => new Uint32Array(4)], ["BigInt64Array", () => new BigInt64Array(4)], ["BigUint64Array", () => new BigUint64Array(4)], + // Node and the specification refuse these two. + ["Uint8Array on a SharedArrayBuffer", () => new Uint8Array(new SharedArrayBuffer(4))], + ["Uint8Array on a resizable ArrayBuffer", () => new Uint8Array(new ArrayBuffer(4, { maxByteLength: 8 }))], ] as const; for (const [name, make] of integerViews) { @@ -1548,24 +1551,9 @@ describe("crypto.getRandomValues argument types", () => { // Web IDL's conversion of the argument to an ArrayBufferView comes first. const notConverted = [ - ["ArrayBuffer", () => new ArrayBuffer(4), "is not an ArrayBufferView"], - ["SharedArrayBuffer", () => new SharedArrayBuffer(4), "is not an ArrayBufferView"], - ["plain object", () => ({}), "is not an ArrayBufferView"], - [ - "view on a SharedArrayBuffer", - () => new Uint8Array(new SharedArrayBuffer(4)), - "is a view on a SharedArrayBuffer, which is not allowed", - ], - [ - "view on a resizable ArrayBuffer", - () => new Uint8Array(new ArrayBuffer(4, { maxByteLength: 8 })), - "is backed by a resizable ArrayBuffer, which is not allowed", - ], - [ - "Float32Array on a SharedArrayBuffer", - () => new Float32Array(new SharedArrayBuffer(4)), - "is a view on a SharedArrayBuffer, which is not allowed", - ], + ["ArrayBuffer", () => new ArrayBuffer(4)], + ["SharedArrayBuffer", () => new SharedArrayBuffer(4)], + ["plain object", () => ({})], ] as const; it("needs an argument", () => { @@ -1579,13 +1567,13 @@ describe("crypto.getRandomValues argument types", () => { ); }); - for (const [name, make, why] of notConverted) { + for (const [name, make] of notConverted) { it(`rejects ${name} with TypeError`, () => { expect(() => crypto.getRandomValues(make() as any)).toThrow( expect.objectContaining({ name: "TypeError", code: "ERR_INVALID_ARG_TYPE", - message: `Failed to execute 'getRandomValues' on 'Crypto': 1st argument ${why}.`, + message: "Failed to execute 'getRandomValues' on 'Crypto': 1st argument is not an ArrayBufferView.", }), ); }); diff --git a/test/js/web/fetch/body.test.ts b/test/js/web/fetch/body.test.ts index 34a4f79a5471..f859a062efd4 100644 --- a/test/js/web/fetch/body.test.ts +++ b/test/js/web/fetch/body.test.ts @@ -41,15 +41,15 @@ const bufferTypes = [ Float64Array, ]; -// getRandomValues takes integer-typed views on unshared memory only, so copy from a Uint8Array -// to cover the ArrayBuffer/SharedArrayBuffer/Float* cases too. +// getRandomValues takes integer-typed views only, so fill through a Uint8Array +// view to cover the ArrayBuffer/SharedArrayBuffer/Float* cases too. function randomFilled(bufferType: (typeof bufferTypes)[number], length: number) { const buffer = new (bufferType as any)(length); const bytes = buffer instanceof ArrayBuffer || buffer instanceof SharedArrayBuffer ? new Uint8Array(buffer) : new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength); - bytes.set(crypto.getRandomValues(new Uint8Array(bytes.length))); + crypto.getRandomValues(bytes); return buffer; } From c88dd16c626d8fa6464c3a0c4b874fef953752a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:30:06 +0000 Subject: [PATCH 19/19] crypto-rsa.test.js: give publicDecrypt a real signature in the OAEP options test A block of 0x01 bytes is refused with or without an option, so that assertion could not fail. It now gets the output of privateEncrypt(), which is checked to decrypt when no option is set. Also say in update() that BoringSSL's bound is exact today, so the copy below it is not taken. Co-authored-by: dylanc --- src/jsc/bindings/node/crypto/JSCipherPrototype.cpp | 3 ++- test/js/node/crypto/crypto-rsa.test.js | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp index e70a9b6d043c..aa9b38e3b67c 100644 --- a/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp +++ b/src/jsc/bindings/node/crypto/JSCipherPrototype.cpp @@ -109,7 +109,8 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj return {}; } - // EVP_CIPHER_CTX_max_next_update() is an upper bound. The result's ArrayBuffer is as long as what was written, like Node's. + // The result's ArrayBuffer is as long as what was written, like Node's. BoringSSL documents + // EVP_CIPHER_CTX_max_next_update() as an upper bound, but it is exact today, so nothing is copied. if (outLen != outBuf->byteLength()) outBuf = ArrayBuffer::tryCreate(outBuf->span().first(outLen)); if (!outBuf) { diff --git a/test/js/node/crypto/crypto-rsa.test.js b/test/js/node/crypto/crypto-rsa.test.js index 818dd047172c..2632b36d9823 100644 --- a/test/js/node/crypto/crypto-rsa.test.js +++ b/test/js/node/crypto/crypto-rsa.test.js @@ -341,6 +341,8 @@ test("RSA-OAEP with an mgf1Hash other than oaepHash", () => { test("the OAEP options under another padding", () => { const options = { oaepHash: "sha256", mgf1Hash: "sha256", oaepLabel: Buffer.from("label") }; const block = Buffer.alloc(rsaKeySize / 8, 1); + const signed = crypto.privateEncrypt(rsaKeyPem, block.subarray(0, 32)); + expect(crypto.publicDecrypt(rsaPubPem, signed)).toEqual(block.subarray(0, 32)); for (const [name, value] of Object.entries(options)) { // Encryption and decryption ignore them. const noPadding = { padding: constants.RSA_NO_PADDING, [name]: value }; @@ -356,7 +358,7 @@ test("the OAEP options under another padding", () => { expect(() => crypto.privateEncrypt({ key: rsaKeyPem, [name]: value }, block.subarray(0, 32))).toThrow( expect.objectContaining({ code: expect.stringMatching(/^ERR_OSSL_/) }), ); - expect(() => crypto.publicDecrypt({ key: rsaPubPem, [name]: value }, block)).toThrow( + expect(() => crypto.publicDecrypt({ key: rsaPubPem, [name]: value }, signed)).toThrow( expect.objectContaining({ code: expect.stringMatching(/^ERR_OSSL_/) }), ); }