diff --git a/src/crypto/fipsmodule/rand/fork_detect.c b/src/crypto/fipsmodule/rand/fork_detect.c index 71a02c89e..a2cf3a05a 100644 --- a/src/crypto/fipsmodule/rand/fork_detect.c +++ b/src/crypto/fipsmodule/rand/fork_detect.c @@ -17,31 +17,34 @@ #endif #include - #include "fork_detect.h" -#if defined(OPENSSL_LINUX) -#include -#include +#if defined(OPENSSL_FORK_DETECTION_MADVISE) #include #include - -#include "../delocate.h" -#include "../../internal.h" - - +#include +#include #if defined(MADV_WIPEONFORK) static_assert(MADV_WIPEONFORK == 18, "MADV_WIPEONFORK is not 18"); #else #define MADV_WIPEONFORK 18 #endif +#elif defined(OPENSSL_FORK_DETECTION_PTHREAD_ATFORK) +#include +#include +#include +#endif // OPENSSL_FORK_DETECTION_MADVISE +#include "../delocate.h" +#include "../../internal.h" + +#if defined(OPENSSL_FORK_DETECTION_MADVISE) +DEFINE_BSS_GET(int, g_force_madv_wipeonfork); +DEFINE_BSS_GET(int, g_force_madv_wipeonfork_enabled); DEFINE_STATIC_ONCE(g_fork_detect_once); DEFINE_STATIC_MUTEX(g_fork_detect_lock); DEFINE_BSS_GET(CRYPTO_atomic_u32 *, g_fork_detect_addr); DEFINE_BSS_GET(uint64_t, g_fork_generation); -DEFINE_BSS_GET(int, g_force_madv_wipeonfork); -DEFINE_BSS_GET(int, g_force_madv_wipeonfork_enabled); static void init_fork_detect(void) { if (*g_force_madv_wipeonfork_bss_get()) { @@ -73,9 +76,12 @@ static void init_fork_detect(void) { CRYPTO_atomic_store_u32(addr, 1); *g_fork_detect_addr_bss_get() = addr; *g_fork_generation_bss_get() = 1; + } uint64_t CRYPTO_get_fork_generation(void) { + CRYPTO_once(g_fork_detect_once_bss_get(), init_fork_detect); + // In a single-threaded process, there are obviously no races because there's // only a single mutator in the address space. // @@ -87,7 +93,6 @@ uint64_t CRYPTO_get_fork_generation(void) { // child process is single-threaded, the child may become multi-threaded // before it observes this. Therefore, we must synchronize the logic below. - CRYPTO_once(g_fork_detect_once_bss_get(), init_fork_detect); CRYPTO_atomic_u32 *const flag_ptr = *g_fork_detect_addr_bss_get(); if (flag_ptr == NULL) { // Our kernel is too old to support |MADV_WIPEONFORK| or @@ -98,6 +103,12 @@ uint64_t CRYPTO_get_fork_generation(void) { // doesn't support it. return 42; } + // With Linux and clone(), we do not believe that pthread_atfork() is + // sufficient for detecting all forms of address space duplication. At this + // point we have a kernel that does not support MADV_WIPEONFORK. We could + // return the generation number from pthread_atfork() here and it would + // probably be safe in almost any situation, but to ensure safety we return + // 0 and force an entropy draw on every call. return 0; } @@ -140,7 +151,34 @@ void CRYPTO_fork_detect_force_madv_wipeonfork_for_testing(int on) { *g_force_madv_wipeonfork_enabled_bss_get() = on; } -#elif defined(OPENSSL_WINDOWS) || defined(OPENSSL_TRUSTY) +#elif defined(OPENSSL_FORK_DETECTION_PTHREAD_ATFORK) + +DEFINE_STATIC_ONCE(g_pthread_fork_detection_once); +DEFINE_BSS_GET(uint64_t, g_atfork_fork_generation); + +static void we_are_forked(void) { + // Immediately after a fork, the process must be single-threaded. + uint64_t value = *g_atfork_fork_generation_bss_get() + 1; + if (value == 0) { + value = 1; + } + *g_atfork_fork_generation_bss_get() = value; +} + +static void init_pthread_fork_detection(void) { + if (pthread_atfork(NULL, NULL, we_are_forked) != 0) { + abort(); + } + *g_atfork_fork_generation_bss_get() = 1; +} + +uint64_t CRYPTO_get_fork_generation(void) { + CRYPTO_once(g_pthread_fork_detection_once_bss_get(), init_pthread_fork_detection); + + return *g_atfork_fork_generation_bss_get(); +} + +#elif defined(OPENSSL_DOES_NOT_FORK) // These platforms are guaranteed not to fork, and therefore do not require // fork detection support. Returning a constant non zero value makes BoringSSL diff --git a/src/crypto/fipsmodule/rand/fork_detect.h b/src/crypto/fipsmodule/rand/fork_detect.h index f9bbe02f7..c26784821 100644 --- a/src/crypto/fipsmodule/rand/fork_detect.h +++ b/src/crypto/fipsmodule/rand/fork_detect.h @@ -17,6 +17,23 @@ #include +#if defined(OPENSSL_LINUX) +// On linux we use MADVISE instead of pthread_atfork(), due +// to concerns about clone() being used for address space +// duplication. +#define OPENSSL_FORK_DETECTION +#define OPENSSL_FORK_DETECTION_MADVISE +#elif defined(OPENSSL_MACOS) || defined(OPENSSL_IOS) || \ + defined(OPENSSL_OPENBSD) || defined(OPENSSL_FREEBSD) +// These platforms may detect address space duplication with pthread_atfork. +// iOS doesn't normally allow fork in apps, but it's there. +#define OPENSSL_FORK_DETECTION +#define OPENSSL_FORK_DETECTION_PTHREAD_ATFORK +#elif defined(OPENSSL_WINDOWS) || defined(OPENSSL_TRUSTY) +// These platforms do not fork. +#define OPENSSL_DOES_NOT_FORK +#endif + #if defined(__cplusplus) extern "C" { #endif diff --git a/src/crypto/fipsmodule/rand/fork_detect_test.cc b/src/crypto/fipsmodule/rand/fork_detect_test.cc index ab4851276..b93126b1c 100644 --- a/src/crypto/fipsmodule/rand/fork_detect_test.cc +++ b/src/crypto/fipsmodule/rand/fork_detect_test.cc @@ -14,9 +14,11 @@ #include +#include "fork_detect.h" + // TSAN cannot cope with this test and complains that "starting new threads // after multi-threaded fork is not supported". -#if defined(OPENSSL_LINUX) && !defined(OPENSSL_TSAN) +#if defined(OPENSSL_FORK_DETECTION) && !defined(OPENSSL_TSAN) #include #include #include @@ -32,8 +34,6 @@ #include -#include "fork_detect.h" - static pid_t WaitpidEINTR(pid_t pid, int *out_status, int options) { pid_t ret; @@ -47,19 +47,20 @@ static pid_t WaitpidEINTR(pid_t pid, int *out_status, int options) { // The *InChild functions run inside a child process and must report errors via // |stderr| and |_exit| rather than GTest. -static void CheckGenerationInChild(const char *name, uint64_t expected) { +static void CheckGenerationAtLeastInChild(const char *name, + uint64_t minimum_expected) { uint64_t generation = CRYPTO_get_fork_generation(); - if (generation != expected) { + if (generation < minimum_expected) { fprintf(stderr, "%s generation (#1) was %" PRIu64 ", wanted %" PRIu64 ".\n", - name, generation, expected); + name, generation, minimum_expected); _exit(1); } // The generation should be stable. - generation = CRYPTO_get_fork_generation(); - if (generation != expected) { + uint64_t new_generation = CRYPTO_get_fork_generation(); + if (new_generation != generation) { fprintf(stderr, "%s generation (#2) was %" PRIu64 ", wanted %" PRIu64 ".\n", - name, generation, expected); + name, new_generation, generation); _exit(1); } } @@ -95,10 +96,9 @@ static void ForkInChild(std::function f) { } TEST(ForkDetect, Test) { - const uint64_t start = CRYPTO_get_fork_generation(); + uint64_t start = CRYPTO_get_fork_generation(); if (start == 0) { - fprintf(stderr, "Fork detection not supported. Skipping test.\n"); - return; + GTEST_SKIP() << "Fork detection not supported. Skipping test.\n"; } // The fork generation should be stable. @@ -111,16 +111,22 @@ TEST(ForkDetect, Test) { // Fork grandchildren before observing the fork generation. The // grandchildren will observe |start| + 1. for (int i = 0; i < 2; i++) { - ForkInChild([&] { CheckGenerationInChild("Grandchild", start + 1); }); + ForkInChild( + [&] { CheckGenerationAtLeastInChild("Grandchild", start + 1); }); } // Now the child also observes |start| + 1. This is fine because it has // already diverged from the grandchild at this point. - CheckGenerationInChild("Child", start + 1); + CheckGenerationAtLeastInChild("Child", start + 1); + + // In the pthread_atfork the value may have changed. + uint64_t child_generation = CRYPTO_get_fork_generation(); // Forked grandchildren will now observe |start| + 2. for (int i = 0; i < 2; i++) { - ForkInChild([&] { CheckGenerationInChild("Grandchild", start + 2); }); + ForkInChild([&] { + CheckGenerationAtLeastInChild("Grandchild", child_generation + 1); + }); } #if defined(OPENSSL_THREADS) @@ -131,8 +137,10 @@ TEST(ForkDetect, Test) { std::vector threads(4); for (int i = 0; i < 2; i++) { for (auto &t : threads) { - t = std::thread( - [&] { CheckGenerationInChild("Grandchild thread", start + 2); }); + t = std::thread([&] { + CheckGenerationAtLeastInChild("Grandchild thread", + child_generation + 1); + }); } for (auto &t : threads) { t.join(); @@ -141,8 +149,15 @@ TEST(ForkDetect, Test) { }); #endif // OPENSSL_THREADS - // The child still observes |start| + 1. - CheckGenerationInChild("Child", start + 1); + // The child's observed value should be unchanged. + if (child_generation != CRYPTO_get_fork_generation()) { + fprintf(stderr, + "Child generation (final stable check) was %" PRIu64 + ", wanted %" PRIu64 ".\n", + child_generation, CRYPTO_get_fork_generation()); + _exit(1); + } + _exit(0); } @@ -157,4 +172,4 @@ TEST(ForkDetect, Test) { EXPECT_EQ(start, CRYPTO_get_fork_generation()); } -#endif // OPENSSL_LINUX && !OPENSSL_TSAN +#endif // OPENSSL_FORK_DETECTION && !OPENSSL_TSAN diff --git a/src/crypto/rand_extra/forkunsafe.c b/src/crypto/rand_extra/forkunsafe.c index 356afddf8..5bdcad38f 100644 --- a/src/crypto/rand_extra/forkunsafe.c +++ b/src/crypto/rand_extra/forkunsafe.c @@ -33,6 +33,10 @@ void RAND_enable_fork_unsafe_buffering(int fd) { CRYPTO_atomic_store_u32(&g_buffering_enabled, 1); } + +void RAND_disable_fork_unsafe_buffering(void) { + CRYPTO_atomic_store_u32(&g_buffering_enabled, 0); +} #endif int rand_fork_unsafe_buffering_enabled(void) { diff --git a/src/include/openssl/rand.h b/src/include/openssl/rand.h index 6193c0011..215798e52 100644 --- a/src/include/openssl/rand.h +++ b/src/include/openssl/rand.h @@ -43,6 +43,11 @@ OPENSSL_EXPORT int RAND_bytes(uint8_t *buf, size_t len); // It has an unusual name because the buffer is unsafe across calls to |fork|. // Hence, this function should never be called by libraries. OPENSSL_EXPORT void RAND_enable_fork_unsafe_buffering(int fd); + +// RAND_disable_fork_unsafe_buffering disables efficient buffered reading of +// /dev/urandom, causing BoringSSL to always draw entropy on every request +// for random bytes. +OPENSSL_EXPORT void RAND_disable_fork_unsafe_buffering(void); #endif #if defined(BORINGSSL_UNSAFE_DETERMINISTIC_MODE)