summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDr. Matthias St. Pierre <matthias.st.pierre@ncp-e.com>2020-08-31 23:36:22 +0200
committerDr. Matthias St. Pierre <matthias.st.pierre@ncp-e.com>2020-09-10 22:57:34 +0200
commit958fec77928a28350f6af252ac5e8d0e6e081faa (patch)
tree9ce5e0770b69c76d26cab6a9ffd452c8c2496ec7
parent526cf60408e1a356ec712b6c88a88864fdbe73af (diff)
Fix the DRBG seed propagation
In a nutshell, reseed propagation is a compatibility feature with the sole purpose to support the traditional way of (re-)seeding manually by calling 'RAND_add()' before 'RAND_bytes(). It ensures that the former has an immediate effect on the latter *within the same thread*, but it does not care about immediate reseed propagation to other threads. The implementation is lock-free, i.e., it works without taking the lock of the primary DRBG. Pull request #7399 not only fixed the data race issue #7394 but also changed the original implementation of the seed propagation unnecessarily. This commit reverts most of the changes of commit 1f98527659b8 and intends to fix the data race while retaining the original simplicity of the seed propagation. - use atomics with relaxed semantics to load and store the seed counter - add a new member drbg->enable_reseed_propagation to simplify the overflow treatment of the seed propagation counter - don't handle races between different threads This partially reverts commit 1f98527659b8290d442c4e1532452b9ba6463f1e. Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/12759)
-rw-r--r--crypto/rand/drbg_lib.c46
-rw-r--r--crypto/rand/rand_lib.c2
-rw-r--r--crypto/rand/rand_local.h9
3 files changed, 27 insertions, 30 deletions
diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index 73fd4394a3..e3666afdcc 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -327,13 +327,6 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
max_entropylen += drbg->max_noncelen;
}
- drbg->reseed_next_counter = tsan_load(&drbg->reseed_prop_counter);
- if (drbg->reseed_next_counter) {
- drbg->reseed_next_counter++;
- if (!drbg->reseed_next_counter)
- drbg->reseed_next_counter = 1;
- }
-
if (drbg->get_entropy != NULL)
entropylen = drbg->get_entropy(drbg, &entropy, min_entropy,
min_entropylen, max_entropylen, 0);
@@ -361,7 +354,13 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
drbg->state = DRBG_READY;
drbg->reseed_gen_counter = 1;
drbg->reseed_time = time(NULL);
- tsan_store(&drbg->reseed_prop_counter, drbg->reseed_next_counter);
+ if (drbg->enable_reseed_propagation) {
+ if (drbg->parent == NULL)
+ tsan_counter(&drbg->reseed_prop_counter);
+ else
+ tsan_store(&drbg->reseed_prop_counter,
+ tsan_load(&drbg->parent->reseed_prop_counter));
+ }
end:
if (entropy != NULL && drbg->cleanup_entropy != NULL)
@@ -428,14 +427,6 @@ int RAND_DRBG_reseed(RAND_DRBG *drbg,
}
drbg->state = DRBG_ERROR;
-
- drbg->reseed_next_counter = tsan_load(&drbg->reseed_prop_counter);
- if (drbg->reseed_next_counter) {
- drbg->reseed_next_counter++;
- if (!drbg->reseed_next_counter)
- drbg->reseed_next_counter = 1;
- }
-
if (drbg->get_entropy != NULL)
entropylen = drbg->get_entropy(drbg, &entropy, drbg->strength,
drbg->min_entropylen,
@@ -453,7 +444,13 @@ int RAND_DRBG_reseed(RAND_DRBG *drbg,
drbg->state = DRBG_READY;
drbg->reseed_gen_counter = 1;
drbg->reseed_time = time(NULL);
- tsan_store(&drbg->reseed_prop_counter, drbg->reseed_next_counter);
+ if (drbg->enable_reseed_propagation) {
+ if (drbg->parent == NULL)
+ tsan_counter(&drbg->reseed_prop_counter);
+ else
+ tsan_store(&drbg->reseed_prop_counter,
+ tsan_load(&drbg->parent->reseed_prop_counter));
+ }
end:
if (entropy != NULL && drbg->cleanup_entropy != NULL)
@@ -623,11 +620,8 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
|| now - drbg->reseed_time >= drbg->reseed_time_interval)
reseed_required = 1;
}
- if (drbg->parent != NULL) {
- unsigned int reseed_counter = tsan_load(&drbg->reseed_prop_counter);
- if (reseed_counter > 0
- && tsan_load(&drbg->parent->reseed_prop_counter)
- != reseed_counter)
+ if (drbg->enable_reseed_propagation && drbg->parent != NULL) {
+ if (drbg->reseed_prop_counter != tsan_load(&drbg->parent->reseed_prop_counter))
reseed_required = 1;
}
@@ -708,8 +702,7 @@ int RAND_DRBG_set_callbacks(RAND_DRBG *drbg,
RAND_DRBG_get_nonce_fn get_nonce,
RAND_DRBG_cleanup_nonce_fn cleanup_nonce)
{
- if (drbg->state != DRBG_UNINITIALISED
- || drbg->parent != NULL)
+ if (drbg->state != DRBG_UNINITIALISED)
return 0;
drbg->get_entropy = get_entropy;
drbg->cleanup_entropy = cleanup_entropy;
@@ -885,8 +878,9 @@ static RAND_DRBG *drbg_setup(RAND_DRBG *parent)
if (parent == NULL && rand_drbg_enable_locking(drbg) == 0)
goto err;
- /* enable seed propagation */
- tsan_store(&drbg->reseed_prop_counter, 1);
+ /* enable reseed propagation */
+ drbg->enable_reseed_propagation = 1;
+ drbg->reseed_prop_counter = 1;
/*
* Ignore instantiation error to support just-in-time instantiation.
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index ab4e9b5486..ba3a29e584 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -174,8 +174,6 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
prediction_resistance,
(unsigned char *)&drbg, sizeof(drbg)) != 0)
bytes = bytes_needed;
- drbg->reseed_next_counter
- = tsan_load(&drbg->parent->reseed_prop_counter);
rand_drbg_unlock(drbg->parent);
rand_pool_add_end(pool, bytes, 8 * bytes);
diff --git a/crypto/rand/rand_local.h b/crypto/rand/rand_local.h
index 0cdfb3332e..a04f9b0067 100644
--- a/crypto/rand/rand_local.h
+++ b/crypto/rand/rand_local.h
@@ -248,9 +248,15 @@ struct rand_drbg_st {
* This value is ignored if it is zero.
*/
time_t reseed_time_interval;
+
+ /*
+ * Enables reseed propagation (see following comment)
+ */
+ unsigned int enable_reseed_propagation;
+
/*
* Counts the number of reseeds since instantiation.
- * This value is ignored if it is zero.
+ * This value is ignored if enable_reseed_propagation is zero.
*
* This counter is used only for seed propagation from the <master> DRBG
* to its two children, the <public> and <private> DRBG. This feature is
@@ -259,7 +265,6 @@ struct rand_drbg_st {
* the output of RAND_bytes() resp. RAND_priv_bytes().
*/
TSAN_QUALIFIER unsigned int reseed_prop_counter;
- unsigned int reseed_next_counter;
size_t seedlen;
DRBG_STATUS state;