From ac7750bb5ec4238c4f6514eb174c1bd584728f05 Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Wed, 25 Nov 2020 15:21:52 +1000 Subject: [PATCH] Fix Segfault in EVP_PKEY_CTX_dup when the ctx has an undefined operation. Fixes #12438 Note: This worked in 1.1.1 so just returning an error is not valid. Reviewed-by: Ben Kaduk (Merged from https://github.com/openssl/openssl/pull/13505) --- crypto/evp/pmeth_lib.c | 73 ++++++++++++++++++++--------------- doc/man3/EVP_PKEY_CTX_new.pod | 5 ++- test/evp_pkey_provided_test.c | 66 +++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 33 deletions(-) diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index 2c2d9395381..7364a148a63 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -488,87 +488,79 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx) return NULL; } } + rctx->legacy_keytype = pctx->legacy_keytype; if (EVP_PKEY_CTX_IS_DERIVE_OP(pctx)) { if (pctx->op.kex.exchange != NULL) { rctx->op.kex.exchange = pctx->op.kex.exchange; - if (!EVP_KEYEXCH_up_ref(rctx->op.kex.exchange)) { - OPENSSL_free(rctx); - return NULL; - } + if (!EVP_KEYEXCH_up_ref(rctx->op.kex.exchange)) + goto end; } if (pctx->op.kex.exchprovctx != NULL) { if (!ossl_assert(pctx->op.kex.exchange != NULL)) - return NULL; + goto end; rctx->op.kex.exchprovctx = pctx->op.kex.exchange->dupctx(pctx->op.kex.exchprovctx); if (rctx->op.kex.exchprovctx == NULL) { EVP_KEYEXCH_free(rctx->op.kex.exchange); - OPENSSL_free(rctx); - return NULL; + goto end; } return rctx; } } else if (EVP_PKEY_CTX_IS_SIGNATURE_OP(pctx)) { if (pctx->op.sig.signature != NULL) { rctx->op.sig.signature = pctx->op.sig.signature; - if (!EVP_SIGNATURE_up_ref(rctx->op.sig.signature)) { - OPENSSL_free(rctx); - return NULL; - } + if (!EVP_SIGNATURE_up_ref(rctx->op.sig.signature)) + goto end; } if (pctx->op.sig.sigprovctx != NULL) { if (!ossl_assert(pctx->op.sig.signature != NULL)) - return NULL; + goto end; rctx->op.sig.sigprovctx = pctx->op.sig.signature->dupctx(pctx->op.sig.sigprovctx); if (rctx->op.sig.sigprovctx == NULL) { EVP_SIGNATURE_free(rctx->op.sig.signature); - OPENSSL_free(rctx); - return NULL; + goto end; } return rctx; } } else if (EVP_PKEY_CTX_IS_ASYM_CIPHER_OP(pctx)) { if (pctx->op.ciph.cipher != NULL) { rctx->op.ciph.cipher = pctx->op.ciph.cipher; - if (!EVP_ASYM_CIPHER_up_ref(rctx->op.ciph.cipher)) { - OPENSSL_free(rctx); - return NULL; - } + if (!EVP_ASYM_CIPHER_up_ref(rctx->op.ciph.cipher)) + goto end; } if (pctx->op.ciph.ciphprovctx != NULL) { if (!ossl_assert(pctx->op.ciph.cipher != NULL)) - return NULL; + goto end; rctx->op.ciph.ciphprovctx = pctx->op.ciph.cipher->dupctx(pctx->op.ciph.ciphprovctx); if (rctx->op.ciph.ciphprovctx == NULL) { EVP_ASYM_CIPHER_free(rctx->op.ciph.cipher); - OPENSSL_free(rctx); - return NULL; + goto end; } return rctx; } } else if (EVP_PKEY_CTX_IS_KEM_OP(pctx)) { if (pctx->op.encap.kem != NULL) { rctx->op.encap.kem = pctx->op.encap.kem; - if (!EVP_KEM_up_ref(rctx->op.encap.kem)) { - OPENSSL_free(rctx); - return NULL; - } + if (!EVP_KEM_up_ref(rctx->op.encap.kem)) + goto end; } if (pctx->op.encap.kemprovctx != NULL) { if (!ossl_assert(pctx->op.encap.kem != NULL)) - return NULL; + goto end; rctx->op.encap.kemprovctx = pctx->op.encap.kem->dupctx(pctx->op.encap.kemprovctx); if (rctx->op.encap.kemprovctx == NULL) { EVP_KEM_free(rctx->op.encap.kem); - OPENSSL_free(rctx); - return NULL; + goto end; } return rctx; } + } else if (EVP_PKEY_CTX_IS_GEN_OP(pctx)) { + /* Not supported - This would need a gen_dupctx() to work */ + goto end; } rctx->pmeth = pctx->pmeth; @@ -576,17 +568,36 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx) rctx->engine = pctx->engine; # endif - if (pctx->peerkey) + if (pctx->peerkey != NULL) EVP_PKEY_up_ref(pctx->peerkey); rctx->peerkey = pctx->peerkey; + if (pctx->pmeth == NULL) { + if (rctx->operation == EVP_PKEY_OP_UNDEFINED) { + EVP_KEYMGMT *tmp_keymgmt = pctx->keymgmt; + void *provkey; + + provkey = evp_pkey_export_to_provider(pctx->pkey, pctx->libctx, + &tmp_keymgmt, pctx->propquery); + if (provkey == NULL) + goto err; + if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) + goto err; + EVP_KEYMGMT_free(rctx->keymgmt); + rctx->keymgmt = tmp_keymgmt; + return rctx; + } + goto err; + } if (pctx->pmeth->copy(rctx, pctx) > 0) return rctx; - +err: rctx->pmeth = NULL; EVP_PKEY_CTX_free(rctx); return NULL; - +end: + OPENSSL_free(rctx); + return NULL; } int EVP_PKEY_meth_add0(const EVP_PKEY_METHOD *pmeth) diff --git a/doc/man3/EVP_PKEY_CTX_new.pod b/doc/man3/EVP_PKEY_CTX_new.pod index 1b23c2a403c..3342386d94e 100644 --- a/doc/man3/EVP_PKEY_CTX_new.pod +++ b/doc/man3/EVP_PKEY_CTX_new.pod @@ -47,7 +47,8 @@ used when no B structure is associated with the operations, for example during parameter generation or key generation for some algorithms. -EVP_PKEY_CTX_dup() duplicates the context I. +EVP_PKEY_CTX_dup() duplicates the context I. It is not supported for a +keygen operation. EVP_PKEY_CTX_free() frees up the context I. If I is NULL, nothing is done. @@ -96,7 +97,7 @@ documentation for more information. =head1 RETURN VALUES -EVP_PKEY_CTX_new(), EVP_PKEY_CTX_new_id(), EVP_PKEY_CTX_dup() returns either +EVP_PKEY_CTX_new(), EVP_PKEY_CTX_new_id() and EVP_PKEY_CTX_dup() return either the newly allocated B structure or B if an error occurred. EVP_PKEY_CTX_free() does not return a value. diff --git a/test/evp_pkey_provided_test.c b/test/evp_pkey_provided_test.c index a983d3b5330..bfc9cd2ebc7 100644 --- a/test/evp_pkey_provided_test.c +++ b/test/evp_pkey_provided_test.c @@ -1072,6 +1072,70 @@ err: return ret; } +static int test_ec_dup_no_operation(void) +{ + int ret = 0; + EVP_PKEY_CTX *pctx = NULL, *ctx = NULL, *kctx = NULL; + EVP_PKEY *param = NULL, *pkey = NULL; + + if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL)) + || !TEST_int_gt(EVP_PKEY_paramgen_init(pctx), 0) + || !TEST_int_gt(EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, + NID_X9_62_prime256v1), 0) + || !TEST_int_gt(EVP_PKEY_paramgen(pctx, ¶m), 0) + || !TEST_ptr(param)) + goto err; + + EVP_PKEY_CTX_free(pctx); + pctx = NULL; + + if (!TEST_ptr(ctx = EVP_PKEY_CTX_new_from_pkey(NULL, param, NULL)) + || !TEST_ptr(kctx = EVP_PKEY_CTX_dup(ctx)) + || !TEST_int_gt(EVP_PKEY_keygen_init(kctx), 0) + || !TEST_int_gt(EVP_PKEY_keygen(kctx, &pkey), 0)) + goto err; + ret = 1; +err: + EVP_PKEY_free(pkey); + EVP_PKEY_free(param); + EVP_PKEY_CTX_free(ctx); + EVP_PKEY_CTX_free(kctx); + EVP_PKEY_CTX_free(pctx); + return ret; +} + +/* Test that keygen doesn't support EVP_PKEY_CTX_dup */ +static int test_ec_dup_keygen_operation(void) +{ + int ret = 0; + EVP_PKEY_CTX *pctx = NULL, *ctx = NULL, *kctx = NULL; + EVP_PKEY *param = NULL, *pkey = NULL; + + if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL)) + || !TEST_int_gt(EVP_PKEY_paramgen_init(pctx), 0) + || !TEST_int_gt(EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, + NID_X9_62_prime256v1), 0) + || !TEST_int_gt(EVP_PKEY_paramgen(pctx, ¶m), 0) + || !TEST_ptr(param)) + goto err; + + EVP_PKEY_CTX_free(pctx); + pctx = NULL; + + if (!TEST_ptr(ctx = EVP_PKEY_CTX_new_from_pkey(NULL, param, NULL)) + || !TEST_int_gt(EVP_PKEY_keygen_init(ctx), 0) + || !TEST_ptr_null(kctx = EVP_PKEY_CTX_dup(ctx))) + goto err; + ret = 1; +err: + EVP_PKEY_free(pkey); + EVP_PKEY_free(param); + EVP_PKEY_CTX_free(ctx); + EVP_PKEY_CTX_free(kctx); + EVP_PKEY_CTX_free(pctx); + return ret; +} + #endif /* OPENSSL_NO_EC */ #ifndef OPENSSL_NO_DSA @@ -1343,6 +1407,8 @@ int setup_tests(void) #ifndef OPENSSL_NO_EC ADD_ALL_TESTS(test_fromdata_ecx, 4 * 3); ADD_TEST(test_fromdata_ec); + ADD_TEST(test_ec_dup_no_operation); + ADD_TEST(test_ec_dup_keygen_operation); #endif return 1; } -- 2.47.2