From b328ec24629dbe2403373e1d51638fcc50f6b41c Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 18 Dec 2014 22:40:32 -0800 Subject: [PATCH] Prevent passing of non-POD types to POD-only interfaces: This tidies up the code that produces random numbers to conform to programming best practices and reduce dependencies. * Use std::random_device instead of platform-specific code * Remove RandomNumbers class and use free functions instead --- src/ripple/app/main/Main.cpp | 7 +- src/ripple/app/misc/NetworkOPs.cpp | 3 +- src/ripple/app/shamap/SHAMapNodeID.cpp | 2 +- src/ripple/crypto/RandomNumbers.h | 86 +++----- src/ripple/crypto/impl/ECIES.cpp | 2 +- src/ripple/crypto/impl/RandomNumbers.cpp | 237 +++------------------ src/ripple/net/impl/SNTPClient.cpp | 2 +- src/ripple/protocol/impl/RippleAddress.cpp | 2 +- src/ripple/rpc/handlers/Random.cpp | 2 +- 9 files changed, 60 insertions(+), 283 deletions(-) diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 44501d095..67a9b68d2 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -218,11 +218,8 @@ int run (int argc, char** argv) po::positional_options_description p; p.add ("parameters", -1); - if (! RandomNumbers::getInstance ().initialize ()) - { - std::cerr << "Unable to add system entropy" << std::endl; - iResult = 2; - } + // Seed the RNG early + add_entropy (); if (!iResult) { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 3a3ee9dda..882c46d76 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -2895,7 +2895,8 @@ bool NetworkOPsImp::subServer (InfoSub::ref isrListener, Json::Value& jvResult, if (m_standalone) jvResult[jss::stand_alone] = m_standalone; - RandomNumbers::getInstance ().fillBytes (uRandom.begin (), uRandom.size ()); + // CHECKME: is it necessary to provide a random number here? + random_fill (uRandom.begin (), uRandom.size ()); jvResult[jss::random] = to_string (uRandom); jvResult[jss::server_status] = strOperatingMode (); diff --git a/src/ripple/app/shamap/SHAMapNodeID.cpp b/src/ripple/app/shamap/SHAMapNodeID.cpp index a1c44f024..9b6c1e807 100644 --- a/src/ripple/app/shamap/SHAMapNodeID.cpp +++ b/src/ripple/app/shamap/SHAMapNodeID.cpp @@ -64,7 +64,7 @@ SHAMapNodeID::calculate_hash (uint256 const& node, int depth) HashParams () : golden_ratio (0x9e3779b9) { - RandomNumbers::getInstance ().fill (&cookie_value); + random_fill (&cookie_value); } // The cookie value protects us against algorithmic complexity attacks. diff --git a/src/ripple/crypto/RandomNumbers.h b/src/ripple/crypto/RandomNumbers.h index 7292de374..4c908a0a6 100644 --- a/src/ripple/crypto/RandomNumbers.h +++ b/src/ripple/crypto/RandomNumbers.h @@ -20,72 +20,36 @@ #ifndef RIPPLE_CRYPTO_RANDOMNUMBERS_H_INCLUDED #define RIPPLE_CRYPTO_RANDOMNUMBERS_H_INCLUDED -#include +#include // namespace ripple { -/** Cryptographically secure random number source. +/** Adds entropy to the RNG pool. + + @param buffer An optional buffer that contains random data. + @param count The size of the buffer, in bytes (or 0). + + This can be called multiple times to stir entropy into the pool + without any locks. */ -class RandomNumbers +void add_entropy (void* buffer = nullptr, int count = 0); + +/** Generate random bytes, suitable for cryptography. */ +/**@{*/ +/** + @param buffer The place to store the bytes. + @param count The number of bytes to generate. +*/ +void random_fill (void* buffer, int count); + +/** Fills a POD object with random data */ +template ::value>> +void +random_fill (T* object) { -public: - /** Retrieve the instance of the generator. - */ - static RandomNumbers& getInstance (); - - /** Initialize the generator. - - If the generator is not manually initialized, it will be - automatically initialized on first use. If automatic initialization - fails, an exception is thrown. - - @return `true` if enough entropy could be retrieved. - */ - bool initialize (beast::Journal::Stream stream = beast::Journal::Stream()); - - /** Generate secure random numbers. - - The generated data is suitable for cryptography. - - @invariant The destination buffer must be large enough or - undefined behavior results. - - @param destinationBuffer The place to store the bytes. - @param numberOfBytes The number of bytes to generate. - */ - void fillBytes (void* destinationBuffer, int numberOfBytes); - - /** Generate secure random numbers. - - The generated data is suitable for cryptography. - - Fills the memory for the object with random numbers. - This is a type-safe alternative to the function above. - - @param object A pointer to the object to fill. - - @tparam T The type of `object` - - @note Undefined behavior results if `T` is not a POD type. - */ - template - void fill (T* object) - { - fillBytes (object, sizeof (T)); - } - -private: - RandomNumbers (); - - ~RandomNumbers (); - - bool platformAddEntropy (beast::Journal::Stream stream); - - void platformAddPerformanceMonitorEntropy (); - -private: - bool m_initialized; -}; + random_fill (object, sizeof (T)); +} +/**@}*/ } diff --git a/src/ripple/crypto/impl/ECIES.cpp b/src/ripple/crypto/impl/ECIES.cpp index 02784f4ea..44495119e 100644 --- a/src/ripple/crypto/impl/ECIES.cpp +++ b/src/ripple/crypto/impl/ECIES.cpp @@ -131,7 +131,7 @@ Blob encryptECIES (const openssl::ec_key& secretKey, const openssl::ec_key& publ { ECIES_ENC_IV_TYPE iv; - RandomNumbers::getInstance ().fillBytes (iv.begin (), ECIES_ENC_BLK_SIZE); + random_fill (iv.begin (), ECIES_ENC_BLK_SIZE); ECIES_ENC_KEY_TYPE secret; ECIES_HMAC_KEY_TYPE hmacKey; diff --git a/src/ripple/crypto/impl/RandomNumbers.cpp b/src/ripple/crypto/impl/RandomNumbers.cpp index a203583a1..68273f30b 100644 --- a/src/ripple/crypto/impl/RandomNumbers.cpp +++ b/src/ripple/crypto/impl/RandomNumbers.cpp @@ -18,226 +18,41 @@ //============================================================================== #include -#include -#include -#include #include -#if BEAST_WIN32 -#include -#include -#endif -#if BEAST_LINUX || BEAST_BSD || BEAST_MAC || BEAST_IOS -#include -#else -#include -#endif -#include + +#include +#include namespace ripple { -RandomNumbers::RandomNumbers () - : m_initialized (false) +void add_entropy (void* buffer, int count) { + assert (buffer == nullptr || count != 0); + + // If we are passed data in we use it but conservatively estimate that it + // contains only around 2 bits of entropy per byte. + if (buffer != nullptr && count != 0) + RAND_add (buffer, count, count / 4.0); + + // Try to add a bit more entropy from the system + unsigned int rdbuf[32]; + + std::random_device rd; + + for (auto& x : rdbuf) + x = rd (); + + // In all our supported platforms, std::random_device is non-deterministic + // but we conservatively estimate it has around 4 bits of entropy per byte. + RAND_add (rdbuf, sizeof (rdbuf), sizeof (rdbuf) / 2.0); } -RandomNumbers::~RandomNumbers () +void random_fill (void* buffer, int count) { -} + assert (count > 0); -bool RandomNumbers::initialize (beast::Journal::Stream stream) -{ - assert (!m_initialized); - - bool success = platformAddEntropy (stream); - - if (success) - m_initialized = true; - - return success; -} - -void RandomNumbers::fillBytes (void* destinationBuffer, int numberOfBytes) -{ - // VFALCO NOTE this assert is here to remind us that the code is not yet - // thread safe. - assert (m_initialized); - - // VFALCO NOTE When a spinlock is available in beast, use it here. - if (! m_initialized) - { - if (! initialize ()) - { - char const* message = "Unable to add system entropy"; - throw std::runtime_error (message); - } - } - -#ifdef PURIFY - memset (destinationBuffer, 0, numberOfBytes); -#endif - - if (RAND_bytes (reinterpret_cast (destinationBuffer), numberOfBytes) != 1) - { - assert (false); - - throw std::runtime_error ("Entropy pool not seeded"); - } -} - -RandomNumbers& RandomNumbers::getInstance () -{ - static RandomNumbers instance; - - return instance; -} - -//------------------------------------------------------------------------------ - -#if BEAST_WIN32 - -// Get entropy from the Windows crypto provider -bool RandomNumbers::platformAddEntropy (beast::Journal::Stream stream) -{ - char name[512], rand[128]; - DWORD count = 500; - HCRYPTPROV cryptoHandle; - - if (!CryptGetDefaultProviderA (PROV_RSA_FULL, nullptr, CRYPT_MACHINE_DEFAULT, name, &count)) - { - stream << "Unable to get default crypto provider"; - return false; - } - - if (!CryptAcquireContextA (&cryptoHandle, nullptr, name, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) - { - stream << "Unable to acquire crypto provider"; - return false; - } - - if (!CryptGenRandom (cryptoHandle, 128, reinterpret_cast (rand))) - { - stream << "Unable to get entropy from crypto provider"; - CryptReleaseContext (cryptoHandle, 0); - return false; - } - - CryptReleaseContext (cryptoHandle, 0); - RAND_seed (rand, 128); - - return true; -} - -#else - -bool RandomNumbers::platformAddEntropy (beast::Journal::Stream stream) -{ - char rand[128]; - std::ifstream reader; - - reader.open ("/dev/urandom", std::ios::in | std::ios::binary); - - if (!reader.is_open ()) - { -#ifdef BEAST_DEBUG - stream << "Unable to open random source"; -#endif - return false; - } - - reader.read (rand, 128); - - int bytesRead = reader.gcount (); - - if (bytesRead == 0) - { -#ifdef BEAST_DEBUG - stream << "Unable to read from random source"; -#endif - return false; - } - - RAND_seed (rand, bytesRead); - return bytesRead >= 64; -} - -#endif - -//------------------------------------------------------------------------------ - -// -// "Never go to sea with two chronometers; take one or three." -// Our three time sources are: -// - System clock -// - Median of other nodes's clocks -// - The user (asking the user to fix the system clock if the first two disagree) -// - -void RandomNumbers::platformAddPerformanceMonitorEntropy () -{ - // VFALCO TODO Remove all this fancy stuff - struct - { - std::int64_t operator () () const - { - return time (nullptr); - } - } GetTime; - - struct - { - void operator () () - { - struct - { - // VFALCO TODO clean this up - std::int64_t operator () () const - { - std::int64_t nCounter = 0; -#if BEAST_WIN32 - QueryPerformanceCounter ((LARGE_INTEGER*)&nCounter); -#else - timeval t; - gettimeofday (&t, nullptr); - nCounter = t.tv_sec * 1000000 + t.tv_usec; -#endif - return nCounter; - } - } GetPerformanceCounter; - - // Seed with CPU performance counter - std::int64_t nCounter = GetPerformanceCounter (); - RAND_add (&nCounter, sizeof (nCounter), 1.5); - memset (&nCounter, 0, sizeof (nCounter)); - } - } RandAddSeed; - - RandAddSeed (); - - // This can take up to 2 seconds, so only do it every 10 minutes - static std::int64_t nLastPerfmon; - - if (GetTime () < nLastPerfmon + 10 * 60) - return; - - nLastPerfmon = GetTime (); - -#if BEAST_WIN32 - // Don't need this on Linux, OpenSSL automatically uses /dev/urandom - // Seed with the entire set of perfmon data - unsigned char pdata[250000]; - memset (pdata, 0, sizeof (pdata)); - unsigned long nSize = sizeof (pdata); - long ret = RegQueryValueExA (HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, pdata, &nSize); - RegCloseKey (HKEY_PERFORMANCE_DATA); - - if (ret == ERROR_SUCCESS) - { - RAND_add (pdata, nSize, nSize / 100.0); - memset (pdata, 0, nSize); - //printf("%s RandAddSeed() %d bytes\n", DateTimeStrFormat("%x %H:%M", GetTime()).c_str(), nSize); - } - -#endif + if (RAND_bytes (reinterpret_cast (buffer), count) != 1) + throw std::runtime_error ("Insufficient entropy in pool."); } } diff --git a/src/ripple/net/impl/SNTPClient.cpp b/src/ripple/net/impl/SNTPClient.cpp index 0af7925f0..3246aaf20 100644 --- a/src/ripple/net/impl/SNTPClient.cpp +++ b/src/ripple/net/impl/SNTPClient.cpp @@ -231,7 +231,7 @@ public: query.mReceivedReply = false; query.mLocalTimeSent = now; - RandomNumbers::getInstance ().fill (&query.mQueryNonce); + random_fill (&query.mQueryNonce); reinterpret_cast (SNTPQueryData)[NTP_OFF_XMITTS_INT] = static_cast (time (nullptr)) + NTP_UNIX_OFFSET; reinterpret_cast (SNTPQueryData)[NTP_OFF_XMITTS_FRAC] = query.mQueryNonce; mSocket.async_send_to (boost::asio::buffer (SNTPQueryData, 48), *sel, diff --git a/src/ripple/protocol/impl/RippleAddress.cpp b/src/ripple/protocol/impl/RippleAddress.cpp index ee7238474..eb011aa60 100644 --- a/src/ripple/protocol/impl/RippleAddress.cpp +++ b/src/ripple/protocol/impl/RippleAddress.cpp @@ -867,7 +867,7 @@ void RippleAddress::setSeedRandom () // XXX Maybe we should call MakeNewKey uint128 key; - RandomNumbers::getInstance ().fillBytes (key.begin (), key.size ()); + random_fill (key.begin (), key.size ()); RippleAddress::setSeed (key); } diff --git a/src/ripple/rpc/handlers/Random.cpp b/src/ripple/rpc/handlers/Random.cpp index e0a7acae0..d4ed196df 100644 --- a/src/ripple/rpc/handlers/Random.cpp +++ b/src/ripple/rpc/handlers/Random.cpp @@ -33,7 +33,7 @@ Json::Value doRandom (RPC::Context& context) try { uint256 rand; - RandomNumbers::getInstance ().fillBytes (rand.begin (), rand.size ()); + random_fill (rand.begin (), rand.size ()); Json::Value jvResult; jvResult["random"] = to_string (rand);