Enable locking on the primary DRBG when we create it

The primary DRBG may be shared across multiple threads and therefore
we must use locking to access it. Previously we were enabling that locking
lazily when we attempted to obtain one of the child DRBGs. Part of the
process of enabling the lock, is to create the lock. But if we create the
lock lazily then it is too late - we may race with other threads where each
thread is independently attempting to enable the locking. This results
in multiple locks being created - only one of which "sticks" and the rest
are leaked.

Instead we enable locking on the primary when we first create it. This is
already locked and therefore we cannot race.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13660)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index bb200a796081b7d9cc3fe2a2835f0216b84fe55b..40f93ba0cd84564569525a1f364aac035b8a5084 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2609,7 +2609,7 @@ EVP_R_PRIVATE_KEY_ENCODE_ERROR:146:private key encode error
 EVP_R_PUBLIC_KEY_NOT_RSA:106:public key not rsa
 EVP_R_SET_DEFAULT_PROPERTY_FAILURE:209:set default property failure
 EVP_R_TOO_MANY_RECORDS:183:too many records
-EVP_R_UNABLE_TO_ENABLE_PARENT_LOCKING:212:unable to enable parent locking
+EVP_R_UNABLE_TO_ENABLE_LOCKING:212:unable to enable locking
 EVP_R_UNABLE_TO_GET_MAXIMUM_REQUEST_SIZE:215:unable to get maximum request size
 EVP_R_UNABLE_TO_GET_RANDOM_STRENGTH:216:unable to get random strength
 EVP_R_UNABLE_TO_LOCK_CONTEXT:211:unable to lock context

diff --git a/crypto/evp/evp_err.c b/crypto/evp/evp_err.c
index e08c373b33ef840631a085887af1529fd24d5cf2..ab6e3e879d2b94d2b4b874ae49da27da17d1ff81 100644 (file)
--- a/crypto/evp/evp_err.c
+++ b/crypto/evp/evp_err.c
@@ -152,7 +152,7 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_SET_DEFAULT_PROPERTY_FAILURE),
     "set default property failure"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_TOO_MANY_RECORDS), "too many records"},
-    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_UNABLE_TO_ENABLE_PARENT_LOCKING),
+    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_UNABLE_TO_ENABLE_LOCKING),
     "unable to enable parent locking"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_UNABLE_TO_GET_MAXIMUM_REQUEST_SIZE),
     "unable to get maximum request size"},

diff --git a/crypto/evp/evp_rand.c b/crypto/evp/evp_rand.c
index 6a4f57414c35ab7c3c93b0df83cc2b614768a5d7..42e51b4ea1ef038ce2a62ab315bf2da91d224589 100644 (file)
--- a/crypto/evp/evp_rand.c
+++ b/crypto/evp/evp_rand.c
@@ -333,12 +333,6 @@ EVP_RAND_CTX *EVP_RAND_CTX_new(EVP_RAND *rand, EVP_RAND_CTX *parent)
         return NULL;
     }
     if (parent != NULL) {
-        if (!EVP_RAND_enable_locking(parent)) {
-            ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_PARENT_LOCKING);
-            CRYPTO_THREAD_lock_free(ctx->refcnt_lock);
-            OPENSSL_free(ctx);
-            return NULL;
-        }
         if (!evp_rand_ctx_up_ref(parent)) {
             ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
             CRYPTO_THREAD_lock_free(ctx->refcnt_lock);

diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index f0284aab0897fd84b111e525d33c17bacf961530..01927401abf8a28719c94f9258a874bdb3580611 100644 (file)
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -571,6 +571,17 @@ EVP_RAND_CTX *RAND_get0_primary(OSSL_LIB_CTX *ctx)
             dgbl->primary = rand_new_drbg(ctx, dgbl->seed,
                                           PRIMARY_RESEED_INTERVAL,
                                           PRIMARY_RESEED_TIME_INTERVAL);
+        /*
+         * The primary DRBG may be shared between multiple threads so we must
+         * enable locking.
+         */
+        if (dgbl->primary != NULL && !EVP_RAND_enable_locking(dgbl->primary)) {
+            ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_LOCKING);
+            EVP_RAND_CTX_free(dgbl->primary);
+            dgbl->primary = NULL;
+            CRYPTO_THREAD_lock_free(dgbl->lock);
+            return NULL;
+        }
         CRYPTO_THREAD_unlock(dgbl->lock);
     }
     return dgbl->primary;

diff --git a/doc/man3/EVP_RAND.pod b/doc/man3/EVP_RAND.pod
index e53cddff2ffe1bf5cbbdc0f30d8e3d217f5fd474..4e1eb88afd265334f1f3c703b12eafcfe2c077d6 100644 (file)
--- a/doc/man3/EVP_RAND.pod
+++ b/doc/man3/EVP_RAND.pod
@@ -152,7 +152,9 @@ appropriate size.
 
 EVP_RAND_enable_locking() enables locking for the RAND I<ctx> and all of
 its parents.  After this I<ctx> will operate in a thread safe manner, albeit
-more slowly.
+more slowly. This function is not itself thread safe if called with the same
+I<ctx> from multiple threads. Typically locking should be enabled before a
+I<ctx> is shared across multiple threads.
 
 EVP_RAND_get_params() retrieves details about the implementation
 I<rand>.

diff --git a/include/openssl/evperr.h b/include/openssl/evperr.h
index c25cc49025d52950a4d2a961855d4bcf44673752..309f4cd341e2eb2c954238cc450be693c411fda7 100644 (file)
--- a/include/openssl/evperr.h
+++ b/include/openssl/evperr.h
@@ -243,7 +243,7 @@
 # define EVP_R_PUBLIC_KEY_NOT_RSA                         106
 # define EVP_R_SET_DEFAULT_PROPERTY_FAILURE               209
 # define EVP_R_TOO_MANY_RECORDS                           183
-# define EVP_R_UNABLE_TO_ENABLE_PARENT_LOCKING            212
+# define EVP_R_UNABLE_TO_ENABLE_LOCKING                   212
 # define EVP_R_UNABLE_TO_GET_MAXIMUM_REQUEST_SIZE         215
 # define EVP_R_UNABLE_TO_GET_RANDOM_STRENGTH              216
 # define EVP_R_UNABLE_TO_LOCK_CONTEXT                     211