Make fork-unsafe buffering act via CTR-DRBG.
Fork-unsafe buffering was a mode that could be enabled by applications
that were sure that they didn't need to worry about state duplication.
It saved reads to urandom.
Since everything is now going through the CTR-DRBG, we can get the same
effect by simply not reading additional data from urandom in this case.
This change drops the buffering from urandom.c and, instead, implements
fork-unsafe buffering as a mode that skips reading additional data from
urandom, which only happened when RDRAND wasn't available anyway.
Since we expect the power-on self-tests to call into the PRNG, this
change also makes the flag capable of changing at any point by using a
mutex rather than a once. This is split into a separate file so that it
doesn't have to go into the FIPS module—since it uses r/w data that
would be a pain.
Change-Id: I5fd0ead0422e770e35758f080bb1cffa70d0c8da
Reviewed-on: https://boringssl-review.googlesource.com/14924
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h
index 7ce99a4..162041f 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -445,7 +445,6 @@
typedef enum {
OPENSSL_THREAD_LOCAL_ERR = 0,
OPENSSL_THREAD_LOCAL_RAND,
- OPENSSL_THREAD_LOCAL_URANDOM_BUF,
OPENSSL_THREAD_LOCAL_TEST,
NUM_OPENSSL_THREAD_LOCALS,
} thread_local_data_t;
diff --git a/crypto/rand/CMakeLists.txt b/crypto/rand/CMakeLists.txt
index 1340f72..c2b1fda 100644
--- a/crypto/rand/CMakeLists.txt
+++ b/crypto/rand/CMakeLists.txt
@@ -15,6 +15,7 @@
ctrdrbg.c
deterministic.c
+ forkunsafe.c
fuchsia.c
rand.c
urandom.c
diff --git a/crypto/rand/forkunsafe.c b/crypto/rand/forkunsafe.c
new file mode 100644
index 0000000..e9badc9
--- /dev/null
+++ b/crypto/rand/forkunsafe.c
@@ -0,0 +1,44 @@
+/* Copyright (c) 2017, 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 <stdlib.h>
+
+#include "internal.h"
+
+
+/* g_buffering_enabled is true if fork-unsafe buffering has been enabled. */
+static int g_buffering_enabled = 0;
+
+/* g_lock protects |g_buffering_enabled|. */
+static struct CRYPTO_STATIC_MUTEX g_lock = CRYPTO_STATIC_MUTEX_INIT;
+
+void RAND_enable_fork_unsafe_buffering(int fd) {
+ /* We no longer support setting the file-descriptor with this function. */
+ if (fd != -1) {
+ abort();
+ }
+
+ CRYPTO_STATIC_MUTEX_lock_write(&g_lock);
+ g_buffering_enabled = 1;
+ CRYPTO_STATIC_MUTEX_unlock_write(&g_lock);
+}
+
+int rand_fork_unsafe_buffering_enabled(void) {
+ CRYPTO_STATIC_MUTEX_lock_read(&g_lock);
+ const int ret = g_buffering_enabled;
+ CRYPTO_STATIC_MUTEX_unlock_read(&g_lock);
+ return ret;
+}
diff --git a/crypto/rand/internal.h b/crypto/rand/internal.h
index 9815e2c..58ffaaa 100644
--- a/crypto/rand/internal.h
+++ b/crypto/rand/internal.h
@@ -34,6 +34,10 @@
* system. */
void CRYPTO_sysrand(uint8_t *buf, size_t len);
+/* rand_fork_unsafe_buffering_enabled returns whether fork-unsafe buffering has
+ * been enabled via |RAND_enable_fork_unsafe_buffering|. */
+int rand_fork_unsafe_buffering_enabled(void);
+
/* CTR_DRBG_STATE contains the state of a CTR_DRBG based on AES-256. See SP
* 800-90Ar1. */
typedef struct {
diff --git a/crypto/rand/rand.c b/crypto/rand/rand.c
index e02b40b..b505657 100644
--- a/crypto/rand/rand.c
+++ b/crypto/rand/rand.c
@@ -178,8 +178,14 @@
uint8_t additional_data[32];
if (!hwrand(additional_data, sizeof(additional_data))) {
/* Without a hardware RNG to save us from address-space duplication, the OS
- * entropy is used. */
- CRYPTO_sysrand(additional_data, sizeof(additional_data));
+ * entropy is used. This can be expensive (one read per |RAND_bytes| call)
+ * and so can be disabled by applications that we have ensured don't fork
+ * and aren't at risk of VM cloning. */
+ if (!rand_fork_unsafe_buffering_enabled()) {
+ CRYPTO_sysrand(additional_data, sizeof(additional_data));
+ } else {
+ OPENSSL_memset(additional_data, 0, sizeof(additional_data));
+ }
}
for (size_t i = 0; i < sizeof(additional_data); i++) {
diff --git a/crypto/rand/urandom.c b/crypto/rand/urandom.c
index 23bdcf4..dc31a51 100644
--- a/crypto/rand/urandom.c
+++ b/crypto/rand/urandom.c
@@ -76,18 +76,6 @@
#endif /* OPENSSL_LINUX */
-/* This file implements a PRNG by reading from /dev/urandom, optionally with a
- * buffer, which is unsafe across |fork|. */
-
-#define BUF_SIZE 4096
-
-/* rand_buffer contains unused, random bytes, some of which may have been
- * consumed already. */
-struct rand_buffer {
- size_t used;
- uint8_t rand[BUF_SIZE];
-};
-
/* requested_lock is used to protect the |*_requested| variables. */
static struct CRYPTO_STATIC_MUTEX requested_lock = CRYPTO_STATIC_MUTEX_INIT;
@@ -102,14 +90,6 @@
/* urandom_fd is a file descriptor to /dev/urandom. It's protected by |once|. */
static int urandom_fd = -2 /* kUnset */;
-/* urandom_buffering_requested is set by |RAND_enable_fork_unsafe_buffering|.
- * It's protected by |requested_lock|. */
-static int urandom_buffering_requested = 0;
-
-/* urandom_buffering controls whether buffering is enabled (1) or not (0). This
- * is protected by |once|. */
-static int urandom_buffering = 0;
-
static CRYPTO_once_t once = CRYPTO_ONCE_INIT;
/* init_once initializes the state of this module to values previously
@@ -118,7 +98,6 @@
* once. */
static void init_once(void) {
CRYPTO_STATIC_MUTEX_lock_read(&requested_lock);
- urandom_buffering = urandom_buffering_requested;
int fd = urandom_fd_requested;
CRYPTO_STATIC_MUTEX_unlock_read(&requested_lock);
@@ -190,56 +169,6 @@
}
}
-void RAND_enable_fork_unsafe_buffering(int fd) {
- if (fd >= 0) {
- fd = dup(fd);
- if (fd < 0) {
- abort();
- }
- } else {
- fd = kUnset;
- }
-
- CRYPTO_STATIC_MUTEX_lock_write(&requested_lock);
- urandom_buffering_requested = 1;
- urandom_fd_requested = fd;
- CRYPTO_STATIC_MUTEX_unlock_write(&requested_lock);
-
- CRYPTO_once(&once, init_once);
- if (urandom_buffering != 1) {
- abort(); // Already initialized
- }
-
- if (fd >= 0) {
- if (urandom_fd == kHaveGetrandom) {
- close(fd);
- } else if (urandom_fd != fd) {
- abort(); // Already initialized.
- }
- }
-}
-
-static struct rand_buffer *get_thread_local_buffer(void) {
- struct rand_buffer *buf =
- CRYPTO_get_thread_local(OPENSSL_THREAD_LOCAL_URANDOM_BUF);
- if (buf != NULL) {
- return buf;
- }
-
- buf = OPENSSL_malloc(sizeof(struct rand_buffer));
- if (buf == NULL) {
- return NULL;
- }
- buf->used = BUF_SIZE; /* To trigger a |fill_with_entropy| on first use. */
- if (!CRYPTO_set_thread_local(OPENSSL_THREAD_LOCAL_URANDOM_BUF, buf,
- OPENSSL_free)) {
- OPENSSL_free(buf);
- return NULL;
- }
-
- return buf;
-}
-
#if defined(USE_SYS_getrandom) && defined(__has_feature)
#if __has_feature(memory_sanitizer)
void __msan_unpoison(void *, size_t);
@@ -287,30 +216,6 @@
return 1;
}
-/* read_from_buffer reads |requested| random bytes from the buffer into |out|,
- * refilling it if necessary to satisfy the request. */
-static void read_from_buffer(struct rand_buffer *buf,
- uint8_t *out, size_t requested) {
- size_t remaining = BUF_SIZE - buf->used;
-
- while (requested > remaining) {
- OPENSSL_memcpy(out, &buf->rand[buf->used], remaining);
- buf->used += remaining;
- out += remaining;
- requested -= remaining;
-
- if (!fill_with_entropy(buf->rand, BUF_SIZE)) {
- abort();
- return;
- }
- buf->used = 0;
- remaining = BUF_SIZE;
- }
-
- OPENSSL_memcpy(out, &buf->rand[buf->used], requested);
- buf->used += requested;
-}
-
/* CRYPTO_sysrand puts |requested| random bytes into |out|. */
void CRYPTO_sysrand(uint8_t *out, size_t requested) {
if (requested == 0) {
@@ -318,13 +223,6 @@
}
CRYPTO_once(&once, init_once);
- if (urandom_buffering && requested < BUF_SIZE) {
- struct rand_buffer *buf = get_thread_local_buffer();
- if (buf != NULL) {
- read_from_buffer(buf, out, requested);
- return;
- }
- }
if (!fill_with_entropy(out, requested)) {
abort();
