test/evp_extra_test.c: Refactor test_fromdata()

test_fromdata() turns out to be a bit inflexible, so we split it into
two functions, make_key_fromdata() and test_selection(), and adjust
test_EVP_PKEY_ffc_priv_pub() and test_EC_priv_pub() accordingly.  This
allows us to check the resulting keys further, not only to check that
the bits we expect are there, but also that the bits that we expect
not to be there to actually not be there!

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

1 files changed, 93 insertions, 43 deletions

diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index 62b61bdd45..bd6ff414d0 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -679,25 +679,35 @@ err:
 }
 
 #if !defined(OPENSSL_NO_DH) || !defined(OPENSSL_NO_DSA) || !defined(OPENSSL_NO_EC)
-static int test_fromdata(char *keytype, int selection, OSSL_PARAM *params)
+static EVP_PKEY *make_key_fromdata(char *keytype, OSSL_PARAM *params)
 {
     EVP_PKEY_CTX *pctx = NULL;
-    EVP_PKEY *pkey = NULL;
-    int testresult = 0;
-    int ret;
-    BIO *bio = BIO_new(BIO_s_mem());
+    EVP_PKEY *tmp_pkey = NULL, *pkey = NULL;
 
     if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_from_name(testctx, keytype, testpropq)))
         goto err;
     if (!TEST_int_gt(EVP_PKEY_fromdata_init(pctx), 0)
-        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &pkey, EVP_PKEY_KEYPAIR,
+        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &tmp_pkey, EVP_PKEY_KEYPAIR,
                                           params), 0))
         goto err;
 
-    if (!TEST_ptr(pkey))
+    if (!TEST_ptr(tmp_pkey))
         goto err;
 
-    /* Check we can use the resulting key */
+    pkey = tmp_pkey;
+    tmp_pkey = NULL;
+ err:
+    EVP_PKEY_free(tmp_pkey);
+    EVP_PKEY_CTX_free(pctx);
+    return pkey;
+}
+
+static int test_selection(EVP_PKEY *pkey, int selection)
+{
+    int testresult = 0;
+    int ret;
+    BIO *bio = BIO_new(BIO_s_mem());
+
     ret = PEM_write_bio_PUBKEY(bio, pkey);
     if ((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) {
         if (!TEST_true(ret))
@@ -718,8 +728,6 @@ static int test_fromdata(char *keytype, int selection, OSSL_PARAM *params)
 
     testresult = 1;
  err:
-    EVP_PKEY_free(pkey);
-    EVP_PKEY_CTX_free(pctx);
     BIO_free(bio);
 
     return testresult;
@@ -735,6 +743,10 @@ static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
 {
     OSSL_PARAM_BLD *bld = NULL;
     OSSL_PARAM *params = NULL;
+    EVP_PKEY *just_params = NULL;
+    EVP_PKEY *params_and_priv = NULL;
+    EVP_PKEY *params_and_pub = NULL;
+    EVP_PKEY *params_and_keypair = NULL;
     BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub = NULL, *priv = NULL;
     int ret = 0;
 
@@ -755,14 +767,18 @@ static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_Q, q))
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_G, g)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(just_params = make_key_fromdata(keytype, params)))
         goto err;
 
-    if (!test_fromdata(keytype, OSSL_KEYMGMT_SELECT_ALL_PARAMETERS, params))
-        goto err;
     OSSL_PARAM_free(params);
-    params = NULL;
     OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
+    if (!test_selection(just_params, OSSL_KEYMGMT_SELECT_ALL_PARAMETERS)
+        || test_selection(just_params, OSSL_KEYMGMT_SELECT_KEYPAIR))
+        goto err;
 
     /* Test priv and !pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -772,14 +788,18 @@ static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
                                              priv)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_priv = make_key_fromdata(keytype, params)))
         goto err;
 
-    if (!test_fromdata(keytype, OSSL_KEYMGMT_SELECT_PRIVATE_KEY, params))
-        goto err;
     OSSL_PARAM_free(params);
-    params = NULL;
     OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
+    if (!test_selection(params_and_priv, OSSL_KEYMGMT_SELECT_PRIVATE_KEY)
+        || test_selection(params_and_priv, OSSL_KEYMGMT_SELECT_PUBLIC_KEY))
+        goto err;
 
     /* Test !priv and pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -789,14 +809,18 @@ static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PUB_KEY,
                                              pub)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_pub = make_key_fromdata(keytype, params)))
         goto err;
 
-    if (!test_fromdata(keytype, OSSL_KEYMGMT_SELECT_PUBLIC_KEY, params))
-        goto err;
     OSSL_PARAM_free(params);
-    params = NULL;
     OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
+    if (!test_selection(params_and_pub, OSSL_KEYMGMT_SELECT_PUBLIC_KEY)
+        || test_selection(params_and_pub, OSSL_KEYMGMT_SELECT_PRIVATE_KEY))
+        goto err;
 
     /* Test priv and pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -808,16 +832,21 @@ static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
                                              priv)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_keypair = make_key_fromdata(keytype, params)))
         goto err;
 
-    if (!test_fromdata(keytype, EVP_PKEY_KEYPAIR, params))
+    if (!test_selection(params_and_keypair, EVP_PKEY_KEYPAIR))
         goto err;
 
     ret = 1;
  err:
     OSSL_PARAM_free(params);
     OSSL_PARAM_BLD_free(bld);
+    EVP_PKEY_free(just_params);
+    EVP_PKEY_free(params_and_priv);
+    EVP_PKEY_free(params_and_pub);
+    EVP_PKEY_free(params_and_keypair);
     BN_free(p);
     BN_free(q);
     BN_free(g);
@@ -851,6 +880,10 @@ static int test_EC_priv_pub(void)
 {
     OSSL_PARAM_BLD *bld = NULL;
     OSSL_PARAM *params = NULL;
+    EVP_PKEY *just_params = NULL;
+    EVP_PKEY *params_and_priv = NULL;
+    EVP_PKEY *params_and_pub = NULL;
+    EVP_PKEY *params_and_keypair = NULL;
     BIGNUM *priv = NULL;
     int ret = 0;
 
@@ -867,14 +900,18 @@ static int test_EC_priv_pub(void)
                                                       OSSL_PKEY_PARAM_GROUP_NAME,
                                                       "P-256", 0)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(just_params = make_key_fromdata("EC", params)))
         goto err;
 
-    if (!test_fromdata("EC", OSSL_KEYMGMT_SELECT_ALL_PARAMETERS, params))
-        goto err;
     OSSL_PARAM_free(params);
-    params = NULL;
     OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
+    if (!test_selection(just_params, OSSL_KEYMGMT_SELECT_ALL_PARAMETERS)
+        || test_selection(just_params, OSSL_KEYMGMT_SELECT_KEYPAIR))
+        goto err;
 
     /* Test priv and !pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -884,20 +921,24 @@ static int test_EC_priv_pub(void)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
                                              priv)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_priv = make_key_fromdata("EC", params)))
         goto err;
 
+    OSSL_PARAM_free(params);
+    OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
     /*
-     * We indicate only parameters here. The "EVP_PKEY_fromdata" call will do
-     * the private key anyway, but the subsequent PEM_write_bio_PrivateKey_ex
-     * call is expected to fail because PEM_write_bio_PrivateKey_ex does not
-     * support exporting a private EC key without a corresponding public key
+     * We indicate only parameters here, in spite of having built a key that
+     * has a private part, because the PEM_write_bio_PrivateKey_ex call is
+     * expected to fail because it does not support exporting a private EC
+     * key without a corresponding public key
      */
-    if (!test_fromdata("EC", OSSL_KEYMGMT_SELECT_ALL_PARAMETERS, params))
+    if (!test_selection(params_and_priv, OSSL_KEYMGMT_SELECT_ALL_PARAMETERS)
+        || test_selection(params_and_priv, OSSL_KEYMGMT_SELECT_PUBLIC_KEY))
         goto err;
-    OSSL_PARAM_free(params);
-    params = NULL;
-    OSSL_PARAM_BLD_free(bld);
 
     /* Test !priv and pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -908,14 +949,18 @@ static int test_EC_priv_pub(void)
                                                        OSSL_PKEY_PARAM_PUB_KEY,
                                                        ec_pub, sizeof(ec_pub))))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_pub = make_key_fromdata("EC", params)))
         goto err;
 
-    if (!test_fromdata("EC", OSSL_KEYMGMT_SELECT_PUBLIC_KEY, params))
-        goto err;
     OSSL_PARAM_free(params);
-    params = NULL;
     OSSL_PARAM_BLD_free(bld);
+    params = NULL;
+    bld = NULL;
+
+    if (!test_selection(params_and_pub, OSSL_KEYMGMT_SELECT_PUBLIC_KEY)
+        || test_selection(params_and_pub, OSSL_KEYMGMT_SELECT_PRIVATE_KEY))
+        goto err;
 
     /* Test priv and pub */
     if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
@@ -928,16 +973,21 @@ static int test_EC_priv_pub(void)
         || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
                                              priv)))
         goto err;
-    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))
+        || !TEST_ptr(params_and_keypair = make_key_fromdata("EC", params)))
         goto err;
 
-    if (!test_fromdata("EC", EVP_PKEY_KEYPAIR, params))
+    if (!test_selection(params_and_keypair, EVP_PKEY_KEYPAIR))
         goto err;
 
     ret = 1;
  err:
     OSSL_PARAM_free(params);
     OSSL_PARAM_BLD_free(bld);
+    EVP_PKEY_free(just_params);
+    EVP_PKEY_free(params_and_priv);
+    EVP_PKEY_free(params_and_pub);
+    EVP_PKEY_free(params_and_keypair);
     BN_free(priv);
 
     return ret;