Add support for child provider to up_ref/free their parent

If the ref counts on a child provider change, then this needs to be
reflected in the parent so we add callbacks to do this.

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

diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index b74379031b0443bb60ddf92e506d9b87429ff813..6c701bf1e2bbaa27aed761398cadd8f50eebe289 100644 (file)
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -36,7 +36,7 @@ static void *evp_method_store_new(OSSL_LIB_CTX *ctx)
 
 static const OSSL_LIB_CTX_METHOD evp_method_store_method = {
     /* We want evp_method_store to be cleaned up before the provider store */
-    OSSL_LIB_CTX_METHOD_HIGH_PRIORITY,
+    OSSL_LIB_CTX_METHOD_PRIORITY_2,
     evp_method_store_new,
     evp_method_store_free,
 };

diff --git a/crypto/provider.c b/crypto/provider.c
index 743c2f48467acbcb0e3ef7429aa0551da1e5ab2c..766086a47b3bb7a54116f1cbe984e3ef3e7f0082 100644 (file)
--- a/crypto/provider.c
+++ b/crypto/provider.c
@@ -23,7 +23,7 @@ OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
         && (prov = ossl_provider_new(libctx, name, NULL, 0)) == NULL)
         return NULL;
 
-    if (!ossl_provider_activate(prov, retain_fallbacks)) {
+    if (!ossl_provider_activate(prov, retain_fallbacks, 1)) {
         ossl_provider_free(prov);
         return NULL;
     }

diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index ea86379efc960fffee75f04f26290a89e5240ba8..71ca2bc73140c05180b08098fbeeb12d9d978c0d 100644 (file)
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -7,6 +7,7 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include <assert.h>
 #include <openssl/crypto.h>
 #include <openssl/core_dispatch.h>
 #include <openssl/core_names.h>
@@ -19,7 +20,6 @@ DEFINE_STACK_OF(OSSL_PROVIDER)
 struct child_prov_globals {
     const OSSL_CORE_HANDLE *handle;
     const OSSL_CORE_HANDLE *curr_prov;
-    STACK_OF(OSSL_PROVIDER) *childprovs;
     unsigned int isinited:1;
     CRYPTO_RWLOCK *lock;
     OSSL_FUNC_core_get_libctx_fn *c_get_libctx;
@@ -28,6 +28,8 @@ struct child_prov_globals {
     OSSL_FUNC_provider_name_fn *c_prov_name;
     OSSL_FUNC_provider_get0_provider_ctx_fn *c_prov_get0_provider_ctx;
     OSSL_FUNC_provider_get0_dispatch_fn *c_prov_get0_dispatch;
+    OSSL_FUNC_provider_up_ref_fn *c_prov_up_ref;
+    OSSL_FUNC_provider_free_fn *c_prov_free;
 };
 
 static void *child_prov_ossl_ctx_new(OSSL_LIB_CTX *libctx)
@@ -35,18 +37,11 @@ static void *child_prov_ossl_ctx_new(OSSL_LIB_CTX *libctx)
     return OPENSSL_zalloc(sizeof(struct child_prov_globals));
 }
 
-/* Wrapper with a void return type for use with sk_OSSL_PROVIDER_pop_free */
-static void ossl_prov_free(OSSL_PROVIDER *prov)
-{
-    OSSL_PROVIDER_unload(prov);
-}
-
 static void child_prov_ossl_ctx_free(void *vgbl)
 {
     struct child_prov_globals *gbl = vgbl;
 
     gbl->c_provider_deregister_child_cb(gbl->handle);
-    sk_OSSL_PROVIDER_pop_free(gbl->childprovs, ossl_prov_free);
     CRYPTO_THREAD_lock_free(gbl->lock);
     OPENSSL_free(gbl);
 }
@@ -132,18 +127,24 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
      * 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)
+    /* 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;
 
-    if (!ossl_provider_activate(cprov, 0)) {
-        ossl_provider_free(cprov);
+    /*
+     * 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;
     }
-    ossl_provider_set_child(cprov);
 
-    if (!sk_OSSL_PROVIDER_push(gbl->childprovs, cprov)) {
-        ossl_provider_free(cprov);
+    if (!ossl_provider_set_child(cprov, prov)) {
+        ossl_provider_deactivate(cprov);
         goto err;
     }
 
@@ -168,7 +169,10 @@ 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);
-    OSSL_PROVIDER_unload(cprov);
+    if (!ossl_provider_deactivate(cprov))
+        return 0;
+    /* ossl_provider_find also ups the ref count, so we free it again here */
+    ossl_provider_free(cprov);
 
     return 1;
 }
@@ -249,6 +253,13 @@ int ossl_provider_init_as_child(OSSL_LIB_CTX *ctx,
         case OSSL_FUNC_PROVIDER_GET0_DISPATCH:
             gbl->c_prov_get0_dispatch = OSSL_FUNC_provider_get0_dispatch(in);
             break;
+        case OSSL_FUNC_PROVIDER_UP_REF:
+            gbl->c_prov_up_ref
+                = OSSL_FUNC_provider_up_ref(in);
+            break;
+        case OSSL_FUNC_PROVIDER_FREE:
+            gbl->c_prov_free = OSSL_FUNC_provider_free(in);
+            break;
         default:
             /* Just ignore anything we don't understand */
             break;
@@ -259,15 +270,40 @@ int ossl_provider_init_as_child(OSSL_LIB_CTX *ctx,
             || gbl->c_provider_register_child_cb == NULL
             || gbl->c_prov_name == NULL
             || gbl->c_prov_get0_provider_ctx == NULL
-            || gbl->c_prov_get0_dispatch == NULL)
+            || gbl->c_prov_get0_dispatch == NULL
+            || gbl->c_prov_up_ref == NULL
+            || gbl->c_prov_free == NULL)
         return 0;
 
-    gbl->childprovs = sk_OSSL_PROVIDER_new_null();
-    if (gbl->childprovs == NULL)
-        return 0;
     gbl->lock = CRYPTO_THREAD_lock_new();
     if (gbl->lock == NULL)
         return 0;
 
     return 1;
 }
+
+int ossl_provider_up_ref_parent(OSSL_PROVIDER *prov, int activate)
+{
+    struct child_prov_globals *gbl;
+
+    gbl = ossl_lib_ctx_get_data(ossl_provider_libctx(prov),
+                                OSSL_LIB_CTX_CHILD_PROVIDER_INDEX,
+                                &child_prov_ossl_ctx_method);
+    if (gbl == NULL)
+        return 0;
+
+    return gbl->c_prov_up_ref(ossl_provider_get_parent(prov), activate);
+}
+
+int ossl_provider_free_parent(OSSL_PROVIDER *prov, int deactivate)
+{
+    struct child_prov_globals *gbl;
+
+    gbl = ossl_lib_ctx_get_data(ossl_provider_libctx(prov),
+                                OSSL_LIB_CTX_CHILD_PROVIDER_INDEX,
+                                &child_prov_ossl_ctx_method);
+    if (gbl == NULL)
+        return 0;
+
+    return gbl->c_prov_free(ossl_provider_get_parent(prov), deactivate);
+}

diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 6223d27cbb556375ce1d1d3e95ff198764903c6c..5725ef3c639f735ed62a33166196938a5ab0f7d4 100644 (file)
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -46,7 +46,7 @@ static void prov_conf_ossl_ctx_free(void *vpcgbl)
 
 static const OSSL_LIB_CTX_METHOD provider_conf_ossl_ctx_method = {
     /* Must be freed before the provider store is freed */
-    OSSL_LIB_CTX_METHOD_HIGH_PRIORITY,
+    OSSL_LIB_CTX_METHOD_PRIORITY_2,
     prov_conf_ossl_ctx_new,
     prov_conf_ossl_ctx_free,
 };
@@ -164,7 +164,7 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
     ok = provider_conf_params(prov, NULL, value, cnf);
 
     if (ok && activate) {
-        if (!ossl_provider_activate(prov, 0)) {
+        if (!ossl_provider_activate(prov, 0, 1)) {
             ok = 0;
         } else {
             if (pcgbl->activated_providers == NULL)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 9cf7ca2f7c5a804ea50fb6aa095272eec5fd9c77..f87ccfa4a25b399f3f2ca2c6b4701ad2f0256666 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -101,6 +101,7 @@ struct ossl_provider_st {
     CRYPTO_RWLOCK *opbits_lock;
 
     /* Whether this provider is the child of some other provider */
+    const OSSL_CORE_HANDLE *handle;
     unsigned int ischild:1;
 
     /* Provider side data */
@@ -124,6 +125,7 @@ static int ossl_provider_cmp(const OSSL_PROVIDER * const *a,
  */
 
 struct provider_store_st {
+    OSSL_LIB_CTX *libctx;
     STACK_OF(OSSL_PROVIDER) *providers;
     STACK_OF(OSSL_PROVIDER_CHILD_CB) *child_cbs;
     CRYPTO_RWLOCK *default_path_lock;
@@ -180,6 +182,7 @@ static void *provider_store_new(OSSL_LIB_CTX *ctx)
         provider_store_free(store);
         return NULL;
     }
+    store->libctx = ctx;
     store->use_fallbacks = 1;
 
     for (p = ossl_predefined_providers; p->name != NULL; p++) {
@@ -211,7 +214,8 @@ static void *provider_store_new(OSSL_LIB_CTX *ctx)
 }
 
 static const OSSL_LIB_CTX_METHOD provider_store_method = {
-    OSSL_LIB_CTX_METHOD_DEFAULT_PRIORITY,
+    /* Needs to be freed before the child provider data is freed */
+    OSSL_LIB_CTX_METHOD_PRIORITY_1,
     provider_store_new,
     provider_store_free,
 };
@@ -311,9 +315,38 @@ int ossl_provider_up_ref(OSSL_PROVIDER *prov)
 
     if (CRYPTO_UP_REF(&prov->refcnt, &ref, prov->refcnt_lock) <= 0)
         return 0;
+
+#ifndef FIPS_MODULE
+    if (prov->ischild) {
+        if (!ossl_provider_up_ref_parent(prov, 0)) {
+            ossl_provider_free(prov);
+            return 0;
+        }
+    }
+#endif
+
     return ref;
 }
 
+#ifndef FIPS_MODULE
+static int provider_up_ref_intern(OSSL_PROVIDER *prov, int activate)
+{
+    if (activate)
+        return ossl_provider_activate(prov, 0, 1);
+
+    return ossl_provider_up_ref(prov);
+}
+
+static int provider_free_intern(OSSL_PROVIDER *prov, int deactivate)
+{
+    if (deactivate)
+        return ossl_provider_deactivate(prov);
+
+    ossl_provider_free(prov);
+    return 1;
+}
+#endif
+
 OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name,
                                  OSSL_provider_init_fn *init_function,
                                  int noconfig)
@@ -424,6 +457,11 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
 #endif
             OPENSSL_free(prov);
         }
+#ifndef FIPS_MODULE
+        else if (prov->ischild) {
+            ossl_provider_free_parent(prov, 0);
+        }
+#endif
     }
 }
 
@@ -727,6 +765,16 @@ static int provider_deactivate(OSSL_PROVIDER *prov)
     if (!CRYPTO_THREAD_write_lock(prov->flag_lock))
         return -1;
 
+#ifndef FIPS_MODULE
+    if (prov->activatecnt == 2 && prov->ischild) {
+        /*
+         * We have had a direct activation in this child libctx so we need to
+         * now down the ref count in the parent provider.
+         */
+        ossl_provider_free_parent(prov, 1);
+    }
+#endif
+
     if ((count = --prov->activatecnt) < 1) {
         int i, max = sk_OSSL_PROVIDER_CHILD_CB_num(store->child_cbs);
         OSSL_PROVIDER_CHILD_CB *child_cb;
@@ -749,12 +797,13 @@ static int provider_deactivate(OSSL_PROVIDER *prov)
  * Activate a provider.
  * Return -1 on failure and the activation count on success
  */
-static int provider_activate(OSSL_PROVIDER *prov, int lock)
+static int provider_activate(OSSL_PROVIDER *prov, int lock, int upcalls)
 {
     int count = -1;
 
     if (provider_init(prov, lock)) {
-        int i, max, ret;
+        int ret = 1;
+        int i, max;
         OSSL_PROVIDER_CHILD_CB *child_cb;
         struct provider_store_st *store;
 
@@ -768,19 +817,36 @@ static int provider_activate(OSSL_PROVIDER *prov, int lock)
         max = sk_OSSL_PROVIDER_CHILD_CB_num(store->child_cbs);
         if (lock && !CRYPTO_THREAD_write_lock(prov->flag_lock))
             return 0;
+
+#ifndef FIPS_MODULE
+        if (prov->ischild && upcalls)
+            ret = ossl_provider_up_ref_parent(prov, 1);
+#endif
+
         if (ret) {
             count = ++prov->activatecnt;
             prov->flag_activated = 1;
 
-            for (i = 0; i < max; i++) {
-                child_cb = sk_OSSL_PROVIDER_CHILD_CB_value(store->child_cbs, i);
-                child_cb->create_cb((OSSL_CORE_HANDLE *)prov, child_cb->cbdata);
+            if (prov->activatecnt == 1) {
+                for (i = 0; i < max; i++) {
+                    /*
+                     * This is newly activated (activatecnt == 1), so we need to
+                     * create child providers as necessary.
+                     */
+                    child_cb = sk_OSSL_PROVIDER_CHILD_CB_value(store->child_cbs,
+                                                               i);
+                    ret &= child_cb->create_cb((OSSL_CORE_HANDLE *)prov,
+                                               child_cb->cbdata);
+                }
             }
         }
+
         if (lock) {
             CRYPTO_THREAD_unlock(prov->flag_lock);
             CRYPTO_THREAD_unlock(store->lock);
         }
+        if (!ret)
+            return -1;
     }
 
     return count;
@@ -804,13 +870,14 @@ static int provider_flush_store_cache(const OSSL_PROVIDER *prov)
     return 1;
 }
 
-int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks)
+int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
+                           int upcalls)
 {
     int count;
 
     if (prov == NULL)
         return 0;
-    if ((count = provider_activate(prov, 1)) > 0) {
+    if ((count = provider_activate(prov, 1, upcalls)) > 0) {
         if (!retain_fallbacks) {
             if (!CRYPTO_THREAD_write_lock(prov->store->lock)) {
                 provider_deactivate(prov);
@@ -873,7 +940,7 @@ static void provider_activate_fallbacks(struct provider_store_st *store)
         if (ossl_provider_up_ref(prov)) {
             if (CRYPTO_THREAD_write_lock(prov->flag_lock)) {
                 if (prov->flag_fallback) {
-                    if (provider_activate(prov, 0) > 0)
+                    if (provider_activate(prov, 0, 0) > 0)
                         activated_fallback_count++;
                 }
                 CRYPTO_THREAD_unlock(prov->flag_lock);
@@ -949,7 +1016,7 @@ int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx,
              * It's already activated, but we up the activated count to ensure
              * it remains activated until after we've called the user callback.
              */
-            if (provider_activate(prov, 0) < 0) {
+            if (provider_activate(prov, 0, 1) < 0) {
                 ossl_provider_free(prov);
                 CRYPTO_THREAD_unlock(prov->flag_lock);
                 goto err_unlock;
@@ -1209,9 +1276,26 @@ int ossl_provider_test_operation_bit(OSSL_PROVIDER *provider, size_t bitnum,
     return 1;
 }
 
-void ossl_provider_set_child(OSSL_PROVIDER *prov)
+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)
 {
+    struct provider_store_st *store = NULL;
+
+    if ((store = get_provider_store(prov->libctx)) == NULL)
+        return 0;
+
+    prov->handle = handle;
     prov->ischild = 1;
+
+    if (!CRYPTO_THREAD_write_lock(store->lock))
+        return 0;
+
+    CRYPTO_THREAD_unlock(store->lock);
+    return 1;
 }
 
 #ifndef FIPS_MODULE
@@ -1542,6 +1626,10 @@ static const OSSL_DISPATCH core_dispatch_[] = {
         (void (*)(void))OSSL_PROVIDER_get0_provider_ctx },
     { OSSL_FUNC_PROVIDER_GET0_DISPATCH,
         (void (*)(void))OSSL_PROVIDER_get0_dispatch },
+    { OSSL_FUNC_PROVIDER_UP_REF,
+        (void (*)(void))provider_up_ref_intern },
+    { OSSL_FUNC_PROVIDER_FREE,
+        (void (*)(void))provider_free_intern },
 #endif
     { 0, NULL }
 };

diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index 9245a0b8700a6a021007a87caccce8759de3ccda..d943419a52da7e9612753b32e2489c38b46bfe36 100644 (file)
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -169,7 +169,9 @@ typedef struct ossl_ex_data_global_st {
 # define OSSL_LIB_CTX_MAX_INDEXES                   19
 
 # define OSSL_LIB_CTX_METHOD_DEFAULT_PRIORITY       0
-# define OSSL_LIB_CTX_METHOD_HIGH_PRIORITY          1
+# define OSSL_LIB_CTX_METHOD_PRIORITY_1             1
+# define OSSL_LIB_CTX_METHOD_PRIORITY_2             2
+
 typedef struct ossl_lib_ctx_method {
     int priority;
     void *(*new_func)(OSSL_LIB_CTX *ctx);

diff --git a/include/internal/provider.h b/include/internal/provider.h
index 13b72ad2dec98c1891b96ddd12e0b6ff28b794f9..7d5ccccbd1df703cc3620b91d299b590eac839a1 100644 (file)
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -41,7 +41,10 @@ int ossl_provider_set_fallback(OSSL_PROVIDER *prov);
 int ossl_provider_set_module_path(OSSL_PROVIDER *prov, const char *module_path);
 int ossl_provider_add_parameter(OSSL_PROVIDER *prov, const char *name,
                                 const char *value);
-void ossl_provider_set_child(OSSL_PROVIDER *prov);
+int ossl_provider_set_child(OSSL_PROVIDER *prov, const OSSL_CORE_HANDLE *handle);
+const OSSL_CORE_HANDLE *ossl_provider_get_parent(OSSL_PROVIDER *prov);
+int ossl_provider_up_ref_parent(OSSL_PROVIDER *prov, int activate);
+int ossl_provider_free_parent(OSSL_PROVIDER *prov, int deactivate);
 
 /* Disable fallback loading */
 int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
@@ -50,7 +53,8 @@ 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 ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
+                           int upcalls);
 int ossl_provider_deactivate(OSSL_PROVIDER *prov);
 /* Check if the provider is available (activated) */
 int ossl_provider_available(OSSL_PROVIDER *prov);

diff --git a/include/openssl/core_dispatch.h b/include/openssl/core_dispatch.h
index c2c6c45976d94a62a5da86efd0b0a0e7ec5bec46..5c453eaac0fe91d22a4fe4f790869183f7a8475f 100644 (file)
--- a/include/openssl/core_dispatch.h
+++ b/include/openssl/core_dispatch.h
@@ -189,6 +189,8 @@ OSSL_CORE_MAKE_FUNC(void, cleanup_nonce, (const OSSL_CORE_HANDLE *handle,
 #define OSSL_FUNC_PROVIDER_NAME                107
 #define OSSL_FUNC_PROVIDER_GET0_PROVIDER_CTX   108
 #define OSSL_FUNC_PROVIDER_GET0_DISPATCH       109
+#define OSSL_FUNC_PROVIDER_UP_REF              110
+#define OSSL_FUNC_PROVIDER_FREE                111
 
 OSSL_CORE_MAKE_FUNC(int, provider_register_child_cb,
                     (const OSSL_CORE_HANDLE *handle,
@@ -203,6 +205,10 @@ OSSL_CORE_MAKE_FUNC(void *, provider_get0_provider_ctx,
                     (const OSSL_CORE_HANDLE *prov))
 OSSL_CORE_MAKE_FUNC(const OSSL_DISPATCH *, provider_get0_dispatch,
                     (const OSSL_CORE_HANDLE *prov))
+OSSL_CORE_MAKE_FUNC(int, provider_up_ref,
+                    (const OSSL_CORE_HANDLE *prov, int activate))
+OSSL_CORE_MAKE_FUNC(int, provider_free,
+                    (const OSSL_CORE_HANDLE *prov, int deactivate))
 
 /* Functions provided by the provider to the Core, reserved numbers 1024-1535 */
 # define OSSL_FUNC_PROVIDER_TEARDOWN           1024

diff --git a/test/provider_internal_test.c b/test/provider_internal_test.c
index 7bf2b8e272a436a3da31dd1851e2d3cb3a095990..2341dd3dac5fdaa3cd087aeb3a6210765137d963 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))
+        TEST_true(ossl_provider_activate(prov, 0, 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)