Set use_fallbacks to zero when we add a provider to the store

Update use_fallbacks to zero when we add a provider to the store rather
than when we activate it. Its only at the point that we add it to the store
that it is actually usable and visible to other threads.

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

diff --git a/crypto/provider.c b/crypto/provider.c
index 12336acc57070ea8103cdeb1e59046f9a15a7d2b..f5dbc4f94a4fa8f89ab585e6e2af7794c19d05e9 100644 (file)
--- a/crypto/provider.c
+++ b/crypto/provider.c
@@ -26,12 +26,12 @@ OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
         isnew = 1;
     }
 
-    if (!ossl_provider_activate(prov, retain_fallbacks, 1)) {
+    if (!ossl_provider_activate(prov, 1)) {
         ossl_provider_free(prov);
         return NULL;
     }
 
-    if (isnew && !ossl_provider_add_to_store(prov)) {
+    if (isnew && !ossl_provider_add_to_store(prov, retain_fallbacks)) {
         ossl_provider_deactivate(prov);
         ossl_provider_free(prov);
         return NULL;

diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index cabf3ba19df93380868ab84eadaa77da5c7713c7..b077e95ffcbc0892a9f79a36c1f7fc77fe54e85b 100644 (file)
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -148,11 +148,11 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
                                        1)) == NULL)
             goto err;
 
-        if (!ossl_provider_activate(cprov, 0, 0))
+        if (!ossl_provider_activate(cprov, 0))
             goto err;
 
         if (!ossl_provider_set_child(cprov, prov)
-            || !ossl_provider_add_to_store(cprov)) {
+            || !ossl_provider_add_to_store(cprov, 0)) {
             ossl_provider_deactivate(cprov);
             ossl_provider_free(cprov);
             goto err;

diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 8e83264dc6adf12bf563fc1722b62335b7f80339..1e59e959e3af75a6f456a5f9bcef06ef73e0f3ef 100644 (file)
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -171,9 +171,9 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
         ok = provider_conf_params(prov, NULL, NULL, value, cnf);
 
         if (ok) {
-            if (!ossl_provider_activate(prov, 0, 1)) {
+            if (!ossl_provider_activate(prov, 1)) {
                 ok = 0;
-            } else if (!ossl_provider_add_to_store(prov)) {
+            } else if (!ossl_provider_add_to_store(prov, 0)) {
                 ossl_provider_deactivate(prov);
                 ok = 0;
             } else {

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 62fab92028d1270ae2c148a387695bdde43d3580..83c6bf28f052ea1b5d6f756e08c2dc454ad4d166 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -431,7 +431,7 @@ int ossl_provider_up_ref(OSSL_PROVIDER *prov)
 static int provider_up_ref_intern(OSSL_PROVIDER *prov, int activate)
 {
     if (activate)
-        return ossl_provider_activate(prov, 0, 1);
+        return ossl_provider_activate(prov, 1);
 
     return ossl_provider_up_ref(prov);
 }
@@ -512,7 +512,7 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name,
     return prov;
 }
 
-int ossl_provider_add_to_store(OSSL_PROVIDER *prov)
+int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks)
 {
     struct provider_store_st *store = NULL;
     int ret = 1;
@@ -530,6 +530,8 @@ int ossl_provider_add_to_store(OSSL_PROVIDER *prov)
         ossl_provider_free(prov);
         ret = 0;
     }
+    if (!retain_fallbacks)
+        store->use_fallbacks = 0;
     CRYPTO_THREAD_unlock(store->lock);
 
     return ret;
@@ -1025,24 +1027,15 @@ static int provider_flush_store_cache(const OSSL_PROVIDER *prov)
     return 1;
 }
 
-int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                           int upcalls)
+int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls)
 {
     int count;
 
     if (prov == NULL)
         return 0;
-    if ((count = provider_activate(prov, 1, upcalls)) > 0) {
-        if (!retain_fallbacks) {
-            if (!CRYPTO_THREAD_write_lock(prov->store->lock)) {
-                provider_deactivate(prov);
-                return 0;
-            }
-            prov->store->use_fallbacks = 0;
-            CRYPTO_THREAD_unlock(prov->store->lock);
-        }
+    if ((count = provider_activate(prov, 1, upcalls)) > 0)
         return count == 1 ? provider_flush_store_cache(prov) : 1;
-    }
+
     return 0;
 }
 
@@ -1485,10 +1478,8 @@ int ossl_provider_activate_child(OSSL_PROVIDER *prov,
      * The provider could be in one of two states: (1) Already a child,
      * (2) Not a child (not eligible to be one).
      */
-    if (prov->ischild && provider_activate(prov, 0, 0)) {
+    if (prov->ischild && provider_activate(prov, 0, 0))
         flush = 1;
-        prov->store->use_fallbacks = 0;
-    }
 
     CRYPTO_THREAD_unlock(prov->flag_lock);
     CRYPTO_THREAD_unlock(prov->store->lock);

diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod
index d563d41e72ff8aaf1e72ff51b1bb2b82538a9c13..0a76135281827e049d427adc2fcfcc1403ef8698 100644 (file)
--- a/doc/internal/man3/ossl_provider_new.pod
+++ b/doc/internal/man3/ossl_provider_new.pod
@@ -53,8 +53,7 @@ ossl_provider_get_capabilities
   * Activate the Provider
   * If the Provider is a module, the module will be loaded
   */
- int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                            int upcalls);
+ int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls);
  int ossl_provider_deactivate(OSSL_PROVIDER *prov);
 
  /* Return pointer to the provider's context */
@@ -218,10 +217,8 @@ be located in that module, and called.
 
 =back
 
-If I<retain_fallbacks> is zero, fallbacks are disabled.  If it is nonzero,
-fallbacks are left unchanged. If I<upcalls> is nonzero then, if this is a child
-provider, upcalls to the parent libctx will be made to inform it of an
-up-ref.
+If I<upcalls> is nonzero then, if this is a child provider, upcalls to the
+parent libctx will be made to inform it of an up-ref.
 
 ossl_provider_deactivate() "deactivates" the provider for the given
 provider object I<prov> by decrementing its activation count.  When

diff --git a/include/internal/provider.h b/include/internal/provider.h
index 45ad1a5b00b4a30e3a2e41d2d84233eef99a9639..b6e413f7a4108d53d1ab519ccb09f722a28dae27 100644 (file)
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -59,10 +59,9 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
  * Activate the Provider
  * If the Provider is a module, the module will be loaded
  */
-int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                           int upcalls);
+int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls);
 int ossl_provider_deactivate(OSSL_PROVIDER *prov);
-int ossl_provider_add_to_store(OSSL_PROVIDER *prov);
+int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks);
 
 /* Return pointer to the provider's context */
 void *ossl_provider_ctx(const OSSL_PROVIDER *prov);

diff --git a/test/provider_internal_test.c b/test/provider_internal_test.c
index 7a37ef8c241ee38832ad2832c45f6efda133049f..87906c1bdcb1745e87160cab478e554ac5cb1021 100644 (file)
--- a/test/provider_internal_test.c
+++ b/test/provider_internal_test.c
@@ -26,7 +26,7 @@ static int test_provider(OSSL_PROVIDER *prov, const char *expected_greeting)
     int ret = 0;
 
     ret =
-        TEST_true(ossl_provider_activate(prov, 0, 1))
+        TEST_true(ossl_provider_activate(prov, 1))
         && TEST_true(ossl_provider_get_params(prov, greeting_request))
         && TEST_ptr(greeting = greeting_request[0].data)
         && TEST_size_t_gt(greeting_request[0].data_size, 0)