Add locking for the provider_conf.c

Avoid races where 2 threads attempt to configure activation of providers
at the same time. E.g. via an explicit and an implict load of the config
file at the same time.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16425)

1 files changed, 53 insertions, 36 deletions

diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 7689301b75..da3796d914 100644
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -22,6 +22,7 @@ DEFINE_STACK_OF(OSSL_PROVIDER)
 /* PROVIDER config module */
 
 typedef struct {
+    CRYPTO_RWLOCK *lock;
     STACK_OF(OSSL_PROVIDER) *activated_providers;
 } PROVIDER_CONF_GLOBAL;
 
@@ -32,6 +33,12 @@ static void *prov_conf_ossl_ctx_new(OSSL_LIB_CTX *libctx)
     if (pcgbl == NULL)
         return NULL;
 
+    pcgbl->lock = CRYPTO_THREAD_lock_new();
+    if (pcgbl->lock == NULL) {
+        OPENSSL_free(pcgbl);
+        return NULL;
+    }
+
     return pcgbl;
 }
 
@@ -43,6 +50,7 @@ static void prov_conf_ossl_ctx_free(void *vpcgbl)
                               ossl_provider_free);
 
     OSSL_TRACE(CONF, "Cleaned up providers\n");
+    CRYPTO_THREAD_lock_free(pcgbl->lock);
     OPENSSL_free(pcgbl);
 }
 
@@ -176,48 +184,57 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
             activate = 1;
     }
 
-    if (activate && !prov_already_activated(name, pcgbl->activated_providers)) {
-        /*
-        * There is an attempt to activate a provider, so we should disable
-        * loading of fallbacks. Otherwise a misconfiguration could mean the
-        * intended provider does not get loaded. Subsequent fetches could then
-        * fallback to the default provider - which may be the wrong thing.
-        */
-        if (!ossl_provider_disable_fallback_loading(libctx)) {
+    if (activate) {
+        if (!CRYPTO_THREAD_write_lock(pcgbl->lock)) {
             ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
             return 0;
         }
-        prov = ossl_provider_find(libctx, name, 1);
-        if (prov == NULL)
-            prov = ossl_provider_new(libctx, name, NULL, 1);
-        if (prov == NULL) {
-            if (soft)
-                ERR_clear_error();
-            return 0;
-        }
-
-        if (path != NULL)
-            ossl_provider_set_module_path(prov, path);
-
-        ok = provider_conf_params(prov, NULL, NULL, value, cnf);
+        if (!prov_already_activated(name, pcgbl->activated_providers)) {
+            /*
+            * There is an attempt to activate a provider, so we should disable
+            * loading of fallbacks. Otherwise a misconfiguration could mean the
+            * intended provider does not get loaded. Subsequent fetches could
+            * then fallback to the default provider - which may be the wrong
+            * thing.
+            */
+            if (!ossl_provider_disable_fallback_loading(libctx)) {
+                CRYPTO_THREAD_unlock(pcgbl->lock);
+                ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
+            prov = ossl_provider_find(libctx, name, 1);
+            if (prov == NULL)
+                prov = ossl_provider_new(libctx, name, NULL, 1);
+            if (prov == NULL) {
+                CRYPTO_THREAD_unlock(pcgbl->lock);
+                if (soft)
+                    ERR_clear_error();
+                return 0;
+            }
 
-        if (ok) {
-            if (!ossl_provider_activate(prov, 1, 0)) {
-                ok = 0;
-            } else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
-                ossl_provider_deactivate(prov);
-                ok = 0;
-            } else {
-                if (pcgbl->activated_providers == NULL)
-                    pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
-                sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
-                ok = 1;
+            if (path != NULL)
+                ossl_provider_set_module_path(prov, path);
+
+            ok = provider_conf_params(prov, NULL, NULL, value, cnf);
+
+            if (ok) {
+                if (!ossl_provider_activate(prov, 1, 0)) {
+                    ok = 0;
+                } else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
+                    ossl_provider_deactivate(prov);
+                    ok = 0;
+                } else {
+                    if (pcgbl->activated_providers == NULL)
+                        pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
+                    sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
+                    ok = 1;
+                }
             }
+            if (!ok)
+                ossl_provider_free(prov);
         }
-
-        if (!ok)
-            ossl_provider_free(prov);
-    } else if (!activate) {
+        CRYPTO_THREAD_unlock(pcgbl->lock);
+    } else {
         OSSL_PROVIDER_INFO entry;
 
         memset(&entry, 0, sizeof(entry));