diff --git a/crypto/fipsmodule/rand/rand.c b/crypto/fipsmodule/rand/rand.c index bf6b0469f..a3fc6880d 100644 --- a/crypto/fipsmodule/rand/rand.c +++ b/crypto/fipsmodule/rand/rand.c @@ -65,6 +65,9 @@ struct rand_thread_state { // last_block_valid is non-zero iff |last_block| contains data from // |get_seed_entropy|. int last_block_valid; + // fork_unsafe_buffering is non-zero iff, when |drbg| was last (re)seeded, + // fork-unsafe buffering was enabled. + int fork_unsafe_buffering; #if defined(BORINGSSL_FIPS) // last_block contains the previous block from |get_seed_entropy|. @@ -331,6 +334,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len, } const uint64_t fork_generation = CRYPTO_get_fork_generation(); + const int fork_unsafe_buffering = rand_fork_unsafe_buffering_enabled(); // Additional data is mixed into every CTR-DRBG call to protect, as best we // can, against forks & VM clones. We do not over-read this information and @@ -345,7 +349,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len, // entropy is used. This can be expensive (one read per |RAND_bytes| call) // and so is disabled when we have fork detection, or if the application has // promised not to fork. - if (fork_generation != 0 || rand_fork_unsafe_buffering_enabled()) { + if (fork_generation != 0 || fork_unsafe_buffering) { OPENSSL_memset(additional_data, 0, sizeof(additional_data)); } else if (!have_rdrand()) { // No alternative so block for OS entropy. @@ -388,6 +392,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len, } state->calls = 0; state->fork_generation = fork_generation; + state->fork_unsafe_buffering = fork_unsafe_buffering; #if defined(BORINGSSL_FIPS) CRYPTO_MUTEX_init(&state->clear_drbg_lock); @@ -406,7 +411,14 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len, } if (state->calls >= kReseedInterval || - state->fork_generation != fork_generation) { + // If we've forked since |state| was last seeded, reseed. + state->fork_generation != fork_generation || + // If |state| was seeded from a state with different fork-safety + // preferences, reseed. Suppose |state| was fork-safe, then forked into + // two children, but each of the children never fork and disable fork + // safety. The children must reseed to avoid working from the same PRNG + // state. + state->fork_unsafe_buffering != fork_unsafe_buffering) { uint8_t seed[CTR_DRBG_ENTROPY_LEN]; uint8_t reseed_additional_data[CTR_DRBG_ENTROPY_LEN] = {0}; size_t reseed_additional_data_len = 0; @@ -424,6 +436,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len, } state->calls = 0; state->fork_generation = fork_generation; + state->fork_unsafe_buffering = fork_unsafe_buffering; } else { #if defined(BORINGSSL_FIPS) CRYPTO_MUTEX_lock_read(&state->clear_drbg_lock); diff --git a/crypto/rand_extra/rand_test.cc b/crypto/rand_extra/rand_test.cc index 2ed1deb41..1af28b04a 100644 --- a/crypto/rand_extra/rand_test.cc +++ b/crypto/rand_extra/rand_test.cc @@ -56,7 +56,7 @@ TEST(RandTest, NotObviouslyBroken) { #if !defined(OPENSSL_WINDOWS) && !defined(OPENSSL_IOS) && \ !defined(OPENSSL_FUCHSIA) && !defined(BORINGSSL_UNSAFE_DETERMINISTIC_MODE) -static bool ForkAndRand(bssl::Span out) { +static bool ForkAndRand(bssl::Span out, bool fork_unsafe_buffering) { int pipefds[2]; if (pipe(pipefds) < 0) { perror("pipe"); @@ -76,6 +76,9 @@ static bool ForkAndRand(bssl::Span out) { if (child == 0) { // This is the child. Generate entropy and write it to the parent. close(pipefds[0]); + if (fork_unsafe_buffering) { + RAND_enable_fork_unsafe_buffering(-1); + } RAND_bytes(out.data(), out.size()); while (!out.empty()) { ssize_t ret = write(pipefds[1], out.data(), out.size()); @@ -136,18 +139,27 @@ TEST(RandTest, Fork) { // intentionally uses smaller buffers than the others, to minimize the chance // of sneaking by with a large enough buffer that we've since reseeded from // the OS. - uint8_t buf1[16], buf2[16], buf3[16]; - ASSERT_TRUE(ForkAndRand(buf1)); - ASSERT_TRUE(ForkAndRand(buf2)); - RAND_bytes(buf3, sizeof(buf3)); + // + // All child processes should have different PRNGs, including the ones that + // disavow fork-safety. Although they are produced by fork, they themselves do + // not fork after that call. + uint8_t bufs[5][16]; + ASSERT_TRUE(ForkAndRand(bufs[0], /*fork_unsafe_buffering=*/false)); + ASSERT_TRUE(ForkAndRand(bufs[1], /*fork_unsafe_buffering=*/false)); + ASSERT_TRUE(ForkAndRand(bufs[2], /*fork_unsafe_buffering=*/true)); + ASSERT_TRUE(ForkAndRand(bufs[3], /*fork_unsafe_buffering=*/true)); + RAND_bytes(bufs[4], sizeof(bufs[4])); - // All should be different. - EXPECT_NE(Bytes(buf1), Bytes(buf2)); - EXPECT_NE(Bytes(buf2), Bytes(buf3)); - EXPECT_NE(Bytes(buf1), Bytes(buf3)); - EXPECT_NE(Bytes(buf1), Bytes(kZeros)); - EXPECT_NE(Bytes(buf2), Bytes(kZeros)); - EXPECT_NE(Bytes(buf3), Bytes(kZeros)); + // All should be different and non-zero. + for (const auto &buf : bufs) { + EXPECT_NE(Bytes(buf), Bytes(kZeros)); + } + for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(bufs); i++) { + for (size_t j = 0; j < i; j++) { + EXPECT_NE(Bytes(bufs[i]), Bytes(bufs[j])) + << "buffers " << i << " and " << j << " matched"; + } + } } #endif // !OPENSSL_WINDOWS && !OPENSSL_IOS && // !OPENSSL_FUCHSIA && !BORINGSSL_UNSAFE_DETERMINISTIC_MODE