The EC private key in uadk_prov_ec_kmgmt.c uses BN_new() instead of BN_secure_new(), leaving sensitive material in non-secure memory. The EC gen_init error path calls OPENSSL_free() instead of the proper cleanup function, skipping propq and ikm cleanup. The ECX gen_cleanup functions delegate to the default provider cleanup via get_default_x25519/x448_keymgmt().gen_cleanup(), which does not handle our extended fields (propq, dhkem_ikm). Replace both with a local uadk_keymgmt_ecx_cleanup() that properly frees all fields. Also add a comment in uadk_ec.c explaining why pkey_meth->ec must not be freed in uadk_ec_delete_meth() to prevent a double-free. Signed-off-by: Weili Qian <qianweili@huawei.com> --- src/uadk_ec.c | 9 +++++++++ src/uadk_prov_ec_kmgmt.c | 4 ++-- src/uadk_prov_ecx.c | 25 +++++++++++++++---------- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/uadk_ec.c b/src/uadk_ec.c index d241e79..1114434 100644 --- a/src/uadk_ec.c +++ b/src/uadk_ec.c @@ -1464,6 +1464,15 @@ int uadk_ec_create_pmeth(struct uadk_pkey_meth *pkey_meth) if (pkey_meth->ec) return 1; + /* + * The meth is returned to OpenSSL via the pkey_meths callback + * (get_pkey_meths in uadk_pkey.c) and registered by ENGINE_set_pkey_meths. + * On ENGINE_free, engine_pkey_meths_free() (tb_pkmeth.c) iterates all + * registered nids, obtains the meth via the callback, and calls + * EVP_PKEY_meth_free() on it. Thus pkey_meth->ec must NOT be freed in + * uadk_ec_delete_meth() - doing so would cause a double-free because + * engine_pkey_meths_free() runs before the engine's destroy callback. + */ meth = EVP_PKEY_meth_new(EVP_PKEY_EC, 0); if (meth == NULL) { fprintf(stderr, "failed to EVP_PKEY_meth_new\n"); diff --git a/src/uadk_prov_ec_kmgmt.c b/src/uadk_prov_ec_kmgmt.c index 48c7b18..9896d09 100644 --- a/src/uadk_prov_ec_kmgmt.c +++ b/src/uadk_prov_ec_kmgmt.c @@ -202,7 +202,7 @@ static int ec_set_private_key(EC_KEY *ec, BIGNUM *priv_key) if (priv_k) goto set_key; - priv_k = BN_new(); + priv_k = BN_secure_new(); if (!priv_k) { UADK_ERR("failed to BN_new priv_k!\n"); return UADK_P_FAIL; @@ -450,7 +450,7 @@ static void *uadk_keymgmt_ec_gen_init(void *provctx, int selection, ret = uadk_keymgmt_ec_gen_set_params(gctx, params); if (!ret) { - OPENSSL_free(gctx); + uadk_keymgmt_ec_gen_cleanup(gctx); return NULL; } diff --git a/src/uadk_prov_ecx.c b/src/uadk_prov_ecx.c index 5550af8..1818219 100644 --- a/src/uadk_prov_ecx.c +++ b/src/uadk_prov_ecx.c @@ -446,6 +446,19 @@ static void uadk_prov_ecx_free_sess(handle_t sess) wd_ecc_free_sess(sess); } +static void uadk_keymgmt_ecx_cleanup(void *genctx) +{ + PROV_ECX_KEYMGMT_CTX *gctx = genctx; + + if (gctx) { + OPENSSL_free(gctx->propq); +# if OPENSSL_VERSION_NUMBER >= 0x30200000L + OPENSSL_clear_free(gctx->dhkem_ikm, gctx->dhkem_ikmlen); +# endif + OPENSSL_free(gctx); + } +} + static void *ossl_ecx_gen_init(void *provctx, int selection, const OSSL_PARAM params[], ECX_KEY_TYPE type) { @@ -475,11 +488,7 @@ static void *ossl_ecx_gen_init(void *provctx, int selection, const OSSL_PARAM pa static void uadk_keymgmt_x448_gen_cleanup(void *genctx) { - /* genctx will be freed in cleanup function */ - if (get_default_x448_keymgmt().gen_cleanup == NULL) - return; - - get_default_x448_keymgmt().gen_cleanup(genctx); + uadk_keymgmt_ecx_cleanup(genctx); } static void *uadk_keymgmt_x448_gen_init(void *provctx, int selection, @@ -1481,11 +1490,7 @@ static int uadk_keymgmt_x25519_get_params(void *key, OSSL_PARAM params[]) static void uadk_keymgmt_x25519_gen_cleanup(void *genctx) { - /* genctx will be freed in cleanup function */ - if (get_default_x25519_keymgmt().gen_cleanup == NULL) - return; - - get_default_x25519_keymgmt().gen_cleanup(genctx); + uadk_keymgmt_ecx_cleanup(genctx); } static void *uadk_keymgmt_x25519_gen_init(void *provctx, int selection, -- 2.43.0