From cfe271e2b89658e6980e7d054bde9c164f2e8687 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Fri, 10 Dec 2021 13:15:10 +0100 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/16765) (cherry picked from commit 5fbe15fd3b7c90a0cfb9f00be16225d8ed18b0dd) --- test/evp_extra_test.c | 136 +++++++++++++++++++++++++++++------------- 1 file changed, 93 insertions(+), 43 deletions(-) diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index e6582315c00..0b4f058c7b4 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; -- 2.47.2