C9S/boringssl-antelopeio
2
0
Fork 0
mirror of https://github.com/AntelopeIO/boringssl.git synced 2026-07-21 14:43:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix RAND_enable_fork_unsafe_buffering when called after fork

Browse Source 
If a process calls fork(), then the child process never forks again, the
child may wish to call RAND_enable_fork_unsafe_buffering(). However,
doing so exposes a bug: we assume that, if the flag is set, we don't
need to worry about fork-safety. But it is possible that the PRNG state
was cloned from another process which does not work.

Concretely, consider a zygote process, e.g. Chromium's. A zygote process
would retain fork-safety, but pass along its PRNG state to each of its
children. If the children never fork, they might disable fork-safety,
hitting this bug. (Chromium does not call this API. This is just a
hypothetical scenario.)

Fix this by reseeding whenever the fork-safety bit changes. This fix
does not strictly depend on the atomics work, but it causes us to
unconditionally sample rand_fork_unsafe_buffering_enabled(). This no
longer causes contention because it's just another atomic load.

This only affects systems without MADV_WIPEONFORK and without fast
RDRAND. If RDRAND is fast, we're always fork-safe and MADV_WIPEONFORK
allows us to efficiently detect forks.

Cq-Include-Trybots: luci.boringssl.try:linux_clang_rel_tsan
Change-Id: I6d0c471c62c951254faf85420a7dc3f4a9d65ee0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59850
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin
2023-01-08 18:49:42 -08:00
committed by Boringssl LUCI CQ
parent 2eaf07075a
commit dd5219451c
2 changed files with 39 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crypto/fipsmodule/rand/rand.c
+15 -2
View File
@@ -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);
crypto/rand_extra/rand_test.cc
+24 -12
View File
@@ -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<uint8_t> out) {
static bool ForkAndRand(bssl::Span<uint8_t> out, bool fork_unsafe_buffering) {
int pipefds[2];
if (pipe(pipefds) < 0) {
perror("pipe");
@@ -76,6 +76,9 @@ static bool ForkAndRand(bssl::Span<uint8_t> 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