property: do not overwrite the NULL-provider cache entry on set

ossl_method_store_cache_set inserts two entries per method: one keyed
on (nid, prop_query, prov) and one keyed on (nid, prop_query) with a
NULL provider, used to match "any provider" lookups.

Previously the set path always replaced the NULL-provider entry. When a
second provider cached the same nid, its method became the result for
"any provider" lookups, even though an earlier provider was already
cached. A shared nid could then resolve to the wrong provider: a
certificate's SPKI would decode through that provider's keymgmt and
X509_check_private_key would fail with a key value mismatch.

Only insert the NULL-provider entry when one does not already exist, so
the first provider to cache the nid owns it, matching the order
ossl_method_store_fetch would select.

Assisted-by: Claude:claude-opus-4-8

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Bob Beck <beck@openssl.org>
Reviewed-by: Simo Sorce <simo@redhat.com>
MergeDate: Thu Jun 18 18:02:32 2026
(Merged from https://github.com/openssl/openssl/pull/31568)
This commit is contained in:
Jakub Zelenka 2026-06-17 17:59:40 +02:00 committed by Neil Horman
parent 3989ab04c6
commit 9d476175d7

View file

@ -1190,7 +1190,6 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
{
QUERY *p = NULL;
int res = 1;
int skip_providerless = 0;
if (method == NULL) {
p = ossl_method_store_atomic_find_in_list(sa, nid, prov, prop_query);
@ -1200,19 +1199,9 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
}
p = ossl_method_store_atomic_find_in_list(sa, nid, prov, prop_query);
if (p != NULL) {
if (p != NULL)
ossl_method_store_atomic_archive(sa, p);
p = ossl_method_store_atomic_find_in_list(sa, nid, NULL, prop_query);
/*
* Note: We want to preserve previous behavior here. Namely, if we load multiple
* providers we don't want to change the algorithm implementation we return if we've
* already potentially returned one from another provider. So if an alg exists for
* a given nid/prop query with a NULL provider, don't replace the one we have.
* Instead, let the oldest one win
*/
if (p != NULL)
skip_providerless = 1;
}
p = QUERY_new(strlen(prop_query));
if (p != NULL) {
TSAN_BENIGN(p, "Unpublished value is safe on subsequent read");
@ -1232,12 +1221,20 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
goto err;
}
if (skip_providerless == 0) {
/*
* We also want to add this method into the cache against a key computed _only_
* from nid and property query. This lets us match in the event someone does a lookup
* against a NULL provider (i.e. the "any provided alg will do" match
*/
/*
* We also want to add this method into the cache against a key computed
* _only_ from nid and property query. This lets us match in the event
* someone does a lookup against a NULL provider (i.e. the "any provided
* alg will do" match).
*
* Only insert it if no NULL-provider entry exists yet for this nid and
* property query. The first provider to cache this nid owns that
* entry, which matches the provider ossl_method_store_fetch would pick
* by implementation order. Without this check, a later cache_set from
* a different provider would overwrite it and change which provider an
* "any provider" lookup resolves to.
*/
if (ossl_method_store_atomic_find_in_list(sa, nid, NULL, prop_query) == NULL) {
p = QUERY_new(strlen(prop_query));
if (p == NULL)
goto err;
@ -1257,6 +1254,7 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
goto err;
}
}
goto end;
}
err: