Buffer reads of urandom, if you promise no forking.
Callers that lack hardware random may obtain a speed improvement by
calling |RAND_enable_fork_unsafe_buffering|, which enables a
thread-local buffer around reads from /dev/urandom.
Change-Id: I46e675d1679b20434dd520c58ece0f888f38a241
Reviewed-on: https://boringssl-review.googlesource.com/5792
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h
index a502d20..713659d 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -452,6 +452,7 @@
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 35d5290..8e639c2 100644
--- a/crypto/rand/CMakeLists.txt
+++ b/crypto/rand/CMakeLists.txt
@@ -22,3 +22,12 @@
)
perlasm(rdrand-x86_64.${ASM_EXT} asm/rdrand-x86_64.pl)
+
+add_executable(
+ urandom_test
+
+ urandom_test.cc
+ $<TARGET_OBJECTS:test_support>
+)
+
+target_link_libraries(urandom_test crypto)
diff --git a/crypto/rand/urandom.c b/crypto/rand/urandom.c
index d7ed5c6..1cc5260 100644
--- a/crypto/rand/urandom.c
+++ b/crypto/rand/urandom.c
@@ -30,109 +30,126 @@
/* This file implements a PRNG by reading from /dev/urandom, optionally with a
- * fork-safe buffer.
- *
- * If buffering is enabled then it maintains a global, linked list of buffers.
- * Threads which need random bytes grab a buffer from the list under a lock and
- * copy out the bytes that they need. In the rare case that the buffer is
- * empty, it's refilled from /dev/urandom outside of the lock.
- *
- * Large requests are always serviced from /dev/urandom directly.
- *
- * Each buffer contains the PID of the process that created it and it's tested
- * against the current PID each time. Thus processes that fork will discard all
- * the buffers filled by the parent process. There are two problems with this:
- *
- * 1) glibc maintains a cache of the current PID+PPID and, if this cache isn't
- * correctly invalidated, the getpid() will continue to believe that
- * it's the old process. Glibc depends on the glibc wrappers for fork,
- * vfork and clone being used in order to invalidate the getpid() cache.
- *
- * 2) If a process forks, dies and then its child forks, it's possible that
- * the third process will end up with the same PID as the original process.
- * If the second process never used any random values then this will mean
- * that the third process has stale, cached values and won't notice.
- */
+ * buffer, which is unsafe across |fork|. */
-/* BUF_SIZE is intended to be a 4K allocation with malloc overhead. struct
- * rand_buffer also fits in this space and the remainder is entropy. */
-#define BUF_SIZE (4096 - 16)
+#define BUF_SIZE 4096
-/* rand_buffer contains unused, random bytes. These structures form a linked
- * list via the |next| pointer, which is NULL in the final element. */
+/* rand_buffer contains unused, random bytes, some of which may have been
+ * consumed already. */
struct rand_buffer {
- size_t used; /* used contains the number of bytes of |rand| that have
- been consumed. */
- struct rand_buffer *next;
- pid_t pid; /* pid contains the pid at the time that the buffer was
- created so that data is not duplicated after a fork. */
- pid_t ppid; /* ppid contains the parent pid in order to try and reduce
- the possibility of duplicated PID confusing the
- detection of a fork. */
- uint8_t rand[];
+ size_t used;
+ uint8_t rand[BUF_SIZE];
};
-/* rand_bytes_per_buf is the number of actual entropy bytes in a buffer. */
-static const size_t rand_bytes_per_buf = BUF_SIZE - sizeof(struct rand_buffer);
+/* requested_lock is used to protect the |*_requested| variables. */
+static struct CRYPTO_STATIC_MUTEX requested_lock = CRYPTO_STATIC_MUTEX_INIT;
-static struct CRYPTO_STATIC_MUTEX global_lock = CRYPTO_STATIC_MUTEX_INIT;
+/* urandom_fd_requested is set by |RAND_set_urandom_fd|. It's protected by
+ * |requested_lock|. */
+static int urandom_fd_requested = -2;
-/* list_head is the start of a global, linked-list of rand_buffer objects. It's
- * protected by |global_lock|. */
-static struct rand_buffer *list_head;
-
-/* urandom_fd is a file descriptor to /dev/urandom. It's protected by
- * |global_lock|. */
+/* urandom_fd is a file descriptor to /dev/urandom. It's protected by |once|. */
static int urandom_fd = -2;
+/* 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 |global_lock|. */
+ * is protected by |once|. */
static int urandom_buffering = 0;
-/* urandom_get_fd_locked returns a file descriptor to /dev/urandom. The caller
- * of this function must hold |global_lock|. */
-static int urandom_get_fd_locked(void) {
- if (urandom_fd != -2) {
- return urandom_fd;
+static CRYPTO_once_t once = CRYPTO_ONCE_INIT;
+
+/* init_once initializes the state of this module to values previously
+ * requested. This is the only function that modifies |urandom_fd| and
+ * |urandom_buffering|, whose values may be read safely after calling the
+ * 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(&requested_lock);
+
+ if (fd == -2) {
+ do {
+ fd = open("/dev/urandom", O_RDONLY);
+ } while (fd == -1 && errno == EINTR);
}
- do {
- urandom_fd = open("/dev/urandom", O_RDONLY);
- } while (urandom_fd == -1 && errno == EINTR);
- return urandom_fd;
+ if (fd < 0) {
+ abort();
+ }
+
+ int flags = fcntl(fd, F_GETFD);
+ if (flags == -1) {
+ abort();
+ }
+ flags |= FD_CLOEXEC;
+ if (fcntl(fd, F_SETFD, flags) == -1) {
+ abort();
+ }
+ urandom_fd = fd;
}
-/* RAND_cleanup frees all buffers, closes any cached file descriptor
- * and resets the global state. */
-void RAND_cleanup(void) {
- struct rand_buffer *cur;
-
- CRYPTO_STATIC_MUTEX_lock_write(&global_lock);
- while ((cur = list_head)) {
- list_head = cur->next;
- OPENSSL_free(cur);
- }
- if (urandom_fd >= 0) {
- close(urandom_fd);
- }
- urandom_fd = -2;
- list_head = NULL;
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
-}
+void RAND_cleanup(void) {}
void RAND_set_urandom_fd(int fd) {
- CRYPTO_STATIC_MUTEX_lock_write(&global_lock);
- if (urandom_fd != -2) {
- /* |RAND_set_urandom_fd| may not be called after the RNG is used. */
+ fd = dup(fd);
+ if (fd < 0) {
abort();
}
- do {
- urandom_fd = dup(fd);
- } while (urandom_fd == -1 && errno == EINTR);
- if (urandom_fd < 0) {
- abort();
+
+ CRYPTO_STATIC_MUTEX_lock_write(&requested_lock);
+ urandom_fd_requested = fd;
+ CRYPTO_STATIC_MUTEX_unlock(&requested_lock);
+
+ CRYPTO_once(&once, init_once);
+ if (urandom_fd != fd) {
+ abort(); // Already initialized.
}
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
+}
+
+void RAND_enable_fork_unsafe_buffering(int fd) {
+ if (fd >= 0) {
+ fd = dup(fd);
+ if (fd < 0) {
+ abort();
+ }
+ } else {
+ fd = -2;
+ }
+
+ CRYPTO_STATIC_MUTEX_lock_write(&requested_lock);
+ urandom_buffering_requested = 1;
+ urandom_fd_requested = fd;
+ CRYPTO_STATIC_MUTEX_unlock(&requested_lock);
+
+ CRYPTO_once(&once, init_once);
+ if (urandom_buffering != 1 || (fd >= 0 && 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 |read_full| on first use. */
+ if (!CRYPTO_set_thread_local(OPENSSL_THREAD_LOCAL_URANDOM_BUF, buf,
+ OPENSSL_free)) {
+ OPENSSL_free(buf);
+ return NULL;
+ }
+
+ return buf;
}
/* read_full reads exactly |len| bytes from |fd| into |out| and returns 1. In
@@ -155,110 +172,48 @@
return 1;
}
-/* CRYPTO_sysrand puts |num| random bytes into |out|. */
-void CRYPTO_sysrand(uint8_t *out, size_t requested) {
- int fd;
- struct rand_buffer *buf;
- size_t todo;
- pid_t pid, ppid;
+/* 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) {
+ memcpy(out, &buf->rand[buf->used], remaining);
+ buf->used += remaining;
+ out += remaining;
+ requested -= remaining;
+
+ if (!read_full(urandom_fd, buf->rand, BUF_SIZE)) {
+ abort();
+ return;
+ }
+ buf->used = 0;
+ remaining = BUF_SIZE;
+ }
+
+ 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) {
return;
}
- CRYPTO_STATIC_MUTEX_lock_write(&global_lock);
- fd = urandom_get_fd_locked();
+ 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 (fd < 0) {
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
+ if (!read_full(urandom_fd, out, requested)) {
abort();
- return;
}
-
- /* If buffering is not enabled, or if the request is large, then the
- * result comes directly from urandom. */
- if (!urandom_buffering || requested > BUF_SIZE / 2) {
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
- if (!read_full(fd, out, requested)) {
- abort();
- }
- return;
- }
-
- pid = getpid();
- ppid = getppid();
-
- for (;;) {
- buf = list_head;
- if (buf && buf->pid == pid && buf->ppid == ppid &&
- rand_bytes_per_buf - buf->used >= requested) {
- memcpy(out, &buf->rand[buf->used], requested);
- buf->used += requested;
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
- return;
- }
-
- /* If we don't immediately have enough entropy with the correct
- * PID, remove the buffer from the list in order to gain
- * exclusive access and unlock. */
- if (buf) {
- list_head = buf->next;
- }
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
-
- if (!buf) {
- buf = (struct rand_buffer *)OPENSSL_malloc(BUF_SIZE);
- if (!buf) {
- abort();
- return;
- }
- /* The buffer doesn't contain any random bytes yet
- * so we mark it as fully used so that it will be
- * filled below. */
- buf->used = rand_bytes_per_buf;
- buf->next = NULL;
- buf->pid = pid;
- buf->ppid = ppid;
- }
-
- if (buf->pid == pid && buf->ppid == ppid) {
- break;
- }
-
- /* We have forked and so cannot use these bytes as they
- * may have been used in another process. */
- OPENSSL_free(buf);
- CRYPTO_STATIC_MUTEX_lock_write(&global_lock);
- }
-
- while (requested > 0) {
- todo = rand_bytes_per_buf - buf->used;
- if (todo > requested) {
- todo = requested;
- }
- memcpy(out, &buf->rand[buf->used], todo);
- requested -= todo;
- out += todo;
- buf->used += todo;
-
- if (buf->used < rand_bytes_per_buf) {
- break;
- }
-
- if (!read_full(fd, buf->rand, rand_bytes_per_buf)) {
- OPENSSL_free(buf);
- abort();
- return;
- }
-
- buf->used = 0;
- }
-
- CRYPTO_STATIC_MUTEX_lock_write(&global_lock);
- assert(list_head != buf);
- buf->next = list_head;
- list_head = buf;
- CRYPTO_STATIC_MUTEX_unlock(&global_lock);
}
#endif /* !OPENSSL_WINDOWS */
diff --git a/crypto/rand/urandom_test.cc b/crypto/rand/urandom_test.cc
new file mode 100644
index 0000000..956efdf
--- /dev/null
+++ b/crypto/rand/urandom_test.cc
@@ -0,0 +1,52 @@
+/* Copyright (c) 2015, 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 <stdio.h>
+#include <string.h>
+
+#include <openssl/rand.h>
+#include <openssl/crypto.h>
+
+
+extern "C" {
+extern void CRYPTO_sysrand(uint8_t *out, size_t requested);
+}
+
+static bool TestBuffering() {
+ RAND_enable_fork_unsafe_buffering(-1);
+ uint8_t buf[4096];
+
+ memset(buf, 0, sizeof(buf));
+ static const size_t kBufSize = 4096;
+ CRYPTO_sysrand(buf, 0);
+ CRYPTO_sysrand(buf, 2048); /* fills the buffer */
+ CRYPTO_sysrand(buf, kBufSize - 1); /* triggers a second fill */
+ CRYPTO_sysrand(buf, kBufSize - 2048 + 1); /* consumes the remainder */
+ CRYPTO_sysrand(buf, 4096); /* bypasses the buffer */
+
+ /* Lame, but might as well sanity check that something happened. */
+ uint8_t cmp[4096];
+ memset(cmp, 0, sizeof(cmp));
+ return memcmp(buf, cmp, 4096) != 0;
+}
+
+int main() {
+ CRYPTO_library_init();
+ if (!TestBuffering()) {
+ return false;
+ }
+
+ printf("PASS\n");
+ return 0;
+}
diff --git a/include/openssl/rand.h b/include/openssl/rand.h
index 335c76e..de1bd8d 100644
--- a/include/openssl/rand.h
+++ b/include/openssl/rand.h
@@ -40,11 +40,26 @@
* randomness rather opening /dev/urandom internally. The caller retains
* ownership of |fd| and is at liberty to close it at any time. This is useful
* if, due to a sandbox, /dev/urandom isn't available. If used, it must be
- * called before |RAND_bytes| is called in the current address space.
+ * called before the first call to |RAND_bytes|, and it is mutually exclusive
+ * with |RAND_enable_fork_unsafe_buffering|.
*
* |RAND_set_urandom_fd| does not buffer any entropy, so it is safe to call
- * |fork| between |RAND_set_urandom_fd| and the first call to |RAND_bytes|. */
+ * |fork| at any time after calling |RAND_set_urandom_fd|. */
OPENSSL_EXPORT void RAND_set_urandom_fd(int fd);
+
+/* RAND_enable_fork_unsafe_buffering enables efficient buffered reading of
+ * /dev/urandom. It adds an overhead of a few KB of memory per thread. It must
+ * be called before the first call to |RAND_bytes| and it is mutually exclusive
+ * with calls to |RAND_set_urandom_fd|.
+ *
+ * If |fd| is non-negative then a copy of |fd| will be used rather than opening
+ * /dev/urandom internally. Like |RAND_set_urandom_fd|, the caller retains
+ * ownership of |fd|. If |fd| is negative then /dev/urandom will be opened and
+ * any error from open(2) crashes the address space.
+ *
+ * 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);
#endif
diff --git a/util/all_tests.json b/util/all_tests.json
index a6daa2f..41924c6 100644
--- a/util/all_tests.json
+++ b/util/all_tests.json
@@ -47,6 +47,7 @@
["crypto/pkcs8/pkcs8_test"],
["crypto/pkcs8/pkcs12_test"],
["crypto/poly1305/poly1305_test", "crypto/poly1305/poly1305_test.txt"],
+ ["crypto/rand/urandom_test"],
["crypto/refcount_test"],
["crypto/rsa/rsa_test"],
["crypto/thread_test"],