From 2924910eeea5c86720149bc48d799ccb69e59797 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 1/2] 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) --- lib/isc/include/isc/random.h | 2 +- lib/isc/random.c | 109 ++++++++--------------------------- tests/isc/random_test.c | 4 +- 3 files changed, 29 insertions(+), 86 deletions(-) diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h index 79f1f9de2c..577cf530e6 100644 --- a/lib/isc/include/isc/random.h +++ b/lib/isc/include/isc/random.h @@ -20,7 +20,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 88efd21c4e..666975c119 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -31,125 +31,66 @@ */ #include -#include -#include -#include +#include #include +#include #include -#include #include -#include #include -/* - * 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 . - */ +#define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t)) -/* - * 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 }; +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 -rotl(const uint32_t x, int k) { - return (x << k) | (x >> (32 - k)); -} - -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); + REQUIRE(buflen == 0 || buf != NULL); - int i; - uint32_t r; - - isc__random_initialize(); - - 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 21a33167d9..13fdb89c79 100644 --- a/tests/isc/random_test.c +++ b/tests/isc/random_test.c @@ -320,7 +320,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; From 26c77915d52a577be6f421fd351506c29185ab97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 25 Aug 2025 19:02:54 +0200 Subject: [PATCH 2/2] Use arc4random for CSPRNG when available Use arc4random on platforms where available. arc4random() provides high quality cryptographically-secure pseudo-random numbers and is generally recommended for application use. The uv_random() call unfortunately uses getentropy() on platforms like MacOS, OpenBSD or NetBSD which is not recommended for application use. (cherry picked from commit 4db9e5d90e290f77718f8e1e0a64c3e1a8fe34f9) --- configure.ac | 2 +- lib/isc/Makefile.am | 3 --- lib/isc/entropy.c | 24 ------------------------ lib/isc/hash.c | 3 +-- lib/isc/hashmap.c | 1 - lib/isc/include/isc/entropy.h | 35 ----------------------------------- lib/isc/include/isc/nonce.h | 7 +++++-- lib/isc/include/isc/random.h | 34 ++++++++++++++++++++++------------ lib/isc/nonce.c | 20 -------------------- lib/isc/random.c | 34 ++++++++++++---------------------- 10 files changed, 41 insertions(+), 122 deletions(-) delete mode 100644 lib/isc/entropy.c delete mode 100644 lib/isc/include/isc/entropy.h delete mode 100644 lib/isc/nonce.c diff --git a/configure.ac b/configure.ac index 34895e9b46..85c53a4d97 100644 --- a/configure.ac +++ b/configure.ac @@ -653,7 +653,7 @@ AS_IF([test "$enable_pthread_rwlock" = "yes"], ]) AM_CONDITIONAL([USE_ISC_RWLOCK], [test "$enable_pthread_rwlock" != "yes"]) -CRYPTO=OpenSSL +AC_CHECK_FUNCS_ONCE([arc4random]) # # OpenSSL/LibreSSL is mandatory diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index 1ea03acfb0..c0769ec527 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -21,7 +21,6 @@ libisc_la_HEADERS = \ include/isc/dir.h \ include/isc/dnsstream.h \ include/isc/endian.h \ - include/isc/entropy.h \ include/isc/errno.h \ include/isc/error.h \ include/isc/file.h \ @@ -127,7 +126,6 @@ libisc_la_SOURCES = \ counter.c \ crc64.c \ dir.c \ - entropy.c \ errno.c \ errno2result.c \ errno2result.h \ @@ -165,7 +163,6 @@ libisc_la_SOURCES = \ net.c \ netaddr.c \ netscope.c \ - nonce.c \ openssl_shim.c \ openssl_shim.h \ os.c \ diff --git a/lib/isc/entropy.c b/lib/isc/entropy.c deleted file mode 100644 index a037960bd1..0000000000 --- a/lib/isc/entropy.c +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#include -#include -#include -#include - -void -isc_entropy_get(void *buf, size_t buflen) { - int r = uv_random(NULL, NULL, buf, buflen, 0, NULL); - - UV_RUNTIME_CHECK(uv_random, r); -} diff --git a/lib/isc/hash.c b/lib/isc/hash.c index 387e2373cd..49b942ef5b 100644 --- a/lib/isc/hash.c +++ b/lib/isc/hash.c @@ -16,7 +16,6 @@ #include #include -#include #include /* IWYU pragma: keep */ #include #include @@ -35,7 +34,7 @@ isc__hash_initialize(void) { */ uint8_t key[16] = { 1 }; #if !FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - isc_entropy_get(key, sizeof(key)); + isc_random_buf(key, sizeof(key)); #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ STATIC_ASSERT(sizeof(key) >= sizeof(isc_hash_key), "sizeof(key) < sizeof(isc_hash_key)"); diff --git a/lib/isc/hashmap.c b/lib/isc/hashmap.c index d8c2bd58ae..87b1a3f21a 100644 --- a/lib/isc/hashmap.c +++ b/lib/isc/hashmap.c @@ -32,7 +32,6 @@ #include #include -#include #include #include #include diff --git a/lib/isc/include/isc/entropy.h b/lib/isc/include/isc/entropy.h deleted file mode 100644 index 4e2dc5f884..0000000000 --- a/lib/isc/include/isc/entropy.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#pragma once - -#include - -#include - -/*! \file isc/entropy.h - * \brief Implements wrapper around CSPRNG cryptographic library calls - * for getting cryptographically secure pseudo-random numbers. - * - * Uses synchronous version of uv_random(). - */ - -ISC_LANG_BEGINDECLS - -void -isc_entropy_get(void *buf, size_t buflen); -/*!< - * \brief Get cryptographically-secure pseudo-random data. - */ - -ISC_LANG_ENDDECLS diff --git a/lib/isc/include/isc/nonce.h b/lib/isc/include/isc/nonce.h index b593e41445..10fb8b10a0 100644 --- a/lib/isc/include/isc/nonce.h +++ b/lib/isc/include/isc/nonce.h @@ -16,6 +16,7 @@ #include #include +#include /*! \file isc/nonce.h * \brief Provides a function for generating an arbitrarily long nonce. @@ -23,8 +24,10 @@ ISC_LANG_BEGINDECLS -void -isc_nonce_buf(void *buf, size_t buflen); +static inline void +isc_nonce_buf(void *buf, size_t buflen) { + isc_random_buf(buf, buflen); +} /*!< * Fill 'buf', up to 'buflen' bytes, with random data from the * crypto provider's random function. diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h index 577cf530e6..78035fbe9a 100644 --- a/lib/isc/include/isc/random.h +++ b/lib/isc/include/isc/random.h @@ -27,18 +27,11 @@ ISC_LANG_BEGINDECLS -uint8_t -isc_random8(void); -/*!< - * \brief Returns a single 8-bit random value. - */ - -uint16_t -isc_random16(void); -/*!< - * \brief Returns a single 16-bit random value. - */ - +#if HAVE_ARC4RANDOM && !defined(__linux__) +#define isc_random32() arc4random() +#define isc_random_buf(buf, buflen) arc4random_buf(buf, buflen) +#define isc_random_uniform(upper_bound) arc4random_uniform(upper_bound) +#else /* HAVE_ARC4RANDOM && !defined(__linux__) */ uint32_t isc_random32(void); /*!< @@ -68,4 +61,21 @@ isc_random_uniform(uint32_t upper_bound); * when upper_bound is UINT32_MAX/2. */ +#endif /* HAVE_ARC4RANDOM && !defined(__linux__) */ + +static inline uint8_t +isc_random8(void) { + return (uint8_t)isc_random32(); +} +/*!< + * \brief Returns a single 8-bit random value. + */ + +static inline uint16_t +isc_random16(void) { + return (uint16_t)isc_random32(); +} +/*!< + * \brief Returns a single 16-bit random value. + */ ISC_LANG_ENDDECLS diff --git a/lib/isc/nonce.c b/lib/isc/nonce.c deleted file mode 100644 index 316498a613..0000000000 --- a/lib/isc/nonce.c +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#include -#include - -void -isc_nonce_buf(void *buf, size_t buflen) { - isc_entropy_get(buf, buflen); -} diff --git a/lib/isc/random.c b/lib/isc/random.c index 666975c119..1367fd5a12 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -30,22 +30,24 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#if !HAVE_ARC4RANDOM || defined(__linux__) + #include #include -#include #include #include #include #include +#include #define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t)) 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 -random_u32(void) { +uint32_t +isc_random32(void) { #if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION /* * A fixed stream of numbers helps with problem reproduction when @@ -56,28 +58,13 @@ random_u32(void) { #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ if (isc__random_pos == ISC_RANDOM_BUFSIZE) { - isc_entropy_get(isc__random_pool, sizeof(isc__random_pool)); + isc_random_buf(isc__random_pool, sizeof(isc__random_pool)); isc__random_pos = 0; } return isc__random_pool[isc__random_pos++]; } -uint8_t -isc_random8(void) { - return (uint8_t)random_u32(); -} - -uint16_t -isc_random16(void) { - return (uint16_t)random_u32(); -} - -uint32_t -isc_random32(void) { - return random_u32(); -} - void isc_random_buf(void *buf, size_t buflen) { REQUIRE(buflen == 0 || buf != NULL); @@ -86,7 +73,8 @@ isc_random_buf(void *buf, size_t buflen) { return; } - isc_entropy_get(buf, buflen); + int r = uv_random(NULL, NULL, buf, buflen, 0, NULL); + UV_RUNTIME_CHECK(uv_random, r); } uint32_t @@ -102,7 +90,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)random_u32() * (uint64_t)limit; + uint64_t num = (uint64_t)isc_random32() * (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 @@ -154,7 +142,7 @@ isc_random_uniform(uint32_t limit) { * our valid range, it is superfluous, and we resample. */ while ((uint32_t)(num) < residue) { - num = (uint64_t)random_u32() * (uint64_t)limit; + num = (uint64_t)isc_random32() * (uint64_t)limit; } } /* @@ -162,3 +150,5 @@ isc_random_uniform(uint32_t limit) { */ return (uint32_t)(num >> 32); } + +#endif /* HAVE_ARC4RANDOM && !defined(__linux__) */