From 274ee6744daba4cddfd9dee14fe42f8160b996eb Mon Sep 17 00:00:00 2001 From: Markus Theil Date: Thu, 5 Sep 2024 22:26:38 +0200 Subject: [PATCH] Randomizer: always use CSPRNG from OpenSSL The Randomizer class provided an insecure mersenne twister PRNG as a convenience method to draw things like PINs and serial numbers from it. I changed this to always use a secure OpenSSL-based CSPRNG. Furthermore, the OpenSSL PRNG was insecurely seeded from the mersenne twister RNG. Fix this, by combining several input sources via a cryptographic hash function and seed OpenSSL from it. The code now tries to read 256 Bit from different sources and combines them, with SHA-512. When OpenSSL aims for 256 Bit security strength, seed it with at least 1.5x this security strength. Please note, that OpenSSL in typical configurations is automatically seeded and the seeding strategy here probably did no harm by accident in the past. Signed-off-by: Markus Theil --- src/global/Randomizer.cpp | 163 +++++++++++++++++++++-------- src/global/Randomizer.h | 58 +++++----- test/qt/global/test_Randomizer.cpp | 40 ------- 3 files changed, 145 insertions(+), 116 deletions(-) diff --git a/src/global/Randomizer.cpp b/src/global/Randomizer.cpp index e2082750..ae273c6f 100644 --- a/src/global/Randomizer.cpp +++ b/src/global/Randomizer.cpp @@ -10,9 +10,11 @@ #include #include -#include #include +#include +#include #include +#include #ifdef Q_OS_WIN #include @@ -37,71 +39,125 @@ using namespace governikus; defineSingleton(Randomizer) +const size_t ENTROPY_FETCH_SIZE = 256 / 8; -template QList Randomizer::getEntropy() +OpenSSLGenerator::result_type OpenSSLGenerator::operator()() { + result_type buffer{0}; + + int ret = RAND_bytes(reinterpret_cast(&buffer), sizeof(buffer)); + Q_ASSERT(ret == 1); + + return buffer; +} + + +size_t Randomizer::getEntropy(std::shared_ptr mdCtx) { - QList entropy; + int ret; + size_t numSources = 3; + + auto clockTicks = std::chrono::high_resolution_clock::now().time_since_epoch().count(); + ret = EVP_DigestUpdate(mdCtx.get(), &clockTicks, sizeof(clockTicks)); + Q_ASSERT(ret == 1); + + for (size_t i = 0; i < ENTROPY_FETCH_SIZE / sizeof(std::random_device::result_type); ++i) { + auto randomDeviceRand = std::random_device()(); + ret = EVP_DigestUpdate(mdCtx.get(), &randomDeviceRand, sizeof(randomDeviceRand)); + Q_ASSERT(ret == 1); + OPENSSL_cleanse(&randomDeviceRand, sizeof(randomDeviceRand)); + } - entropy += static_cast(std::chrono::system_clock::now().time_since_epoch().count()); - entropy += std::random_device()(); - entropy += QRandomGenerator::securelySeeded().generate(); + quint64 qtBuffer[ENTROPY_FETCH_SIZE / sizeof(quint64)]; + QRandomGenerator::securelySeeded().fillRange(qtBuffer); + ret = EVP_DigestUpdate(mdCtx.get(), &qtBuffer, ENTROPY_FETCH_SIZE); + Q_ASSERT(ret == 1); + OPENSSL_cleanse(&qtBuffer, ENTROPY_FETCH_SIZE); - if (UniversalBuffer buffer; RAND_bytes(buffer.data, sizeof(buffer.data))) + if (unsigned char buffer[ENTROPY_FETCH_SIZE]; RAND_bytes(buffer, sizeof(buffer))) { - entropy += buffer.get(); + ret = EVP_DigestUpdate(mdCtx.get(), buffer, sizeof(buffer)); + Q_ASSERT(ret == 1); + ++numSources; + OPENSSL_cleanse(buffer, sizeof(buffer)); } - entropy += getEntropyWin(); - entropy += getEntropyUnixoid(); - entropy += getEntropyApple(); + if (getEntropyWin(mdCtx)) + ++numSources; + if (getEntropyUnixoid(mdCtx)) + ++numSources; + if (getEntropyApple(mdCtx)) + ++numSources; - return entropy; + return numSources; } -template QList Randomizer::getEntropyWin() +bool Randomizer::getEntropyWin(std::shared_ptr mdCtx) { - QList entropy; + bool hasEntropy = false; #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) - UniversalBuffer buffer; + unsigned char buffer[ENTROPY_FETCH_SIZE]; HCRYPTPROV provider = 0; if (CryptAcquireContext(&provider, 0, 0, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) { - if (CryptGenRandom(provider, sizeof(buffer.data), buffer.data)) + if (CryptGenRandom(provider, sizeof(buffer), buffer)) { - entropy += buffer.get(); + int ret = EVP_DigestUpdate(mdCtx.get(), buffer, sizeof(buffer)); + Q_ASSERT(ret == 1); + hasEntropy = true; } CryptReleaseContext(provider, 0); } + OPENSSL_cleanse(buffer, sizeof(buffer)); +#else + Q_UNUSED(mdCtx) #endif - return entropy; + return hasEntropy; } -template QList Randomizer::getEntropyUnixoid() +bool Randomizer::getEntropyUnixoid(std::shared_ptr mdCtx) { - QList entropy; + bool hasEntropy = false; #ifdef SYS_getrandom - if (UniversalBuffer buffer; syscall(SYS_getrandom, buffer.data, sizeof(buffer.data), GRND_NONBLOCK)) { - entropy += buffer.get(); + unsigned char buffer[ENTROPY_FETCH_SIZE]; + long int sys_ret; + ssize_t bytesToRead = ENTROPY_FETCH_SIZE; + while (bytesToRead > 0) { + sys_ret = syscall(SYS_getrandom, buffer + ENTROPY_FETCH_SIZE - bytesToRead, bytesToRead, GRND_NONBLOCK); + if (sys_ret > 0 && sys_ret <= static_cast(bytesToRead)) { + bytesToRead -= sys_ret; + } + if (sys_ret == -1 && !(errno == EINTR || errno == EAGAIN)) { + // return from loop, if something goes wrong unexpectedly + break; + } + }; + Q_ASSERT(bytesToRead == 0); + + int ret = EVP_DigestUpdate(mdCtx.get(), buffer, sizeof(buffer)); + Q_ASSERT(ret == 1); + + hasEntropy = true; + + OPENSSL_cleanse(buffer, sizeof(buffer)); } #elif defined(Q_OS_UNIX) // Fallback for unixoid systems without SYS_getrandom (like linux before version 3.17 ). // This is mainly relevant for older androids. - UniversalBuffer buffer; QFile file(QStringLiteral("/dev/urandom")); if (file.open(QIODevice::ReadOnly)) { - qint64 bytesToRead = sizeof(buffer.data); + qint64 bytesToRead = sizeof(buffer); do { - const qint64 bytesRead = file.read(buffer.data, bytesToRead); + const qint64 bytesRead = file.read(buffer + sizeof(buffer) - bytesToRead, bytesToRead); if (bytesRead < 0) { qCritical() << "Failed to read" << file.fileName(); @@ -111,52 +167,75 @@ template QList Randomizer::getEntropyUnixoid() } while (bytesToRead > 0); - entropy += buffer.get(); + int ret = EVP_DigestUpdate(mdCtx.get(), buffer, sizeof(buffer)); + Q_ASSERT(ret == 1); + hasEntropy = bytesToRead == 0; file.close(); + + OPENSSL_cleanse(buffer, sizeof(buffer)); } else { qCritical() << "Failed to open" << file.fileName(); } +#else + Q_UNUSED(mdCtx) #endif - return entropy; + return hasEntropy; } -template QList Randomizer::getEntropyApple() +bool Randomizer::getEntropyApple(std::shared_ptr mdCtx) { - QList entropy; + bool hasEntropy = false; #if defined(Q_OS_IOS) || defined(Q_OS_MACOS) - if (UniversalBuffer buffer; SecRandomCopyBytes(kSecRandomDefault, sizeof(buffer.data), buffer.data) == 0) + if (unsigned char buffer[ENTROPY_FETCH_SIZE]; SecRandomCopyBytes(kSecRandomDefault, sizeof(buffer), buffer) == 0) { - entropy += buffer.get(); + int ret = EVP_DigestUpdate(mdCtx.get(), buffer, sizeof(buffer)); + Q_ASSERT(ret == 1); + hasEntropy = true; + OPENSSL_cleanse(buffer, sizeof(buffer)); } +#else + Q_UNUSED(mdCtx) #endif - return entropy; + return hasEntropy; } Randomizer::Randomizer() { - const auto& entropy = getEntropy(); - std::seed_seq seed(entropy.cbegin(), entropy.cend()); - mGenerator.seed(seed); + auto mdCtx = std::shared_ptr(EVP_MD_CTX_new(), EVP_MD_CTX_free); + + int ret = EVP_DigestInit_ex(mdCtx.get(), EVP_sha512(), nullptr); + Q_ASSERT(ret == 1); + + size_t numSources = getEntropy(mdCtx); + + const size_t SHA512_DIGEST_SIZE = 512 / 8; + unsigned char seedBuffer[SHA512_DIGEST_SIZE]; + unsigned int mdLen = SHA512_DIGEST_SIZE; + + ret = EVP_DigestFinal_ex(mdCtx.get(), seedBuffer, &mdLen); + Q_ASSERT(ret == 1); + Q_ASSERT(mdLen = SHA512_DIGEST_SIZE); - // We need to seed pseudo random pool of openssl. - // yes, OpenSSL is an entropy source, too. But not the only one! - UniversalBuffer buffer; - buffer.set(mGenerator()); - RAND_seed(buffer.data, sizeof(std::mt19937_64::result_type)); + /* + * OpenSSL automatically seeds itself, unless configured otherwise. + * This is just defensive programming. + */ + RAND_seed(seedBuffer, sizeof(seedBuffer)); + OPENSSL_cleanse(seedBuffer, sizeof(seedBuffer)); static const int MINIMUM_ENTROPY_SOURCES = 4; - mSecureRandom = seed.size() >= MINIMUM_ENTROPY_SOURCES; + mSecureRandom = numSources >= MINIMUM_ENTROPY_SOURCES; } -std::mt19937_64& Randomizer::getGenerator() +OpenSSLGenerator& Randomizer::getGenerator() { return mGenerator; } diff --git a/src/global/Randomizer.h b/src/global/Randomizer.h index efb0034d..49779ef3 100644 --- a/src/global/Randomizer.h +++ b/src/global/Randomizer.h @@ -8,56 +8,46 @@ #pragma once -#include #include +#include +#include #include +#include class test_Randomizer; namespace governikus { +class OpenSSLGenerator { +public: + using result_type = uint64_t; + + static constexpr result_type min() { + return std::numeric_limits::min(); + } + + static constexpr result_type max() { + return std::numeric_limits::max(); + } + + result_type operator()(); +}; + class Randomizer { Q_DISABLE_COPY(Randomizer) friend class ::test_Randomizer; private: - template struct UniversalBuffer - { - U data[sizeof(T)] = {}; - - T get() - { -#if __cpp_lib_bit_cast >= 201806 - return std::bit_cast(data); - -#else - T number; - memcpy(&number, &data, sizeof(T)); - return number; - -#endif - } - - - void set(T pNumber) - { - memcpy(&data, &pNumber, sizeof(T)); - } - - - static_assert(sizeof(T) == sizeof(data)); - }; - - std::mt19937_64 mGenerator; + OpenSSLGenerator mGenerator; bool mSecureRandom; - template static QList getEntropy(); - template static QList getEntropyWin(); - template static QList getEntropyUnixoid(); - template static QList getEntropyApple(); + [[nodiscard]] static size_t getEntropy(std::shared_ptr mdCtx); + [[nodiscard]] static bool getEntropyWin(std::shared_ptr mdCtx); + [[nodiscard]] static bool getEntropyUnixoid(std::shared_ptr mdCtx); + [[nodiscard]] static bool getEntropyApple(std::shared_ptr mdCtx); protected: Randomizer(); @@ -66,7 +56,7 @@ class Randomizer public: static Randomizer& getInstance(); - [[nodiscard]] std::mt19937_64& getGenerator(); + [[nodiscard]] OpenSSLGenerator& getGenerator(); [[nodiscard]] bool isSecureRandom() const; [[nodiscard]] QUuid createUuid(); diff --git a/test/qt/global/test_Randomizer.cpp b/test/qt/global/test_Randomizer.cpp index 196392c6..40d57917 100644 --- a/test/qt/global/test_Randomizer.cpp +++ b/test/qt/global/test_Randomizer.cpp @@ -20,12 +20,6 @@ class test_Randomizer Q_OBJECT private Q_SLOTS: - void defaultSeed() - { - QVERIFY(Randomizer::getInstance().getGenerator().default_seed == 5489u); - } - - void secureSeeded() { Randomizer& randomizer = Randomizer::getInstance(); @@ -33,22 +27,6 @@ class test_Randomizer } - void defaultSeedNotUsed() - { - std::mt19937 unseeded; - std::mt19937_64 unseeded64; - - auto& randomizer = Randomizer::getInstance(); - - for (int i = 0; i < 10; ++i) - { - const auto fromRandomizer = randomizer.getGenerator()(); - QVERIFY(fromRandomizer != unseeded()); - QVERIFY(fromRandomizer != unseeded64()); - } - } - - void createUuid() { auto& randomizer = Randomizer::getInstance(); @@ -61,24 +39,6 @@ class test_Randomizer QVERIFY(uuid1 != uuid2); } - - - void universalBuffer() - { - Randomizer::UniversalBuffer buffer; - QCOMPARE(buffer.get(), 0); - - buffer.set(666); - QCOMPARE(buffer.get(), 666); - - memcpy(buffer.data, "1234567", sizeof(buffer.data)); - QCOMPARE(buffer.get(), 15540725856023089); - - buffer.set(13847469359445559); - QCOMPARE(QLatin1String(reinterpret_cast(buffer.data)), QLatin1String("7654321")); - } - - }; QTEST_GUILESS_MAIN(test_Randomizer)