CORE: Separate OSSL_PROVIDER activation from OSSL_PROVIDER reference
authorRichard Levitte <levitte@openssl.org>
Wed, 16 Dec 2020 14:15:06 +0000 (15:15 +0100)
committerRichard Levitte <levitte@openssl.org>
Thu, 17 Dec 2020 11:02:08 +0000 (12:02 +0100)
This introduces a separate activation counter, and the function
ossl_provider_deactivate() for provider deactivation.

Something to be noted is that if the reference count goes down to
zero, we don't care if the activation count is non-zero (i.e. someone
forgot to call ossl_provider_deactivate()).  Since there are no more
references to the provider, it doesn't matter.
The important thing is that deactivation doesn't remove the provider
as long as there are references to it, for example because there are
live methods associated with that provider, but still makes the
provider unavailable to create new methods from.

Fixes #13503
Fixes #12157

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13661)

crypto/provider.c
crypto/provider_core.c
doc/internal/man3/ossl_provider_new.pod
include/internal/provider.h
test/provider_internal_test.c

index 0441fb2f2ac1c3a1194340fb0645c196c436b3a7..bd8f75a2c1da820ea01921c22ab3cfde86bc2ac4 100644 (file)
@@ -40,6 +40,8 @@ OSSL_PROVIDER *OSSL_PROVIDER_load(OSSL_LIB_CTX *libctx, const char *name)
 
 int OSSL_PROVIDER_unload(OSSL_PROVIDER *prov)
 {
+    if (!ossl_provider_deactivate(prov))
+        return 0;
     ossl_provider_free(prov);
     return 1;
 }
index 954befd4a2129ea21f7ef3aa7e16e6ff7e0fa058..f0d6fb20f84110a0d096def52fb4cf72bcf89f91 100644 (file)
@@ -44,12 +44,15 @@ struct provider_store_st;        /* Forward declaration */
 struct ossl_provider_st {
     /* Flag bits */
     unsigned int flag_initialized:1;
+    unsigned int flag_activated:1;
     unsigned int flag_fallback:1; /* Can be used as fallback */
     unsigned int flag_activated_as_fallback:1;
 
     /* OpenSSL library side data */
     CRYPTO_REF_COUNT refcnt;
     CRYPTO_RWLOCK *refcnt_lock;  /* For the ref counter */
+    CRYPTO_REF_COUNT activatecnt;
+    CRYPTO_RWLOCK *activatecnt_lock; /* For the activate counter */
     char *name;
     char *path;
     DSO *module;
@@ -110,20 +113,15 @@ struct provider_store_st {
 };
 
 /*
- * provider_deactivate_free() is a wrapper around ossl_provider_free()
- * that also makes sure that activated fallback providers are deactivated.
- * This is simply done by freeing them an extra time, to compensate for the
- * refcount that provider_activate_fallbacks() gives them.
+ * provider_deactivate_free() is a wrapper around ossl_provider_deactivate()
+ * and ossl_provider_free(), called as needed.
  * Since this is only called when the provider store is being emptied, we
  * don't need to care about any lock.
  */
 static void provider_deactivate_free(OSSL_PROVIDER *prov)
 {
-    int extra_free = (prov->flag_initialized
-                      && prov->flag_activated_as_fallback);
-
-    if (extra_free)
-        ossl_provider_free(prov);
+    if (prov->flag_activated)
+        ossl_provider_deactivate(prov);
     ossl_provider_free(prov);
 }
 
@@ -251,6 +249,7 @@ static OSSL_PROVIDER *provider_new(const char *name,
     if ((prov = OPENSSL_zalloc(sizeof(*prov))) == NULL
 #ifndef HAVE_ATOMICS
         || (prov->refcnt_lock = CRYPTO_THREAD_lock_new()) == NULL
+        || (prov->activatecnt_lock = CRYPTO_THREAD_lock_new()) == NULL
 #endif
         || !ossl_provider_up_ref(prov) /* +1 One reference to be returned */
         || (prov->name = OPENSSL_strdup(name)) == NULL) {
@@ -337,38 +336,35 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
         CRYPTO_DOWN_REF(&prov->refcnt, &ref, prov->refcnt_lock);
 
         /*
-         * When the refcount drops below two, the store is the only
-         * possible reference, or it has already been taken away from
-         * the store (this may happen if a provider was activated
-         * because it's a fallback, but isn't currently used)
-         * When that happens, the provider is inactivated.
+         * When the refcount drops to zero, we clean up the provider.
+         * Note that this also does teardown, which may seem late,
+         * considering that init happens on first activation.  However,
+         * there may be other structures hanging on to the provider after
+         * the last deactivation and may therefore need full access to the
+         * provider's services.  Therefore, we deinit late.
          */
-        if (ref < 2 && prov->flag_initialized) {
+        if (ref == 0) {
+            if (prov->flag_initialized) {
 #ifndef FIPS_MODULE
-            ossl_init_thread_deregister(prov);
+                ossl_init_thread_deregister(prov);
 #endif
-            if (prov->teardown != NULL)
-                prov->teardown(prov->provctx);
+                if (prov->teardown != NULL)
+                    prov->teardown(prov->provctx);
 #ifndef OPENSSL_NO_ERR
 # ifndef FIPS_MODULE
-            if (prov->error_strings != NULL) {
-                ERR_unload_strings(prov->error_lib, prov->error_strings);
-                OPENSSL_free(prov->error_strings);
-                prov->error_strings = NULL;
-            }
+                if (prov->error_strings != NULL) {
+                    ERR_unload_strings(prov->error_lib, prov->error_strings);
+                    OPENSSL_free(prov->error_strings);
+                    prov->error_strings = NULL;
+                }
 # endif
 #endif
-            OPENSSL_free(prov->operation_bits);
-            prov->operation_bits = NULL;
-            prov->operation_bits_sz = 0;
-            prov->flag_initialized = 0;
-        }
+                OPENSSL_free(prov->operation_bits);
+                prov->operation_bits = NULL;
+                prov->operation_bits_sz = 0;
+                prov->flag_initialized = 0;
+            }
 
-        /*
-         * When the refcount drops to zero, it has been taken out of
-         * the store.  All we have to do here is clean it out.
-         */
-        if (ref == 0) {
 #ifndef FIPS_MODULE
             DSO_free(prov->module);
 #endif
@@ -377,6 +373,7 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
             sk_INFOPAIR_pop_free(prov->parameters, free_infopair);
 #ifndef HAVE_ATOMICS
             CRYPTO_THREAD_lock_free(prov->refcnt_lock);
+            CRYPTO_THREAD_lock_free(prov->activatecnt_lock);
 #endif
             OPENSSL_free(prov);
         }
@@ -460,7 +457,7 @@ int OSSL_PROVIDER_set_default_search_path(OSSL_LIB_CTX *libctx,
  * locking.  Direct callers must remember to set the store flags when
  * appropriate.
  */
-static int provider_activate(OSSL_PROVIDER *prov)
+static int provider_init(OSSL_PROVIDER *prov)
 {
     const OSSL_DISPATCH *provider_dispatch = NULL;
     void *tmp_provctx = NULL;    /* safety measure */
@@ -633,18 +630,60 @@ static int provider_activate(OSSL_PROVIDER *prov)
     return 1;
 }
 
+static int provider_deactivate(OSSL_PROVIDER *prov)
+{
+    int ref = 0;
+
+    if (!ossl_assert(prov != NULL))
+        return 0;
+
+    if (CRYPTO_DOWN_REF(&prov->activatecnt, &ref, prov->activatecnt_lock) <= 0)
+        return 0;
+
+    if (ref < 1)
+        prov->flag_activated = 0;
+
+    /* We don't deinit here, that's done in ossl_provider_free() */
+    return 1;
+}
+
+static int provider_activate(OSSL_PROVIDER *prov)
+{
+    int ref = 0;
+
+    if (CRYPTO_UP_REF(&prov->activatecnt, &ref, prov->activatecnt_lock) <= 0)
+        return 0;
+
+    if (provider_init(prov)) {
+        prov->flag_activated = 1;
+
+        return 1;
+    }
+
+    provider_deactivate(prov);
+    return 0;
+}
+
 int ossl_provider_activate(OSSL_PROVIDER *prov)
 {
+    if (prov == NULL)
+        return 0;
     if (provider_activate(prov)) {
         CRYPTO_THREAD_write_lock(prov->store->lock);
         prov->store->use_fallbacks = 0;
         CRYPTO_THREAD_unlock(prov->store->lock);
         return 1;
     }
-
     return 0;
 }
 
+int ossl_provider_deactivate(OSSL_PROVIDER *prov)
+{
+    if (prov == NULL)
+        return 0;
+    return provider_deactivate(prov);
+}
+
 void *ossl_provider_ctx(const OSSL_PROVIDER *prov)
 {
     return prov->provctx;
@@ -669,7 +708,7 @@ static int provider_forall_loaded(struct provider_store_st *store,
         OSSL_PROVIDER *prov =
             sk_OSSL_PROVIDER_value(store->providers, i);
 
-        if (prov->flag_initialized) {
+        if (prov->flag_activated) {
             if (found_activated != NULL)
                 *found_activated = 1;
             if (!(ret = cb(prov, cbdata)))
@@ -695,23 +734,14 @@ static void provider_activate_fallbacks(struct provider_store_st *store)
         for (i = 0; i < num_provs; i++) {
             OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i);
 
-            /*
-             * Activated fallback providers get an extra refcount, to
-             * simulate a regular load.
-             * Note that we don't care if the activation succeeds or not,
-             * other than to maintain a correct refcount.  If the activation
-             * doesn't succeed, then any future attempt to use the fallback
-             * provider will fail anyway.
-             */
-            if (prov->flag_fallback) {
-                if (ossl_provider_up_ref(prov)) {
-                    if (!provider_activate(prov)) {
-                        ossl_provider_free(prov);
-                    } else {
+            if (ossl_provider_up_ref(prov)) {
+                if (prov->flag_fallback) {
+                    if (provider_activate(prov)) {
                         prov->flag_activated_as_fallback = 1;
                         activated_fallback_count++;
                     }
                 }
+                ossl_provider_free(prov);
             }
         }
 
@@ -765,7 +795,7 @@ int ossl_provider_available(OSSL_PROVIDER *prov)
         provider_activate_fallbacks(prov->store);
         CRYPTO_THREAD_unlock(prov->store->lock);
 
-        return prov->flag_initialized;
+        return prov->flag_activated;
     }
     return 0;
 }
index dc7717062c21b034b51d3d453aea4593f393fd06..d01673e7674c369034e43c63b5797c38622e3d1b 100644 (file)
@@ -6,7 +6,7 @@ ossl_provider_find, ossl_provider_new, ossl_provider_up_ref,
 ossl_provider_free,
 ossl_provider_set_fallback, ossl_provider_set_module_path,
 ossl_provider_add_parameter,
-ossl_provider_activate, ossl_provider_available,
+ossl_provider_activate, ossl_provider_deactivate, ossl_provider_available,
 ossl_provider_ctx,
 ossl_provider_forall_loaded,
 ossl_provider_name, ossl_provider_dso,
@@ -36,9 +36,13 @@ ossl_provider_get_capabilities
  int ossl_provider_add_parameter(OSSL_PROVIDER *prov, const char *name,
                                  const char *value);
 
- /* Load and initialize the Provider */
+ /*
+  * Activate the Provider
+  * If the Provider is a module, the module will be loaded
+  */
  int ossl_provider_activate(OSSL_PROVIDER *prov);
- /* Check if provider is available */
+ int ossl_provider_deactivate(OSSL_PROVIDER *prov);
+ /* Check if provider is available (activated) */
  int ossl_provider_available(OSSL_PROVIDER *prov);
 
  /* Return pointer to the provider's context */
@@ -89,8 +93,8 @@ Provider objects are reference counted.
 Provider objects are initially inactive, i.e. they are only recorded
 in the store, but are not used.
 They are activated with the first call to ossl_provider_activate(),
-and are inactivated when ossl_provider_free() has been called as many
-times as ossl_provider_activate() has.
+and are deactivated with the last call to ossl_provider_deactivate().
+Activation affects a separate counter.
 
 =head2 Functions
 
@@ -127,11 +131,10 @@ ossl_provider_up_ref() increments the provider object I<prov>'s
 reference count.
 
 ossl_provider_free() decrements the provider object I<prov>'s
-reference count; if it drops below 2, the provider object is assumed
-to have fallen out of use and will be deactivated (its I<teardown>
-function is called); if it drops down to zero, I<prov> is assumed to
-have been taken out of the store, and the associated module will be
-unloaded if one was loaded, and I<prov> itself will be freed.
+reference count; when it drops to zero, the provider object is assumed
+to have fallen out of use and will be deinitialized (its I<teardown>
+function is called), and the associated module will be unloaded if one
+was loaded, and I<prov> itself will be freed.
 
 ossl_provider_set_fallback() marks an available provider I<prov> as
 fallback.
@@ -155,9 +158,9 @@ Only text parameters can be given, and it's up to the provider to
 interpret them.
 
 ossl_provider_activate() "activates" the provider for the given
-provider object I<prov>.
-What "activates" means depends on what type of provider object it
-is:
+provider object I<prov> by incrementing its activation count, flagging
+it as activated, and initializing it if it isn't already initialized.
+Initializing means one of the following:
 
 =over 4
 
@@ -175,6 +178,10 @@ be located in that module, and called.
 
 =back
 
+ossl_provider_deactivate() "deactivates" the provider for the given
+provider object I<prov> by decrementing its activation count.  When
+that count reaches zero, the activation flag is cleared.
+
 ossl_provider_available() activates all fallbacks if no provider is
 activated yet, then checks if given provider object I<prov> is
 activated.
@@ -269,8 +276,9 @@ it has been incremented.
 
 ossl_provider_free() doesn't return any value.
 
-ossl_provider_set_module_path(), ossl_provider_set_fallback() and
-ossl_provider_activate() return 1 on success, or 0 on error.
+ossl_provider_set_module_path(), ossl_provider_set_fallback(),
+ossl_provider_activate() and ossl_provider_deactivate() return 1 on
+success, or 0 on error.
 
 ossl_provider_available() return 1 if the provider is available,
 otherwise 0.
index ab36c93b3298cf18004140536ac41a38b79b77d5..7a0fc848757da18454b16944dd3550e441d0644a 100644 (file)
@@ -47,10 +47,10 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
 /*
  * Activate the Provider
  * If the Provider is a module, the module will be loaded
- * Inactivation is done by freeing the Provider
  */
 int ossl_provider_activate(OSSL_PROVIDER *prov);
-/* Check if the provider is available */
+int ossl_provider_deactivate(OSSL_PROVIDER *prov);
+/* Check if the provider is available (activated) */
 int ossl_provider_available(OSSL_PROVIDER *prov);
 
 /* Return pointer to the provider's context */
index 478f28182d32a30f7224d8afcfc7653305555ee0..4b2b6d5349fe6c9f54bf2491cf51575445f1e085 100644 (file)
@@ -30,7 +30,8 @@ static int test_provider(OSSL_PROVIDER *prov, const char *expected_greeting)
         && 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)
-        && TEST_str_eq(greeting, expected_greeting);
+        && TEST_str_eq(greeting, expected_greeting)
+        && TEST_true(ossl_provider_deactivate(prov));
 
     TEST_info("Got this greeting: %s\n", greeting);
     ossl_provider_free(prov);