From 6254679e5250a9f0d6079ec082cffdad4315372d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 19 Aug 2025 19:22:18 +0200 Subject: [PATCH] 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! (cherry picked from commit cffcab9d5f3e709002f331b72498fcc229786ae2) (cherry picked from commit d9b5ef342916462bfd63391831d96afc80c12df3) --- lib/isc/include/isc/os.h | 5 + lib/isc/include/isc/random.h | 2 +- lib/isc/random.c | 181 ++++++++++++++++++----------------- lib/isc/tests/random_test.c | 4 +- 4 files changed, 102 insertions(+), 90 deletions(-) diff --git a/lib/isc/include/isc/os.h b/lib/isc/include/isc/os.h index ce7615a0e3..5def473d2b 100644 --- a/lib/isc/include/isc/os.h +++ b/lib/isc/include/isc/os.h @@ -18,6 +18,11 @@ ISC_LANG_BEGINDECLS +/*%< + * Hardcode the L1 cacheline size of the CPU to 64. + */ +#define ISC_OS_CACHELINE_SIZE 64 + unsigned int isc_os_ncpus(void); /*%< diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h index 556e74754a..2c16472a5d 100644 --- a/lib/isc/include/isc/random.h +++ b/lib/isc/include/isc/random.h @@ -18,7 +18,7 @@ #include /*! \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 753453ff3c..717e1f0dcd 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -29,131 +29,136 @@ */ #include -#include -#include -#include +#include -#include +#include #include #include -#include #include -#include #include #include "entropy_private.h" -/* - * The specific implementation for PRNG is included as a C file - * that has to provide a static variable named seed, and a function - * uint32_t next(void) that provides next random number. - * - * The implementation must be thread-safe. - */ - -/* - * Two contestants have been considered: the xoroshiro family of the - * functions by Villa&Blackman, and PCG by O'Neill. After - * consideration, the xoshiro128starstar function has been chosen as - * the uint32_t random number provider because it is very fast and has - * good enough properties for our usage pattern. - */ -#include "xoshiro128starstar.c" +#define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t)) -ISC_THREAD_LOCAL isc_once_t isc_random_once = ISC_ONCE_INIT; +ISC_THREAD_LOCAL uint32_t isc__random_pool[ISC_RANDOM_BUFSIZE]; +ISC_THREAD_LOCAL size_t isc__random_pos = ISC_RANDOM_BUFSIZE; -static void -isc_random_initialize(void) { - int useed[4] = { 0, 0, 0, 1 }; +static uint32_t +random_u32(void) { #if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION /* - * Set a constant seed to help in problem reproduction should fuzzing - * find a crash or a hang. The seed array must be non-zero else - * xoshiro128starstar will generate an infinite series of zeroes. + * 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). */ -#else /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ - isc_entropy_get(useed, sizeof(useed)); + return (uint32_t)(isc__random_pos++); #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ - memmove(seed, useed, sizeof(seed)); + + if (isc__random_pos == ISC_RANDOM_BUFSIZE) { + isc_entropy_get(isc__random_pool, sizeof(isc__random_pool)); + isc__random_pos = 0; + } + + return isc__random_pool[isc__random_pos++]; } uint8_t isc_random8(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return (next() & 0xff); + return (uint8_t)random_u32(); } uint16_t isc_random16(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return (next() & 0xffff); + return (uint16_t)random_u32(); } uint32_t isc_random32(void) { - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - return (next()); + return random_u32(); } void isc_random_buf(void *buf, size_t buflen) { - int i; - uint32_t r; + REQUIRE(buflen == 0 || buf != NULL); - REQUIRE(buf != NULL); - REQUIRE(buflen > 0); - - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - - 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 upper_bound) { - /* Copy of arc4random_uniform from OpenBSD */ - uint32_t r, min; - - RUNTIME_CHECK(isc_once_do(&isc_random_once, isc_random_initialize) == - ISC_R_SUCCESS); - - if (upper_bound < 2) { - return (0); - } - -#if (ULONG_MAX > 0xffffffffUL) - min = 0x100000000UL % upper_bound; -#else /* if (ULONG_MAX > 0xffffffffUL) */ - /* Calculate (2**32 % upper_bound) avoiding 64-bit math */ - if (upper_bound > 0x80000000) { - min = 1 + ~upper_bound; /* 2**32 - upper_bound */ - } else { - /* (2**32 - (x * 2)) % x == 2**32 % x when x <= 2**31 */ - min = ((0xffffffff - (upper_bound * 2)) + 1) % upper_bound; - } -#endif /* if (ULONG_MAX > 0xffffffffUL) */ - +isc_random_uniform(uint32_t limit) { /* - * This could theoretically loop forever but each retry has - * p > 0.5 (worst case, usually far better) of selecting a - * number inside the range we need, so it should rarely need - * to re-roll. + * Daniel Lemire's nearly-divisionless unbiased bounded random numbers. + * + * https://lemire.me/blog/?p=17551 + * + * The raw random number generator `next()` returns a 32-bit value. + * We do a 64-bit multiply `next() * limit` and treat the product as a + * 32.32 fixed-point value less than the limit. Our result will be the + * integer part (upper 32 bits), and we will use the fraction part + * (lower 32 bits) to determine whether or not we need to resample. */ - for (;;) { - r = next(); - if (r >= min) { - break; + 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 + * a slight over-estimate for the exact resample threshold. + */ + if ((uint32_t)(num) < limit) { + /* + * We are in the slow path where we re-do the approximate test + * more accurately. The exact threshold for the resample loop + * is the remainder after dividing the raw RNG limit `1 << 32` + * by the caller's limit. We use a trick to calculate it + * within 32 bits: + * + * (1 << 32) % limit + * == ((1 << 32) - limit) % limit + * == (uint32_t)(-limit) % limit + * + * This division is safe: we know that `limit` is strictly + * greater than zero because of the slow-path test above. + */ + uint32_t residue = (uint32_t)(-limit) % limit; + /* + * Unless we get one of `N = (1 << 32) - residue` valid + * values, we reject the sample. This `N` is a multiple of + * `limit`, so our results will be unbiased; and `N` is the + * largest multiple that fits in 32 bits, so rejections are as + * rare as possible. + * + * There are `limit` possible values for the integer part of + * our fixed-point number. Each one corresponds to `N/limit` + * or `N/limit + 1` possible fraction parts. For our result to + * be unbiased, every possible integer part must have the same + * number of possible valid fraction parts. So, when we get + * the superfluous value in the `N/limit + 1` cases, we need + * to reject and resample. + * + * Because of the multiplication, the possible values in the + * fraction part are equally spaced by `limit`, with varying + * gaps at each end of the fraction's 32-bit range. We will + * choose a range of size `N` (a multiple of `limit`) into + * which valid fraction values must fall, with the rest of the + * 32-bit range covered by the `residue`. Lemire's paper says + * that exactly `N/limit` possible values spaced apart by + * `limit` will fit into our size `N` valid range, regardless + * of the size of the end gaps, the phase alignment of the + * values, or the position of the range. + * + * So, when a fraction value falls in the `residue` outside + * our valid range, it is superfluous, and we resample. + */ + while ((uint32_t)(num) < residue) { + num = (uint64_t)random_u32() * (uint64_t)limit; } } - - return (r % upper_bound); + /* + * Return the integer part (upper 32 bits). + */ + return (uint32_t)(num >> 32); } diff --git a/lib/isc/tests/random_test.c b/lib/isc/tests/random_test.c index 7161cd96a9..f47137d3ac 100644 --- a/lib/isc/tests/random_test.c +++ b/lib/isc/tests/random_test.c @@ -345,7 +345,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; -- 2.51.1