mirror of
https://github.com/termux-pacman/termux-packages.git
synced 2026-01-07 11:23:23 +00:00
331 lines
13 KiB
Diff
331 lines
13 KiB
Diff
From 0c2a5723beff39d1f62daec96b5389da3d427e79 Mon Sep 17 00:00:00 2001
|
|
From: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Date: Sun, 11 Sep 2022 10:48:34 +0200
|
|
Subject: [PATCH] crypto: fix weak randomness in WebCrypto keygen
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
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 <rafael.nunu@hotmail.com>
|
|
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|
|
PR-URL: #346
|
|
Backport-PR-URL: #351
|
|
CVE-ID: CVE-2022-35255
|
|
---
|
|
node.gyp | 2 ++
|
|
src/crypto/crypto_context.cc | 19 ++++++++++--------
|
|
src/crypto/crypto_keygen.cc | 8 +++++++-
|
|
src/crypto/crypto_keygen.h | 3 ---
|
|
src/crypto/crypto_random.cc | 15 +++++++--------
|
|
src/crypto/crypto_util.cc | 28 ++++++++-------------------
|
|
src/crypto/crypto_util.h | 37 ++++++++++++------------------------
|
|
src/inspector_io.cc | 3 +--
|
|
src/node.cc | 30 ++++++++++++++++++-----------
|
|
9 files changed, 67 insertions(+), 78 deletions(-)
|
|
|
|
diff --git a/node.gyp b/node.gyp
|
|
index 347d82d2e1e1..8f131acf661b 100644
|
|
--- a/node.gyp
|
|
+++ b/node.gyp
|
|
@@ -683,6 +683,8 @@
|
|
'openssl_default_cipher_list%': '',
|
|
},
|
|
|
|
+ 'cflags': ['-Werror=unused-result'],
|
|
+
|
|
'defines': [
|
|
'NODE_ARCH="<(target_arch)"',
|
|
'NODE_PLATFORM="<(OS)"',
|
|
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
|
|
index 7eab9de37cb3..e70ddc64f368 100644
|
|
--- a/src/crypto/crypto_context.cc
|
|
+++ b/src/crypto/crypto_context.cc
|
|
@@ -541,9 +541,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 (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 THROW_ERR_CRYPTO_OPERATION_FAILED(
|
|
env, "Error generating ticket keys");
|
|
}
|
|
@@ -1241,11 +1241,14 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl,
|
|
|
|
if (enc) {
|
|
memcpy(name, sc->ticket_key_name_, sizeof(sc->ticket_key_name_));
|
|
- if (RAND_bytes(iv, 16) <= 0 ||
|
|
- 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_),
|
|
- EVP_sha256(), nullptr) <= 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_),
|
|
+ EVP_sha256(),
|
|
+ nullptr) <= 0) {
|
|
return -1;
|
|
}
|
|
return 1;
|
|
diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc
|
|
index af489967144d..8def58a5f10c 100644
|
|
--- a/src/crypto/crypto_keygen.cc
|
|
+++ b/src/crypto/crypto_keygen.cc
|
|
@@ -81,7 +81,13 @@ KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(
|
|
SecretKeyGenConfig* params) {
|
|
CHECK_LE(params->length, INT_MAX);
|
|
params->out = MallocOpenSSL<char>(params->length);
|
|
- EntropySource(reinterpret_cast<unsigned char*>(params->out), params->length);
|
|
+ if (CSPRNG(reinterpret_cast<unsigned char*>(params->out),
|
|
+ params->length).is_err()) {
|
|
+ OPENSSL_clear_free(params->out, params->length);
|
|
+ params->out = nullptr;
|
|
+ params->length = 0;
|
|
+ return KeyGenJobStatus::FAILED;
|
|
+ }
|
|
return KeyGenJobStatus::OK;
|
|
}
|
|
|
|
diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h
|
|
index ed7d5c0601ff..1a933a247800 100644
|
|
--- a/src/crypto/crypto_keygen.h
|
|
+++ b/src/crypto/crypto_keygen.h
|
|
@@ -75,9 +75,6 @@ class KeyGenJob final : public CryptoJob<KeyGenTraits> {
|
|
std::move(params)) {}
|
|
|
|
void DoThreadPoolWork() override {
|
|
- // Make sure the CSPRNG is properly seeded so the results are secure.
|
|
- CheckEntropy();
|
|
-
|
|
AdditionalParams* params = CryptoJob<KeyGenTraits>::params();
|
|
|
|
switch (KeyGenTraits::DoKeyGen(AsyncWrap::env(), params)) {
|
|
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
|
|
index d0736a9cf127..2f9e9aacb1e6 100644
|
|
--- a/src/crypto/crypto_random.cc
|
|
+++ b/src/crypto/crypto_random.cc
|
|
@@ -66,8 +66,7 @@ bool RandomBytesTraits::DeriveBits(
|
|
Environment* env,
|
|
const RandomBytesConfig& params,
|
|
ByteSource* unused) {
|
|
- CheckEntropy(); // Ensure that OpenSSL's PRNG is properly seeded.
|
|
- return RAND_bytes(params.buffer, params.size) != 0;
|
|
+ return CSPRNG(params.buffer, params.size).is_ok();
|
|
}
|
|
|
|
void RandomPrimeConfig::MemoryInfo(MemoryTracker* tracker) const {
|
|
@@ -156,12 +155,12 @@ Maybe<bool> RandomPrimeTraits::AdditionalConfig(
|
|
return Just(true);
|
|
}
|
|
|
|
-bool RandomPrimeTraits::DeriveBits(
|
|
- Environment* env,
|
|
- const RandomPrimeConfig& params,
|
|
- ByteSource* unused) {
|
|
-
|
|
- CheckEntropy();
|
|
+bool RandomPrimeTraits::DeriveBits(Environment* env,
|
|
+ const RandomPrimeConfig& params,
|
|
+ ByteSource* unused) {
|
|
+ // BN_generate_prime_ex() calls RAND_bytes_ex() internally.
|
|
+ // Make sure the CSPRNG is properly seeded.
|
|
+ CHECK(CSPRNG(nullptr, 0).is_ok());
|
|
|
|
if (BN_generate_prime_ex(
|
|
params.prime.get(),
|
|
diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
|
|
index 805f8978b2ad..e878c5ea15d5 100644
|
|
--- a/src/crypto/crypto_util.cc
|
|
+++ b/src/crypto/crypto_util.cc
|
|
@@ -60,26 +60,14 @@ int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx) {
|
|
return 1;
|
|
}
|
|
|
|
-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;
|
|
+MUST_USE_RESULT CSPRNGResult CSPRNG(void* buffer, size_t length) {
|
|
+ do {
|
|
+ if (1 == RAND_status())
|
|
+ if (1 == RAND_bytes(static_cast<unsigned char*>(buffer), length))
|
|
+ return {true};
|
|
+ } while (1 == RAND_poll());
|
|
+
|
|
+ return {false};
|
|
}
|
|
|
|
int PasswordCallback(char* buf, int size, int rwflag, void* u) {
|
|
diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h
|
|
index 4afae1884fe4..dc3bb15cfb48 100644
|
|
--- a/src/crypto/crypto_util.h
|
|
+++ b/src/crypto/crypto_util.h
|
|
@@ -111,31 +111,18 @@ struct MarkPopErrorOnReturn {
|
|
~MarkPopErrorOnReturn() { ERR_pop_to_mark(); }
|
|
};
|
|
|
|
-// 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.
|
|
-void CheckEntropy();
|
|
-
|
|
-// Generate length bytes of random data. If this returns false, the data
|
|
-// may not be truly random but it's still generally good enough.
|
|
-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);
|
|
|
|
int PasswordCallback(char* buf, int size, int rwflag, void* u);
|
|
|
|
diff --git a/src/inspector_io.cc b/src/inspector_io.cc
|
|
index 7f52fc605933..7e0b3ea63cff 100644
|
|
--- a/src/inspector_io.cc
|
|
+++ b/src/inspector_io.cc
|
|
@@ -46,8 +46,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::EntropySource(reinterpret_cast<unsigned char*>(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",
|
|
diff --git a/src/node.cc b/src/node.cc
|
|
index 71b3f7dcfe3c..75423472f0f3 100644
|
|
--- a/src/node.cc
|
|
+++ b/src/node.cc
|
|
@@ -1056,11 +1056,10 @@ InitializationResult InitializeOncePerProcess(
|
|
// fipsinstall command. If the path to this file is incorrect no error
|
|
// will be reported.
|
|
//
|
|
- // For Node.js this will mean that EntropySource will be called by V8 as
|
|
- // part of its initialization process, and EntropySource will in turn call
|
|
- // CheckEntropy. CheckEntropy will call RAND_status which will now always
|
|
- // return 0, leading to an endless loop and the node process will appear to
|
|
- // hang/freeze.
|
|
+ // For Node.js this will mean that CSPRNG() will be called by V8 as
|
|
+ // part of its initialization process, and CSPRNG() will in turn call
|
|
+ // call RAND_status which will now always return 0, leading to an endless
|
|
+ // loop and the node process will appear to hang/freeze.
|
|
|
|
// Passing NULL as the config file will allow the default openssl.cnf file
|
|
// to be loaded, but the default section in that file will not be used,
|
|
@@ -1105,19 +1104,28 @@ InitializationResult InitializeOncePerProcess(
|
|
OPENSSL_init();
|
|
}
|
|
#endif
|
|
- if (!crypto::ProcessFipsOptions()) {
|
|
+ if (!crypto::ProcessFipsOptions()) {
|
|
result.exit_code = ERR_GET_REASON(ERR_peek_error());
|
|
result.early_return = true;
|
|
fprintf(stderr, "OpenSSL error when trying to enable FIPS:\n");
|
|
ERR_print_errors_fp(stderr);
|
|
return result;
|
|
- }
|
|
+ }
|
|
|
|
- // 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 && !defined(OPENSSL_IS_BORINGSSL)
|
|
-}
|
|
+ }
|
|
+
|
|
per_process::v8_platform.Initialize(
|
|
static_cast<int>(per_process::cli_options->v8_thread_pool_size));
|
|
if (init_flags & kInitializeV8) {
|