From abaa2dd2981ba3c15456016c6248f539242cfb49 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Tue, 4 May 2021 16:23:31 +0100
Subject: 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)
---
 crypto/provider_child.c | 71 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 25 deletions(-)

(limited to 'crypto/provider_child.c')

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;
 }
-- 
cgit v1.2.3