Fix EVP_PKEY_can_sign() handling of NULL from query_operation_name()

EVP_PKEY_can_sign() assumed query_operation_name(OSSL_OP_SIGNATURE)
always returns a non-NULL string. According to the documentation,
query_operation_name() may return NULL, in which case
EVP_KEYMGMT_get0_name() should be used as a fallback.

Fixes #27790

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28641)

(cherry picked from commit 0c2d67f417a2c2e0a63272e8d7753489b4958c0b)

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index aaba87170586e317447d52c5fa183ccc647a9fce..db1b696e9c6a2d6d780e3e541365904422e8c84f 100644 (file)
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1103,15 +1103,14 @@ int EVP_PKEY_can_sign(const EVP_PKEY *pkey)
     } else {
         const OSSL_PROVIDER *prov = EVP_KEYMGMT_get0_provider(pkey->keymgmt);
         OSSL_LIB_CTX *libctx = ossl_provider_libctx(prov);
-        const char *supported_sig =
-            pkey->keymgmt->query_operation_name != NULL
-            ? pkey->keymgmt->query_operation_name(OSSL_OP_SIGNATURE)
-            : EVP_KEYMGMT_get0_name(pkey->keymgmt);
-        EVP_SIGNATURE *signature = NULL;
-
-        signature = EVP_SIGNATURE_fetch(libctx, supported_sig, NULL);
-        if (signature != NULL) {
-            EVP_SIGNATURE_free(signature);
+        EVP_SIGNATURE *sig;
+        const char *name;
+
+        name = evp_keymgmt_util_query_operation_name(pkey->keymgmt,
+                                                     OSSL_OP_SIGNATURE);
+        sig = EVP_SIGNATURE_fetch(libctx, name, NULL);
+        if (sig != NULL) {
+            EVP_SIGNATURE_free(sig);
             return 1;
         }
     }

diff --git a/test/fake_rsaprov.c b/test/fake_rsaprov.c
index c1b8e28286143f4994dc5e17924e6ab6221dc9d4..4514df0effdfcff5e525babd9c504791e8c86820 100644 (file)
--- a/test/fake_rsaprov.c
+++ b/test/fake_rsaprov.c
@@ -32,6 +32,8 @@ static int exptypes_selection;
 static int query_id;
 static int key_deleted;
 
+unsigned fake_rsa_query_operation_name = 0;
+
 struct fake_rsa_keydata {
     int selection;
     int status;
@@ -77,7 +79,7 @@ static const char *fake_rsa_keymgmt_query(int id)
     /* record global for checking */
     query_id = id;
 
-    return "RSA";
+    return fake_rsa_query_operation_name ? NULL: "RSA";
 }
 
 static int fake_rsa_keymgmt_import(void *keydata, int selection,

diff --git a/test/fake_rsaprov.h b/test/fake_rsaprov.h
index cb2e66eb68ef200c5e71aa75872270474a7014ce..b2bc5d9ab5203b59b9092dfde8e3241f5ba42544 100644 (file)
--- a/test/fake_rsaprov.h
+++ b/test/fake_rsaprov.h
@@ -14,5 +14,14 @@
 /* Fake RSA provider implementation */
 OSSL_PROVIDER *fake_rsa_start(OSSL_LIB_CTX *libctx);
 void fake_rsa_finish(OSSL_PROVIDER *p);
+
 OSSL_PARAM *fake_rsa_key_params(int priv);
 void fake_rsa_restore_store_state(void);
+
+/*
+ * When fake_rsa_query_operation_name is set to a non-zero value,
+ * query_operation_name() will return NULL.
+ *
+ * By default, it is 0, in which case query_operation_name() will return "RSA".
+ */
+extern unsigned fake_rsa_query_operation_name;

diff --git a/test/provider_pkey_test.c b/test/provider_pkey_test.c
index 4abbdd33ec4d6a1077c0bc05fe839abbf974481d..2519689b769396987cf49fbbad2bcbebcd5774fe 100644 (file)
--- a/test/provider_pkey_test.c
+++ b/test/provider_pkey_test.c
@@ -239,6 +239,77 @@ end:
     return ret;
 }
 
+static int test_pkey_can_sign(void)
+{
+    OSSL_PROVIDER *fake_rsa = NULL;
+    EVP_PKEY *pkey_fake = NULL;
+    EVP_PKEY_CTX *ctx = NULL;
+    OSSL_PARAM *params = NULL;
+    int ret = 0;
+
+    if (!TEST_ptr(fake_rsa = fake_rsa_start(libctx)))
+        return 0;
+
+    /*
+     * Ensure other tests did not forget to reset fake_rsa_query_operation_name
+     * to its default value: 0
+     */
+    if (!TEST_int_eq(fake_rsa_query_operation_name, 0))
+        goto end;
+
+    if (!TEST_ptr(params = fake_rsa_key_params(0))
+        || !TEST_ptr(ctx = EVP_PKEY_CTX_new_from_name(libctx, "RSA",
+                                                      "provider=fake-rsa"))
+        || !TEST_true(EVP_PKEY_fromdata_init(ctx))
+        || !TEST_true(EVP_PKEY_fromdata(ctx, &pkey_fake, EVP_PKEY_PUBLIC_KEY,
+                                        params))
+        || !TEST_true(EVP_PKEY_can_sign(pkey_fake))
+        || !TEST_ptr(pkey_fake))
+        goto end;
+
+    EVP_PKEY_CTX_free(ctx);
+    ctx = NULL;
+    EVP_PKEY_free(pkey_fake);
+    pkey_fake = NULL;
+    OSSL_PARAM_free(params);
+    params = NULL;
+
+    /*
+     * Documented behavior for OSSL_FUNC_keymgmt_query_operation_name()
+     * allows it to return NULL, in which case the fallback should be to use
+     * EVP_KEYMGMT_get0_name(). That is exactly the thing we are testing here.
+     */
+    fake_rsa_query_operation_name = 1;
+
+    if (!TEST_ptr(params = fake_rsa_key_params(0))
+        || !TEST_ptr(ctx = EVP_PKEY_CTX_new_from_name(libctx, "RSA",
+                                                      "provider=fake-rsa"))
+        || !TEST_true(EVP_PKEY_fromdata_init(ctx))
+        || !TEST_true(EVP_PKEY_fromdata(ctx, &pkey_fake, EVP_PKEY_PUBLIC_KEY,
+                                        params))
+        || !TEST_true(EVP_PKEY_can_sign(pkey_fake))
+        || !TEST_ptr(pkey_fake))
+        goto end;
+
+    EVP_PKEY_CTX_free(ctx);
+    ctx = NULL;
+    EVP_PKEY_free(pkey_fake);
+    pkey_fake = NULL;
+    OSSL_PARAM_free(params);
+    params = NULL;
+
+    ret = 1;
+end:
+
+    EVP_PKEY_CTX_free(ctx);
+    EVP_PKEY_free(pkey_fake);
+    OSSL_PARAM_free(params);
+    fake_rsa_query_operation_name = 0;
+
+    fake_rsa_finish(fake_rsa);
+    return ret;
+}
+
 static int test_pkey_store(int idx)
 {
     OSSL_PROVIDER *deflt = NULL;
@@ -433,6 +504,7 @@ int setup_tests(void)
     ADD_TEST(test_pkey_sig);
     ADD_TEST(test_alternative_keygen_init);
     ADD_TEST(test_pkey_eq);
+    ADD_TEST(test_pkey_can_sign);
     ADD_ALL_TESTS(test_pkey_store, 2);
     ADD_TEST(test_pkey_delete);
     ADD_TEST(test_pkey_store_open_ex);