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 <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5294)

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: