From dba764386b38a5b4ac974eb74f142c6ae107acb0 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 21 Jul 2020 14:24:50 -0700 Subject: [PATCH] GH-1076: Fix use of getrandom() The availability and use of getrandom() actually caused unrandom and deterministic results in terms of Zeek's random number generation. --- src/util.cc | 20 ++++++------- testing/btest/Baseline/bifs.rand/out.3 | 1 + testing/btest/bifs/rand.zeek | 41 ++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 testing/btest/Baseline/bifs.rand/out.3 diff --git a/src/util.cc b/src/util.cc index 80af723c72..4b84cdee7f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1105,16 +1105,15 @@ void init_random_seed(const char* read_file, const char* write_file, else if ( use_empty_seeds ) seeds_done = true; + if ( ! seeds_done ) + { #ifdef HAVE_GETRANDOM - if ( ! seeds_done ) - { - ssize_t nbytes = getrandom(buf.data(), sizeof(buf), 0); - seeds_done = nbytes == ssize_t(sizeof(buf)); - } -#endif - - if ( ! seeds_done ) - { + // getrandom() guarantees reads up to 256 bytes are always successful, + assert(sizeof(buf) < 256); + auto nbytes = getrandom(buf.data(), sizeof(buf), 0); + assert(nbytes == sizeof(buf)); + pos += nbytes / sizeof(uint32_t); +#else // Gather up some entropy. gettimeofday((struct timeval *)(buf.data() + pos), 0); pos += sizeof(struct timeval) / sizeof(uint32_t); @@ -1141,9 +1140,10 @@ void init_random_seed(const char* read_file, const char* write_file, // systems due to a lack of entropy. errno = 0; } +#endif if ( pos < KeyedHash::SEED_INIT_SIZE ) - reporter->FatalError("Could not read enough random data from /dev/urandom. Wanted %d, got %lu", KeyedHash::SEED_INIT_SIZE, pos); + reporter->FatalError("Could not read enough random data. Wanted %d, got %lu", KeyedHash::SEED_INIT_SIZE, pos); if ( ! seed ) { diff --git a/testing/btest/Baseline/bifs.rand/out.3 b/testing/btest/Baseline/bifs.rand/out.3 new file mode 100644 index 0000000000..cf84443e49 --- /dev/null +++ b/testing/btest/Baseline/bifs.rand/out.3 @@ -0,0 +1 @@ +F diff --git a/testing/btest/bifs/rand.zeek b/testing/btest/bifs/rand.zeek index b4b0facabc..2254cf912a 100644 --- a/testing/btest/bifs/rand.zeek +++ b/testing/btest/bifs/rand.zeek @@ -1,13 +1,19 @@ # # @TEST-EXEC: zeek -b %INPUT >out # @TEST-EXEC: zeek -b %INPUT do_seed=F >out.2 +# @TEST-EXEC: unset ZEEK_SEED_FILE && zeek -b %INPUT real_random=T >out.3 # @TEST-EXEC: btest-diff out # @TEST-EXEC: btest-diff out.2 +# @TEST-EXEC: btest-diff out.3 const do_seed = T &redef; +const real_random = F &redef; event zeek_init() { + if ( real_random ) + return; + local a = rand(1000); local b = rand(1000); local c = rand(1000); @@ -27,3 +33,38 @@ event zeek_init() print e; print f; } + +event zeek_init() &priority=-10 + { + if ( ! real_random ) + return; + + local v1: vector of count = vector(); + local v2: vector of count = vector(); + local i = 0; + + while ( i < 20 ) + { + v1 += rand(65535); + i += 1; + } + + i = 0; + + while ( i < 20 ) + { + v2 += rand(65535); + i += 1; + } + + # Note: this is expected to be F with high probability, but + # technically could all be the same because, well, that's a + # valid "random" sequence, too + print all_set(v1 == v2); + + if ( all_set(v1 == v2) ) + { + print v1; + print v2; + } + }