The branch master has been updated
via 7f0a8dc7f9c5c35af0f66aca553304737931d55f (commit)
via 0768b38b80d7636910c129ef8954d4f13a574ff6 (commit)
via 5562dbb39cbf9db41dad9b8d3ae643262e458c63 (commit)
via 849529257c9979c7ca0d28e8b80a47bc4a36d4f2 (commit)
from dc64dc2edd215d6cc5843c1bfe1f0b64bff26adc (commit)
- Log -----------------------------------------------------------------
commit 7f0a8dc7f9c5c35af0f66aca553304737931d55f
Author: Dr. Matthias St. Pierre <matthias.st.pie...@ncp-e.com>
Date: Wed Sep 11 10:40:18 2019 +0200
crypto/threads_win.c: fix preprocessor indentation
Reviewed-by: Matt Caswell <m...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
commit 0768b38b80d7636910c129ef8954d4f13a574ff6
Author: Dr. Matthias St. Pierre <matthias.st.pie...@ncp-e.com>
Date: Thu May 30 18:37:29 2019 +0200
drbg: fix issue where DRBG_CTR fails if NO_DF is used (2nd attempt)
Since commit 7c226dfc434d a chained DRBG does not add additional
data anymore when reseeding from its parent. The reason is that
the size of the additional data exceeded the allowed size when
no derivation function was used.
This commit provides an alternative fix: instead of adding the
entire DRBG's complete state, we just add the DRBG's address
in memory, thereby providing some distinction between the different
DRBG instances.
Reviewed-by: Paul Dale <paul.d...@oracle.com>
Reviewed-by: Matt Caswell <m...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
commit 5562dbb39cbf9db41dad9b8d3ae643262e458c63
Author: Dr. Matthias St. Pierre <matthias.st.pie...@ncp-e.com>
Date: Thu May 30 18:52:39 2019 +0200
drbg: add fork id to additional data on UNIX systems
Provides a little extra fork-safety on UNIX systems, adding to the
fact that all DRBGs reseed automatically when the fork_id changes.
Reviewed-by: Paul Dale <paul.d...@oracle.com>
Reviewed-by: Matt Caswell <m...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
commit 849529257c9979c7ca0d28e8b80a47bc4a36d4f2
Author: Dr. Matthias St. Pierre <matthias.st.pie...@ncp-e.com>
Date: Mon May 27 21:03:09 2019 +0200
drbg: ensure fork-safety without using a pthread_atfork handler
When the new OpenSSL CSPRNG was introduced in version 1.1.1,
it was announced in the release notes that it would be fork-safe,
which the old CSPRNG hadn't been.
The fork-safety was implemented using a fork count, which was
incremented by a pthread_atfork handler. Initially, this handler
was enabled by default. Unfortunately, the default behaviour
had to be changed for other reasons in commit b5319bdbd095, so
the new OpenSSL CSPRNG failed to keep its promise.
This commit restores the fork-safety using a different approach.
It replaces the fork count by a fork id, which coincides with
the process id on UNIX-like operating systems and is zero on other
operating systems. It is used to detect when an automatic reseed
after a fork is necessary.
To prevent a future regression, it also adds a test to verify that
the child reseeds after fork.
CVE-2019-1549
Reviewed-by: Paul Dale <paul.d...@oracle.com>
Reviewed-by: Matt Caswell <m...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
-----------------------------------------------------------------------
Summary of changes:
crypto/include/internal/rand_int.h | 1 -
crypto/init.c | 1 -
crypto/rand/drbg_lib.c | 9 +++++---
crypto/rand/rand_lcl.h | 23 +++++--------------
crypto/rand/rand_lib.c | 13 ++++-------
crypto/rand/rand_unix.c | 3 +++
crypto/threads_none.c | 13 +++++++++++
crypto/threads_pthread.c | 10 +++++++++
crypto/threads_win.c | 10 ++++++---
include/internal/cryptlib.h | 1 +
test/drbgtest.c | 45 ++++++++++++++++++++++++++++++++++++++
11 files changed, 94 insertions(+), 35 deletions(-)
diff --git a/crypto/include/internal/rand_int.h
b/crypto/include/internal/rand_int.h
index c5d0c20551..bc427e3cf4 100644
--- a/crypto/include/internal/rand_int.h
+++ b/crypto/include/internal/rand_int.h
@@ -24,7 +24,6 @@
typedef struct rand_pool_st RAND_POOL;
void rand_cleanup_int(void);
-void rand_fork(void);
/* Hardware-based seeding functions. */
size_t rand_acquire_entropy_from_tsc(RAND_POOL *pool);
diff --git a/crypto/init.c b/crypto/init.c
index 36c6333877..6536bd5266 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -676,7 +676,6 @@ void OPENSSL_fork_parent(void)
void OPENSSL_fork_child(void)
{
- rand_fork();
/* TODO(3.0): Inform all providers about a fork event */
}
#endif
diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index f8b58d7245..c24222188f 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -415,7 +415,7 @@ static RAND_DRBG *rand_drbg_new(OPENSSL_CTX *ctx,
drbg->libctx = ctx;
drbg->secure = secure && CRYPTO_secure_allocated(drbg);
- drbg->fork_count = rand_fork_count;
+ drbg->fork_id = openssl_get_fork_id();
drbg->parent = parent;
if (parent == NULL) {
@@ -829,6 +829,7 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out,
size_t outlen,
int prediction_resistance,
const unsigned char *adin, size_t adinlen)
{
+ int fork_id;
int reseed_required = 0;
if (drbg->state != DRBG_READY) {
@@ -854,8 +855,10 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char
*out, size_t outlen,
return 0;
}
- if (drbg->fork_count != rand_fork_count) {
- drbg->fork_count = rand_fork_count;
+ fork_id = openssl_get_fork_id();
+
+ if (drbg->fork_id != fork_id) {
+ drbg->fork_id = fork_id;
reseed_required = 1;
}
diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h
index c954cf5f39..0c92d75666 100644
--- a/crypto/rand/rand_lcl.h
+++ b/crypto/rand/rand_lcl.h
@@ -225,12 +225,12 @@ struct rand_drbg_st {
int secure; /* 1: allocated on the secure heap, 0: otherwise */
int type; /* the nid of the underlying algorithm */
/*
- * Stores the value of the rand_fork_count global as of when we last
- * reseeded. The DRBG reseeds automatically whenever drbg->fork_count !=
- * rand_fork_count. Used to provide fork-safety and reseed this DRBG in
- * the child process.
+ * Stores the return value of openssl_get_fork_id() as of when we last
+ * reseeded. The DRBG reseeds automatically whenever drbg->fork_id !=
+ * openssl_get_fork_id(). Used to provide fork-safety and reseed this
+ * DRBG in the child process.
*/
- int fork_count;
+ int fork_id;
unsigned short flags; /* various external flags */
/*
@@ -331,19 +331,6 @@ struct rand_drbg_st {
/* The global RAND method, and the global buffer and DRBG instance. */
extern RAND_METHOD rand_meth;
-/*
- * A "generation count" of forks. Incremented in the child process after a
- * fork. Since rand_fork_count is increment-only, and only ever written to in
- * the child process of the fork, which is guaranteed to be single-threaded, no
- * locking is needed for normal (read) accesses; the rest of pthread fork
- * processing is assumed to introduce the necessary memory barriers. Sibling
- * children of a given parent will produce duplicate values, but this is not
- * problematic because the reseeding process pulls input from the system CSPRNG
- * and/or other global sources, so the siblings will end up generating
- * different output streams.
- */
-extern int rand_fork_count;
-
/* DRBG helpers */
int rand_drbg_restart(RAND_DRBG *drbg,
const unsigned char *buffer, size_t len, size_t entropy);
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index 1ab2a8246c..b70b60a650 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -30,8 +30,6 @@ static CRYPTO_ONCE rand_init = CRYPTO_ONCE_STATIC_INIT;
static int rand_inited = 0;
#endif /* FIPS_MODE */
-int rand_fork_count;
-
#ifdef OPENSSL_RAND_SEED_RDTSC
/*
* IMPORTANT NOTE: It is not currently possible to use this code
@@ -160,7 +158,9 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
size_t bytes = 0;
/*
- * Get random from parent, include our state as additional input.
+ * Get random data from parent. Include our address as additional
input,
+ * in order to provide some additional distinction between
different
+ * DRBG child instances.
* Our lock is already held, but we need to lock our parent before
* generating bits from it. (Note: taking the lock will be a no-op
* if locking if drbg->parent->lock == NULL.)
@@ -169,7 +169,7 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
if (RAND_DRBG_generate(drbg->parent,
buffer, bytes_needed,
prediction_resistance,
- NULL, 0) != 0)
+ (unsigned char *)&drbg, sizeof(drbg)) != 0)
bytes = bytes_needed;
drbg->reseed_next_counter
= tsan_load(&drbg->parent->reseed_prop_counter);
@@ -238,11 +238,6 @@ void rand_drbg_cleanup_additional_data(RAND_POOL *pool,
unsigned char *out)
rand_pool_reattach(pool, out);
}
-void rand_fork(void)
-{
- rand_fork_count++;
-}
-
#ifndef FIPS_MODE
DEFINE_RUN_ONCE_STATIC(do_rand_init)
{
diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c
index 813964665f..8641badbff 100644
--- a/crypto/rand/rand_unix.c
+++ b/crypto/rand/rand_unix.c
@@ -704,6 +704,7 @@ int rand_pool_add_nonce_data(RAND_POOL *pool)
int rand_pool_add_additional_data(RAND_POOL *pool)
{
struct {
+ int fork_id;
CRYPTO_THREAD_ID tid;
uint64_t time;
} data;
@@ -713,9 +714,11 @@ int rand_pool_add_additional_data(RAND_POOL *pool)
/*
* Add some noise from the thread id and a high resolution timer.
+ * The fork_id adds some extra fork-safety.
* The thread id adds a little randomness if the drbg is accessed
* concurrently (which is the case for the <master> drbg).
*/
+ data.fork_id = openssl_get_fork_id();
data.tid = CRYPTO_THREAD_get_current_id();
data.time = get_timer_bits();
diff --git a/crypto/threads_none.c b/crypto/threads_none.c
index ff95556c17..c12d5610aa 100644
--- a/crypto/threads_none.c
+++ b/crypto/threads_none.c
@@ -12,6 +12,11 @@
#if !defined(OPENSSL_THREADS) || defined(CRYPTO_TDEBUG)
+# if defined(OPENSSL_SYS_UNIX)
+# include <sys/types.h>
+# include <unistd.h>
+# endif
+
CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void)
{
CRYPTO_RWLOCK *lock;
@@ -133,4 +138,12 @@ int openssl_init_fork_handlers(void)
return 0;
}
+int openssl_get_fork_id(void)
+{
+# if defined(OPENSSL_SYS_UNIX)
+ return getpid();
+# else
+ return 0;
+# endif
+}
#endif
diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index c3fd2411db..762da60a87 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -16,6 +16,11 @@
#if defined(OPENSSL_THREADS) && !defined(CRYPTO_TDEBUG) &&
!defined(OPENSSL_SYS_WINDOWS)
+# if defined(OPENSSL_SYS_UNIX)
+# include <sys/types.h>
+# include <unistd.h>
+#endif
+
# ifdef PTHREAD_RWLOCK_INITIALIZER
# define USE_RWLOCK
# endif
@@ -207,4 +212,9 @@ int openssl_init_fork_handlers(void)
return 0;
}
# endif /* FIPS_MODE */
+
+int openssl_get_fork_id(void)
+{
+ return getpid();
+}
#endif
diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index 73203834c1..43bd0f51d9 100644
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -24,15 +24,15 @@ CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void)
return NULL;
}
-#if !defined(_WIN32_WCE)
+# if !defined(_WIN32_WCE)
/* 0x400 is the spin count value suggested in the documentation */
if (!InitializeCriticalSectionAndSpinCount(lock, 0x400)) {
OPENSSL_free(lock);
return NULL;
}
-#else
+# else
InitializeCriticalSection(lock);
-#endif
+# endif
return lock;
}
@@ -164,4 +164,8 @@ int openssl_init_fork_handlers(void)
return 0;
}
+int openssl_get_fork_id(void)
+{
+ return 0;
+}
#endif
diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index d591f203d2..cfac74a328 100644
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -93,6 +93,7 @@ void OPENSSL_showfatal(const char *fmta, ...);
int do_ex_data_init(OPENSSL_CTX *ctx);
void crypto_cleanup_all_ex_data_int(OPENSSL_CTX *ctx);
int openssl_init_fork_handlers(void);
+int openssl_get_fork_id(void);
char *ossl_safe_getenv(const char *name);
diff --git a/test/drbgtest.c b/test/drbgtest.c
index 9efdd87a4d..4559ffd635 100644
--- a/test/drbgtest.c
+++ b/test/drbgtest.c
@@ -22,6 +22,13 @@
# include <windows.h>
#endif
+
+#if defined(OPENSSL_SYS_UNIX)
+# include <sys/types.h>
+# include <sys/wait.h>
+# include <unistd.h>
+#endif
+
#include "testutil.h"
#include "drbgtest.h"
@@ -708,6 +715,40 @@ static int test_drbg_reseed(int expect_success,
return 1;
}
+
+#if defined(OPENSSL_SYS_UNIX)
+/*
+ * Test whether master, public and private DRBG are reseeded after
+ * forking the process.
+ */
+static int test_drbg_reseed_after_fork(RAND_DRBG *master,
+ RAND_DRBG *public,
+ RAND_DRBG *private)
+{
+ pid_t pid;
+ int status=0;
+
+ pid = fork();
+ if (!TEST_int_ge(pid, 0))
+ return 0;
+
+ if (pid > 0) {
+ /* I'm the parent; wait for the child and check its exit code */
+ return TEST_int_eq(waitpid(pid, &status, 0), pid) &&
TEST_int_eq(status, 0);
+ }
+
+ /* I'm the child; check whether all three DRBGs reseed. */
+ if (!TEST_true(test_drbg_reseed(1, master, public, private, 1, 1, 1, 0)))
+ status = 1;
+
+ /* Remove hooks */
+ unhook_drbg(master);
+ unhook_drbg(public);
+ unhook_drbg(private);
+ exit(status);
+}
+#endif
+
/*
* Test whether the default rand_method (RAND_OpenSSL()) is
* setup correctly, in particular whether reseeding works
@@ -798,6 +839,10 @@ static int test_rand_drbg_reseed(void)
goto error;
reset_drbg_hook_ctx();
+#if defined(OPENSSL_SYS_UNIX)
+ if (!TEST_true(test_drbg_reseed_after_fork(master, public, private)))
+ goto error;
+#endif
/* fill 'randomness' buffer with some arbitrary data */
memset(rand_add_buf, 'r', sizeof(rand_add_buf));
