src: start using ncrypto for CSPRNG calls

PR-URL: #53984 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Aug 5, 2024 · bd4a9ff · bd4a9ff
1 parent 3fdcf7a
commit bd4a9ff
Show file tree

Hide file tree

Showing 12 changed files with 29 additions and 67 deletions.
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
@@ -547,9 +547,9 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) {
   // OpenSSL 1.1.0 changed the ticket key size, but the OpenSSL 1.0.x size was
   // exposed in the public API. To retain compatibility, install a callback
   // which restores the old algorithm.
-  if (CSPRNG(sc->ticket_key_name_, sizeof(sc->ticket_key_name_)).is_err() ||
-      CSPRNG(sc->ticket_key_hmac_, sizeof(sc->ticket_key_hmac_)).is_err() ||
-      CSPRNG(sc->ticket_key_aes_, sizeof(sc->ticket_key_aes_)).is_err()) {
+  if (!ncrypto::CSPRNG(sc->ticket_key_name_, sizeof(sc->ticket_key_name_)) ||
+      !ncrypto::CSPRNG(sc->ticket_key_hmac_, sizeof(sc->ticket_key_hmac_)) ||
+      !ncrypto::CSPRNG(sc->ticket_key_aes_, sizeof(sc->ticket_key_aes_))) {
     return THROW_ERR_CRYPTO_OPERATION_FAILED(
         env, "Error generating ticket keys");
   }
@@ -1324,7 +1324,7 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl,
 
   if (enc) {
     memcpy(name, sc->ticket_key_name_, sizeof(sc->ticket_key_name_));
-    if (CSPRNG(iv, 16).is_err() ||
+    if (!ncrypto::CSPRNG(iv, 16) ||
         EVP_EncryptInit_ex(
             ectx, EVP_aes_128_cbc(), nullptr, sc->ticket_key_aes_, iv) <= 0 ||
         HMAC_Init_ex(hctx,

diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc
@@ -4,6 +4,7 @@
 #include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
+#include "ncrypto.h"
 #include "threadpoolwork-inl.h"
 #include "v8.h"
 
@@ -71,7 +72,7 @@ Maybe<bool> SecretKeyGenTraits::AdditionalConfig(
 KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env,
                                              SecretKeyGenConfig* params) {
   ByteSource::Builder bytes(params->length);
-  if (CSPRNG(bytes.data<unsigned char>(), params->length).is_err())
+  if (!ncrypto::CSPRNG(bytes.data<unsigned char>(), params->length))
     return KeyGenJobStatus::FAILED;
   params->out = std::move(bytes).release();
   return KeyGenJobStatus::OK;

diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
@@ -3,6 +3,7 @@
 #include "crypto/crypto_util.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
+#include "ncrypto.h"
 #include "threadpoolwork-inl.h"
 #include "v8.h"
 
@@ -60,7 +61,7 @@ bool RandomBytesTraits::DeriveBits(
     Environment* env,
     const RandomBytesConfig& params,
     ByteSource* unused) {
-  return CSPRNG(params.buffer, params.size).is_ok();
+  return ncrypto::CSPRNG(params.buffer, params.size);
 }
 
 void RandomPrimeConfig::MemoryInfo(MemoryTracker* tracker) const {
@@ -154,7 +155,7 @@ bool RandomPrimeTraits::DeriveBits(Environment* env,
                                    ByteSource* unused) {
   // BN_generate_prime_ex() calls RAND_bytes_ex() internally.
   // Make sure the CSPRNG is properly seeded.
-  CHECK(CSPRNG(nullptr, 0).is_ok());
+  CHECK(ncrypto::CSPRNG(nullptr, 0));
 
   if (BN_generate_prime_ex(
           params.prime.get(),

diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
@@ -49,40 +49,6 @@ using v8::Value;
 
 namespace crypto {
 
-MUST_USE_RESULT CSPRNGResult CSPRNG(void* buffer, size_t length) {
-  unsigned char* buf = static_cast<unsigned char*>(buffer);
-  do {
-    if (1 == RAND_status()) {
-#if OPENSSL_VERSION_MAJOR >= 3
-      if (1 == RAND_bytes_ex(nullptr, buf, length, 0)) return {true};
-#else
-      while (length > INT_MAX && 1 == RAND_bytes(buf, INT_MAX)) {
-        buf += INT_MAX;
-        length -= INT_MAX;
-      }
-      if (length <= INT_MAX && 1 == RAND_bytes(buf, static_cast<int>(length)))
-        return {true};
-#endif
-    }
-#if OPENSSL_VERSION_MAJOR >= 3
-    const auto code = ERR_peek_last_error();
-    // A misconfigured OpenSSL 3 installation may report 1 from RAND_poll()
-    // and RAND_status() but fail in RAND_bytes() if it cannot look up
-    // a matching algorithm for the CSPRNG.
-    if (ERR_GET_LIB(code) == ERR_LIB_RAND) {
-      const auto reason = ERR_GET_REASON(code);
-      if (reason == RAND_R_ERROR_INSTANTIATING_DRBG ||
-          reason == RAND_R_UNABLE_TO_FETCH_DRBG ||
-          reason == RAND_R_UNABLE_TO_CREATE_DRBG) {
-        return {false};
-      }
-    }
-#endif
-  } while (1 == RAND_poll());
-
-  return {false};
-}
-
 int PasswordCallback(char* buf, int size, int rwflag, void* u) {
   const ByteSource* passphrase = *static_cast<const ByteSource**>(u);
   if (passphrase != nullptr) {

diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h
@@ -91,19 +91,6 @@ void InitCrypto(v8::Local<v8::Object> target);
 
 extern void UseExtraCaCerts(const std::string& file);
 
-struct CSPRNGResult {
-  const bool ok;
-  MUST_USE_RESULT bool is_ok() const { return ok; }
-  MUST_USE_RESULT bool is_err() const { return !ok; }
-};
-
-// Either succeeds with exactly |length| bytes of cryptographically
-// strong pseudo-random data, or fails. This function may block.
-// Don't assume anything about the contents of |buffer| on error.
-// As a special case, |length == 0| can be used to check if the CSPRNG
-// is properly seeded without consuming entropy.
-MUST_USE_RESULT CSPRNGResult CSPRNG(void* buffer, size_t length);
-
 int PasswordCallback(char* buf, int size, int rwflag, void* u);
 
 int NoPasswordCallback(char* buf, int size, int rwflag, void* u);

diff --git a/src/inspector_io.cc b/src/inspector_io.cc
@@ -1,16 +1,17 @@
 #include "inspector_io.h"
 
-#include "inspector_socket_server.h"
-#include "inspector/main_thread_interface.h"
-#include "inspector/node_string.h"
-#include "crypto/crypto_util.h"
 #include "base_object-inl.h"
+#include "crypto/crypto_util.h"
 #include "debug_utils-inl.h"
+#include "inspector/main_thread_interface.h"
+#include "inspector/node_string.h"
+#include "inspector_socket_server.h"
+#include "ncrypto.h"
 #include "node.h"
 #include "node_internals.h"
 #include "node_mutex.h"
-#include "v8-inspector.h"
 #include "util-inl.h"
+#include "v8-inspector.h"
 #include "zlib.h"
 
 #include <deque>
@@ -46,7 +47,7 @@ std::string ScriptPath(uv_loop_t* loop, const std::string& script_name) {
 // Used ver 4 - with numbers
 std::string GenerateID() {
   uint16_t buffer[8];
-  CHECK(crypto::CSPRNG(buffer, sizeof(buffer)).is_ok());
+  CHECK(ncrypto::CSPRNG(buffer, sizeof(buffer)));
 
   char uuid[256];
   snprintf(uuid, sizeof(uuid), "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",

diff --git a/src/node.cc b/src/node.cc
@@ -47,6 +47,7 @@
 #include "node_version.h"
 
 #if HAVE_OPENSSL
+#include "ncrypto.h"
 #include "node_crypto.h"
 #endif
 
@@ -1191,14 +1192,14 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
     }
 
     // Ensure CSPRNG is properly seeded.
-    CHECK(crypto::CSPRNG(nullptr, 0).is_ok());
+    CHECK(ncrypto::CSPRNG(nullptr, 0));
 
     V8::SetEntropySource([](unsigned char* buffer, size_t length) {
       // V8 falls back to very weak entropy when this function fails
       // and /dev/urandom isn't available. That wouldn't be so bad if
       // the entropy was only used for Math.random() but it's also used for
       // hash table and address space layout randomization. Better to abort.
-      CHECK(crypto::CSPRNG(buffer, length).is_ok());
+      CHECK(ncrypto::CSPRNG(buffer, length));
       return true;
     });
 #endif  // !defined(OPENSSL_IS_BORINGSSL)

diff --git a/src/quic/cid.cc b/src/quic/cid.cc
@@ -5,6 +5,7 @@
 #include <node_mutex.h>
 #include <string_bytes.h>
 #include "nbytes.h"
+#include "ncrypto.h"
 #include "quic/defs.h"
 
 namespace node {
@@ -132,7 +133,7 @@ class RandomCIDFactory : public CID::Factory {
     // a CID of the requested size, we regenerate the pool
     // and reset it to zero.
     if (pos_ + length_hint > kPoolSize) {
-      CHECK(crypto::CSPRNG(pool_, kPoolSize).is_ok());
+      CHECK(ncrypto::CSPRNG(pool_, kPoolSize));
       pos_ = 0;
     }
   }

diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc
@@ -19,6 +19,7 @@
 #include "application.h"
 #include "bindingdata.h"
 #include "defs.h"
+#include "ncrypto.h"
 
 namespace node {
 
@@ -87,7 +88,7 @@ namespace {
 bool is_diagnostic_packet_loss(double probability) {
   if (LIKELY(probability == 0.0)) return false;
   unsigned char c = 255;
-  CHECK(crypto::CSPRNG(&c, 1).is_ok());
+  CHECK(ncrypto::CSPRNG(&c, 1));
   return (static_cast<double>(c) / 255) < probability;
 }
 #endif  // DEBUG

diff --git a/src/quic/packet.cc b/src/quic/packet.cc
@@ -14,6 +14,7 @@
 #include "bindingdata.h"
 #include "cid.h"
 #include "defs.h"
+#include "ncrypto.h"
 #include "tokens.h"
 
 namespace node {
@@ -331,7 +332,7 @@ Packet* Packet::CreateStatelessResetPacket(
 
   StatelessResetToken token(token_secret, path_descriptor.dcid);
   uint8_t random[kRandlen];
-  CHECK(crypto::CSPRNG(random, kRandlen).is_ok());
+  CHECK(ncrypto::CSPRNG(random, kRandlen));
 
   auto packet = Create(env,
                        listener,

diff --git a/src/quic/session.cc b/src/quic/session.cc
@@ -24,6 +24,7 @@
 #include "defs.h"
 #include "endpoint.h"
 #include "logstream.h"
+#include "ncrypto.h"
 #include "packet.h"
 #include "preferredaddress.h"
 #include "sessionticket.h"
@@ -2163,7 +2164,7 @@ struct Session::Impl {
   static void on_rand(uint8_t* dest,
                       size_t destlen,
                       const ngtcp2_rand_ctx* rand_ctx) {
-    CHECK(crypto::CSPRNG(dest, destlen).is_ok());
+    CHECK(ncrypto::CSPRNG(dest, destlen));
   }
 
   static int on_early_data_rejected(ngtcp2_conn* conn, void* user_data) {

diff --git a/src/quic/tokens.cc b/src/quic/tokens.cc
@@ -8,6 +8,7 @@
 #include <util-inl.h>
 #include <algorithm>
 #include "nbytes.h"
+#include "ncrypto.h"
 
 namespace node {
 namespace quic {
@@ -22,7 +23,7 @@ TokenSecret::TokenSecret() : buf_() {
   // If someone manages to get visibility into that cache then they would know
   // the secrets for a larger number of tokens, which could be bad. For now,
   // generating on each call is safer, even if less performant.
-  CHECK(crypto::CSPRNG(buf_, QUIC_TOKENSECRET_LEN).is_ok());
+  CHECK(ncrypto::CSPRNG(buf_, QUIC_TOKENSECRET_LEN));
 }
 
 TokenSecret::TokenSecret(const uint8_t* secret) : buf_() {