Fix reseeding issues of the public RAND_DRBG

Reseeding is handled very differently by the classic RAND_METHOD API
and the new RAND_DRBG api. These differences led to some problems when
the new RAND_DRBG was made the default OpenSSL RNG. In particular,
RAND_add() did not work as expected anymore. These issues are discussed
on the thread '[openssl-dev] Plea for a new public OpenSSL RNG API'
and in Pull Request #4328. This commit fixes the mentioned issues,
introducing the following changes:

- Replace the fixed size RAND_BYTES_BUFFER by a new RAND_POOL API which
  facilitates collecting entropy by the get_entropy() callback.
- Don't use RAND_poll()/RAND_add() for collecting entropy from the
  get_entropy() callback anymore. Instead, replace RAND_poll() by
  RAND_POOL_acquire_entropy().
- Add a new function rand_drbg_restart() which tries to get the DRBG
  in an instantiated state by all means, regardless of the current
  state (uninstantiated, error, ...) the DRBG is in. If the caller
  provides entropy or additional input, it will be used for reseeding.
- Restore the original documented behaviour of RAND_add() and RAND_poll()
  (namely to reseed the DRBG immediately) by a new implementation based
  on rand_drbg_restart().
- Add automatic error recovery from temporary failures of the entropy
  source to RAND_DRBG_generate() using the rand_drbg_restart() function.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4328)

1 files changed, 249 insertions, 62 deletions

diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index b7f7e4c341..eef5e11cc5 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -18,6 +18,9 @@
 static RAND_DRBG rand_drbg; /* The default global DRBG. */
 static RAND_DRBG priv_drbg; /* The global private-key DRBG. */
 
+/* NIST SP 800-90A DRBG recommends the use of a personalization string. */
+static const char ossl_pers_string[] = "OpenSSL NIST SP 800-90A DRBG";
+
 /*
  * Support framework for NIST SP 800-90A DRBG, AES-CTR mode.
  * The RAND_DRBG is OpenSSL's pointer to an instance of the DRBG.
@@ -30,7 +33,9 @@ static RAND_DRBG priv_drbg; /* The global private-key DRBG. */
  * a much bigger deal than just re-setting an allocated resource.)
  */
 
-static CRYPTO_ONCE rand_init_drbg = CRYPTO_ONCE_STATIC_INIT;
+static CRYPTO_ONCE rand_drbg_init = CRYPTO_ONCE_STATIC_INIT;
+
+static int drbg_setup(RAND_DRBG *drbg, const char *name);
 
 /*
  * Set/initialize |drbg| to be of type |nid|, with optional |flags|.
@@ -76,15 +81,14 @@ RAND_DRBG *RAND_DRBG_new(int type, unsigned int flags, RAND_DRBG *parent)
         RANDerr(RAND_F_RAND_DRBG_NEW, ERR_R_MALLOC_FAILURE);
         goto err;
     }
-    drbg->size = RANDOMNESS_NEEDED;
     drbg->fork_count = rand_fork_count;
     drbg->parent = parent;
     if (RAND_DRBG_set(drbg, type, flags) < 0)
         goto err;
 
     if (parent != NULL) {
-        if (!RAND_DRBG_set_callbacks(drbg, drbg_entropy_from_parent,
-                                     drbg_release_entropy,
+        if (!RAND_DRBG_set_callbacks(drbg, rand_drbg_get_entropy,
+                                     rand_drbg_cleanup_entropy,
                                      NULL, NULL))
             goto err;
     }
@@ -101,8 +105,7 @@ err:
  */
 void RAND_DRBG_free(RAND_DRBG *drbg)
 {
-    /* The global DRBG is free'd by rand_cleanup_drbg_int() */
-    if (drbg == NULL || drbg == &rand_drbg)
+    if (drbg == NULL)
         return;
 
     ctr_uninstantiate(drbg);
@@ -136,7 +139,8 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
     if (drbg->get_entropy != NULL)
         entropylen = drbg->get_entropy(drbg, &entropy, drbg->strength,
                                    drbg->min_entropylen, drbg->max_entropylen);
-    if (entropylen < drbg->min_entropylen || entropylen > drbg->max_entropylen) {
+    if (entropylen < drbg->min_entropylen
+        || entropylen > drbg->max_entropylen) {
         RANDerr(RAND_F_RAND_DRBG_INSTANTIATE, RAND_R_ERROR_RETRIEVING_ENTROPY);
         goto end;
     }
@@ -145,7 +149,8 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
         noncelen = drbg->get_nonce(drbg, &nonce, drbg->strength / 2,
                                    drbg->min_noncelen, drbg->max_noncelen);
         if (noncelen < drbg->min_noncelen || noncelen > drbg->max_noncelen) {
-            RANDerr(RAND_F_RAND_DRBG_INSTANTIATE, RAND_R_ERROR_RETRIEVING_NONCE);
+            RANDerr(RAND_F_RAND_DRBG_INSTANTIATE,
+                    RAND_R_ERROR_RETRIEVING_NONCE);
             goto end;
         }
     }
@@ -164,6 +169,15 @@ end:
         drbg->cleanup_entropy(drbg, entropy, entropylen);
     if (nonce != NULL && drbg->cleanup_nonce!= NULL )
         drbg->cleanup_nonce(drbg, nonce, noncelen);
+    if (drbg->pool != NULL) {
+        if (drbg->state == DRBG_READY) {
+            RANDerr(RAND_F_RAND_DRBG_INSTANTIATE,
+                    RAND_R_ERROR_ENTROPY_POOL_WAS_IGNORED);
+            drbg->state = DRBG_ERROR;
+        }
+        RAND_POOL_free(drbg->pool);
+        drbg->pool = NULL;
+    }
     if (drbg->state == DRBG_READY)
         return 1;
     return 0;
@@ -182,7 +196,7 @@ int RAND_DRBG_uninstantiate(RAND_DRBG *drbg)
 }
 
 /*
- * Mix in the specified data to reseed |drbg|.
+ * Reseed |drbg|, mixing in the specified data
  */
 int RAND_DRBG_reseed(RAND_DRBG *drbg,
                      const unsigned char *adin, size_t adinlen)
@@ -210,7 +224,8 @@ int RAND_DRBG_reseed(RAND_DRBG *drbg,
     if (drbg->get_entropy != NULL)
         entropylen = drbg->get_entropy(drbg, &entropy, drbg->strength,
                                    drbg->min_entropylen, drbg->max_entropylen);
-    if (entropylen < drbg->min_entropylen || entropylen > drbg->max_entropylen) {
+    if (entropylen < drbg->min_entropylen
+        || entropylen > drbg->max_entropylen) {
         RANDerr(RAND_F_RAND_DRBG_RESEED, RAND_R_ERROR_RETRIEVING_ENTROPY);
         goto end;
     }
@@ -229,22 +244,132 @@ end:
 }
 
 /*
+ * Restart |drbg|, using the specified entropy or additional input
+ *
+ * Tries its best to get the drbg instantiated by all means,
+ * regardless of its current state.
+ *
+ * Optionally, a |buffer| of |len| random bytes can be passed,
+ * which is assumed to contain at least |entropy| bits of entropy.
+ *
+ * If |entropy| > 0, the buffer content is used as entropy input.
+ *
+ * If |entropy| == 0, the buffer content is used as additional input
+ *
+ * Returns 1 on success, 0 on failure.
+ *
+ * This function is used internally only.
+ */
+int rand_drbg_restart(RAND_DRBG *drbg,
+                      const unsigned char *buffer, size_t len, size_t entropy)
+{
+    int reseeded = 0;
+    const unsigned char *adin = NULL;
+    size_t adinlen = 0;
+
+    if (drbg->pool != NULL) {
+        RANDerr(RAND_F_RAND_DRBG_RESTART, ERR_R_INTERNAL_ERROR);
+        RAND_POOL_free(drbg->pool);
+        drbg->pool = NULL;
+    }
+
+    if (buffer != NULL) {
+        if (entropy > 0) {
+            if (drbg->max_entropylen < len) {
+                RANDerr(RAND_F_RAND_DRBG_RESTART,
+                    RAND_R_ENTROPY_INPUT_TOO_LONG);
+                return 0;
+            }
+
+            if (entropy > 8 * len) {
+                RANDerr(RAND_F_RAND_DRBG_RESTART, RAND_R_ENTROPY_OUT_OF_RANGE);
+                return 0;
+            }
+
+            /* will be picked up by the rand_drbg_get_entropy() callback */
+            drbg->pool = RAND_POOL_new(entropy, len, len);
+            if (drbg->pool == NULL)
+                return 0;
+
+            RAND_POOL_add(drbg->pool, buffer, len, entropy);
+        } else {
+            if (drbg->max_adinlen < len) {
+                RANDerr(RAND_F_RAND_DRBG_RESTART,
+                        RAND_R_ADDITIONAL_INPUT_TOO_LONG);
+                return 0;
+            }
+            adin = buffer;
+            adinlen = len;
+        }
+    }
+
+    /* repair error state */
+    if (drbg->state == DRBG_ERROR)
+        RAND_DRBG_uninstantiate(drbg);
+
+    /* repair uninitialized state */
+    if (drbg->state == DRBG_UNINITIALISED) {
+        drbg_setup(drbg, NULL);
+        /* already reseeded. prevent second reseeding below */
+        reseeded = (drbg->state == DRBG_READY);
+    }
+
+    /* refresh current state if entropy or additional input has been provided */
+    if (drbg->state == DRBG_READY) {
+        if (adin != NULL) {
+            /*
+             * mix in additional input without reseeding
+             *
+             * Similar to RAND_DRBG_reseed(), but the provided additional
+             * data |adin| is mixed into the current state without pulling
+             * entropy from the trusted entropy source using get_entropy().
+             * This is not a reseeding in the strict sense of NIST SP 800-90A.
+             */
+            ctr_reseed(drbg, adin, adinlen, NULL, 0);
+        } else if (reseeded == 0) {
+            /* do a full reseeding if it has not been done yet above */
+            RAND_DRBG_reseed(drbg, NULL, 0);
+        }
+    }
+
+    /* check whether a given entropy pool was cleared properly during reseed */
+    if (drbg->pool != NULL) {
+        drbg->state = DRBG_ERROR;
+        RANDerr(RAND_F_RAND_DRBG_RESTART, ERR_R_INTERNAL_ERROR);
+        RAND_POOL_free(drbg->pool);
+        drbg->pool = NULL;
+        return 0;
+    }
+
+    return drbg->state == DRBG_READY;
+}
+
+/*
  * Generate |outlen| bytes into the buffer at |out|.  Reseed if we need
  * to or if |prediction_resistance| is set.  Additional input can be
  * sent in |adin| and |adinlen|.
+ *
+ * Returns 1 on success, 0 on failure.
+ *
  */
 int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
                        int prediction_resistance,
                        const unsigned char *adin, size_t adinlen)
 {
-    if (drbg->state == DRBG_ERROR) {
-        RANDerr(RAND_F_RAND_DRBG_GENERATE, RAND_R_IN_ERROR_STATE);
-        return 0;
-    }
-    if (drbg->state == DRBG_UNINITIALISED) {
-        RANDerr(RAND_F_RAND_DRBG_GENERATE, RAND_R_NOT_INSTANTIATED);
-        return 0;
+    if (drbg->state != DRBG_READY) {
+        /* try to recover from previous errors */
+        rand_drbg_restart(drbg, NULL, 0, 0);
+
+        if (drbg->state == DRBG_ERROR) {
+            RANDerr(RAND_F_RAND_DRBG_GENERATE, RAND_R_IN_ERROR_STATE);
+            return 0;
+        }
+        if (drbg->state == DRBG_UNINITIALISED) {
+            RANDerr(RAND_F_RAND_DRBG_GENERATE, RAND_R_NOT_INSTANTIATED);
+            return 0;
+        }
     }
+
     if (outlen > drbg->max_request) {
         RANDerr(RAND_F_RAND_DRBG_GENERATE, RAND_R_REQUEST_TOO_LARGE_FOR_DRBG);
         return 0;
@@ -285,21 +410,55 @@ int RAND_DRBG_generate(RAND_DRBG *drbg, unsigned char *out, size_t outlen,
 }
 
 /*
- * Set the callbacks for entropy and nonce.  We currently don't use
- * the nonce; that's mainly for the KATs
+ * Set the RAND_DRBG callbacks for obtaining entropy and nonce.
+ *
+ * In the following, the signature and the semantics of the
+ * get_entropy() and cleanup_entropy() callbacks are explained.
+ *
+ * GET_ENTROPY
+ *
+ *     size_t get_entropy(RAND_DRBG *ctx,
+ *                        unsigned char **pout,
+ *                        int entropy,
+ *                        size_t min_len, size_t max_len);
+ *
+ * This is a request to allocate and fill a buffer of size
+ * |min_len| <= size <= |max_len| (in bytes) which contains
+ * at least |entropy| bits of randomness. The buffer's address is
+ * to be returned in |*pout| and the number of collected
+ * randomness bytes (which may be less than the allocated size
+ * of the buffer) as return value.
+ *
+ * If the callback fails to acquire at least |entropy| bits of
+ * randomness, it shall return a buffer length of 0.
+ *
+ * CLEANUP_ENTROPY
+ *
+ *     void cleanup_entropy(RAND_DRBG *ctx,
+ *                          unsigned char *out, size_t outlen);
+ *
+ * A request to clear and free the buffer allocated by get_entropy().
+ * The values |out| and |outlen| are expected to be the random buffer's
+ * address and length, as returned by the get_entropy() callback.
+ *
+ * GET_NONCE, CLEANUP_NONCE
+ *
+ * Signature and semantics of the get_nonce() and cleanup_nonce()
+ * callbacks are analogous to get_entropy() and cleanup_entropy().
+ * Currently, the nonce is used only for the known answer tests.
  */
 int RAND_DRBG_set_callbacks(RAND_DRBG *drbg,
-                            RAND_DRBG_get_entropy_fn cb_get_entropy,
-                            RAND_DRBG_cleanup_entropy_fn cb_cleanup_entropy,
-                            RAND_DRBG_get_nonce_fn cb_get_nonce,
-                            RAND_DRBG_cleanup_nonce_fn cb_cleanup_nonce)
+                            RAND_DRBG_get_entropy_fn get_entropy,
+                            RAND_DRBG_cleanup_entropy_fn cleanup_entropy,
+                            RAND_DRBG_get_nonce_fn get_nonce,
+                            RAND_DRBG_cleanup_nonce_fn cleanup_nonce)
 {
     if (drbg->state != DRBG_UNINITIALISED)
         return 0;
-    drbg->get_entropy = cb_get_entropy;
-    drbg->cleanup_entropy = cb_cleanup_entropy;
-    drbg->get_nonce = cb_get_nonce;
-    drbg->cleanup_nonce = cb_cleanup_nonce;
+    drbg->get_entropy = get_entropy;
+    drbg->cleanup_entropy = cleanup_entropy;
+    drbg->get_nonce = get_nonce;
+    drbg->cleanup_nonce = cleanup_nonce;
     return 1;
 }
 
@@ -334,23 +493,40 @@ void *RAND_DRBG_get_ex_data(const RAND_DRBG *drbg, int idx)
  */
 
 /*
- * Creates a global DRBG with default settings.
+ * Initializes the DRBG with default settings.
+ * For global DRBGs a global lock is created with the given name
  * Returns 1 on success, 0 on failure
  */
-static int setup_drbg(RAND_DRBG *drbg, const char *name)
+static int drbg_setup(RAND_DRBG *drbg, const char *name)
 {
     int ret = 1;
 
-    drbg->lock = CRYPTO_THREAD_glock_new(name);
-    ret &= drbg->lock != NULL;
-    drbg->size = RANDOMNESS_NEEDED;
-    drbg->secure = CRYPTO_secure_malloc_initialized();
-    /* If you change these parameters, see RANDOMNESS_NEEDED */
+    if (name != NULL) {
+        if (drbg->lock != NULL) {
+            RANDerr(RAND_F_DRBG_SETUP, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+
+        drbg->lock = CRYPTO_THREAD_glock_new(name);
+        if (drbg->lock == NULL) {
+            RANDerr(RAND_F_DRBG_SETUP, RAND_R_FAILED_TO_CREATE_LOCK);
+            return 0;
+        }
+    }
+
     ret &= RAND_DRBG_set(drbg,
-                         NID_aes_128_ctr, RAND_DRBG_FLAG_CTR_USE_DF) == 1;
-    ret &= RAND_DRBG_set_callbacks(drbg, drbg_entropy_from_system,
-                                   drbg_release_entropy, NULL, NULL) == 1;
-    ret &= RAND_DRBG_instantiate(drbg, NULL, 0) == 1;
+                         RAND_DRBG_NID, RAND_DRBG_FLAG_CTR_USE_DF) == 1;
+    ret &= RAND_DRBG_set_callbacks(drbg, rand_drbg_get_entropy,
+                                   rand_drbg_cleanup_entropy, NULL, NULL) == 1;
+    /*
+     * Ignore instantiation error so support just-in-time instantiation.
+     *
+     * The state of the drbg will be checked in RAND_DRBG_generate() and
+     * an automatic recovery is attempted.
+     */
+    RAND_DRBG_instantiate(drbg,
+                          (const unsigned char *) ossl_pers_string,
+                          sizeof(ossl_pers_string) - 1);
     return ret;
 }
 
@@ -358,30 +534,31 @@ static int setup_drbg(RAND_DRBG *drbg, const char *name)
  * Initialize the global DRBGs on first use.
  * Returns 1 on success, 0 on failure.
  */
-DEFINE_RUN_ONCE_STATIC(do_rand_init_drbg)
+DEFINE_RUN_ONCE_STATIC(do_rand_drbg_init)
 {
     int ret = 1;
 
-    ret &= setup_drbg(&rand_drbg, "rand_drbg");
-    ret &= setup_drbg(&priv_drbg, "priv_drbg");
+    ret &= drbg_setup(&rand_drbg, "rand_drbg");
+    ret &= drbg_setup(&priv_drbg, "priv_drbg");
 
     return ret;
 }
 
-/* Clean up a DRBG and free it */
-static void free_drbg(RAND_DRBG *drbg)
+/* Cleans up the given global DRBG  */
+static void drbg_cleanup(RAND_DRBG *drbg)
 {
     CRYPTO_THREAD_lock_free(drbg->lock);
     RAND_DRBG_uninstantiate(drbg);
 }
 
 /* Clean up the global DRBGs before exit */
-void rand_cleanup_drbg_int(void)
+void rand_drbg_cleanup_int(void)
 {
-    free_drbg(&rand_drbg);
-    free_drbg(&priv_drbg);
+    drbg_cleanup(&rand_drbg);
+    drbg_cleanup(&priv_drbg);
 }
 
+/* Implements the default OpenSSL RAND_bytes() method */
 static int drbg_bytes(unsigned char *out, int count)
 {
     int ret = 0;
@@ -410,34 +587,44 @@ err:
     return ret;
 }
 
+/* Implements the default OpenSSL RAND_add() method */
 static int drbg_add(const void *buf, int num, double randomness)
 {
-    unsigned char *in = (unsigned char *)buf;
-    unsigned char *out, *end;
+    int ret = 0;
+    RAND_DRBG *drbg = RAND_DRBG_get0_global();
 
-    CRYPTO_THREAD_write_lock(rand_bytes.lock);
-    out = &rand_bytes.buff[rand_bytes.curr];
-    end = &rand_bytes.buff[rand_bytes.size];
+    if (drbg == NULL)
+        return 0;
 
-    /* Copy whatever fits into the end of the buffer. */
-    for ( ; --num >= 0 && out < end; rand_bytes.curr++)
-        *out++ = *in++;
+    if (num < 0 || randomness < 0.0)
+        return 0;
 
-    /* XOR any the leftover. */
-    while (num > 0) {
-        for (out = rand_bytes.buff; --num >= 0 && out < end; )
-            *out++ ^= *in++;
+    if (randomness > (double)drbg->max_entropylen) {
+        /*
+         * The purpose of this check is to bound |randomness| by a
+         * relatively small value in order to prevent an integer
+         * overflow when multiplying by 8 in the rand_drbg_restart()
+         * call below.
+         */
+        return 0;
     }
 
-    CRYPTO_THREAD_unlock(rand_bytes.lock);
-    return 1;
+    CRYPTO_THREAD_write_lock(drbg->lock);
+    ret = rand_drbg_restart(drbg, buf,
+                            (size_t)(unsigned int)num,
+                            (size_t)(8*randomness));
+    CRYPTO_THREAD_unlock(drbg->lock);
+
+    return ret;
 }
 
+/* Implements the default OpenSSL RAND_seed() method */
 static int drbg_seed(const void *buf, int num)
 {
     return drbg_add(buf, num, num);
 }
 
+/* Implements the default OpenSSL RAND_status() method */
 static int drbg_status(void)
 {
     int ret;
@@ -458,7 +645,7 @@ static int drbg_status(void)
  */
 RAND_DRBG *RAND_DRBG_get0_global(void)
 {
-    if (!RUN_ONCE(&rand_init_drbg, do_rand_init_drbg))
+    if (!RUN_ONCE(&rand_drbg_init, do_rand_drbg_init))
         return NULL;
 
     return &rand_drbg;
@@ -470,7 +657,7 @@ RAND_DRBG *RAND_DRBG_get0_global(void)
  */
 RAND_DRBG *RAND_DRBG_get0_priv_global(void)
 {
-    if (!RUN_ONCE(&rand_init_drbg, do_rand_init_drbg))
+    if (!RUN_ONCE(&rand_drbg_init, do_rand_drbg_init))
         return NULL;
 
     return &priv_drbg;