Unwind wait_for_entropy diagnostic function in getrandom code In non-FIPS config, getrandom's blocking behavior has stuck for many, many years now. In the FIPS config, we've had the logging for as long, and as of 63f42a00cf28e3b5d2c5eddfa06ed16f532bde88, will be as visible as in the FIPS config. That all has stuck without much trouble, so start cleaning this code up. This change does not impact whether we block on getrandom because we already pass a flags of 0 into the actual getrandom call. The wait_for_entropy machinery was diagnostic-only. Also inline some helper functions now that we only have one entrypoint into the OS PRNG. Bug: 446280903 Change-Id: I3cd735046df9b2c662a695f35f0d1c9c5acecef6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/83728 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Lily Chen <chlily@google.com>
diff --git a/crypto/rand/urandom.cc b/crypto/rand/urandom.cc index 5387b18..4c6d38d 100644 --- a/crypto/rand/urandom.cc +++ b/crypto/rand/urandom.cc
@@ -23,31 +23,12 @@ #if defined(OPENSSL_RAND_URANDOM) -#include <assert.h> #include <errno.h> #include <fcntl.h> #include <stdio.h> -#include <string.h> #include <sys/syscall.h> #include <unistd.h> -#if !defined(OPENSSL_ANDROID) -#define OPENSSL_HAS_GETAUXVAL -#endif -// glibc prior to 2.16 does not have getauxval and sys/auxv.h. Android has some -// host builds (i.e. not building for Android itself, so |OPENSSL_ANDROID| is -// unset) which are still using a 2.15 sysroot. -// -// TODO(davidben): Remove this once Android updates their sysroot. -#if defined(__GLIBC_PREREQ) -#if !__GLIBC_PREREQ(2, 16) -#undef OPENSSL_HAS_GETAUXVAL -#endif -#endif -#if defined(OPENSSL_HAS_GETAUXVAL) -#include <sys/auxv.h> -#endif - #include "../internal.h" #include "getrandom_fillin.h" @@ -86,14 +67,6 @@ // urandom_fd is a file descriptor to /dev/urandom. It's protected by |once|. static int urandom_fd; -#if defined(USE_NR_getrandom) - -// getrandom_ready is one if |getrandom| had been initialized by the time -// |init_once| was called and zero otherwise. -static int getrandom_ready; - -#endif // USE_NR_getrandom - static CRYPTO_once_t rand_once = CRYPTO_ONCE_INIT; // init_once initializes the state of this module to values previously @@ -106,7 +79,6 @@ ssize_t getrandom_ret = boringssl_getrandom(&dummy, sizeof(dummy), GRND_NONBLOCK); if (getrandom_ret == 1) { - getrandom_ready = 1; have_getrandom = 1; } else if (getrandom_ret == -1 && errno == EAGAIN) { // We have getrandom, but the entropy pool has not been initialized yet. @@ -145,67 +117,15 @@ urandom_fd = fd; } -static CRYPTO_once_t wait_for_entropy_once = CRYPTO_ONCE_INIT; +void CRYPTO_init_sysrand(void) { CRYPTO_once(&rand_once, init_once); } -static void wait_for_entropy(void) { - int fd = urandom_fd; - if (fd == kHaveGetrandom) { - // |getrandom| and |getentropy| support blocking in |fill_with_entropy| - // directly. For |getrandom|, we first probe with a non-blocking call to aid - // debugging. -#if defined(USE_NR_getrandom) - if (getrandom_ready) { - // The entropy pool was already initialized in |init_once|. - return; - } - - uint8_t dummy; - ssize_t getrandom_ret = - boringssl_getrandom(&dummy, sizeof(dummy), GRND_NONBLOCK); - if (getrandom_ret == -1 && errno == EAGAIN) { - // Attempt to get the path of the current process to aid in debugging when - // something blocks. - const char *current_process = "<unknown>"; -#if defined(OPENSSL_HAS_GETAUXVAL) - const unsigned long getauxval_ret = getauxval(AT_EXECFN); - if (getauxval_ret != 0) { - current_process = (const char *)getauxval_ret; - } -#endif - - fprintf( - stderr, - "%s: getrandom indicates that the entropy pool has not been " - "initialized. Rather than continue with poor entropy, this process " - "will block until entropy is available.\n", - current_process); - - getrandom_ret = - boringssl_getrandom(&dummy, sizeof(dummy), 0 /* no flags */); - } - - if (getrandom_ret != 1) { - perror("getrandom"); - abort(); - } -#endif // USE_NR_getrandom - return; - } -} - -// fill_with_entropy writes |len| bytes of entropy into |out|. It returns one -// on success and zero on error. This function will block until the entropy pool -// is initialized. -static int fill_with_entropy(uint8_t *out, size_t len) { +// CRYPTO_sysrand writes |len| bytes of entropy into |out|. +void CRYPTO_sysrand(uint8_t *out, size_t len) { if (len == 0) { - return 1; + return; } CRYPTO_init_sysrand(); - // TODO(crbug.com/446280903): After the change to uniformly use OS entropy has - // stuck, remove this |wait_for_entropy| hook. The |getrandom| calls below - // already wait for entropy. |wait_for_entropy| just added more useful errors. - CRYPTO_once(&wait_for_entropy_once, wait_for_entropy); // Clear |errno| so it has defined value if |read| or |getrandom| // "successfully" returns zero. @@ -227,23 +147,12 @@ } if (r <= 0) { - return 0; + perror("entropy fill failed"); + abort(); } out += r; len -= r; } - - return 1; -} - -void CRYPTO_init_sysrand(void) { CRYPTO_once(&rand_once, init_once); } - -// CRYPTO_sysrand puts |requested| random bytes into |out|. -void CRYPTO_sysrand(uint8_t *out, size_t requested) { - if (!fill_with_entropy(out, requested)) { - perror("entropy fill failed"); - abort(); - } } #endif // OPENSSL_RAND_URANDOM
diff --git a/crypto/rand/urandom_test.cc b/crypto/rand/urandom_test.cc index e610607..1dc8810 100644 --- a/crypto/rand/urandom_test.cc +++ b/crypto/rand/urandom_test.cc
@@ -639,7 +639,6 @@ // |GetTrace| will observe the real code making. static std::vector<Event> TestFunctionPRNGModel(unsigned flags) { std::vector<Event> ret; - bool getrandom_ready = false; bool used_daemon = false; if (have_fork_detection()) { @@ -648,7 +647,6 @@ // Probe for getrandom support ret.push_back(Event::GetRandom(1, GRND_NONBLOCK)); - std::function<void()> wait_for_entropy; std::function<bool(size_t)> sysrand; if (flags & NO_GETRANDOM) { @@ -678,18 +676,7 @@ return ret; } - getrandom_ready = (flags & GETRANDOM_NOT_READY) == 0; - wait_for_entropy = [&ret, &getrandom_ready] { - if (getrandom_ready) { - return; - } - - ret.push_back(Event::GetRandom(1, GRND_NONBLOCK)); - ret.push_back(Event::GetRandom(1, 0)); - getrandom_ready = true; - }; - sysrand = [&ret, &wait_for_entropy](size_t len) { - wait_for_entropy(); + sysrand = [&ret](size_t len) { ret.push_back(Event::GetRandom(len, 0)); return true; };