From: Shane Lontis <shane.lontis@oracle.com>
Date: Fri, 12 Mar 2021 02:32:44 +0000 (+1000)
Subject: Remove TODO in rsa_ameth.c
X-Git-Tag: openssl-3.0.0-alpha14~288
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0fc39c9030df3e313c7ee08a3aefec8ab44bcd9a;p=thirdparty%2Fopenssl.git

Remove TODO in rsa_ameth.c

Fixes #14390

The only caller of this function tests EVP_KEYMGMT_is_a() beforehand
which will fail if the RSA key types do not match. So the test is not
necessary. The assert has been removed when it does the test.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14524)
---

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 30ba8d64283..74eb4330af2 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1714,7 +1714,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx,
         }
 
         /* Make sure that the keymgmt key type matches the legacy NID */
-        if (!ossl_assert(EVP_KEYMGMT_is_a(tmp_keymgmt, OBJ_nid2sn(pk->type))))
+        if (!EVP_KEYMGMT_is_a(tmp_keymgmt, OBJ_nid2sn(pk->type)))
             goto end;
 
         if ((keydata = evp_keymgmt_newdata(tmp_keymgmt)) == NULL)
diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
index 479155b90b2..e9e442606d2 100644
--- a/crypto/rsa/rsa_ameth.c
+++ b/crypto/rsa/rsa_ameth.c
@@ -856,15 +856,8 @@ static size_t rsa_pkey_dirty_cnt(const EVP_PKEY *pkey)
 }
 
 /*
- * For the moment, we trust the call path, where keys going through
- * rsa_pkey_export_to() match a KEYMGMT for the "RSA" keytype, while
- * keys going through rsa_pss_pkey_export_to() match a KEYMGMT for the
- * "RSA-PSS" keytype.
- * TODO(3.0) Investigate whether we should simply continue to trust the
- * call path, or if we should strengthen this function by checking that
- * |rsa_type| matches the RSA key subtype.  The latter requires ensuring
- * that the type flag for the RSA key is properly set by other functions
- * in this file.
+ * There is no need to do RSA_test_flags(rsa, RSA_FLAG_TYPE_RSASSAPSS)
+ * checks in this method since the caller tests EVP_KEYMGMT_is_a() first.
  */
 static int rsa_int_export_to(const EVP_PKEY *from, int rsa_type,
                              void *to_keydata, EVP_KEYMGMT *to_keymgmt,
diff --git a/test/keymgmt_internal_test.c b/test/keymgmt_internal_test.c
index 77414dbc271..e309c9e6547 100644
--- a/test/keymgmt_internal_test.c
+++ b/test/keymgmt_internal_test.c
@@ -142,8 +142,8 @@ static int test_pass_rsa(FIXTURE *fixture)
     RSA *rsa = NULL;
     BIGNUM *bn1 = NULL, *bn2 = NULL, *bn3 = NULL;
     EVP_PKEY *pk = NULL;
-    EVP_KEYMGMT *km1 = NULL, *km2 = NULL;
-    void *provkey = NULL;
+    EVP_KEYMGMT *km = NULL, *km1 = NULL, *km2 = NULL, *km3 = NULL;
+    void *provkey = NULL, *provkey2 = NULL;
     BIGNUM *bn_primes[1] = { NULL };
     BIGNUM *bn_exps[1] = { NULL };
     BIGNUM *bn_coeffs[1] = { NULL };
@@ -216,9 +216,16 @@ static int test_pass_rsa(FIXTURE *fixture)
 
     if (!TEST_ptr(km1 = EVP_KEYMGMT_fetch(fixture->ctx1, "RSA", NULL))
         || !TEST_ptr(km2 = EVP_KEYMGMT_fetch(fixture->ctx2, "RSA", NULL))
+        || !TEST_ptr(km3 = EVP_KEYMGMT_fetch(fixture->ctx1, "RSA-PSS", NULL))
         || !TEST_ptr_ne(km1, km2))
         goto err;
 
+    km = km3;
+    /* Check that we can't export an RSA key into a RSA-PSS keymanager */
+    if (!TEST_ptr_null(provkey2 = evp_pkey_export_to_provider(pk, NULL, &km,
+                                                              NULL)))
+        goto err;
+
     if (!TEST_ptr(provkey = evp_pkey_export_to_provider(pk, NULL, &km1, NULL))
         || !TEST_true(evp_keymgmt_export(km2, provkey,
                                          OSSL_KEYMGMT_SELECT_KEYPAIR,
@@ -249,6 +256,7 @@ static int test_pass_rsa(FIXTURE *fixture)
     EVP_PKEY_free(pk);
     EVP_KEYMGMT_free(km1);
     EVP_KEYMGMT_free(km2);
+    EVP_KEYMGMT_free(km3);
 
     return ret;
 }