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 <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4266)

4 files changed, 21 insertions, 31 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