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)

diff --git a/crypto/property/property.c b/crypto/property/property.c
index 59238bfcd2ff7542f38e1d648954e543f14f2f9c..e807f06995be707d7d91f9b47f75548b22184828 100644 (file)
--- 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: