Always try to construct methods as new provider might be added

Otherwise optional properties can be incorrectly ignored.

Fixes #18262

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18269)

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index 6b25379f7bbce899a770a83052012ff7826369b0..faa6ebdefd1784224fa6bfea4e3d1d4eb46a250d 100644 (file)
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -123,31 +123,38 @@ void *ossl_method_construct(OSSL_LIB_CTX *libctx, int operation_id,
                             OSSL_METHOD_CONSTRUCT_METHOD *mcm, void *mcm_data)
 {
     void *method = NULL;
+    OSSL_PROVIDER *provider = provider_rw != NULL ? *provider_rw : NULL;
+    struct construct_data_st cbdata;
 
-    if ((method = mcm->get(NULL, (const OSSL_PROVIDER **)provider_rw,
-                           mcm_data)) == NULL) {
-        OSSL_PROVIDER *provider = provider_rw != NULL ? *provider_rw : NULL;
-        struct construct_data_st cbdata;
-
-        cbdata.store = NULL;
-        cbdata.force_store = force_store;
-        cbdata.mcm = mcm;
-        cbdata.mcm_data = mcm_data;
-        ossl_algorithm_do_all(libctx, operation_id, provider,
-                              ossl_method_construct_precondition,
-                              ossl_method_construct_this,
-                              ossl_method_construct_postcondition,
-                              &cbdata);
-
-        /* If there is a temporary store, try there first */
-        if (cbdata.store != NULL)
-            method = mcm->get(cbdata.store, (const OSSL_PROVIDER **)provider_rw,
-                              mcm_data);
-
-        /* If no method was found yet, try the global store */
-        if (method == NULL)
-            method = mcm->get(NULL, (const OSSL_PROVIDER **)provider_rw, mcm_data);
-    }
+    /*
+     * We might be tempted to try to look into the method store without
+     * constructing to see if we can find our method there already.
+     * Unfortunately that does not work well if the query contains
+     * optional properties as newly loaded providers can match them better.
+     * We trust that ossl_method_construct_precondition() and
+     * ossl_method_construct_postcondition() make sure that the
+     * ossl_algorithm_do_all() does very little when methods from
+     * a provider have already been constructed.
+     */
+
+    cbdata.store = NULL;
+    cbdata.force_store = force_store;
+    cbdata.mcm = mcm;
+    cbdata.mcm_data = mcm_data;
+    ossl_algorithm_do_all(libctx, operation_id, provider,
+                          ossl_method_construct_precondition,
+                          ossl_method_construct_this,
+                          ossl_method_construct_postcondition,
+                          &cbdata);
+
+    /* If there is a temporary store, try there first */
+    if (cbdata.store != NULL)
+        method = mcm->get(cbdata.store, (const OSSL_PROVIDER **)provider_rw,
+                          mcm_data);
+
+    /* If no method was found yet, try the global store */
+    if (method == NULL)
+        method = mcm->get(NULL, (const OSSL_PROVIDER **)provider_rw, mcm_data);
 
     return method;
 }