Remove TODO in rsa_ameth.c
authorShane Lontis <shane.lontis@oracle.com>
Fri, 12 Mar 2021 02:32:44 +0000 (12:32 +1000)
committerPauli <ppzgs1@gmail.com>
Sun, 14 Mar 2021 05:37:18 +0000 (15:37 +1000)
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)

crypto/evp/p_lib.c
crypto/rsa/rsa_ameth.c
test/keymgmt_internal_test.c

index 30ba8d64283c8d2ad34d2864e79f9147b78ebe66..74eb4330af226da36531f33921863e89bbb746da 100644 (file)
@@ -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)
index 479155b90b23cdd76c06e1b8fe62f26bf9d39ddc..e9e442606d2931c07e0093d746f6afde6d43a532 100644 (file)
@@ -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,
index 77414dbc271d81946ef5f9469d751eeae5088f8a..e309c9e654757c815afde0e11f466a92ece0de84 100644 (file)
@@ -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;
 }