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>

crypto/CMakeLists.txt

@@ -463,6 +463,7 @@ add_executable(
   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

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

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();
 }

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 @@ inline void SetupGoogleTest() {
     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(

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"]
 ]