Don't convert pre-existing providers into children

If a provider explicitly loads another provider into a child libctx where
it wasn't previously loaded then we don't start treating it like a child
if the parent libctx subsequently loads the same provider.

Fixes #14925

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

2 files changed, 100 insertions, 32 deletions

diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index 71ca2bc731..0ca61c0686 100644
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -123,29 +123,44 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
      */
     gbl->curr_prov = prov;
 
-    /*
-     * Create it - passing 1 as final param so we don't try and recursively init
-     * children
-     */
-    /* Find it or create it */
-    if ((cprov = ossl_provider_find(ctx, provname, 1)) == NULL
-        && (cprov = ossl_provider_new(ctx, provname, ossl_child_provider_init,
-                                      1)) == NULL)
-        goto err;
-
-    /*
-     * We free the newly created ref. We rely on the provider sticking around
-     * in the provider store.
-     */
-    ossl_provider_free(cprov);
-
-    if (!ossl_provider_activate(cprov, 0, 0)){
-        goto err;
-    }
-
-    if (!ossl_provider_set_child(cprov, prov)) {
-        ossl_provider_deactivate(cprov);
-        goto err;
+    if ((cprov = ossl_provider_find(ctx, provname, 1)) != NULL) {
+        /*
+        * We free the newly created ref. We rely on the provider sticking around
+        * in the provider store.
+        */
+        ossl_provider_free(cprov);
+
+        /*
+         * The provider already exists. It could be an unused built-in, or a
+         * previously created child, or it could have been explicitly loaded. If
+         * explicitly loaded it cannot be converted to a child and we ignore it
+         * - i.e. we don't start treating it like a child.
+         */
+        if (!ossl_provider_convert_to_child(cprov, prov,
+                                            ossl_child_provider_init))
+            goto err;
+    } else {
+        /*
+         * Create it - passing 1 as final param so we don't try and recursively
+         * init children
+         */
+        if ((cprov = ossl_provider_new(ctx, provname, ossl_child_provider_init,
+                                       1)) == NULL)
+            goto err;
+
+        /*
+        * We free the newly created ref. We rely on the provider sticking around
+        * in the provider store.
+        */
+        ossl_provider_free(cprov);
+
+        if (!ossl_provider_activate(cprov, 0, 0))
+            goto err;
+
+        if (!ossl_provider_set_child(cprov, prov)) {
+            ossl_provider_deactivate(cprov);
+            goto err;
+        }
     }
 
     ret = 1;
@@ -169,10 +184,16 @@ static int provider_remove_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
 
     provname = gbl->c_prov_name(prov);
     cprov = ossl_provider_find(ctx, provname, 1);
-    if (!ossl_provider_deactivate(cprov))
+    if (cprov == NULL)
         return 0;
-    /* ossl_provider_find also ups the ref count, so we free it again here */
+    /*
+     * ossl_provider_find ups the ref count, so we free it again here. We can
+     * rely on the provider store reference count.
+     */
     ossl_provider_free(cprov);
+    if (ossl_provider_is_child(cprov)
+            && !ossl_provider_deactivate(cprov))
+        return 0;
 
     return 1;
 }
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index f87ccfa4a2..598f1ba214 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -57,6 +57,7 @@ struct ossl_provider_st {
     unsigned int flag_initialized:1;
     unsigned int flag_activated:1;
     unsigned int flag_fallback:1; /* Can be used as fallback */
+    unsigned int flag_couldbechild:1;
 
     /* Getting and setting the flags require synchronization */
     CRYPTO_RWLOCK *flag_lock;
@@ -306,6 +307,7 @@ static OSSL_PROVIDER *provider_new(const char *name,
     }
 
     prov->init_function = init_function;
+    prov->flag_couldbechild = 1;
     return prov;
 }
 
@@ -646,6 +648,7 @@ static int provider_init(OSSL_PROVIDER *prov, int flag_lock)
     }
     prov->provctx = tmp_provctx;
     prov->dispatch = provider_dispatch;
+    prov->flag_couldbechild = 0;
 
     for (; provider_dispatch->function_id != 0; provider_dispatch++) {
         switch (provider_dispatch->function_id) {
@@ -1281,23 +1284,60 @@ const OSSL_CORE_HANDLE *ossl_provider_get_parent(OSSL_PROVIDER *prov)
     return prov->handle;
 }
 
-int ossl_provider_set_child(OSSL_PROVIDER *prov, const OSSL_CORE_HANDLE *handle)
+int ossl_provider_is_child(const OSSL_PROVIDER *prov)
 {
-    struct provider_store_st *store = NULL;
-
-    if ((store = get_provider_store(prov->libctx)) == NULL)
-        return 0;
+    return prov->ischild;
+}
 
+int ossl_provider_set_child(OSSL_PROVIDER *prov, const OSSL_CORE_HANDLE *handle)
+{
     prov->handle = handle;
     prov->ischild = 1;
 
-    if (!CRYPTO_THREAD_write_lock(store->lock))
+    return 1;
+}
+
+int ossl_provider_convert_to_child(OSSL_PROVIDER *prov,
+                                   const OSSL_CORE_HANDLE *handle,
+                                   OSSL_provider_init_fn *init_function)
+{
+    int flush = 0;
+
+    if (!CRYPTO_THREAD_write_lock(prov->store->lock))
         return 0;
+    if (!CRYPTO_THREAD_write_lock(prov->flag_lock)) {
+        CRYPTO_THREAD_unlock(prov->store->lock);
+        return 0;
+    }
+    /*
+     * The provider could be in one of three states: (1) Already a child,
+     * (2) Not a child (but eligible to be one), or (3) Not a child (not
+     * eligible to be one).
+     */
+    if (prov->flag_couldbechild) {
+        ossl_provider_set_child(prov, handle);
+        prov->init_function = init_function;
+    }
+    if (prov->ischild && provider_activate(prov, 0, 0)) {
+        flush = 1;
+        prov->store->use_fallbacks = 0;
+    }
 
-    CRYPTO_THREAD_unlock(store->lock);
+    CRYPTO_THREAD_unlock(prov->flag_lock);
+    CRYPTO_THREAD_unlock(prov->store->lock);
+
+    if (flush)
+        provider_flush_store_cache(prov);
+
+    /*
+     * We report success whether or not the provider was eligible for conversion
+     * to a child. If its not elgibile then it has already been loaded as a non
+     * child provider and we should keep it like that.
+     */
     return 1;
 }
 
+
 #ifndef FIPS_MODULE
 static int ossl_provider_register_child_cb(const OSSL_CORE_HANDLE *handle,
                                            int (*create_cb)(
@@ -1337,6 +1377,13 @@ static int ossl_provider_register_child_cb(const OSSL_CORE_HANDLE *handle,
     max = sk_OSSL_PROVIDER_num(store->providers);
     for (i = 0; i < max; i++) {
         prov = sk_OSSL_PROVIDER_value(store->providers, i);
+        /*
+         * We require register_child_cb to be called during a provider init
+         * function. The currently initing provider will never be activated yet
+         * and we we should not attempt to aquire the flag_lock for it.
+         */
+        if (prov == thisprov)
+            continue;
         if (!CRYPTO_THREAD_read_lock(prov->flag_lock))
             break;
         /*