From: Bernd Edlinger Date: Sat, 27 Oct 2018 09:31:21 +0000 (+0200) Subject: Avoid two memory allocations in each RAND_DRBG_bytes X-Git-Tag: openssl-3.0.0-alpha1~2970 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=54f3e855d48d08e9623a7ced715e263352c95274 Avoid two memory allocations in each RAND_DRBG_bytes Reviewed-by: Matthias St. Pierre Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/7507) --- diff --git a/crypto/include/internal/rand_int.h b/crypto/include/internal/rand_int.h index 3c966ab96e..888cab1b8f 100644 --- a/crypto/include/internal/rand_int.h +++ b/crypto/include/internal/rand_int.h @@ -45,9 +45,9 @@ size_t rand_drbg_get_nonce(RAND_DRBG *drbg, void rand_drbg_cleanup_nonce(RAND_DRBG *drbg, unsigned char *out, size_t outlen); -size_t rand_drbg_get_additional_data(unsigned char **pout, size_t max_len); +size_t rand_drbg_get_additional_data(RAND_POOL *pool, unsigned char **pout); -void rand_drbg_cleanup_additional_data(unsigned char *out, size_t outlen); +void rand_drbg_cleanup_additional_data(RAND_POOL *pool, unsigned char *out); /* * RAND_POOL functions @@ -59,6 +59,7 @@ void rand_pool_free(RAND_POOL *pool); const unsigned char *rand_pool_buffer(RAND_POOL *pool); unsigned char *rand_pool_detach(RAND_POOL *pool); +void rand_pool_reattach(RAND_POOL *pool, unsigned char *buffer); size_t rand_pool_entropy(RAND_POOL *pool); size_t rand_pool_length(RAND_POOL *pool); diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c index c4ecf0c97e..8e372e593e 100644 --- a/crypto/rand/drbg_lib.c +++ b/crypto/rand/drbg_lib.c @@ -158,8 +158,10 @@ int RAND_DRBG_set(RAND_DRBG *drbg, int type, unsigned int flags) } /* If set is called multiple times - clear the old one */ - if (type != drbg->type && drbg->type != 0 && drbg->meth != NULL) { + if (drbg->type != 0 && (type != drbg->type || flags != drbg->flags)) { drbg->meth->uninstantiate(drbg); + rand_pool_free(drbg->adin_pool); + drbg->adin_pool = NULL; } drbg->state = DRBG_UNINITIALISED; @@ -168,6 +170,7 @@ int RAND_DRBG_set(RAND_DRBG *drbg, int type, unsigned int flags) if (type == 0) { /* Uninitialized; that's okay. */ + drbg->meth = NULL; return 1; } else if (is_ctr(type)) { ret = drbg_ctr_init(drbg); @@ -315,6 +318,7 @@ void RAND_DRBG_free(RAND_DRBG *drbg) if (drbg->meth != NULL) drbg->meth->uninstantiate(drbg); + rand_pool_free(drbg->adin_pool); CRYPTO_THREAD_lock_free(drbg->lock); CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DRBG, drbg, &drbg->ex_data); @@ -722,9 +726,18 @@ int RAND_DRBG_bytes(RAND_DRBG *drbg, unsigned char *out, size_t outlen) unsigned char *additional = NULL; size_t additional_len; size_t chunk; - size_t ret; + size_t ret = 0; - additional_len = rand_drbg_get_additional_data(&additional, drbg->max_adinlen); + if (drbg->adin_pool == NULL) { + if (drbg->type == 0) + goto err; + drbg->adin_pool = rand_pool_new(0, 0, drbg->max_adinlen); + if (drbg->adin_pool == NULL) + goto err; + } + + additional_len = rand_drbg_get_additional_data(drbg->adin_pool, + &additional); for ( ; outlen > 0; outlen -= chunk, out += chunk) { chunk = outlen; @@ -736,9 +749,9 @@ int RAND_DRBG_bytes(RAND_DRBG *drbg, unsigned char *out, size_t outlen) } ret = 1; -err: - if (additional_len != 0) - OPENSSL_secure_clear_free(additional, additional_len); + err: + if (additional != NULL) + rand_drbg_cleanup_additional_data(drbg->adin_pool, additional); return ret; } diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h index 8abca4c290..77be0059c9 100644 --- a/crypto/rand/rand_lcl.h +++ b/crypto/rand/rand_lcl.h @@ -205,6 +205,11 @@ struct rand_drbg_st { */ struct rand_pool_st *pool; + /* + * Auxiliary pool for additional data. + */ + struct rand_pool_st *adin_pool; + /* * The following parameters are setup by the per-type "init" function. * diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c index 555fea36d0..884917a36e 100644 --- a/crypto/rand/rand_lib.c +++ b/crypto/rand/rand_lib.c @@ -279,14 +279,9 @@ void rand_drbg_cleanup_nonce(RAND_DRBG *drbg, * On success it allocates a buffer at |*pout| and returns the length of * the data. The buffer should get freed using OPENSSL_secure_clear_free(). */ -size_t rand_drbg_get_additional_data(unsigned char **pout, size_t max_len) +size_t rand_drbg_get_additional_data(RAND_POOL *pool, unsigned char **pout) { size_t ret = 0; - RAND_POOL *pool; - - pool = rand_pool_new(0, 0, max_len); - if (pool == NULL) - return 0; if (rand_pool_add_additional_data(pool) == 0) goto err; @@ -295,14 +290,12 @@ size_t rand_drbg_get_additional_data(unsigned char **pout, size_t max_len) *pout = rand_pool_detach(pool); err: - rand_pool_free(pool); - return ret; } -void rand_drbg_cleanup_additional_data(unsigned char *out, size_t outlen) +void rand_drbg_cleanup_additional_data(RAND_POOL *pool, unsigned char *out) { - OPENSSL_secure_clear_free(out, outlen); + rand_pool_reattach(pool, out); } void rand_fork(void) @@ -536,17 +529,27 @@ size_t rand_pool_length(RAND_POOL *pool) /* * Detach the |pool| buffer and return it to the caller. * It's the responsibility of the caller to free the buffer - * using OPENSSL_secure_clear_free(). + * using OPENSSL_secure_clear_free() or to re-attach it + * again to the pool using rand_pool_reattach(). */ unsigned char *rand_pool_detach(RAND_POOL *pool) { unsigned char *ret = pool->buffer; pool->buffer = NULL; - pool->len = 0; pool->entropy = 0; return ret; } +/* + * Re-attach the |pool| buffer. It is only allowed to pass + * the |buffer| which was previously detached from the same pool. + */ +void rand_pool_reattach(RAND_POOL *pool, unsigned char *buffer) +{ + pool->buffer = buffer; + OPENSSL_cleanse(pool->buffer, pool->len); + pool->len = 0; +} /* * If |entropy_factor| bits contain 1 bit of entropy, how many bytes does one @@ -643,6 +646,11 @@ int rand_pool_add(RAND_POOL *pool, return 0; } + if (pool->buffer == NULL) { + RANDerr(RAND_F_RAND_POOL_ADD, ERR_R_INTERNAL_ERROR); + return 0; + } + if (len > 0) { memcpy(pool->buffer + pool->len, buffer, len); pool->len += len; @@ -674,6 +682,11 @@ unsigned char *rand_pool_add_begin(RAND_POOL *pool, size_t len) return NULL; } + if (pool->buffer == NULL) { + RANDerr(RAND_F_RAND_POOL_ADD_BEGIN, ERR_R_INTERNAL_ERROR); + return 0; + } + return pool->buffer + pool->len; }