Use cryptographically-secure pseudo-random generator everywhere

It was discovered in an upcoming academic paper that a xoshiro128**
internal state can be recovered by an external 3rd party allowing to
predict UDP ports and DNS IDs in the outgoing queries.  This could lead
to an attacker spoofing the DNS answers with great efficiency and
poisoning the DNS cache.

Change the internal random generator to system CSPRNG with buffering to
avoid excessive syscalls.

Thanks Omer Ben Simhon and Amit Klein of Hebrew University of Jerusalem
for responsibly reporting this to us.  Very cool research!

diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h
index 213e979ec6bc78510ea3b092e842f4ba045fd161..f7638905c1f680e47f9f4730533dedea3d0a9edf 100644 (file)
--- a/lib/isc/include/isc/random.h
+++ b/lib/isc/include/isc/random.h
@@ -19,7 +19,7 @@
 #include <isc/types.h>
 
 /*! \file isc/random.h
- * \brief Implements wrapper around a non-cryptographically secure
+ * \brief Implements wrapper around a cryptographically secure
  * pseudo-random number generator.
  *
  */

diff --git a/lib/isc/random.c b/lib/isc/random.c
index 88efd21c4e6f48165cbc843417bdde42a6c697a8..666975c119ee87dc19d7904895e64c6b8b7e7561 100644 (file)
--- a/lib/isc/random.c
+++ b/lib/isc/random.c
@@ -31,125 +31,66 @@
  */
 
 #include <inttypes.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+#include <stdio.h>
 
 #include <isc/entropy.h>
+#include <isc/os.h>
 #include <isc/random.h>
-#include <isc/result.h>
 #include <isc/thread.h>
-#include <isc/types.h>
 #include <isc/util.h>
 
-/*
- * Written in 2018 by David Blackman and Sebastiano Vigna (vigna@acm.org)
- *
- * To the extent possible under law, the author has dedicated all
- * copyright and related and neighboring rights to this software to the
- * public domain worldwide. This software is distributed without any
- * warranty.
- *
- * See <http://creativecommons.org/publicdomain/zero/1.0/>.
- */
-
-/*
- * This is xoshiro128** 1.0, our 32-bit all-purpose, rock-solid generator.
- * It has excellent (sub-ns) speed, a state size (128 bits) that is large
- * enough for mild parallelism, and it passes all tests we are aware of.
- *
- * The state must be seeded so that it is not everywhere zero.
- */
-
-static thread_local bool initialized = false;
-static thread_local uint32_t seed[4] = { 0 };
+#define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t))
 
-static uint32_t
-rotl(const uint32_t x, int k) {
-       return (x << k) | (x >> (32 - k));
-}
+thread_local static uint32_t isc__random_pool[ISC_RANDOM_BUFSIZE];
+thread_local static size_t isc__random_pos = ISC_RANDOM_BUFSIZE;
 
 static uint32_t
-next(void) {
-       uint32_t result_starstar, t;
-
-       result_starstar = rotl(seed[0] * 5, 7) * 9;
-       t = seed[1] << 9;
-
-       seed[2] ^= seed[0];
-       seed[3] ^= seed[1];
-       seed[1] ^= seed[2];
-       seed[0] ^= seed[3];
-
-       seed[2] ^= t;
-
-       seed[3] = rotl(seed[3], 11);
-
-       return result_starstar;
-}
-
-static void
-isc__random_initialize(void) {
-       if (initialized) {
-               return;
-       }
-
+random_u32(void) {
 #if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
        /*
-        * A fixed seed helps with problem reproduction when fuzzing. It must be
-        * non-zero else xoshiro128starstar will generate only zeroes, and the
-        * first result needs to be non-zero as expected by random_test.c
+        * A fixed stream of numbers helps with problem reproduction when
+        * fuzzing.  The first result needs to be non-zero as expected by
+        * random_test.c (it starts with ISC_RANDOM_BUFSIZE, see above).
         */
-       seed[0] = 1;
+       return (uint32_t)(isc__random_pos++);
 #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */
 
-       while (seed[0] == 0 && seed[1] == 0 && seed[2] == 0 && seed[3] == 0) {
-               isc_entropy_get(seed, sizeof(seed));
+       if (isc__random_pos == ISC_RANDOM_BUFSIZE) {
+               isc_entropy_get(isc__random_pool, sizeof(isc__random_pool));
+               isc__random_pos = 0;
        }
-       initialized = true;
+
+       return isc__random_pool[isc__random_pos++];
 }
 
 uint8_t
 isc_random8(void) {
-       isc__random_initialize();
-       return (uint8_t)next();
+       return (uint8_t)random_u32();
 }
 
 uint16_t
 isc_random16(void) {
-       isc__random_initialize();
-       return (uint16_t)next();
+       return (uint16_t)random_u32();
 }
 
 uint32_t
 isc_random32(void) {
-       isc__random_initialize();
-       return next();
+       return random_u32();
 }
 
 void
 isc_random_buf(void *buf, size_t buflen) {
-       REQUIRE(buf != NULL);
-       REQUIRE(buflen > 0);
-
-       int i;
-       uint32_t r;
-
-       isc__random_initialize();
+       REQUIRE(buflen == 0 || buf != NULL);
 
-       for (i = 0; i + sizeof(r) <= buflen; i += sizeof(r)) {
-               r = next();
-               memmove((uint8_t *)buf + i, &r, sizeof(r));
+       if (buf == NULL || buflen == 0) {
+               return;
        }
-       r = next();
-       memmove((uint8_t *)buf + i, &r, buflen % sizeof(r));
-       return;
+
+       isc_entropy_get(buf, buflen);
 }
 
 uint32_t
 isc_random_uniform(uint32_t limit) {
-       isc__random_initialize();
-
        /*
         * Daniel Lemire's nearly-divisionless unbiased bounded random numbers.
         *
@@ -161,7 +102,7 @@ isc_random_uniform(uint32_t limit) {
         * integer part (upper 32 bits), and we will use the fraction part
         * (lower 32 bits) to determine whether or not we need to resample.
         */
-       uint64_t num = (uint64_t)next() * (uint64_t)limit;
+       uint64_t num = (uint64_t)random_u32() * (uint64_t)limit;
        /*
         * In the fast path, we avoid doing a division in most cases by
         * comparing the fraction part of `num` with the limit, which is
@@ -213,7 +154,7 @@ isc_random_uniform(uint32_t limit) {
                 * our valid range, it is superfluous, and we resample.
                 */
                while ((uint32_t)(num) < residue) {
-                       num = (uint64_t)next() * (uint64_t)limit;
+                       num = (uint64_t)random_u32() * (uint64_t)limit;
                }
        }
        /*

diff --git a/tests/isc/random_test.c b/tests/isc/random_test.c
index 70bd4b6a79eab3ab09fc02eb0343742033aa554b..a8cce6ceca347374e763f6d8398ebf736506312f 100644 (file)
--- a/tests/isc/random_test.c
+++ b/tests/isc/random_test.c
@@ -321,7 +321,9 @@ random_test(pvalue_func_t *func, isc_random_func test_func) {
                        }
                        break;
                case ISC_RANDOM_BYTES:
-                       isc_random_buf(values, sizeof(values));
+                       for (i = 0; i < ARRAY_SIZE(values); i++) {
+                               values[i] = isc_random32();
+                       }
                        break;
                case ISC_RANDOM_UNIFORM:
                        uniform_values = (uint16_t *)values;