From bd4a9ffe8c85ad31c96571d3c229acf4a95b129d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 23 Jul 2024 14:20:58 -0700 Subject: [PATCH] src: start using ncrypto for CSPRNG calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/53984 Reviewed-By: Yagiz Nizipli Reviewed-By: Ethan Arrowood Reviewed-By: Tobias Nießen --- src/crypto/crypto_context.cc | 8 ++++---- src/crypto/crypto_keygen.cc | 3 ++- src/crypto/crypto_random.cc | 5 +++-- src/crypto/crypto_util.cc | 34 ---------------------------------- src/crypto/crypto_util.h | 13 ------------- src/inspector_io.cc | 13 +++++++------ src/node.cc | 5 +++-- src/quic/cid.cc | 3 ++- src/quic/endpoint.cc | 3 ++- src/quic/packet.cc | 3 ++- src/quic/session.cc | 3 ++- src/quic/tokens.cc | 3 ++- 12 files changed, 29 insertions(+), 67 deletions(-) diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 7abc1dc433bfce..22443405aca5a1 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -547,9 +547,9 @@ void SecureContext::Init(const FunctionCallbackInfo& 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 index b36f908a0b5a62..7e9c3b0dbc07ec 100644 --- 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 SecretKeyGenTraits::AdditionalConfig( KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env, SecretKeyGenConfig* params) { ByteSource::Builder bytes(params->length); - if (CSPRNG(bytes.data(), params->length).is_err()) + if (!ncrypto::CSPRNG(bytes.data(), 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 index 48154df7dc91ed..843eaeeabcfffc 100644 --- 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 index 9c8f6d6f447294..b164eaadffe705 100644 --- 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(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(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(u); if (passphrase != nullptr) { diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 4d97af48edf6d5..a98d17787887d5 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -91,19 +91,6 @@ void InitCrypto(v8::Local 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 index b7b8f21e3ff16a..4e76e529fcf341 100644 --- 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 @@ -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 index e2e6da2b725c3e..ec703ca4132e61 100644 --- 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& 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 index 404b98c47d6c19..f6eb9301a9215c 100644 --- a/src/quic/cid.cc +++ b/src/quic/cid.cc @@ -5,6 +5,7 @@ #include #include #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 index 7ac50c317c93e8..4c89ecdd91803e 100644 --- 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(c) / 255) < probability; } #endif // DEBUG diff --git a/src/quic/packet.cc b/src/quic/packet.cc index c43ac99b2442cd..1311f4828bd835 100644 --- 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 index 6476688e072924..ec54a67588d9b2 100644 --- 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 index e2c03d49c1b32f..fcb2a24ca3b6f8 100644 --- a/src/quic/tokens.cc +++ b/src/quic/tokens.cc @@ -8,6 +8,7 @@ #include #include #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_() {