From 6969a3f49ae63c8a4fd543a121511a1f0eb64a5e Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Fri, 25 Aug 2017 23:26:53 +0200 Subject: [PATCH] DRBG: Remove 'randomness' buffer from 'RAND_DRBG' The DRBG callbacks 'get_entropy()' and 'cleanup_entropy()' are designed in such a way that the randomness buffer does not have to be allocated by the calling function. It receives the address of a dynamically allocated buffer from get_entropy() and returns this address to cleanup_entropy(), where it is freed. If these two calls are properly paired, the address can be stored in a stack local variable of the calling function, so there is no need for having a 'randomness' member (and a 'filled' member) in 'RAND_DRBG'. Reviewed-by: Paul Dale Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/4266) --- crypto/rand/drbg_lib.c | 6 +++--- crypto/rand/rand_lcl.h | 4 +--- crypto/rand/rand_lib.c | 38 +++++++++++++++----------------------- crypto/rand/rand_unix.c | 4 ++-- include/internal/rand.h | 5 +++-- 5 files changed, 24 insertions(+), 33 deletions(-) diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c index 0560e3baa7..d1f419dddf 100644 --- a/crypto/rand/drbg_lib.c +++ b/crypto/rand/drbg_lib.c @@ -168,9 +168,9 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg, end: if (entropy != NULL && drbg->cleanup_entropy != NULL) - drbg->cleanup_entropy(drbg, entropy); + drbg->cleanup_entropy(drbg, entropy, entropylen); if (nonce != NULL && drbg->cleanup_nonce!= NULL ) - drbg->cleanup_nonce(drbg, nonce); + drbg->cleanup_nonce(drbg, nonce, noncelen); if (drbg->state == DRBG_READY) return 1; return 0; @@ -229,7 +229,7 @@ int RAND_DRBG_reseed(RAND_DRBG *drbg, end: if (entropy != NULL && drbg->cleanup_entropy != NULL) - drbg->cleanup_entropy(drbg, entropy); + drbg->cleanup_entropy(drbg, entropy, entropylen); if (drbg->state == DRBG_READY) return 1; return 0; diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h index 20c0ee930b..f9de279658 100644 --- a/crypto/rand/rand_lcl.h +++ b/crypto/rand/rand_lcl.h @@ -94,14 +94,12 @@ struct rand_drbg_st { int nid; /* the underlying algorithm */ int fork_count; unsigned short flags; /* various external flags */ - char filled; char secure; /* * This is a fixed-size buffer, but we malloc to make it a little * harder to find; a classic security/performance trade-off. */ int size; - unsigned char *randomness; /* * The following parameters are setup by the per-type "init" function. @@ -157,7 +155,7 @@ void rand_read_tsc(RAND_poll_cb rand_add, void *arg); int rand_read_cpu(RAND_poll_cb rand_add, void *arg); /* DRBG entropy callbacks. */ -void drbg_release_entropy(RAND_DRBG *drbg, unsigned char *out); +void drbg_release_entropy(RAND_DRBG *drbg, unsigned char *out, size_t outlen); size_t drbg_entropy_from_parent(RAND_DRBG *drbg, unsigned char **pout, int entropy, size_t min_len, size_t max_len); diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c index 909f30426f..5ed08f1e23 100644 --- a/crypto/rand/rand_lib.c +++ b/crypto/rand/rand_lib.c @@ -105,20 +105,14 @@ size_t drbg_entropy_from_system(RAND_DRBG *drbg, int entropy, size_t min_len, size_t max_len) { int i; - + unsigned char *randomness; if (min_len > (size_t)drbg->size) { /* Should not happen. See comment near RANDOMNESS_NEEDED. */ min_len = drbg->size; } - if (drbg->filled) { - /* Re-use what we have. */ - *pout = drbg->randomness; - return drbg->size; - } - - drbg->randomness = drbg->secure ? OPENSSL_secure_malloc(drbg->size) + randomness = drbg->secure ? OPENSSL_secure_malloc(drbg->size) : OPENSSL_malloc(drbg->size); /* If we don't have enough, try to get more. */ @@ -133,15 +127,14 @@ size_t drbg_entropy_from_system(RAND_DRBG *drbg, if (min_len > rand_bytes.curr) min_len = rand_bytes.curr; if (min_len != 0) { - memcpy(drbg->randomness, rand_bytes.buff, min_len); - drbg->filled = 1; + memcpy(randomness, rand_bytes.buff, min_len); /* Update amount left and shift it down. */ rand_bytes.curr -= min_len; if (rand_bytes.curr != 0) memmove(rand_bytes.buff, &rand_bytes.buff[min_len], rand_bytes.curr); } CRYPTO_THREAD_unlock(rand_bytes.lock); - *pout = drbg->randomness; + *pout = randomness; return min_len; } @@ -150,33 +143,33 @@ size_t drbg_entropy_from_parent(RAND_DRBG *drbg, int entropy, size_t min_len, size_t max_len) { int st; - + unsigned char *randomness; + if (min_len > (size_t)drbg->size) { /* Should not happen. See comment near RANDOMNESS_NEEDED. */ min_len = drbg->size; } - drbg->randomness = drbg->secure ? OPENSSL_secure_malloc(drbg->size) + randomness = drbg->secure ? OPENSSL_secure_malloc(drbg->size) : OPENSSL_malloc(drbg->size); /* Get random from parent, include our state as additional input. */ - st = RAND_DRBG_generate(drbg->parent, drbg->randomness, min_len, 0, + st = RAND_DRBG_generate(drbg->parent, randomness, min_len, 0, (unsigned char *)drbg, sizeof(*drbg)); - if (st == 0) + if (st == 0) { + drbg_release_entropy(drbg, randomness, min_len); return 0; - drbg->filled = 1; - *pout = drbg->randomness; + } + *pout = randomness; return min_len; } -void drbg_release_entropy(RAND_DRBG *drbg, unsigned char *out) +void drbg_release_entropy(RAND_DRBG *drbg, unsigned char *out, size_t outlen) { - drbg->filled = 0; if (drbg->secure) - OPENSSL_secure_clear_free(drbg->randomness, drbg->size); + OPENSSL_secure_clear_free(out, outlen); else - OPENSSL_clear_free(drbg->randomness, drbg->size); - drbg->randomness = NULL; + OPENSSL_clear_free(out, outlen); } @@ -191,7 +184,6 @@ static int setup_drbg(RAND_DRBG *drbg) ret &= drbg->lock != NULL; drbg->size = RANDOMNESS_NEEDED; drbg->secure = CRYPTO_secure_malloc_initialized(); - drbg->randomness = NULL; /* If you change these parameters, see RANDOMNESS_NEEDED */ ret &= RAND_DRBG_set(drbg, NID_aes_128_ctr, RAND_DRBG_FLAG_CTR_USE_DF) == 1; diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c index 8090987a2a..4f01e8aad5 100644 --- a/crypto/rand/rand_unix.c +++ b/crypto/rand/rand_unix.c @@ -178,11 +178,11 @@ int RAND_poll_ex(RAND_poll_cb rand_add, void *arg) # endif # ifdef OPENSSL_RAND_SEED_RDTSC - rand_read_tsc(cb, arg); + rand_read_tsc(rand_add, arg); # endif # ifdef OPENSSL_RAND_SEED_RDCPU - if (rand_read_cpu(cb, arg)) + if (rand_read_cpu(rand_add, arg)) goto done; # endif diff --git a/include/internal/rand.h b/include/internal/rand.h index 4e30e38aa1..444c806475 100644 --- a/include/internal/rand.h +++ b/include/internal/rand.h @@ -50,11 +50,12 @@ typedef size_t (*RAND_DRBG_get_entropy_fn)(RAND_DRBG *ctx, int entropy, size_t min_len, size_t max_len); typedef void (*RAND_DRBG_cleanup_entropy_fn)(RAND_DRBG *ctx, - unsigned char *out); + unsigned char *out, size_t outlen); typedef size_t (*RAND_DRBG_get_nonce_fn)(RAND_DRBG *ctx, unsigned char **pout, int entropy, size_t min_len, size_t max_len); -typedef void (*RAND_DRBG_cleanup_nonce_fn)(RAND_DRBG *ctx, unsigned char *out); +typedef void (*RAND_DRBG_cleanup_nonce_fn)(RAND_DRBG *ctx, + unsigned char *out, size_t outlen); int RAND_DRBG_set_callbacks(RAND_DRBG *dctx, RAND_DRBG_get_entropy_fn get_entropy, -- GitLab