Avoid two memory allocations in each RAND_DRBG_bytes

Reviewed-by: NMatthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: NPaul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7518)

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);

crypto/rand/drbg_lib.c

@@ -109,6 +109,13 @@ int RAND_DRBG_set(RAND_DRBG *drbg, int type, unsigned int flags)
         flags = rand_drbg_flags;
     }
 
+    /* If set is called multiple times - clear the old one */
+    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;
     drbg->flags = flags;
     drbg->type = type;
@@ -122,6 +129,7 @@ int RAND_DRBG_set(RAND_DRBG *drbg, int type, unsigned int flags)
         return 0;
     case 0:
         /* Uninitialized; that's okay. */
+        drbg->meth = NULL;
         return 1;
     case NID_aes_128_ctr:
     case NID_aes_192_ctr:
@@ -259,6 +267,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);
 
@@ -650,9 +659,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;
+
+    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(&additional, drbg->max_adinlen);
+    additional_len = rand_drbg_get_additional_data(drbg->adin_pool,
+                                                   &additional);
 
     for ( ; outlen > 0; outlen -= chunk, out += chunk) {
         chunk = outlen;
@@ -664,9 +682,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;
 }

crypto/rand/rand_lcl.h

@@ -185,6 +185,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.
      *

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;
 }