Modify ossl_method_store_add() to accept an OSSL_PROVIDER and check for it

If ossl_method_store_add() gets called with a method that already exists
(i.e. the store has one with matching provider, nid and properties), that
method should not be stored.  We do this check inside ossl_method_store_add()
because it has all the locking required to do so safely.

Fixes #9561

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9650)

3 files changed, 26 insertions, 13 deletions

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index c1c81588bb..6e4414d831 100644
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -52,14 +52,14 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider,
          * If we haven't been told not to store,
          * add to the global store
          */
-        data->mcm->put(data->libctx, NULL, method, data->operation_id,
-                       algo->algorithm_name,
+        data->mcm->put(data->libctx, NULL, method, provider,
+                       data->operation_id, algo->algorithm_name,
                        algo->property_definition, data->mcm_data);
     }
 
-    data->mcm->put(data->libctx, data->store, method, data->operation_id,
-                   algo->algorithm_name, algo->property_definition,
-                   data->mcm_data);
+    data->mcm->put(data->libctx, data->store, method, provider,
+                   data->operation_id, algo->algorithm_name,
+                   algo->property_definition, data->mcm_data);
 
     /* refcnt-- because we're dropping the reference */
     data->mcm->destruct(method, data->mcm_data);
diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index 41dee721a4..fa85178a7e 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -114,9 +114,9 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store,
 }
 
 static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
-                               void *method, int operation_id,
-                               const char *name, const char *propdef,
-                               void *data)
+                               void *method, const OSSL_PROVIDER *prov,
+                               int operation_id, const char *name,
+                               const char *propdef, void *data)
 {
     struct method_data_st *methdata = data;
     OSSL_NAMEMAP *namemap;
@@ -132,7 +132,7 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store,
         && (store = get_default_method_store(libctx)) == NULL)
         return 0;
 
-    return ossl_method_store_add(store, methid, propdef, method,
+    return ossl_method_store_add(store, prov, methid, propdef, method,
                                  methdata->refcnt_up_method,
                                  methdata->destruct_method);
 }
diff --git a/crypto/property/property.c b/crypto/property/property.c
index 6576ba0fd2..182ea6454b 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -25,6 +25,7 @@
 #define IMPL_CACHE_FLUSH_THRESHOLD  500
 
 typedef struct {
+    const OSSL_PROVIDER *provider;
     OSSL_PROPERTY_LIST *properties;
     void *method;
     void (*method_destruct)(void *);
@@ -173,7 +174,7 @@ static int ossl_method_store_insert(OSSL_METHOD_STORE *store, ALGORITHM *alg)
         return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg);
 }
 
-int ossl_method_store_add(OSSL_METHOD_STORE *store,
+int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
                           int nid, const char *properties, void *method,
                           int (*method_up_ref)(void *),
                           void (*method_destruct)(void *))
@@ -181,6 +182,7 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
     ALGORITHM *alg = NULL;
     IMPLEMENTATION *impl;
     int ret = 0;
+    int i;
 
     if (nid <= 0 || method == NULL || store == NULL)
         return 0;
@@ -191,8 +193,11 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
     impl = OPENSSL_malloc(sizeof(*impl));
     if (impl == NULL)
         return 0;
-    if (method_up_ref != NULL && !method_up_ref(method))
+    if (method_up_ref != NULL && !method_up_ref(method)) {
+        OPENSSL_free(impl);
         return 0;
+    }
+    impl->provider = prov;
     impl->method = method;
     impl->method_destruct = method_destruct;
 
@@ -222,8 +227,16 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store,
             goto err;
     }
 
-    /* Push onto stack */
-    if (sk_IMPLEMENTATION_push(alg->impls, impl))
+    /* Push onto stack if there isn't one there already */
+    for (i = 0; i < sk_IMPLEMENTATION_num(alg->impls); i++) {
+        const IMPLEMENTATION *tmpimpl = sk_IMPLEMENTATION_value(alg->impls, i);
+
+        if (tmpimpl->provider == impl->provider
+            && tmpimpl->properties == impl->properties)
+            break;
+    }
+    if (i == sk_IMPLEMENTATION_num(alg->impls)
+        && sk_IMPLEMENTATION_push(alg->impls, impl))
         ret = 1;
     ossl_property_unlock(store);
     if (ret == 0)