From 8d4dec0d4b3055b4c2e7ece5ac99b67b3e77995e Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 18 Jun 2021 10:08:23 +0100 Subject: [PATCH] Instantiate predefined providers just-in-time Previously we instantiated all the predefined providers at the point that we create the provider store. Instead we move them to be instantiated as we need them. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/15854) --- crypto/provider.c | 12 --- crypto/provider_core.c | 134 +++++++++++++----------- doc/internal/man3/ossl_provider_new.pod | 11 +- include/internal/provider.h | 2 - include/internal/symhacks.h | 3 - 5 files changed, 71 insertions(+), 91 deletions(-) diff --git a/crypto/provider.c b/crypto/provider.c index 52647b2e77e..60664e9e24e 100644 --- a/crypto/provider.c +++ b/crypto/provider.c @@ -47,18 +47,6 @@ int OSSL_PROVIDER_unload(OSSL_PROVIDER *prov) return 1; } -int OSSL_PROVIDER_available(OSSL_LIB_CTX *libctx, const char *name) -{ - OSSL_PROVIDER *prov = NULL; - int available = 0; - - /* Find it or create it */ - prov = ossl_provider_find(libctx, name, 0); - available = ossl_provider_available(prov); - ossl_provider_free(prov); - return available; -} - const OSSL_PARAM *OSSL_PROVIDER_gettable_params(const OSSL_PROVIDER *prov) { return ossl_provider_gettable_params(prov); diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 4c423a6bdac..24c88e431d8 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -184,7 +184,6 @@ static void provider_store_free(void *vstore) static void *provider_store_new(OSSL_LIB_CTX *ctx) { struct provider_store_st *store = OPENSSL_zalloc(sizeof(*store)); - const struct predefined_providers_st *p = NULL; if (store == NULL || (store->providers = sk_OSSL_PROVIDER_new(ossl_provider_cmp)) == NULL @@ -199,31 +198,6 @@ static void *provider_store_new(OSSL_LIB_CTX *ctx) store->libctx = ctx; store->use_fallbacks = 1; - for (p = ossl_predefined_providers; p->name != NULL; p++) { - OSSL_PROVIDER *prov = NULL; - - /* - * We use the internal constructor directly here, - * otherwise we get a call loop - */ - prov = provider_new(p->name, p->init); - - if (prov == NULL - || sk_OSSL_PROVIDER_push(store->providers, prov) == 0) { - ossl_provider_free(prov); - provider_store_free(store); - ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR); - return NULL; - } - prov->libctx = ctx; - prov->store = store; -#ifndef FIPS_MODULE - prov->error_lib = ERR_get_next_error_library(); -#endif - if(p->is_fallback) - ossl_provider_set_fallback(prov); - } - return store; } @@ -379,10 +353,28 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name, return NULL; } + if (init_function == NULL) { + const struct predefined_providers_st *p; + + /* Check if this is a built-in provider */ + for (p = ossl_predefined_providers; p->name != NULL; p++) { + if (strcmp(p->name, name) == 0) { + init_function = p->init; + break; + } + } + } + /* provider_new() generates an error, so no need here */ if ((prov = provider_new(name, init_function)) == NULL) return NULL; + prov->libctx = libctx; + prov->store = store; +#ifndef FIPS_MODULE + prov->error_lib = ERR_get_next_error_library(); +#endif + if (!CRYPTO_THREAD_write_lock(store->lock)) return NULL; if (!ossl_provider_up_ref(prov)) { /* +1 One reference for the store */ @@ -392,12 +384,6 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name, ossl_provider_free(prov); /* -1 Store reference */ ossl_provider_free(prov); /* -1 Reference that was to be returned */ prov = NULL; - } else { - prov->libctx = libctx; - prov->store = store; -#ifndef FIPS_MODULE - prov->error_lib = ERR_get_next_error_library(); -#endif } CRYPTO_THREAD_unlock(store->lock); @@ -939,53 +925,68 @@ void *ossl_provider_ctx(const OSSL_PROVIDER *prov) * and then sets store->use_fallbacks = 0, so the second call and so on is * effectively a no-op. */ -static void provider_activate_fallbacks(struct provider_store_st *store) +static int provider_activate_fallbacks(struct provider_store_st *store) { int use_fallbacks; - int num_provs; int activated_fallback_count = 0; - int i; + int ret = 0; + const struct predefined_providers_st *p; if (!CRYPTO_THREAD_read_lock(store->lock)) - return; + return 0; use_fallbacks = store->use_fallbacks; CRYPTO_THREAD_unlock(store->lock); if (!use_fallbacks) - return; + return 1; if (!CRYPTO_THREAD_write_lock(store->lock)) - return; + return 0; /* Check again, just in case another thread changed it */ use_fallbacks = store->use_fallbacks; if (!use_fallbacks) { CRYPTO_THREAD_unlock(store->lock); - return; + return 1; } - num_provs = sk_OSSL_PROVIDER_num(store->providers); - for (i = 0; i < num_provs; i++) { - OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i); + for (p = ossl_predefined_providers; p->name != NULL; p++) { + OSSL_PROVIDER *prov = NULL; - if (ossl_provider_up_ref(prov)) { - if (CRYPTO_THREAD_write_lock(prov->flag_lock)) { - if (prov->flag_fallback) { - if (provider_activate(prov, 0, 0) > 0) - activated_fallback_count++; - } - CRYPTO_THREAD_unlock(prov->flag_lock); - } + if (!p->is_fallback) + continue; + /* + * We use the internal constructor directly here, + * otherwise we get a call loop + */ + prov = provider_new(p->name, p->init); + if (prov == NULL) + goto err; + prov->libctx = store->libctx; + prov->store = store; +#ifndef FIPS_MODULE + prov->error_lib = ERR_get_next_error_library(); +#endif + + /* + * We are calling provider_activate while holding the store lock. This + * means the init function will be called while holding a lock. Normally + * we try to avoid calling a user callback while holding a lock. + * However, fallbacks are never third party providers so we accept this. + */ + if (provider_activate(prov, 0, 0) < 0 + || sk_OSSL_PROVIDER_push(store->providers, prov) == 0) { ossl_provider_free(prov); + goto err; } + activated_fallback_count++; } - /* - * We assume that all fallbacks have been added to the store before - * any fallback is activated. - */ - if (activated_fallback_count > 0) + if (activated_fallback_count > 0) { store->use_fallbacks = 0; - + ret = 1; + } + err: CRYPTO_THREAD_unlock(store->lock); + return ret; } int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx, @@ -1008,7 +1009,8 @@ int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx, if (store == NULL) return 1; - provider_activate_fallbacks(store); + if (!provider_activate_fallbacks(store)) + return 0; /* * Under lock, grab a copy of the provider list and up_ref each @@ -1085,20 +1087,24 @@ int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx, return ret; } -int ossl_provider_available(OSSL_PROVIDER *prov) +int OSSL_PROVIDER_available(OSSL_LIB_CTX *libctx, const char *name) { - int ret; + OSSL_PROVIDER *prov = NULL; + int available = 0; + struct provider_store_st *store = get_provider_store(libctx); - if (prov != NULL) { - provider_activate_fallbacks(prov->store); + if (store == NULL || !provider_activate_fallbacks(store)) + return 0; + prov = ossl_provider_find(libctx, name, 0); + if (prov != NULL) { if (!CRYPTO_THREAD_read_lock(prov->flag_lock)) return 0; - ret = prov->flag_activated; + available = prov->flag_activated; CRYPTO_THREAD_unlock(prov->flag_lock); - return ret; + ossl_provider_free(prov); } - return 0; + return available; } /* Setters of Provider Object data */ diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod index ed2d6993b30..d563d41e72f 100644 --- a/doc/internal/man3/ossl_provider_new.pod +++ b/doc/internal/man3/ossl_provider_new.pod @@ -9,7 +9,7 @@ ossl_provider_add_parameter, ossl_provider_set_child, ossl_provider_get_parent, ossl_provider_up_ref_parent, ossl_provider_free_parent, ossl_provider_default_props_update, ossl_provider_get0_dispatch, ossl_provider_init_as_child, -ossl_provider_activate, ossl_provider_deactivate, ossl_provider_available, +ossl_provider_activate, ossl_provider_deactivate, ossl_provider_ctx, ossl_provider_doall_activated, ossl_provider_name, ossl_provider_dso, @@ -56,8 +56,6 @@ ossl_provider_get_capabilities int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks, int upcalls); 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 */ void *ossl_provider_ctx(const OSSL_PROVIDER *prov); @@ -229,10 +227,6 @@ ossl_provider_deactivate() "deactivates" the provider for the given provider object I 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 is -activated. - ossl_provider_ctx() returns a context created by the provider. Outside of the provider, it's completely opaque, but it needs to be passed back to some of the provider functions. @@ -348,9 +342,6 @@ ossl_provider_activate(), ossl_provider_activate_leave_fallbacks() and ossl_provider_deactivate(), ossl_provider_default_props_update() return 1 on success, or 0 on error. -ossl_provider_available() return 1 if the provider is available, -otherwise 0. - ossl_provider_name(), ossl_provider_dso(), ossl_provider_module_name(), and ossl_provider_module_path() return a pointer to their respective data if it's available, otherwise NULL diff --git a/include/internal/provider.h b/include/internal/provider.h index df20c76f90e..6432f8a2125 100644 --- a/include/internal/provider.h +++ b/include/internal/provider.h @@ -62,8 +62,6 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx); 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); /* Return pointer to the provider's context */ void *ossl_provider_ctx(const OSSL_PROVIDER *prov); diff --git a/include/internal/symhacks.h b/include/internal/symhacks.h index 564351642f1..33bae51e49c 100644 --- a/include/internal/symhacks.h +++ b/include/internal/symhacks.h @@ -15,9 +15,6 @@ # if defined(OPENSSL_SYS_VMS) -/* ossl_provider_available vs OSSL_PROVIDER_available */ -# undef ossl_provider_available -# define ossl_provider_available ossl_int_prov_available /* ossl_provider_gettable_params vs OSSL_PROVIDER_gettable_params */ # undef ossl_provider_gettable_params # define ossl_provider_gettable_params ossl_int_prov_gettable_params -- 2.47.2