Make sure we use a nonce when a nonce is required

If a nonce is required and the get_nonce callback is NULL, request 50%
more entropy following NIST SP800-90Ar1 section 9.1.

Reviewed-by: Dr. Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
GH: #5503

4 files changed, 106 insertions, 29 deletions

diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index e1b3ddb57f..b9ad1b8867 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -266,6 +266,9 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
 {
     unsigned char *nonce = NULL, *entropy = NULL;
     size_t noncelen = 0, entropylen = 0;
+    size_t min_entropy = drbg->strength;
+    size_t min_entropylen = drbg->min_entropylen;
+    size_t max_entropylen = drbg->max_entropylen;
 
     if (perslen > drbg->max_perslen) {
         RANDerr(RAND_F_RAND_DRBG_INSTANTIATE,
@@ -288,22 +291,33 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
     }
 
     drbg->state = DRBG_ERROR;
+
+    /*
+     * NIST SP800-90Ar1 section 9.1 says you can combine getting the entropy
+     * and nonce in 1 call by increasing the entropy with 50% and increasing
+     * the minimum length to accomadate the length of the nonce.
+     * We do this in case a nonce is require and get_nonce is NULL.
+     */
+    if (drbg->min_noncelen > 0 && drbg->get_nonce == NULL) {
+        min_entropy += drbg->strength / 2;
+        min_entropylen += drbg->min_noncelen;
+        max_entropylen += drbg->max_noncelen;
+    }
+
     if (drbg->get_entropy != NULL)
-        entropylen = drbg->get_entropy(drbg, &entropy, drbg->strength,
-                                       drbg->min_entropylen,
-                                       drbg->max_entropylen, 0);
-    if (entropylen < drbg->min_entropylen
-        || entropylen > drbg->max_entropylen) {
+        entropylen = drbg->get_entropy(drbg, &entropy, min_entropy,
+                                       min_entropylen, max_entropylen, 0);
+    if (entropylen < min_entropylen
+        || entropylen > max_entropylen) {
         RANDerr(RAND_F_RAND_DRBG_INSTANTIATE, RAND_R_ERROR_RETRIEVING_ENTROPY);
         goto end;
     }
 
-    if (drbg->max_noncelen > 0 && drbg->get_nonce != NULL) {
+    if (drbg->min_noncelen > 0 && drbg->get_nonce != NULL) {
         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;
         }
     }
diff --git a/crypto/rand/rand_lcl.h b/crypto/rand/rand_lcl.h
index 0a34aa0b93..94ffc96f20 100644
--- a/crypto/rand/rand_lcl.h
+++ b/crypto/rand/rand_lcl.h
@@ -108,6 +108,27 @@ typedef struct rand_drbg_ctr_st {
 
 
 /*
+ * The 'random pool' acts as a dumb container for collecting random
+ * input from various entropy sources. The pool has no knowledge about
+ * whether its randomness is fed into a legacy RAND_METHOD via RAND_add()
+ * or into a new style RAND_DRBG. It is the callers duty to 1) initialize the
+ * random pool, 2) pass it to the polling callbacks, 3) seed the RNG, and
+ * 4) cleanup the random pool again.
+ *
+ * The random pool contains no locking mechanism because its scope and
+ * lifetime is intended to be restricted to a single stack frame.
+ */
+struct rand_pool_st {
+    unsigned char *buffer;  /* points to the beginning of the random pool */
+    size_t len; /* current number of random bytes contained in the pool */
+
+    size_t min_len; /* minimum number of random bytes requested */
+    size_t max_len; /* maximum number of random bytes (allocated buffer size) */
+    size_t entropy; /* current entropy count in bits */
+    size_t requested_entropy; /* requested entropy count in bits */
+};
+
+/*
  * The state of all types of DRBGs, even though we only have CTR mode
  * right now.
  */
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index defa3ecb53..143dfb0f19 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -467,27 +467,6 @@ err:
 }
 
 /*
- * The 'random pool' acts as a dumb container for collecting random
- * input from various entropy sources. The pool has no knowledge about
- * whether its randomness is fed into a legacy RAND_METHOD via RAND_add()
- * or into a new style RAND_DRBG. It is the callers duty to 1) initialize the
- * random pool, 2) pass it to the polling callbacks, 3) seed the RNG, and
- * 4) cleanup the random pool again.
- *
- * The random pool contains no locking mechanism because its scope and
- * lifetime is intended to be restricted to a single stack frame.
- */
-struct rand_pool_st {
-    unsigned char *buffer;  /* points to the beginning of the random pool */
-    size_t len; /* current number of random bytes contained in the pool */
-
-    size_t min_len; /* minimum number of random bytes requested */
-    size_t max_len; /* maximum number of random bytes (allocated buffer size) */
-    size_t entropy; /* current entropy count in bits */
-    size_t requested_entropy; /* requested entropy count in bits */
-};
-
-/*
  * Allocate memory and initialize a new random pool
  */
 
diff --git a/test/drbgtest.c b/test/drbgtest.c
index bef504ebd1..5426046854 100644
--- a/test/drbgtest.c
+++ b/test/drbgtest.c
@@ -16,6 +16,7 @@
 #include <openssl/evp.h>
 #include <openssl/aes.h>
 #include "../crypto/rand/rand_lcl.h"
+#include "../crypto/include/internal/rand_int.h"
 
 #if defined(_WIN32)
 # include <windows.h>
@@ -864,6 +865,67 @@ static int test_multi_thread(void)
 }
 #endif
 
+/*
+ * This function only returns the entropy already added with RAND_add(),
+ * and does not get entropy from the OS.
+ *
+ * Returns 0 on failure and the size of the buffer on success.
+ */
+static size_t get_pool_entropy(RAND_DRBG *drbg,
+                               unsigned char **pout,
+                               int entropy, size_t min_len, size_t max_len,
+                               int prediction_resistance)
+{
+    if (drbg->pool == NULL)
+        return 0;
+
+    if (drbg->pool->entropy < (size_t)entropy || drbg->pool->len < min_len
+        || drbg->pool->len > max_len)
+        return 0;
+
+    *pout = drbg->pool->buffer;
+    return drbg->pool->len;
+}
+
+/*
+ * Clean up the entropy that get_pool_entropy() returned.
+ */
+static void cleanup_pool_entropy(RAND_DRBG *drbg, unsigned char *out, size_t outlen)
+{
+    OPENSSL_secure_clear_free(drbg->pool->buffer, drbg->pool->max_len);
+    OPENSSL_free(drbg->pool);
+    drbg->pool = NULL;
+}
+
+/*
+ * Test that instantiating works when OS entropy is not available and that
+ * RAND_add() is enough to reseed it.
+ */
+static int test_rand_add(void)
+{
+    RAND_DRBG *master = RAND_DRBG_get0_master();
+    RAND_DRBG_get_entropy_fn old_get_entropy = master->get_entropy;
+    RAND_DRBG_cleanup_entropy_fn old_cleanup_entropy = master->cleanup_entropy;
+    int rv = 0;
+    unsigned char rand_add_buf[256];
+
+    master->get_entropy = get_pool_entropy;
+    master->cleanup_entropy = cleanup_pool_entropy;
+    master->reseed_counter++;
+    RAND_DRBG_uninstantiate(master);
+    memset(rand_add_buf, 0xCD, sizeof(rand_add_buf));
+    RAND_add(rand_add_buf, sizeof(rand_add_buf), sizeof(rand_add_buf));
+    if (!TEST_true(RAND_DRBG_instantiate(master, NULL, 0)))
+        goto error;
+
+    rv = 1;
+
+error:
+    master->get_entropy = old_get_entropy;
+    master->cleanup_entropy = old_cleanup_entropy;
+    return rv;
+}
+
 int setup_tests(void)
 {
     app_data_index = RAND_DRBG_get_ex_new_index(0L, NULL, NULL, NULL, NULL);
@@ -871,6 +933,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_kats, OSSL_NELEM(drbg_test));
     ADD_ALL_TESTS(test_error_checks, OSSL_NELEM(drbg_test));
     ADD_TEST(test_rand_reseed);
+    ADD_TEST(test_rand_add);
 #if defined(OPENSSL_THREADS)
     ADD_TEST(test_multi_thread);
 #endif