gate calling of evp_method_id on having a non-zero name id

If a name is passed to EVP_<OBJ>_fetch of the form: name1:name2:name3 The names are parsed on the separator ':' and added to the store, but during the lookup in inner_evp_generic_fetch, the subsequent search of the store uses the full name1:name2:name3 string, which fails lookup, and causes subsequent assertion failures in evp_method_id. instead catch the failure in inner_evp_generic_fetch and return an error code if the name_id against a colon separated list of names fails. This provides a graceful error return path without asserts, and leaves room for a future feature in which such formatted names can be parsed and searched for iteratively Add a simple test to verify that providing a colon separated name results in an error indicating an invalid lookup. Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Todd Short <[email protected]> (Merged from openssl#23110) (cherry picked from commit 94be985)
t8m · Jan 1, 2024 · ec29062 · ec29062
1 parent a936d94
commit ec29062
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
@@ -317,13 +317,26 @@ inner_evp_generic_fetch(struct evp_method_data_st *methdata,
              * there is a correct name_id and meth_id, since those have
              * already been calculated in get_evp_method_from_store() and
              * put_evp_method_in_store() above.
+             * Note that there is a corner case here, in which, if a user
+             * passes a name of the form name1:name2:..., then the construction
+             * will create a method against all names, but the lookup will fail
+             * as ossl_namemap_name2num treats the name string as a single name
+             * rather than introducing new features where in the EVP_<obj>_fetch
+             * parses the string and querys for each, return an error.
              */
             if (name_id == 0)
                 name_id = ossl_namemap_name2num(namemap, name);
-            meth_id = evp_method_id(name_id, operation_id);
-            if (name_id != 0)
-                ossl_method_store_cache_set(store, prov, meth_id, propq,
-                                            method, up_ref_method, free_method);
+            if (name_id == 0) {
+                ERR_raise_data(ERR_LIB_EVP, ERR_R_FETCH_FAILED,
+                               "Algorithm %s cannot be found", name);
+                free_method(method);
+                method = NULL;
+            } else {
+                meth_id = evp_method_id(name_id, operation_id);
+                if (name_id != 0)
+                    ossl_method_store_cache_set(store, prov, meth_id, propq,
+                                                method, up_ref_method, free_method);
+            }
         }
 
         /*

diff --git a/test/evp_extra_test2.c b/test/evp_extra_test2.c
@@ -1236,6 +1236,24 @@ static int test_evp_pbe_alg_add(void)
 }
 #endif
 
+/*
+ * Currently, EVP_<OBJ>_fetch doesn't support
+ * colon separated alternative names for lookup
+ * so add a test here to ensure that when one is provided
+ * libcrypto returns an error
+ */
+static int evp_test_name_parsing(void)
+{
+    EVP_MD *md;
+
+    if (!TEST_ptr_null(md = EVP_MD_fetch(mainctx, "SHA256:BogusName", NULL))) {
+        EVP_MD_free(md);
+        return 0;
+    }
+
+    return 1;
+}
+
 int setup_tests(void)
 {
     if (!test_get_libctx(&mainctx, &nullprov, NULL, NULL, NULL)) {
@@ -1244,6 +1262,7 @@ int setup_tests(void)
         return 0;
     }
 
+    ADD_TEST(evp_test_name_parsing);
     ADD_TEST(test_alternative_default);
     ADD_ALL_TESTS(test_d2i_AutoPrivateKey_ex, OSSL_NELEM(keydata));
 #ifndef OPENSSL_NO_EC