From: Daniel Kubec <kubec@openssl.org>
Date: Fri, 19 Sep 2025 13:48:41 +0000 (+0200)
Subject: Fix EVP_PKEY_can_sign() handling of NULL from query_operation_name()
X-Git-Tag: openssl-3.5.4~18
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=832c39e915d24f13ea2683479a1c7bc07254aa66;p=thirdparty%2Fopenssl.git

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/28620)

(cherry picked from commit 051108ee53d5b0ff5a125d32acfbc7e20899b022)
---

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 7f4508169df..63953a84e1f 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1146,15 +1146,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 46fc9104ef9..6ed12155433 100644
--- a/test/fake_rsaprov.c
+++ b/test/fake_rsaprov.c
@@ -35,6 +35,8 @@ static int exptypes_selection;
 static int query_id;
 static int key_deleted;
 
+unsigned fake_rsa_query_operation_name = 0;
+
 typedef struct {
     OSSL_LIB_CTX *libctx;
 } PROV_FAKE_RSA_CTX;
@@ -90,7 +92,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 cb2e66eb68e..b2bc5d9ab52 100644
--- 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 cb656a62a65..9ffe3581d62 100644
--- 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;
@@ -719,6 +790,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);