diff options
author | Matt Caswell <matt@openssl.org> | 2021-08-24 17:41:39 +0100 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2021-08-27 09:51:00 +1000 |
commit | f38af1258561eb0213b344c607037a528136099f (patch) | |
tree | 2f7a3c8b6ba66fa571f82484ce3f8ac478a092f3 /crypto/provider_conf.c | |
parent | 6f25d3c47995c6e4948212950566dfbe541904df (diff) |
Add locking for the provider_conf.c
Avoid races where 2 threads attempt to configure activation of providers
at the same time. E.g. via an explicit and an implict load of the config
file at the same time.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16425)
Diffstat (limited to 'crypto/provider_conf.c')
-rw-r--r-- | crypto/provider_conf.c | 89 |
1 files changed, 53 insertions, 36 deletions
diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c index 7689301b75..da3796d914 100644 --- a/crypto/provider_conf.c +++ b/crypto/provider_conf.c @@ -22,6 +22,7 @@ DEFINE_STACK_OF(OSSL_PROVIDER) /* PROVIDER config module */ typedef struct { + CRYPTO_RWLOCK *lock; STACK_OF(OSSL_PROVIDER) *activated_providers; } PROVIDER_CONF_GLOBAL; @@ -32,6 +33,12 @@ static void *prov_conf_ossl_ctx_new(OSSL_LIB_CTX *libctx) if (pcgbl == NULL) return NULL; + pcgbl->lock = CRYPTO_THREAD_lock_new(); + if (pcgbl->lock == NULL) { + OPENSSL_free(pcgbl); + return NULL; + } + return pcgbl; } @@ -43,6 +50,7 @@ static void prov_conf_ossl_ctx_free(void *vpcgbl) ossl_provider_free); OSSL_TRACE(CONF, "Cleaned up providers\n"); + CRYPTO_THREAD_lock_free(pcgbl->lock); OPENSSL_free(pcgbl); } @@ -176,48 +184,57 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name, activate = 1; } - if (activate && !prov_already_activated(name, pcgbl->activated_providers)) { - /* - * There is an attempt to activate a provider, so we should disable - * loading of fallbacks. Otherwise a misconfiguration could mean the - * intended provider does not get loaded. Subsequent fetches could then - * fallback to the default provider - which may be the wrong thing. - */ - if (!ossl_provider_disable_fallback_loading(libctx)) { + if (activate) { + if (!CRYPTO_THREAD_write_lock(pcgbl->lock)) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR); return 0; } - prov = ossl_provider_find(libctx, name, 1); - if (prov == NULL) - prov = ossl_provider_new(libctx, name, NULL, 1); - if (prov == NULL) { - if (soft) - ERR_clear_error(); - return 0; - } - - if (path != NULL) - ossl_provider_set_module_path(prov, path); - - ok = provider_conf_params(prov, NULL, NULL, value, cnf); + if (!prov_already_activated(name, pcgbl->activated_providers)) { + /* + * There is an attempt to activate a provider, so we should disable + * loading of fallbacks. Otherwise a misconfiguration could mean the + * intended provider does not get loaded. Subsequent fetches could + * then fallback to the default provider - which may be the wrong + * thing. + */ + if (!ossl_provider_disable_fallback_loading(libctx)) { + CRYPTO_THREAD_unlock(pcgbl->lock); + ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR); + return 0; + } + prov = ossl_provider_find(libctx, name, 1); + if (prov == NULL) + prov = ossl_provider_new(libctx, name, NULL, 1); + if (prov == NULL) { + CRYPTO_THREAD_unlock(pcgbl->lock); + if (soft) + ERR_clear_error(); + return 0; + } - if (ok) { - if (!ossl_provider_activate(prov, 1, 0)) { - ok = 0; - } else if (!ossl_provider_add_to_store(prov, &actual, 0)) { - ossl_provider_deactivate(prov); - ok = 0; - } else { - if (pcgbl->activated_providers == NULL) - pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null(); - sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual); - ok = 1; + if (path != NULL) + ossl_provider_set_module_path(prov, path); + + ok = provider_conf_params(prov, NULL, NULL, value, cnf); + + if (ok) { + if (!ossl_provider_activate(prov, 1, 0)) { + ok = 0; + } else if (!ossl_provider_add_to_store(prov, &actual, 0)) { + ossl_provider_deactivate(prov); + ok = 0; + } else { + if (pcgbl->activated_providers == NULL) + pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null(); + sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual); + ok = 1; + } } + if (!ok) + ossl_provider_free(prov); } - - if (!ok) - ossl_provider_free(prov); - } else if (!activate) { + CRYPTO_THREAD_unlock(pcgbl->lock); + } else { OSSL_PROVIDER_INFO entry; memset(&entry, 0, sizeof(entry)); |