From 81bdd5f61a8f8da53e22d68190be700902b7dbae Mon Sep 17 00:00:00 2001 From: Nils Maier Date: Tue, 30 Dec 2014 18:17:44 +0100 Subject: [PATCH] Revise getRandom facilities Use one of the following to provide random bytes: - Windows CryptGenRandom - Linux getrandom (syscall interface to urandom, without nasty corner cases such as file descriptor exhaustion or re-linked /dev/urandom) - std::device_random (C++ random device, which usually will be urandom) This also equalizes util::getRandom and SimpleRandomizer (the former will now use the latter) instead of having essentially two different PRNG interfaces with potentially different quality. Closes GH-320 --- configure.ac | 14 +++++++ src/Context.cc | 1 - src/Makefile.am | 4 ++ src/SimpleRandomizer.cc | 82 +++++++++++++++-------------------------- src/SimpleRandomizer.h | 51 +++++++++++++++++++------ src/getrandom_linux.c | 50 +++++++++++++++++++++++++ src/getrandom_linux.h | 49 ++++++++++++++++++++++++ src/util.cc | 39 +------------------- test/UtilTest.cc | 48 +++++++++++++++++++++++- 9 files changed, 234 insertions(+), 104 deletions(-) create mode 100644 src/getrandom_linux.c create mode 100644 src/getrandom_linux.h diff --git a/configure.ac b/configure.ac index 18c8374b..41ca35df 100644 --- a/configure.ac +++ b/configure.ac @@ -720,6 +720,7 @@ AC_CHECK_FUNCS([__argz_count \ gethostbyname \ getifaddrs \ getpagesize \ + getrandom \ memchr \ memmove \ mempcpy \ @@ -755,6 +756,19 @@ AC_CHECK_FUNCS([__argz_count \ utime \ utimes]) +AC_MSG_CHECKING([for getrandom linux syscall interface]) +AC_LINK_IFELSE([AC_LANG_PROGRAM([[ +#include +]], +[[ +int x = GRND_NONBLOCK; +]])], + [have_getrandom_interface=yes + AC_DEFINE([HAVE_GETRANDOM_INTERFACE], [1], [Define to 1 if getrandom linux syscall interface is available.])], + [have_getrandom_interface=no]) +AC_MSG_RESULT([$have_getrandom_interface]) +AM_CONDITIONAL([HAVE_GETRANDOM_INTERFACE], [test "x$have_getrandom_interface" = "xyes"]) + dnl Put tcmalloc/jemalloc checks after the posix_memalign check. dnl These libraries may implement posix_memalign, while the usual CRT may not dnl (e.g. mingw). Since we aren't including the corresponding library headers diff --git a/src/Context.cc b/src/Context.cc index f538fabd..204e0fc8 100644 --- a/src/Context.cc +++ b/src/Context.cc @@ -163,7 +163,6 @@ Context::Context(bool standalone, throw DL_ABORT_EX("Option processing failed"); } } - SimpleRandomizer::getInstance()->init(); #ifdef ENABLE_BITTORRENT bittorrent::generateStaticPeerId(op->get(PREF_PEER_ID_PREFIX)); #endif // ENABLE_BITTORRENT diff --git a/src/Makefile.am b/src/Makefile.am index cd49f94f..236a055e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -687,6 +687,10 @@ if HAVE_KQUEUE SRCS += KqueueEventPoll.cc KqueueEventPoll.h endif # HAVE_KQUEUE +if HAVE_GETRANDOM_INTERFACE +SRCS += getrandom_linux.c getrandom_linux.h +endif # HAVE_GETRANDOM_INTERFACE + if HAVE_LIBUV SRCS += LibuvEventPoll.cc LibuvEventPoll.h endif # HAVE_LIBUV diff --git a/src/SimpleRandomizer.cc b/src/SimpleRandomizer.cc index 713a5b5e..746491f1 100644 --- a/src/SimpleRandomizer.cc +++ b/src/SimpleRandomizer.cc @@ -43,6 +43,10 @@ #include "a2time.h" #include "a2functional.h" +#ifdef HAVE_GETRANDOM_INTERFACE +# include "getrandom_linux.h" +#endif + namespace aria2 { std::unique_ptr SimpleRandomizer::randomizer_; @@ -55,75 +59,49 @@ const std::unique_ptr& SimpleRandomizer::getInstance() return randomizer_; } -void SimpleRandomizer::init() -{ -#ifndef __MINGW32__ - // Just in case std::random_device() is fixed, add time and pid too. - eng_.seed(std::random_device()()^time(nullptr)^getpid()); -#endif // !__MINGW32__ -} - SimpleRandomizer::SimpleRandomizer() { #ifdef __MINGW32__ - BOOL r = CryptAcquireContext(&cryProvider_, 0, 0, PROV_RSA_FULL, - CRYPT_VERIFYCONTEXT|CRYPT_SILENT); + BOOL r = ::CryptAcquireContext( + &provider_, + 0, 0, PROV_RSA_FULL, + CRYPT_VERIFYCONTEXT | CRYPT_SILENT); assert(r); -#endif // __MINGW32__ +#endif } SimpleRandomizer::~SimpleRandomizer() { #ifdef __MINGW32__ - CryptReleaseContext(cryProvider_, 0); -#endif // __MINGW32__ + CryptReleaseContext(provider_, 0); +#endif } long int SimpleRandomizer::getRandomNumber(long int to) { assert(to > 0); + return std::uniform_int_distribution(0, to - 1)(*this); +} + +void SimpleRandomizer::getRandomBytes(unsigned char* buf, size_t len) +{ #ifdef __MINGW32__ - int32_t val; - BOOL r = CryptGenRandom(cryProvider_, sizeof(val), - reinterpret_cast(&val)); + BOOL r = CryptGenRandom(provider_, len, reinterpret_cast(buf)); assert(r); - if(val == INT32_MIN) { - val = INT32_MAX; - } else if(val < 0) { - val = -val; +#elif defined(HAVE_GETRANDOM_INTERFACE) + auto rv = getrandom_linux(buf, len); + assert(rv >= 0 && (size_t)rv == len); +#else // ! __MINGW32__ + auto ubuf = reinterpret_cast(buf); + size_t q = len / sizeof(result_type); + auto gen = std::uniform_int_distribution(); + for(; q > 0; --q, ++ubuf) { + *ubuf = gen(dev_); } - return val % to; -#else // !__MINGW32__ - return std::uniform_int_distribution(0, to - 1)(eng_); -#endif // !__MINGW32__ -} - -long int SimpleRandomizer::operator()(long int to) -{ - return getRandomNumber(to); -} - -void SimpleRandomizer::getRandomBytes(unsigned char *buf, size_t len) -{ -#ifdef __MINGW32__ - if (!CryptGenRandom(cryProvider_, len, (PBYTE)buf)) { - throw std::bad_alloc(); - } -#else - uint32_t val; - size_t q = len / sizeof(val); - size_t r = len % sizeof(val); - auto gen = std::bind(std::uniform_int_distribution - (0, std::numeric_limits::max()), - eng_); - for(; q > 0; --q) { - val = gen(); - memcpy(buf, &val, sizeof(val)); - buf += sizeof(val); - } - val = gen(); - memcpy(buf, &val, r); -#endif + const size_t r = len % sizeof(result_type); + auto last = gen(dev_); + memcpy(ubuf, &last, r); +#endif // ! __MINGW32__ } } // namespace aria2 diff --git a/src/SimpleRandomizer.h b/src/SimpleRandomizer.h index fa576306..65ac2094 100644 --- a/src/SimpleRandomizer.h +++ b/src/SimpleRandomizer.h @@ -41,30 +41,32 @@ #include #ifdef __MINGW32__ -# include -#endif // __MINGW32__ +# include +#endif namespace aria2 { class SimpleRandomizer : public Randomizer { private: static std::unique_ptr randomizer_; - -#ifdef __MINGW32__ - HCRYPTPROV cryProvider_; -#else // !__MINGW32__ - std::minstd_rand eng_; -#endif //!__MINGW32__ - SimpleRandomizer(); + +private: +#ifdef __MINGW32__ + HCRYPTPROV provider_; +#elif defined(HAVE_GETRANDOM_INTERFACE) + // Nothing special needed +#else + std::random_device dev_; +#endif // ! __MINGW32__ + public: + typedef std::random_device::result_type result_type; static const std::unique_ptr& getInstance(); virtual ~SimpleRandomizer(); - void init(); - /** * Returns random number in [0, to). */ @@ -72,7 +74,32 @@ public: void getRandomBytes(unsigned char *buf, size_t len); - long int operator()(long int to); + long int operator()(long int to) + { + return getRandomNumber(to); + } + + result_type operator()() + { + result_type rv; + getRandomBytes(reinterpret_cast(&rv), sizeof(rv)); + return rv; + } + + static constexpr result_type min() + { + return std::numeric_limits::min(); + } + + static constexpr result_type max() + { + return std::numeric_limits::max(); + } + + static double entropy() + { + return 0.0; + } }; } // namespace aria2 diff --git a/src/getrandom_linux.c b/src/getrandom_linux.c new file mode 100644 index 00000000..85314fd6 --- /dev/null +++ b/src/getrandom_linux.c @@ -0,0 +1,50 @@ +/* */ + +#define _GNUSOURCE +#include +#include +#include + +#include "config.h" +#include "getrandom_linux.h" + +int getrandom_linux(void *buf, size_t buflen) { +#ifdef HAVE_GETRANDOM + return getrandom(buf, buflen, 0); +#else // HAVE_GETRANDOM + return syscall(SYS_getrandom, buf, buflen, 0); +#endif // HAVE_GETRANDOM +} diff --git a/src/getrandom_linux.h b/src/getrandom_linux.h new file mode 100644 index 00000000..d1d935c7 --- /dev/null +++ b/src/getrandom_linux.h @@ -0,0 +1,49 @@ +/* */ + +#ifndef D_GETRANDOM_LINUX_H +#define D_GETRANDOM_LINUX_H + +#ifdef __cplusplus +extern "C" { +#endif + +int getrandom_linux(void *buf, size_t buflen); + +#ifdef __cplusplus +} +#endif + +#endif /* D_GETRANDOM_LINUX_H */ diff --git a/src/util.cc b/src/util.cc index 9332ae47..ea2767f7 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1545,45 +1545,10 @@ std::vector > createIndexPaths(std::istream& i) return indexPaths; } -namespace { -void generateRandomDataRandom(unsigned char* data, size_t length) -{ - const auto& rd = SimpleRandomizer::getInstance(); - rd->getRandomBytes(data, length); -} -} // namespace - -#ifndef __MINGW32__ -namespace { -void generateRandomDataUrandom -(unsigned char* data, size_t length, std::ifstream& devUrand) -{ - devUrand.read(reinterpret_cast(data), length); -} -} // namespace -#endif - void generateRandomData(unsigned char* data, size_t length) { -#ifdef __MINGW32__ - generateRandomDataRandom(data, length); -#else // !__MINGW32__ - static int method = -1; - static std::ifstream devUrand; - if(method == 0) { - generateRandomDataUrandom(data, length, devUrand); - } else if(method == 1) { - generateRandomDataRandom(data, length); - } else { - devUrand.open("/dev/urandom"); - if(devUrand) { - method = 0; - } else { - method = 1; - } - generateRandomData(data, length); - } -#endif // !__MINGW32__ + const auto& rd = SimpleRandomizer::getInstance(); + return rd->getRandomBytes(data, length); } bool saveAs diff --git a/test/UtilTest.cc b/test/UtilTest.cc index 65b1d9c5..513aeefc 100644 --- a/test/UtilTest.cc +++ b/test/UtilTest.cc @@ -1,5 +1,6 @@ #include "util.h" +#include #include #include #include @@ -1954,11 +1955,54 @@ void UtilTest::testCreateIndexPaths() void UtilTest::testGenerateRandomData() { - unsigned char data1[20]; + using namespace std; + + // Simple sanity check + unsigned char data1[25]; + memset(data1, 0, sizeof(data1)); util::generateRandomData(data1, sizeof(data1)); - unsigned char data2[20]; + + unsigned char data2[25]; + memset(data2, 0, sizeof(data2)); util::generateRandomData(data2, sizeof(data2)); + CPPUNIT_ASSERT(memcmp(data1, data2, sizeof(data1)) != 0); + + // Simple stddev/mean tests + map counts; + uint8_t bytes[1 << 20]; + for (auto i = 0; i < 10; ++i) { + util::generateRandomData(bytes, sizeof(bytes)); + for (auto b : bytes) { + counts[b]++; + } + } + CPPUNIT_ASSERT_MESSAGE("Should see all kinds of bytes", counts.size() == 256); + if (counts.size() != 256) { + throw std::domain_error( + "Would have expected to see at one of each possible byte value!"); + } + double sum = accumulate( + counts.begin(), + counts.end(), + 0.0, + [](double total, const decltype(counts)::value_type & elem) { + return total + elem.second; + }); + double mean = sum / counts.size(); + vector diff(counts.size()); + transform( + counts.begin(), + counts.end(), + diff.begin(), + [&](const decltype(counts)::value_type & elem) -> double { + return (double)elem.second - mean; + }); + double sq_sum = inner_product(diff.begin(), diff.end(), diff.begin(), 0.0); + double stddev = sqrt(sq_sum / counts.size()); + cout << "stddev: " << fixed << stddev << endl; + CPPUNIT_ASSERT_MESSAGE("stddev makes sense (lower)", stddev <= 320); + CPPUNIT_ASSERT_MESSAGE("stddev makes sense (upper)", stddev >= 100); } void UtilTest::testFromHex()