From 4f9dabbfe30c3539dd6cb0bd861ddb0127c11c20 Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Thu, 8 Feb 2018 22:46:23 +0100 Subject: [PATCH] DRBG: unify initialization and cleanup code The functions drbg_setup() and drbg_cleanup() used to duplicate a lot of code from RAND_DRBG_new() and RAND_DRBG_free(). This duplication has been removed, which simplifies drbg_setup() and makes drbg_cleanup() obsolete. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/5294) --- crypto/rand/drbg_lib.c | 79 +++++++++++++++++++++++------------------ crypto/rand/rand_lcl.h | 1 + include/internal/rand.h | 1 + util/libcrypto.num | 1 + 4 files changed, 47 insertions(+), 35 deletions(-) diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c index 13b640bb9b..c7a8dde7dc 100644 --- a/crypto/rand/drbg_lib.c +++ b/crypto/rand/drbg_lib.c @@ -114,7 +114,11 @@ static const char ossl_pers_string[] = "OpenSSL NIST SP 800-90A DRBG"; static CRYPTO_ONCE rand_drbg_init = CRYPTO_ONCE_STATIC_INIT; static RAND_DRBG *drbg_setup(RAND_DRBG *parent); -static void drbg_cleanup(RAND_DRBG *drbg); + +static RAND_DRBG *rand_drbg_new(int secure, + int type, + unsigned int flags, + RAND_DRBG *parent); /* * Set/initialize |drbg| to be of type |nid|, with optional |flags|. @@ -149,19 +153,26 @@ int RAND_DRBG_set(RAND_DRBG *drbg, int nid, unsigned int flags) } /* - * Allocate memory and initialize a new DRBG. The |parent|, if not - * NULL, will be used to auto-seed this RAND_DRBG as needed. + * Allocate memory and initialize a new DRBG. The DRBG is allocated on + * the secure heap if |secure| is nonzero and the secure heap is enabled. + * The |parent|, if not NULL, will be used as random source for reseeding. * * Returns a pointer to the new DRBG instance on success, NULL on failure. */ -RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent) +static RAND_DRBG *rand_drbg_new(int secure, + int type, + unsigned int flags, + RAND_DRBG *parent) { - RAND_DRBG *drbg = OPENSSL_zalloc(sizeof(*drbg)); + RAND_DRBG *drbg = secure ? + OPENSSL_secure_zalloc(sizeof(*drbg)) : OPENSSL_zalloc(sizeof(*drbg)); if (drbg == NULL) { RANDerr(RAND_F_RAND_DRBG_NEW, ERR_R_MALLOC_FAILURE); goto err; } + + drbg->secure = secure && CRYPTO_secure_allocated(drbg); drbg->fork_count = rand_fork_count; drbg->parent = parent; if (RAND_DRBG_set(drbg, type, flags) == 0) @@ -175,10 +186,24 @@ RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent) return drbg; err: - OPENSSL_free(drbg); + if (drbg->secure) + OPENSSL_secure_free(drbg); + else + OPENSSL_free(drbg); + return NULL; } +RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent) +{ + return rand_drbg_new(0, type, flags, parent); +} + +RAND_DRBG *RAND_DRBG_secure_new(int type, unsigned int flags, RAND_DRBG *parent) +{ + return rand_drbg_new(1, type, flags, parent); +} + /* * Uninstantiate |drbg| and free all memory. */ @@ -189,8 +214,13 @@ void RAND_DRBG_free(RAND_DRBG *drbg) if (drbg->meth != NULL) drbg->meth->uninstantiate(drbg); + CRYPTO_THREAD_lock_free(drbg->lock); CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DRBG, drbg, &drbg->ex_data); - OPENSSL_clear_free(drbg, sizeof(*drbg)); + + if (drbg->secure) + OPENSSL_secure_clear_free(drbg, sizeof(*drbg)); + else + OPENSSL_clear_free(drbg, sizeof(*drbg)); } /* @@ -717,7 +747,7 @@ int RAND_DRBG_enable_locking(RAND_DRBG *drbg) } if (drbg->lock == NULL) { - if (drbg->parent != NULL && drbg->lock == NULL) { + if (drbg->parent != NULL && drbg->parent->lock == NULL) { RANDerr(RAND_F_RAND_DRBG_ENABLE_LOCKING, RAND_R_PARENT_LOCKING_NOT_ENABLED); return 0; @@ -763,28 +793,17 @@ static RAND_DRBG *drbg_setup(RAND_DRBG *parent) { RAND_DRBG *drbg; - drbg = OPENSSL_secure_zalloc(sizeof(RAND_DRBG)); + drbg = RAND_DRBG_secure_new(RAND_DRBG_NID, RAND_DRBG_FLAG_CTR_USE_DF, parent); if (drbg == NULL) return NULL; - drbg->lock = CRYPTO_THREAD_lock_new(); - if (drbg->lock == NULL) { - RANDerr(RAND_F_DRBG_SETUP, RAND_R_FAILED_TO_CREATE_LOCK); - goto err; - } - - if (RAND_DRBG_set(drbg, - RAND_DRBG_NID, RAND_DRBG_FLAG_CTR_USE_DF) != 1) - goto err; - if (RAND_DRBG_set_callbacks(drbg, rand_drbg_get_entropy, - rand_drbg_cleanup_entropy, NULL, NULL) != 1) + if (RAND_DRBG_enable_locking(drbg) == 0) goto err; if (parent == NULL) { drbg->reseed_interval = MASTER_RESEED_INTERVAL; drbg->reseed_time_interval = MASTER_RESEED_TIME_INTERVAL; } else { - drbg->parent = parent; drbg->reseed_interval = SLAVE_RESEED_INTERVAL; drbg->reseed_time_interval = SLAVE_RESEED_TIME_INTERVAL; } @@ -804,7 +823,7 @@ static RAND_DRBG *drbg_setup(RAND_DRBG *parent) return drbg; err: - drbg_cleanup(drbg); + RAND_DRBG_free(drbg); return NULL; } @@ -831,22 +850,12 @@ DEFINE_RUN_ONCE_STATIC(do_rand_drbg_init) return 1; } -/* Cleans up the given global DRBG */ -static void drbg_cleanup(RAND_DRBG *drbg) -{ - if (drbg != NULL) { - RAND_DRBG_uninstantiate(drbg); - CRYPTO_THREAD_lock_free(drbg->lock); - OPENSSL_secure_clear_free(drbg, sizeof(RAND_DRBG)); - } -} - /* Clean up the global DRBGs before exit */ void rand_drbg_cleanup_int(void) { - drbg_cleanup(drbg_private); - drbg_cleanup(drbg_public); - drbg_cleanup(drbg_master); + RAND_DRBG_free(drbg_private); + RAND_DRBG_free(drbg_public); + RAND_DRBG_free(drbg_master); drbg_private = drbg_public = drbg_master = NULL; } diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h index e3c0b76cde..a63b28be4e 100644 --- a/crypto/rand/rand_lcl.h +++ b/crypto/rand/rand_lcl.h @@ -115,6 +115,7 @@ typedef struct rand_drbg_ctr_st { struct rand_drbg_st { CRYPTO_RWLOCK *lock; RAND_DRBG *parent; + int secure; /* 1: allocated on the secure heap, 0: otherwise */ int nid; /* the underlying algorithm */ int fork_count; unsigned short flags; /* various external flags */ diff --git a/include/internal/rand.h b/include/internal/rand.h index a7d2912069..1dde659716 100644 --- a/include/internal/rand.h +++ b/include/internal/rand.h @@ -28,6 +28,7 @@ * Object lifetime functions. */ RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent); +RAND_DRBG *RAND_DRBG_secure_new(int type, unsigned int flags, RAND_DRBG *parent); int RAND_DRBG_set(RAND_DRBG *drbg, int type, unsigned int flags); int RAND_DRBG_instantiate(RAND_DRBG *drbg, const unsigned char *pers, size_t perslen); diff --git a/util/libcrypto.num b/util/libcrypto.num index b133c66546..c7be54087c 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -4504,3 +4504,4 @@ RAND_DRBG_bytes 4445 1_1_1 EXIST::FUNCTION: RAND_DRBG_lock 4446 1_1_1 EXIST::FUNCTION: RAND_DRBG_unlock 4447 1_1_1 EXIST::FUNCTION: RAND_DRBG_enable_locking 4448 1_1_1 EXIST::FUNCTION: +RAND_DRBG_secure_new 4449 1_1_1 EXIST::FUNCTION: -- 2.25.1