restore "oldest wins" behavior in method store

We expressly define EVP_*fetch apis as not guaranteeing which provider
and algorithm is sourced from.  However, its likely that some users have
some inadvertent reliance on getting the same provider for a non
provider specific (and non property specific) fetch.  While thats
generally bad practice (since we don't guarantee it), its particularly
hard to provide that behavior in the new cache infrastructure, so lets
save everyone some trouble by not changing that behavior needlessly.

Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed Jun 17 14:38:43 2026
(Merged from https://github.com/openssl/openssl/pull/31487)

diff --git a/crypto/property/property.c b/crypto/property/property.c
index ee85031e6c68a03960ea57d5af8d8483e7b6a688..59238bfcd2ff7542f38e1d648954e543f14f2f9c 100644 (file)
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -1190,6 +1190,7 @@ 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);
@@ -1202,8 +1203,15 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
     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)
-            ossl_method_store_atomic_archive(sa, p);
+            skip_providerless = 1;
     }
     p = QUERY_new(strlen(prop_query));
     if (p != NULL) {
@@ -1224,30 +1232,31 @@ static ossl_inline int ossl_method_store_cache_set_atomic(OSSL_METHOD_STORE *sto
             goto err;
         }
 
-        /*
-         * 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
-         */
-        p = QUERY_new(strlen(prop_query));
-        if (p == NULL)
-            goto err;
-        TSAN_BENIGN(p, "Unpublished value is safe on subsequent read");
-        p->saptr = sa;
-        p->nid = nid;
-        p->prov = NULL;
-        p->archived = 0;
-        strcpy(p->prop_query, prop_query);
-        p->method.method = method;
-        p->method.up_ref = method_up_ref;
-        p->method.free = method_destruct;
-        if (!ossl_method_up_ref(&p->method))
-            goto err;
-        if (!ossl_method_store_atomic_insert_to_list(sa, p)) {
-            ossl_method_free(&p->method);
-            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
+             */
+            p = QUERY_new(strlen(prop_query));
+            if (p == NULL)
+                goto err;
+            TSAN_BENIGN(p, "Unpublished value is safe on subsequent read");
+            p->saptr = sa;
+            p->nid = nid;
+            p->prov = NULL;
+            p->archived = 0;
+            strcpy(p->prop_query, prop_query);
+            p->method.method = method;
+            p->method.up_ref = method_up_ref;
+            p->method.free = method_destruct;
+            if (!ossl_method_up_ref(&p->method))
+                goto err;
+            if (!ossl_method_store_atomic_insert_to_list(sa, p)) {
+                ossl_method_free(&p->method);
+                goto err;
+            }
         }
-
         goto end;
     }
 err:

diff --git a/test/property_test.c b/test/property_test.c
index 467647bf9083bd63424c38d54963e030fb60f1ef..f827d3c4354bcee93bdaeca882deb2276f0d0a2b 100644 (file)
--- a/test/property_test.c
+++ b/test/property_test.c
@@ -634,8 +634,8 @@ static int test_query_cache_stochastic(void)
             || result != v + i)
             errors++;
     }
-    /* There is a tiny probability that this will fail when it shouldn't */
-    res = !TEST_int_gt(errors, 0);
+
+    res = TEST_int_eq(errors, 0);
 
 err:
     ossl_method_store_free(store);
@@ -672,7 +672,7 @@ static int test_query_cache_set_duplicate(void)
      */
     ossl_method_store_cache_set(store, &prov, 1, "", &refs, counted_up_ref,
         counted_down_ref);
-    if (!TEST_int_eq(refs, 5)
+    if (!TEST_int_eq(refs, 4)
         || !TEST_true(ossl_method_store_cache_get(store, &prov, 1, "",
             &result))
         || !TEST_ptr_eq(result, &refs))