From 9d476175d777c4da1a1b4f218d14b54ef4ba7505 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Wed, 17 Jun 2026 17:59:40 +0200 Subject: [PATCH] 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 Reviewed-by: Bob Beck Reviewed-by: Simo Sorce MergeDate: Thu Jun 18 18:02:32 2026 (Merged from https://github.com/openssl/openssl/pull/31568) --- crypto/property/property.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/crypto/property/property.c b/crypto/property/property.c index 59238bfcd2f..e807f06995b 100644 --- a/crypto/property/property.c +++ b/crypto/property/property.c @@ -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: -- 2.47.3