Add some RAND_bytes tests. We're a far cry from the good old days when we just read from /dev/urandom without any fuss... In particular, the threading logic is slightly non-trivial and probably worth some basic sanity checks. Also write a fork-safety test, and test the fork-unsafe-buffering path. The last one is less useful right now, since fork-unsafe-buffering is a no-op with RDRAND enabled (although we do have an SDE bot...), but it's probably worth exercising the code in https://boringssl-review.googlesource.com/c/boringssl/+/31564. Change-Id: I14b1fc5216f2a93183286aa9b35f5f2309107fb2 Reviewed-on: https://boringssl-review.googlesource.com/31684 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt index ee7f8b6..78b835c 100644 --- a/crypto/CMakeLists.txt +++ b/crypto/CMakeLists.txt
@@ -463,6 +463,7 @@ pkcs8/pkcs12_test.cc poly1305/poly1305_test.cc pool/pool_test.cc + rand_extra/rand_test.cc refcount_test.cc rsa_extra/rsa_test.cc self_test.cc
diff --git a/crypto/rand_extra/rand_test.cc b/crypto/rand_extra/rand_test.cc new file mode 100644 index 0000000..bd2eb18 --- /dev/null +++ b/crypto/rand_extra/rand_test.cc
@@ -0,0 +1,184 @@ +/* Copyright (c) 2018, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#include <openssl/rand.h> + +#include <gtest/gtest.h> + +#include <openssl/span.h> + +#include "../test/test_util.h" + +#if !defined(OPENSSL_NO_THREADS) +#include <array> +#include <thread> +#include <vector> +#endif + +#if !defined(OPENSSL_WINDOWS) +#include <errno.h> +#include <stdio.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> +#endif + + +// These tests are, strictly speaking, flaky, but we use large enough buffers +// that the probability of failing when we should pass is negligible. + +TEST(RandTest, NotObviouslyBroken) { + static const uint8_t kZeros[256] = {0}; + + uint8_t buf1[256], buf2[256]; + RAND_bytes(buf1, sizeof(buf1)); + RAND_bytes(buf2, sizeof(buf2)); + + EXPECT_NE(Bytes(buf1), Bytes(buf2)); + EXPECT_NE(Bytes(buf1), Bytes(kZeros)); + EXPECT_NE(Bytes(buf2), Bytes(kZeros)); +} + +#if !defined(OPENSSL_WINDOWS) && !defined(BORINGSSL_UNSAFE_DETERMINISTIC_MODE) +static bool ForkAndRand(bssl::Span<uint8_t> out) { + int pipefds[2]; + if (pipe(pipefds) < 0) { + perror("pipe"); + return false; + } + + // This is a multi-threaded process, but GTest does not run tests concurrently + // and there currently are no threads, so this should be safe. + pid_t child = fork(); + if (child < 0) { + perror("fork"); + close(pipefds[0]); + close(pipefds[1]); + return false; + } + + if (child == 0) { + // This is the child. Generate entropy and write it to the parent. + close(pipefds[0]); + RAND_bytes(out.data(), out.size()); + while (!out.empty()) { + ssize_t ret = write(pipefds[1], out.data(), out.size()); + if (ret < 0) { + if (errno == EINTR) { + continue; + } + perror("write"); + _exit(1); + } + out = out.subspan(static_cast<size_t>(ret)); + } + _exit(0); + } + + // This is the parent. Read the entropy from the child. + close(pipefds[1]); + while (!out.empty()) { + ssize_t ret = read(pipefds[0], out.data(), out.size()); + if (ret <= 0) { + if (ret == 0) { + fprintf(stderr, "Unexpected EOF from child.\n"); + } else { + if (errno == EINTR) { + continue; + } + perror("read"); + } + close(pipefds[0]); + return false; + } + out = out.subspan(static_cast<size_t>(ret)); + } + close(pipefds[0]); + + // Wait for the child to exit. + int status; + if (waitpid(child, &status, 0) < 0) { + perror("waitpid"); + return false; + } + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { + fprintf(stderr, "Child did not exit cleanly.\n"); + return false; + } + + return true; +} + +TEST(RandTest, Fork) { + static const uint8_t kZeros[16] = {0}; + + // Draw a little entropy to initialize any internal PRNG buffering. + uint8_t byte; + RAND_bytes(&byte, 1); + + // Draw entropy in two child processes and the parent process. This test + // 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 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)); +} +#endif // !OPENSSL_WINDOWS && !BORINGSSL_UNSAFE_DETERMINISTIC_MODE + +#if !defined(OPENSSL_NO_THREADS) +static void RunConcurrentRands(size_t num_threads) { + static const uint8_t kZeros[256] = {0}; + + std::vector<std::array<uint8_t, 256>> bufs(num_threads); + std::vector<std::thread> threads(num_threads); + + for (size_t i = 0; i < num_threads; i++) { + threads[i] = + std::thread([i, &bufs] { RAND_bytes(bufs[i].data(), bufs[i].size()); }); + } + for (size_t i = 0; i < num_threads; i++) { + threads[i].join(); + } + + for (size_t i = 0; i < num_threads; i++) { + EXPECT_NE(Bytes(bufs[i]), Bytes(kZeros)); + for (size_t j = i + 1; j < num_threads; j++) { + EXPECT_NE(Bytes(bufs[i]), Bytes(bufs[j])); + } + } +} + +// Test that threads may concurrently draw entropy without tripping TSan. +TEST(RandTest, Threads) { + constexpr size_t kFewerThreads = 10; + constexpr size_t kMoreThreads = 20; + + // Draw entropy in parallel. + RunConcurrentRands(kFewerThreads); + // Draw entropy in parallel with higher concurrency than the previous maximum. + RunConcurrentRands(kMoreThreads); + // Draw entropy in parallel with lower concurrency than the previous maximum. + RunConcurrentRands(kFewerThreads); +} +#endif
diff --git a/crypto/test/gtest_main.cc b/crypto/test/gtest_main.cc index 4071040..5dc8b23 100644 --- a/crypto/test/gtest_main.cc +++ b/crypto/test/gtest_main.cc
@@ -12,13 +12,26 @@ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include <string.h> + #include <gtest/gtest.h> +#include <openssl/rand.h> + #include "gtest_main.h" int main(int argc, char **argv) { testing::InitGoogleTest(&argc, argv); bssl::SetupGoogleTest(); + +#if !defined(OPENSSL_WINDOWS) + for (int i = 1; i < argc; i++) { + if (strcmp(argv[i], "--fork_unsafe_buffering") == 0) { + RAND_enable_fork_unsafe_buffering(-1); + } + } +#endif + return RUN_ALL_TESTS(); }
diff --git a/crypto/test/gtest_main.h b/crypto/test/gtest_main.h index 927ab17..20ccf21 100644 --- a/crypto/test/gtest_main.h +++ b/crypto/test/gtest_main.h
@@ -27,6 +27,8 @@ OPENSSL_MSVC_PRAGMA(warning(push, 3)) #include <winsock2.h> OPENSSL_MSVC_PRAGMA(warning(pop)) +#else +#include <signal.h> #endif @@ -67,6 +69,10 @@ fprintf(stderr, "Didn't get expected version: %x\n", wsa_data.wVersion); exit(1); } +#else + // Some tests create pipes. We check return values, so avoid being killed by + // |SIGPIPE|. + signal(SIGPIPE, SIG_IGN); #endif testing::UnitTest::GetInstance()->listeners().Append(
diff --git a/util/all_tests.json b/util/all_tests.json index 01d6fd0..7152ec1 100644 --- a/util/all_tests.json +++ b/util/all_tests.json
@@ -1,5 +1,6 @@ [ ["crypto/crypto_test"], + ["crypto/crypto_test", "--fork_unsafe_buffering", "--gtest_filter=RandTest.*:-RandTest.Fork"], ["decrepit/decrepit_test"], ["ssl/ssl_test"] ]