commit 785c3f84de958e90c45e82d6126a8c66958be4e1 Author: Nick Mathewson nickm@torproject.org Date: Wed Mar 6 11:03:42 2019 -0500
fast_rng: if noinherit has failed, then check getpid() for bad forks
getpid() can be really expensive sometimes, and it can fail to detect some kind of fork+prng mistakes, so we need to avoid it if it's safe to do so.
This patch might slow down fast_prng a lot on any old operating system that lacks a way to prevent ram from being inherited, AND requires a syscall for any getpid() calls. But it should make sure that we either crash or continue safely on incorrect fork+prng usage elsewhere in the future. --- src/lib/crypt_ops/crypto_rand_fast.c | 63 ++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/src/lib/crypt_ops/crypto_rand_fast.c b/src/lib/crypt_ops/crypto_rand_fast.c index f1acc9faf..384c172c3 100644 --- a/src/lib/crypt_ops/crypto_rand_fast.c +++ b/src/lib/crypt_ops/crypto_rand_fast.c @@ -46,8 +46,25 @@
#include "lib/log/util_bug.h"
+#ifdef HAVE_SYS_TYPES_H +#include <sys/types.h> +#endif +#ifdef HAVE_UNISTD_H +#include <unistd.h> +#endif + #include <string.h>
+#ifdef NOINHERIT_CAN_FAIL +#define CHECK_PID +#endif + +#ifdef CHECK_PID +#define PID_FIELD_LEN sizeof(pid_t) +#else +#define PID_FIELD_LEN 0 +#endif + /* Alias for CRYPTO_FAST_RNG_SEED_LEN to make our code shorter. */ #define SEED_LEN (CRYPTO_FAST_RNG_SEED_LEN) @@ -59,7 +76,7 @@ /* The number of random bytes that we can yield to the user after each * time we fill a crypto_fast_rng_t's buffer. */ -#define BUFLEN (MAPLEN - 2*sizeof(uint16_t) - SEED_LEN) +#define BUFLEN (MAPLEN - 2*sizeof(uint16_t) - SEED_LEN - PID_FIELD_LEN)
/* The number of buffer refills after which we should fetch more * entropy from crypto_strongest_rand(). @@ -82,6 +99,11 @@ struct crypto_fast_rng_t { uint16_t n_till_reseed; /** How many bytes are remaining in cbuf.bytes? */ uint16_t bytes_left; +#ifdef CHECK_PID + /** Which process owns this fast_rng? If this value is zero, we do not + * need to test the owner. */ + pid_t owner; +#endif struct cbuf { /** The seed (key and IV) that we will use the next time that we refill * cbuf. */ @@ -130,16 +152,32 @@ crypto_fast_rng_new(void) crypto_fast_rng_t * crypto_fast_rng_new_from_seed(const uint8_t *seed) { + unsigned inherit = INHERIT_KEEP; /* We try to allocate this object as securely as we can, to avoid * having it get dumped, swapped, or shared after fork. */ crypto_fast_rng_t *result = tor_mmap_anonymous(sizeof(*result), ANONMAP_PRIVATE | ANONMAP_NOINHERIT, - NULL); + &inherit); memcpy(result->buf.seed, seed, SEED_LEN); /* Causes an immediate refill once the user asks for data. */ result->bytes_left = 0; result->n_till_reseed = RESEED_AFTER; +#ifdef CHECK_PID + if (inherit == INHERIT_KEEP) { + /* This value will neither be dropped nor zeroed after fork, so we need to + * check our pid to make sure we are not sharing it across a fork. This + * can be expensive if the pid value isn't cached, sadly. + */ + result->owner = getpid(); + } +#elif defined(_WIN32) + /* Windows can't fork(), so there's no need to noinherit. */ +#else + /* We decided above that noinherit would always do _something_. Assert here + * that we were correct. */ + tor_assert(inherit != INHERIT_KEEP); +#endif return result; }
@@ -211,6 +249,27 @@ static void crypto_fast_rng_getbytes_impl(crypto_fast_rng_t *rng, uint8_t *out, const size_t n) { +#ifdef CHECK_PID + if (rng->owner) { + /* Note that we only need to do this check when we have owner set: that + * is, when our attempt to block inheriting failed, and the result was + * INHERIT_KEEP. + * + * If the result was INHERIT_DROP, then any attempt to access the rng + * memory after forking will crash. + * + * If the result was INHERIT_ZERO, then forking will set the bytes_left + * and n_till_reseed fields to zero. This function will call + * crypto_fast_rng_refill(), which will in turn reseed the PRNG. + * + * So we only need to do this test in the case when mmap_anonymous() + * returned INHERIT_KEEP. We avoid doing it needlessly, since getpid() is + * often a system call, and that can be slow. + */ + tor_assert(rng->owner == getpid()); + } +#endif + size_t bytes_to_yield = n;
while (bytes_to_yield) {