Merge c5ef9164a6 into f12f8cc035

This function holds a drbgs write lock much longer than it needs to,
and does so unconditionally.  by doing a little re-organization we can,
in the common case just hold the read lock on the drbg, shrinking the
write side critical section such that the write lock need only be held,
in the optimal case, during the actual generation call, and in the
pessimal case, only when the restart/reseed is taking place

Fixes openssl/project#1272

providers/implementations/rands/drbg.c

@@ -627,28 +627,17 @@ int ossl_prov_drbg_generate(PROV_DRBG *drbg, unsigned char *out, size_t outlen,
                             const unsigned char *adin, size_t adinlen)
 {
     int fork_id;
-    int reseed_required = 0;
+    int reseed_required = 1;
     int ret = 0;
 
     if (!ossl_prov_is_running())
         return 0;
 
-    if (drbg->lock != NULL && !CRYPTO_THREAD_write_lock(drbg->lock))
+    fork_id = openssl_get_fork_id();
+
+    if (drbg->lock != NULL && !CRYPTO_THREAD_read_lock(drbg->lock))
         return 0;
 
-    if (drbg->state != EVP_RAND_STATE_READY) {
-        /* try to recover from previous errors */
-        rand_drbg_restart(drbg);
-
-        if (drbg->state == EVP_RAND_STATE_ERROR) {
-            ERR_raise(ERR_LIB_PROV, PROV_R_IN_ERROR_STATE);
-            goto err;
-        }
-        if (drbg->state == EVP_RAND_STATE_UNINITIALISED) {
-            ERR_raise(ERR_LIB_PROV, PROV_R_NOT_INSTANTIATED);
-            goto err;
-        }
-    }
     if (strength > drbg->strength) {
         ERR_raise(ERR_LIB_PROV, PROV_R_INSUFFICIENT_DRBG_STRENGTH);
         goto err;
@@ -663,28 +652,49 @@ int ossl_prov_drbg_generate(PROV_DRBG *drbg, unsigned char *out, size_t outlen,
         goto err;
     }
 
-    fork_id = openssl_get_fork_id();
+    if (drbg->state != EVP_RAND_STATE_READY)
+        goto reseed;
 
-    if (drbg->fork_id != fork_id) {
-        drbg->fork_id = fork_id;
-        reseed_required = 1;
-    }
+    if (prediction_resistance)
+        goto reseed;
+
+    if (drbg->fork_id != fork_id)
+        goto reseed;
+
+    if (drbg->reseed_interval > 0
+        && drbg->generate_counter >= drbg->reseed_interval)
+        goto reseed;
 
-    if (drbg->reseed_interval > 0) {
-        if (drbg->generate_counter >= drbg->reseed_interval)
-            reseed_required = 1;
-    }
     if (drbg->reseed_time_interval > 0) {
         time_t now = time(NULL);
         if (now < drbg->reseed_time
             || now - drbg->reseed_time >= drbg->reseed_time_interval)
-            reseed_required = 1;
+            goto reseed;
     }
+
     if (drbg->parent != NULL
             && get_parent_reseed_count(drbg) != drbg->parent_reseed_counter)
-        reseed_required = 1;
+        goto reseed;
 
-    if (reseed_required || prediction_resistance) {
+    reseed_required = 0;
+    /*
+     * From here to the end of the function, if we are doing locking, we do so
+     * under the write lock
+     */
+reseed:
+    if (drbg->lock != NULL) {
+        /* switch to write lock */
+        CRYPTO_THREAD_unlock(drbg->lock);
+        if (!CRYPTO_THREAD_write_lock(drbg->lock))
+            return 0;
+    }
+
+    if (reseed_required) {
+        drbg->fork_id = fork_id;
+        /*
+         * Note that ossl_prov_drbg_reseed_unlocked also handles
+         * the need for restart for us
+         */
         if (!ossl_prov_drbg_reseed_unlocked(drbg, prediction_resistance, NULL,
                                             0, adin, adinlen)) {
             ERR_raise(ERR_LIB_PROV, PROV_R_RESEED_ERROR);

providers/implementations/rands/drbg_local.h

@@ -81,15 +81,6 @@ struct prov_drbg_st {
     OSSL_FUNC_rand_get_seed_fn *parent_get_seed;
     OSSL_FUNC_rand_clear_seed_fn *parent_clear_seed;
 
-    /*
-     * Stores the return value of openssl_get_fork_id() as of when we last
-     * reseeded.  The DRBG reseeds automatically whenever drbg->fork_id !=
-     * openssl_get_fork_id().  Used to provide fork-safety and reseed this
-     * DRBG in the child process.
-     */
-    int fork_id;
-    unsigned short flags; /* various external flags */
-
     /*
      * The following parameters are setup by the per-type "init" function.
      *
@@ -110,11 +101,11 @@ struct prov_drbg_st {
      * clarification.
      */
 
-    unsigned int strength;
     size_t max_request;
     size_t min_entropylen, max_entropylen;
     size_t min_noncelen, max_noncelen;
     size_t max_perslen, max_adinlen;
+    unsigned int strength;
 
     /*
      * Counts the number of generate requests since the last reseed
@@ -127,6 +118,15 @@ struct prov_drbg_st {
      * This value is ignored if it is zero.
      */
     unsigned int reseed_interval;
+
+    /*
+     * Stores the return value of openssl_get_fork_id() as of when we last
+     * reseeded.  The DRBG reseeds automatically whenever drbg->fork_id !=
+     * openssl_get_fork_id().  Used to provide fork-safety and reseed this
+     * DRBG in the child process.
+     */
+    int fork_id;
+
     /* Stores the time when the last reseeding occurred */
     time_t reseed_time;
     /*
@@ -148,8 +148,8 @@ struct prov_drbg_st {
     unsigned int reseed_next_counter;
     unsigned int parent_reseed_counter;
 
-    size_t seedlen;
     DRBG_STATUS state;
+    size_t seedlen;
 
     /* DRBG specific data */
     void *data;
@@ -161,6 +161,8 @@ struct prov_drbg_st {
     OSSL_INOUT_CALLBACK *get_nonce_fn;
     OSSL_CALLBACK *cleanup_nonce_fn;
 
+    unsigned short flags; /* various external flags */
+
     OSSL_FIPS_IND_DECLARE
 };