Several error paths in the engine DH/EC and provider DH/ECDH/SM2/X25519 code returned the wrong code or an uninitialized value: the DH soft generate/compute key check treated 0 as success, set_sess_setup_cv fell through with an uninitialized ret on a NULL generator/order, ecdh_init_req/ecdh_compkey_init_iot returned -ENOMEM and ignored affine/BN_bn2binpad failures, and several provider alloc-sess branches returned UADK_P_FAIL instead of UADK_DO_SOFT. The x25519 keygen path also returned NULL directly instead of falling back to software. Also add NULL checks for BN_bin2bn in sm2_set_key_to_ec_key. Signed-off-by: Weili Qian <qianweili@huawei.com> --- src/uadk_dh.c | 4 ++-- src/uadk_ec.c | 42 +++++++++++++++++++++++++++++++++------ src/uadk_prov_dh.c | 2 +- src/uadk_prov_ecdh_exch.c | 23 +++++++++++++++++---- src/uadk_prov_ecx.c | 10 +++++----- src/uadk_prov_pkey.c | 8 ++++++-- src/uadk_prov_sm2_enc.c | 2 +- src/uadk_prov_sm2_kmgmt.c | 9 +++++++++ src/uadk_prov_sm2_sign.c | 4 ++-- 9 files changed, 81 insertions(+), 23 deletions(-) diff --git a/src/uadk_dh.c b/src/uadk_dh.c index ce00953..fbb6dc9 100644 --- a/src/uadk_dh.c +++ b/src/uadk_dh.c @@ -108,7 +108,7 @@ static int uadk_e_dh_soft_generate_key(DH *dh) } ret = dh_soft_generate_key(dh); - if (ret < 0) { + if (ret <= 0) { fprintf(stderr, "failed to do dh soft generate key\n"); return UADK_E_FAIL; } @@ -137,7 +137,7 @@ static int uadk_e_dh_soft_compute_key(unsigned char *key, } ret = dh_soft_compute_key(key, pub_key, dh); - if (ret < 0) { + if (ret <= 0) { fprintf(stderr, "failed to do dh soft compute key\n"); return UADK_E_FAIL; } diff --git a/src/uadk_ec.c b/src/uadk_ec.c index 1965461..c7d50ac 100644 --- a/src/uadk_ec.c +++ b/src/uadk_ec.c @@ -151,16 +151,20 @@ static int set_sess_setup_cv(const EC_GROUP *group, goto free_cv; cv_param->g = EC_GROUP_get0_generator(group); - if (!cv_param->g) + if (!cv_param->g) { + ret = -1; goto free_cv; + } ret = uadk_get_affine_coordinates(group, cv_param->g, g_x, g_y, ctx); if (ret) goto free_cv; cv_param->order = EC_GROUP_get0_order(group); - if (!cv_param->order) + if (!cv_param->order) { + ret = -1; goto free_cv; + } fill_ecc_cv_param(pparam, cv_param, g_x, g_y); cv->type = WD_CV_CFG_PARAM; @@ -791,6 +795,10 @@ static int sm2_set_key_to_ec_key(EC_KEY *ec, struct wd_ecc_req *req) } tmp = BN_bin2bn((unsigned char *)privkey->data, privkey->dsize, NULL); + if (!tmp) { + fprintf(stderr, "failed to BN_bin2bn privkey\n"); + return -EINVAL; + } ret = EC_KEY_set_private_key(ec, tmp); BN_free(tmp); if (!ret) { @@ -812,6 +820,12 @@ static int sm2_set_key_to_ec_key(EC_KEY *ec, struct wd_ecc_req *req) memcpy(buff + x_offset, pubkey->x.data, pubkey->x.dsize); memcpy(buff + y_offset, pubkey->y.data, pubkey->y.dsize); tmp = BN_bin2bn(buff, ECC_POINT_SIZE(SM2_KEY_BYTES) + 1, NULL); + if (!tmp) { + fprintf(stderr, "failed to BN_bin2bn pubkey\n"); + EC_POINT_free(point); + return -EINVAL; + } + ptr = EC_POINT_bn2point(group, tmp, point, NULL); BN_free(tmp); if (!ptr) { @@ -1034,13 +1048,14 @@ static int ecdh_compkey_init_iot(handle_t sess, struct wd_ecc_req *req, struct wd_ecc_in *ecdh_in; BIGNUM *pkey_x, *pkey_y; const EC_GROUP *group; + int xlen, ylen; size_t ec_size; BN_CTX *ctx; int ret = 0; ctx = BN_CTX_new(); if (!ctx) - return -ENOMEM; + return 0; BN_CTX_start(ctx); pkey_x = BN_CTX_get(ctx); @@ -1056,16 +1071,30 @@ static int ecdh_compkey_init_iot(handle_t sess, struct wd_ecc_req *req, goto free_ctx; ec_size = ecdh_get_ec_size(group); - uadk_get_affine_coordinates(group, pubkey, pkey_x, pkey_y, ctx); + ret = uadk_get_affine_coordinates(group, pubkey, pkey_x, pkey_y, ctx); + if (ret) { + ret = 0; + goto free_ctx; + } + in_pkey.x.data = buf_x; in_pkey.y.data = buf_y; - in_pkey.x.dsize = BN_bn2binpad(pkey_x, (unsigned char *)in_pkey.x.data, ec_size); - in_pkey.y.dsize = BN_bn2binpad(pkey_y, (unsigned char *)in_pkey.y.data, ec_size); + xlen = BN_bn2binpad(pkey_x, (unsigned char *)in_pkey.x.data, ec_size); + ylen = BN_bn2binpad(pkey_y, (unsigned char *)in_pkey.y.data, ec_size); + if (xlen < 0 || ylen < 0) { + fprintf(stderr, "failed to BN_bn2binpad, xlen = %d, ylen = %d\n", + xlen, ylen); + ret = 0; + goto free_ctx; + } + in_pkey.x.dsize = xlen; + in_pkey.y.dsize = ylen; /* Set public key */ ecdh_in = wd_ecxdh_new_in(sess, &in_pkey); if (!ecdh_in) { fprintf(stderr, "failed to new ecxdh in\n"); + ret = 0; goto free_ctx; } @@ -1073,6 +1102,7 @@ static int ecdh_compkey_init_iot(handle_t sess, struct wd_ecc_req *req, if (!ecdh_out) { fprintf(stderr, "failed to new ecxdh out\n"); wd_ecc_del_in(sess, ecdh_in); + ret = 0; goto free_ctx; } diff --git a/src/uadk_prov_dh.c b/src/uadk_prov_dh.c index cf31971..3f49a6f 100644 --- a/src/uadk_prov_dh.c +++ b/src/uadk_prov_dh.c @@ -782,7 +782,7 @@ static int uadk_prov_dh_prepare_data(const BIGNUM *g, DH *dh, struct uadk_dh_ses *dh_sess = uadk_prov_dh_new_session(dh, bits, is_g2); if (*dh_sess == NULL) { UADK_ERR("failed to get session\n"); - return UADK_P_FAIL; + return UADK_DO_SOFT; } ret = uadk_prov_dh_prepare_prikey(*dh_sess, dh, prikey); diff --git a/src/uadk_prov_ecdh_exch.c b/src/uadk_prov_ecdh_exch.c index f2d09fb..01473a8 100644 --- a/src/uadk_prov_ecdh_exch.c +++ b/src/uadk_prov_ecdh_exch.c @@ -216,12 +216,13 @@ static int ecdh_init_req(struct ecdh_sess_ctx *sess_ctx, struct wd_ecc_in *ecdh_in; BIGNUM *pkey_x, *pkey_y; int ret = UADK_P_FAIL; + int xlen, ylen; size_t ec_size; BN_CTX *ctx; ctx = BN_CTX_new(); if (!ctx) - return -ENOMEM; + return UADK_P_FAIL; BN_CTX_start(ctx); pkey_x = BN_CTX_get(ctx); @@ -233,16 +234,29 @@ static int ecdh_init_req(struct ecdh_sess_ctx *sess_ctx, goto free_ctx; ec_size = ecdh_get_ec_size(sess_ctx->group); - uadk_prov_get_affine_coordinates(sess_ctx->group, sess_ctx->pub_key, pkey_x, pkey_y, ctx); + ret = uadk_prov_get_affine_coordinates(sess_ctx->group, + sess_ctx->pub_key, pkey_x, pkey_y, ctx); + if (ret != UADK_P_SUCCESS) + goto free_ctx; + in_pkey.x.data = buf_x; in_pkey.y.data = buf_y; - in_pkey.x.dsize = BN_bn2binpad(pkey_x, (unsigned char *)in_pkey.x.data, ec_size); - in_pkey.y.dsize = BN_bn2binpad(pkey_y, (unsigned char *)in_pkey.y.data, ec_size); + xlen = BN_bn2binpad(pkey_x, (unsigned char *)in_pkey.x.data, ec_size); + ylen = BN_bn2binpad(pkey_y, (unsigned char *)in_pkey.y.data, ec_size); + if (xlen < 0 || ylen < 0) { + UADK_ERR("failed to BN_bn2binpad, xlen = %d, ylen = %d\n", + xlen, ylen); + ret = UADK_P_FAIL; + goto free_ctx; + } + in_pkey.x.dsize = xlen; + in_pkey.y.dsize = ylen; /* Set public key */ ecdh_in = wd_ecxdh_new_in(sess, &in_pkey); if (!ecdh_in) { UADK_ERR("failed to new ecxdh in\n"); + ret = UADK_P_FAIL; goto free_ctx; } @@ -250,6 +264,7 @@ static int ecdh_init_req(struct ecdh_sess_ctx *sess_ctx, if (!ecdh_out) { UADK_ERR("failed to new ecxdh out\n"); wd_ecc_del_in(sess, ecdh_in); + ret = UADK_P_FAIL; goto free_ctx; } diff --git a/src/uadk_prov_ecx.c b/src/uadk_prov_ecx.c index 69494cc..5550af8 100644 --- a/src/uadk_prov_ecx.c +++ b/src/uadk_prov_ecx.c @@ -888,7 +888,7 @@ static void *uadk_keymgmt_x448_gen(void *genctx, OSSL_CALLBACK *cb, void *cb_par gctx->sess = uadk_prov_ecx_alloc_sess(ECX_KEY_TYPE_X448); if (gctx->sess == (handle_t)0) { UADK_ERR("failed to alloc x448 sess\n"); - ret = UADK_P_FAIL; + ret = UADK_DO_SOFT; goto exe_soft; } @@ -1273,7 +1273,7 @@ static int uadk_keyexch_x448_derive(void *vecxctx, unsigned char *secret, size_t ecxctx->sess = uadk_prov_ecx_alloc_sess(ECX_KEY_TYPE_X448); if (ecxctx->sess == (handle_t)0) { UADK_ERR("failed to alloc sess\n"); - ret = UADK_P_FAIL; + ret = UADK_DO_SOFT; goto exe_soft; } @@ -1532,8 +1532,8 @@ static void *uadk_keymgmt_x25519_gen(void *genctx, OSSL_CALLBACK *cb, void *cb_p gctx->sess = uadk_prov_ecx_alloc_sess(ECX_KEY_TYPE_X25519); if (gctx->sess == (handle_t)0) { UADK_ERR("failed to alloc x25519 sess\n"); - ret = UADK_P_FAIL; - return NULL; + ret = UADK_DO_SOFT; + goto exe_soft; } ret = uadk_prov_ecx_keygen(gctx, &ecx_key); @@ -1675,7 +1675,7 @@ static int uadk_keyexch_x25519_derive(void *vecxctx, unsigned char *secret, size ecxctx->sess = uadk_prov_ecx_alloc_sess(ECX_KEY_TYPE_X25519); if (ecxctx->sess == (handle_t)0) { UADK_ERR("failed to alloc sess\n"); - ret = UADK_P_FAIL; + ret = UADK_DO_SOFT; goto exe_soft; } diff --git a/src/uadk_prov_pkey.c b/src/uadk_prov_pkey.c index 2e2ca0b..41d0102 100644 --- a/src/uadk_prov_pkey.c +++ b/src/uadk_prov_pkey.c @@ -271,16 +271,20 @@ static int uadk_prov_set_sess_setup_cv(const EC_GROUP *group, goto free_cv; cv_param->g = EC_GROUP_get0_generator(group); - if (cv_param->g == NULL) + if (cv_param->g == NULL) { + ret = UADK_P_FAIL; goto free_cv; + } ret = uadk_prov_get_affine_coordinates(group, cv_param->g, g_x, g_y, bn_ctx); if (ret == 0) goto free_cv; cv_param->order = EC_GROUP_get0_order(group); - if (cv_param->order == NULL) + if (cv_param->order == NULL) { + ret = UADK_P_FAIL; goto free_cv; + } uadk_prov_fill_ecc_cv_param(ecc_param, cv_param, g_x, g_y); cv->type = WD_CV_CFG_PARAM; diff --git a/src/uadk_prov_sm2_enc.c b/src/uadk_prov_sm2_enc.c index e987202..16e85ae 100644 --- a/src/uadk_prov_sm2_enc.c +++ b/src/uadk_prov_sm2_enc.c @@ -348,7 +348,7 @@ static int sm2_prov_alloc_sess(PROV_SM2_ASYM_CTX *vpsm2ctx, handle_t *sess) *sess = wd_ecc_alloc_sess(&setup); if (*sess == (handle_t)0) { UADK_ERR("failed to alloc sess\n"); - return UADK_P_FAIL; + return UADK_DO_SOFT; } return UADK_P_SUCCESS; diff --git a/src/uadk_prov_sm2_kmgmt.c b/src/uadk_prov_sm2_kmgmt.c index 1791cdd..8f36dfa 100644 --- a/src/uadk_prov_sm2_kmgmt.c +++ b/src/uadk_prov_sm2_kmgmt.c @@ -337,6 +337,10 @@ static int uadk_prov_sm2_set_key_to_ec_key(EC_KEY *ec, struct wd_ecc_req *req) } bn_key = BN_bin2bn((unsigned char *)privkey->data, privkey->dsize, NULL); + if (!bn_key) { + UADK_ERR("failed to BN_bin2bn privkey\n"); + return UADK_P_FAIL; + } ret = EC_KEY_set_private_key(ec, bn_key); BN_free(bn_key); if (ret == 0) { @@ -358,6 +362,11 @@ static int uadk_prov_sm2_set_key_to_ec_key(EC_KEY *ec, struct wd_ecc_req *req) memcpy(key_buff + x_offset, pubkey->x.data, pubkey->x.dsize); memcpy(key_buff + y_offset, pubkey->y.data, pubkey->y.dsize); bn_key = BN_bin2bn(key_buff, ECC_POINT_SIZE(SM2_KEY_BYTES) + 1, NULL); + if (!bn_key) { + UADK_ERR("failed to BN_bin2bn pubkey\n"); + EC_POINT_free(point); + return UADK_P_FAIL; + } ptr = EC_POINT_bn2point(group, bn_key, point, NULL); BN_free(bn_key); if (ptr == NULL) { diff --git a/src/uadk_prov_sm2_sign.c b/src/uadk_prov_sm2_sign.c index aa7049e..aa94875 100644 --- a/src/uadk_prov_sm2_sign.c +++ b/src/uadk_prov_sm2_sign.c @@ -538,7 +538,7 @@ static int sm2_sign_hw(PROV_SM2_SIGN_CTX *psm2ctx, sess = sm2_alloc_sess(psm2ctx->key); if (sess == (handle_t)0) { UADK_ERR("failed to alloc sess in sign\n"); - return UADK_P_FAIL; + return UADK_DO_SOFT; } ret = sm2_sign_init_iot(sess, &req, (void *)tbs, tbslen); @@ -687,7 +687,7 @@ static int sm2_verify_hw(PROV_SM2_SIGN_CTX *psm2ctx, sess = sm2_alloc_sess(psm2ctx->key); if (sess == (handle_t)0) { UADK_ERR("failed to alloc sess in verify\n"); - return UADK_P_FAIL; + return UADK_DO_SOFT; } ret = sm2_verify_init_iot(sess, &req, sig, siglen, tbs, tbslen); -- 2.53.0.windows.2