node:crypto, webcrypto: a third batch of Node compatibility fixes

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 {};
 }

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&&);

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);
 }
 

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<uint8_t*>(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_t>(size) < dp.size()) {
-        const size_t padding = dp.size() - size;
-        uint8_t* data = static_cast<uint8_t*>(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<uint8_t*>(dp.get()), peer.get(), dh_.get()) < 0) return {};
 
     return dp;
 }
@@ -3292,6 +3283,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);
@@ -3420,7 +3418,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 {};
     }
 
@@ -3481,6 +3479,23 @@ const Rsa::PrivateKey Rsa::getPrivateKey() const
     return key;
 }
 
+// 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) {
+        return false;
+    }
+    return BN_cmp(product.get(), pub.n) == 0;
+}
+
 const std::optional<Rsa::PssParams> Rsa::getPssParams() const
 {
     if (rsa_ == nullptr) return std::nullopt;

src/jsc/bindings/ncrypto.h

@@ -321,6 +321,7 @@ class Cipher final {
     struct CipherParams {
         int padding;
         Digest digest;
+        Digest mgf1_digest;
         const Buffer<const void> label;
     };
 
@@ -401,6 +402,8 @@ 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<PssParams> getPssParams() const;
 
     bool setPublicKey(BignumPointer&& n, BignumPointer&& e);
@@ -673,6 +676,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);

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<WTF::StringView> 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,9 +92,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
+    // 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;
     int32_t padding = defaultPadding;
     std::optional<WTF::Vector<uint8_t>> oaepLabel;
     JSValue encodingValue = jsUndefined();
@@ -90,21 +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, {});
+        getDigestOption(lexicalGlobalObject, scope, options, "mgf1Hash"_s, "key.mgf1Hash"_s, mgf1Digest);
         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<WTF::StringView> oaepHashView = oaepHashString->view(lexicalGlobalObject);
-            RETURN_IF_EXCEPTION(scope, {});
-            digest = ncrypto::Digest::FromName(oaepHashView);
-            if (!digest) {
-                ERR::OSSL_EVP_INVALID_DIGEST(scope, lexicalGlobalObject);
-                return {};
-            }
-        }
 
         encodingValue = options->get(lexicalGlobalObject, Identifier::fromString(vm, "encoding"_s));
         RETURN_IF_EXCEPTION(scope, {});
@@ -114,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 {};
@@ -184,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<const void> labelBuf = {};
     if (oaepLabel) {
         labelBuf = {
@@ -195,6 +208,7 @@ JSValue rsaFunction(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* ca
     ncrypto::Cipher::CipherParams cipherParams {
         .padding = padding,
         .digest = digest,
+        .mgf1_digest = mgf1Digest,
         .label = labelBuf,
     };
 

src/jsc/bindings/node/crypto/JSCipherPrototype.cpp

@@ -56,18 +56,21 @@ 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<const uint8_t> 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) {
         throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state");
         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());
@@ -78,56 +81,50 @@ JSC_DEFINE_HOST_FUNCTION(jsCipherUpdate, (JSC::JSGlobalObject * lexicalGlobalObj
         ASSERT_UNUSED(passed, passed);
     }
 
-    const int32_t blockSize = cipher->m_ctx.getBlockSize();
-    if (dataView->byteLength() + 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<const uint8_t> buf {
-        .data = reinterpret_cast<uint8_t*>(dataView->vector()),
-        .len = dataView->byteLength(),
-    };
-
-    if (cipher->m_kind == CipherKind::Cipher && cipher->m_ctx.isWrapMode() && !cipher->m_ctx.update(buf, nullptr, &bufLen)) {
+    if (data.size() + cipher->m_ctx.getBlockSize() > INT_MAX) {
         throwCryptoError(lexicalGlobalObject, scope, ERR_peek_error(), "Trying to add data in unsupported state");
         return {};
     }
 
-    RefPtr<ArrayBuffer> outBuf = JSC::ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + bufLen, 1);
+    RefPtr<ArrayBuffer> 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 {};
     }
-    const size_t heldBack = cipher->takeHeldBackBlock(outBuf->mutableSpan());
+    size_t outLen = cipher->takeHeldBackBlock(outBuf->mutableSpan());
 
-    buf = {
-        .data = reinterpret_cast<uint8_t*>(dataView->vector()),
-        .len = dataView->byteLength(),
-    };
-
-    bool res = cipher->m_ctx.update(buf, static_cast<unsigned char*>(outBuf->data()) + heldBack, &bufLen);
-    ASSERT(heldBack + bufLen <= outBuf->byteLength());
+    auto out = outBuf->mutableSpan().subspan(outLen);
+    size_t written = 0;
+    bool res = EVP_CipherUpdate_ex(cipher->m_ctx, out.data(), &written, out.size(), data.data(), data.size()) == 1;
+    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)));
+    // 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) {
+        throwOutOfMemoryError(lexicalGlobalObject, scope);
+        return {};
+    }
+
+    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;
 
@@ -149,13 +146,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");
     };
 
-    int32_t outLen = cipher->m_ctx.getBlockSize();
-    RefPtr<ArrayBuffer> outBuf = ArrayBuffer::tryCreateUninitialized(cipher->heldBackBlock().size() + outLen, 1);
-    if (!outBuf) {
-        throwOutOfMemoryError(lexicalGlobalObject, scope);
-        return {};
-    }
-    const size_t heldBack = 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();
@@ -169,10 +162,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<unsigned char*>(outBuf->data()) + heldBack, &outLen, true);
-        ASSERT(heldBack + outLen <= outBuf->byteLength());
+        size_t written = 0;
+        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()) {
             ASSERT(cipher->m_authTagLen.has_value());
@@ -188,7 +181,7 @@ 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)));
+    RELEASE_AND_RETURN(scope, JSValue::encode(createBuffer(lexicalGlobalObject, std::span { out }.first(outLen))));
 }
 
 JSC_DEFINE_HOST_FUNCTION(jsCipherSetAutoPadding, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))