Origin: https://github.com/nodejs/node/commit/0c2a5723beff39d1f62daec96b5389da3d427e79 Reviewed-by: Aron Xu Last-Update: 2022-01-05 Comment: Although WebCrypto is not implemented in 12.x series, this fix is introducing enhancment to the crypto setup of V8:EntropySource(). commit 0c2a5723beff39d1f62daec96b5389da3d427e79 Author: Ben Noordhuis Date: Sun Sep 11 10:48:34 2022 +0200 crypto: fix weak randomness in WebCrypto keygen Commit dae283d96f from August 2020 introduced a call to EntropySource() in SecretKeyGenTraits::DoKeyGen() in src/crypto/crypto_keygen.cc. There are two problems with that: 1. It does not check the return value, it assumes EntropySource() always succeeds, but it can (and sometimes will) fail. 2. The random data returned byEntropySource() may not be cryptographically strong and therefore not suitable as keying material. An example is a freshly booted system or a system without /dev/random or getrandom(2). EntropySource() calls out to openssl's RAND_poll() and RAND_bytes() in a best-effort attempt to obtain random data. OpenSSL has a built-in CSPRNG but that can fail to initialize, in which case it's possible either: 1. No random data gets written to the output buffer, i.e., the output is unmodified, or 2. Weak random data is written. It's theoretically possible for the output to be fully predictable because the CSPRNG starts from a predictable state. Replace EntropySource() and CheckEntropy() with new function CSPRNG() that enforces checking of the return value. Abort on startup when the entropy pool fails to initialize because that makes it too easy to compromise the security of the process. Refs: https://hackerone.com/bugs?report_id=1690000 Refs: https://github.com/nodejs/node/pull/35093 Reviewed-By: Rafael Gonzaga Reviewed-By: Tobias Nießen PR-URL: #346 Backport-PR-URL: #351 CVE-ID: CVE-2022-35255 CVE: CVE-2022-35255 Upstream-Status: Backport [https://sources.debian.org/src/nodejs/12.22.12~dfsg-1~deb11u3/debian/patches/cve-2022-35255.patch] Comment: No hunks refreshed Signed-off-by: Poonam Jadhav Index: nodejs-12.22.12~dfsg/node.gyp =================================================================== --- nodejs-12.22.12~dfsg.orig/node.gyp +++ nodejs-12.22.12~dfsg/node.gyp @@ -743,6 +743,8 @@ 'openssl_default_cipher_list%': '', }, + 'cflags': ['-Werror=unused-result'], + 'defines': [ 'NODE_ARCH="<(target_arch)"', 'NODE_PLATFORM="<(OS)"', Index: nodejs-12.22.12~dfsg/src/node_crypto.cc =================================================================== --- nodejs-12.22.12~dfsg.orig/src/node_crypto.cc +++ nodejs-12.22.12~dfsg/src/node_crypto.cc @@ -386,48 +386,14 @@ void ThrowCryptoError(Environment* env, env->isolate()->ThrowException(exception); } +MUST_USE_RESULT CSPRNGResult CSPRNG(void* buffer, size_t length) { + do { + if (1 == RAND_status()) + if (1 == RAND_bytes(static_cast(buffer), length)) + return {true}; + } while (1 == RAND_poll()); -// Ensure that OpenSSL has enough entropy (at least 256 bits) for its PRNG. -// The entropy pool starts out empty and needs to fill up before the PRNG -// can be used securely. Once the pool is filled, it never dries up again; -// its contents is stirred and reused when necessary. -// -// OpenSSL normally fills the pool automatically but not when someone starts -// generating random numbers before the pool is full: in that case OpenSSL -// keeps lowering the entropy estimate to thwart attackers trying to guess -// the initial state of the PRNG. -// -// When that happens, we will have to wait until enough entropy is available. -// That should normally never take longer than a few milliseconds. -// -// OpenSSL draws from /dev/random and /dev/urandom. While /dev/random may -// block pending "true" randomness, /dev/urandom is a CSPRNG that doesn't -// block under normal circumstances. -// -// The only time when /dev/urandom may conceivably block is right after boot, -// when the whole system is still low on entropy. That's not something we can -// do anything about. -inline void CheckEntropy() { - for (;;) { - int status = RAND_status(); - CHECK_GE(status, 0); // Cannot fail. - if (status != 0) - break; - - // Give up, RAND_poll() not supported. - if (RAND_poll() == 0) - break; - } -} - - -bool EntropySource(unsigned char* buffer, size_t length) { - // Ensure that OpenSSL's PRNG is properly seeded. - CheckEntropy(); - // RAND_bytes() can return 0 to indicate that the entropy data is not truly - // random. That's okay, it's still better than V8's stock source of entropy, - // which is /dev/urandom on UNIX platforms and the current time on Windows. - return RAND_bytes(buffer, length) != -1; + return {false}; } void SecureContext::Initialize(Environment* env, Local target) { @@ -649,9 +615,9 @@ void SecureContext::Init(const FunctionC // 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 (RAND_bytes(sc->ticket_key_name_, sizeof(sc->ticket_key_name_)) <= 0 || - RAND_bytes(sc->ticket_key_hmac_, sizeof(sc->ticket_key_hmac_)) <= 0 || - RAND_bytes(sc->ticket_key_aes_, sizeof(sc->ticket_key_aes_)) <= 0) { + 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()) { return env->ThrowError("Error generating ticket keys"); } SSL_CTX_set_tlsext_ticket_key_cb(sc->ctx_.get(), TicketCompatibilityCallback); @@ -1643,7 +1609,7 @@ int SecureContext::TicketCompatibilityCa if (enc) { memcpy(name, sc->ticket_key_name_, sizeof(sc->ticket_key_name_)); - if (RAND_bytes(iv, 16) <= 0 || + if (CSPRNG(iv, 16).is_err() || EVP_EncryptInit_ex(ectx, EVP_aes_128_cbc(), nullptr, sc->ticket_key_aes_, iv) <= 0 || HMAC_Init_ex(hctx, sc->ticket_key_hmac_, sizeof(sc->ticket_key_hmac_), @@ -5867,8 +5833,7 @@ struct RandomBytesJob : public CryptoJob : CryptoJob(env), rc(Nothing()) {} inline void DoThreadPoolWork() override { - CheckEntropy(); // Ensure that OpenSSL's PRNG is properly seeded. - rc = Just(RAND_bytes(data, size)); + rc = Just(int(CSPRNG(data, size).is_ok())); if (0 == rc.FromJust()) errors.Capture(); } @@ -6318,8 +6283,8 @@ class GenerateKeyPairJob : public Crypto } inline bool GenerateKey() { - // Make sure that the CSPRNG is properly seeded so the results are secure. - CheckEntropy(); + // Make sure that the CSPRNG is properly seeded. + CHECK(CSPRNG(nullptr, 0).is_ok()); // Create the key generation context. EVPKeyCtxPointer ctx = config_->Setup(); Index: nodejs-12.22.12~dfsg/src/node_crypto.h =================================================================== --- nodejs-12.22.12~dfsg.orig/src/node_crypto.h +++ nodejs-12.22.12~dfsg/src/node_crypto.h @@ -840,7 +840,19 @@ class ECDH final : public BaseObject { const EC_GROUP* group_; }; -bool EntropySource(unsigned char* buffer, size_t length); +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); + #ifndef OPENSSL_NO_ENGINE void SetEngine(const v8::FunctionCallbackInfo& args); #endif // !OPENSSL_NO_ENGINE Index: nodejs-12.22.12~dfsg/src/inspector_io.cc =================================================================== --- nodejs-12.22.12~dfsg.orig/src/inspector_io.cc +++ nodejs-12.22.12~dfsg/src/inspector_io.cc @@ -46,8 +46,7 @@ std::string ScriptPath(uv_loop_t* loop, // Used ver 4 - with numbers std::string GenerateID() { uint16_t buffer[8]; - CHECK(crypto::EntropySource(reinterpret_cast(buffer), - sizeof(buffer))); + CHECK(crypto::CSPRNG(buffer, sizeof(buffer)).is_ok()); char uuid[256]; snprintf(uuid, sizeof(uuid), "%04x%04x-%04x-%04x-%04x-%04x%04x%04x", Index: nodejs-12.22.12~dfsg/src/node.cc =================================================================== --- nodejs-12.22.12~dfsg.orig/src/node.cc +++ nodejs-12.22.12~dfsg/src/node.cc @@ -969,9 +969,17 @@ InitializationResult InitializeOncePerPr // the random source is properly initialized first. OPENSSL_init(); #endif // NODE_FIPS_MODE - // V8 on Windows doesn't have a good source of entropy. Seed it from - // OpenSSL's pool. - V8::SetEntropySource(crypto::EntropySource); + // Ensure CSPRNG is properly seeded. + CHECK(crypto::CSPRNG(nullptr, 0).is_ok()); + + 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()); + return true; + }); #endif // HAVE_OPENSSL per_process::v8_platform.Initialize(