Bugfix about ecc alg of uadk_engine.
Zhiqi Song (5): ecc: bugfix about magic number ecc: bugfix about variable declaration scope ecc: bugfix about return value check ecc: cleanup unused structure ecc: bugfix multiple definition of ecx structure
src/uadk_ec.c | 181 +++++++++++++++++++++++++++++++----------------- src/uadk_ecx.c | 43 ++++++------ src/uadk_pkey.c | 9 --- src/uadk_sm2.c | 21 ++---- 4 files changed, 148 insertions(+), 106 deletions(-)
Use macros instead of specific numbers to represent the key size.
Signed-off-by: Zhiqi Song songzhiqi1@huawei.com --- src/uadk_ec.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/uadk_ec.c b/src/uadk_ec.c index db69871..53cce00 100644 --- a/src/uadk_ec.c +++ b/src/uadk_ec.c @@ -129,18 +129,18 @@ err: static int get_smallest_hw_keybits(int bits) { /* ec curve order width */ - if (bits > 384) - return 521; - else if (bits > 320) - return 384; - else if (bits > 256) - return 320; - else if (bits > 192) - return 256; - else if (bits > 128) - return 192; + if (bits > ECC384BITS) + return ECC521BITS; + else if (bits > ECC320BITS) + return ECC384BITS; + else if (bits > ECC256BITS) + return ECC320BITS; + else if (bits > ECC192BITS) + return ECC256BITS; + else if (bits > ECC128BITS) + return ECC192BITS; else - return 128; + return ECC128BITS; }
static handle_t ecc_alloc_sess(const EC_KEY *eckey, char *alg)
Global variable 'sm2_order' only used in function 'sm2_update_sess' should be declared in function scope.
Signed-off-by: Zhiqi Song songzhiqi1@huawei.com --- src/uadk_sm2.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/uadk_sm2.c b/src/uadk_sm2.c index 8c75611..75d2b11 100644 --- a/src/uadk_sm2.c +++ b/src/uadk_sm2.c @@ -115,12 +115,6 @@ typedef int (*PFUNC_DEC)(EVP_PKEY_CTX *ctx, const unsigned char *in, size_t inlen);
-const unsigned char sm2_order[] = { - 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,\ - 0xff, 0xff, 0xff, 0xff, 0x72, 0x03, 0xdf, 0x6b, 0x21, 0xc6, 0x05, 0x2b,\ - 0x53, 0xbb, 0xf4, 0x09, 0x39, 0xd5, 0x41, 0x23 -}; - static int get_hash_type(int nid_hash) { switch (nid_hash) { @@ -166,6 +160,12 @@ static int compute_hash(const char *in, size_t in_len,
static int sm2_update_sess(struct sm2_ctx *smctx) { + const unsigned char sm2_order[] = { + 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0x72, 0x03, 0xdf, 0x6b, 0x21, 0xc6, 0x05, 0x2b, + 0x53, 0xbb, 0xf4, 0x09, 0x39, 0xd5, 0x41, 0x23 + }; int nid_hash = smctx->ctx.md ? EVP_MD_type(smctx->ctx.md) : NID_sm3; struct wd_ecc_sess_setup setup; handle_t sess;
Check the return value of BN_CTX_get(), and add a new structure to wrap the curve parameters to make the logic of return value check clearer.
Signed-off-by: Zhiqi Song songzhiqi1@huawei.com --- src/uadk_ec.c | 159 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 108 insertions(+), 51 deletions(-)
diff --git a/src/uadk_ec.c b/src/uadk_ec.c index 53cce00..2ed5755 100644 --- a/src/uadk_ec.c +++ b/src/uadk_ec.c @@ -34,6 +34,19 @@ #define ECC384BITS 384 #define ECC521BITS 521
+struct curve_param { + /* prime */ + BIGNUM *p; + /* ecc coefficient 'a' */ + BIGNUM *a; + /* ecc coefficient 'b' */ + BIGNUM *b; + /* base point */ + const EC_POINT *g; + /* order of base point */ + const BIGNUM *order; +}; + typedef ECDSA_SIG* (*PFUNC_SIGN_SIG)(const unsigned char *, int, const BIGNUM *, @@ -70,58 +83,92 @@ static void init_dtb_param(void *dtb, char *start, } }
+static void fill_ecc_cv_param(struct wd_ecc_curve *pparam, + struct curve_param *cv_param, + BIGNUM *g_x, BIGNUM *g_y) +{ + pparam->p.dsize = BN_bn2bin(cv_param->p, (void *)pparam->p.data); + pparam->a.dsize = BN_bn2bin(cv_param->a, (void *)pparam->a.data); + if (!pparam->a.dsize) { + pparam->a.dsize = 1; + pparam->a.data[0] = 0; + } + + pparam->b.dsize = BN_bn2bin(cv_param->b, (void *)pparam->b.data); + if (!pparam->b.dsize) { + pparam->b.dsize = 1; + pparam->b.data[0] = 0; + } + + pparam->g.x.dsize = BN_bn2bin(g_x, (void *)pparam->g.x.data); + pparam->g.y.dsize = BN_bn2bin(g_y, (void *)pparam->g.y.data); + pparam->n.dsize = BN_bn2bin(cv_param->order, (void *)pparam->n.data); +} + static int set_sess_setup_cv(const EC_GROUP *group, struct wd_ecc_curve_cfg *cv) { struct wd_ecc_curve *pparam = cv->cfg.pparam; - BIGNUM *p, *a, *b, *xg, *yg, *order; - const EC_POINT *g; + struct curve_param *cv_param; + BIGNUM *g_x, *g_y; + int ret = -1; BN_CTX *ctx; - int ret;
ctx = BN_CTX_new(); if (!ctx) - return -ENOMEM; + return ret;
BN_CTX_start(ctx); - p = BN_CTX_get(ctx); - a = BN_CTX_get(ctx); - b = BN_CTX_get(ctx); - xg = BN_CTX_get(ctx); - yg = BN_CTX_get(ctx);
- ret = uadk_get_curve(group, p, a, b, ctx); + cv_param = OPENSSL_malloc(sizeof(struct curve_param)); + if (!cv_param) + goto free_ctx; + + cv_param->p = BN_CTX_get(ctx); + if (!cv_param->p) + goto free_cv; + + cv_param->a = BN_CTX_get(ctx); + if (!cv_param->a) + goto free_cv; + + cv_param->b = BN_CTX_get(ctx); + if (!cv_param->b) + goto free_cv; + + g_x = BN_CTX_get(ctx); + if (!g_x) + goto free_cv; + + g_y = BN_CTX_get(ctx); + if (!g_y) + goto free_cv; + + ret = uadk_get_curve(group, cv_param->p, cv_param->a, cv_param->b, ctx); if (ret) - goto err; + goto free_cv;
- g = EC_GROUP_get0_generator(group); - ret = uadk_get_affine_coordinates(group, g, xg, yg, ctx); + cv_param->g = EC_GROUP_get0_generator(group); + if (!cv_param->g) + goto free_cv; + + ret = uadk_get_affine_coordinates(group, cv_param->g, g_x, g_y, ctx); if (ret) - goto err; + goto free_cv;
- order = (BIGNUM *)EC_GROUP_get0_order(group); - pparam->p.dsize = BN_bn2bin(p, (void *)pparam->p.data); - pparam->a.dsize = BN_bn2bin(a, (void *)pparam->a.data); - /* a or b is all zero, but uadk not allow parameter length is zero */ - if (!pparam->a.dsize) { - pparam->a.dsize = 1; - pparam->a.data[0] = 0; - } - pparam->b.dsize = BN_bn2bin(b, (void *)pparam->b.data); - if (!pparam->b.dsize) { - pparam->b.dsize = 1; - pparam->b.data[0] = 0; - } - pparam->g.x.dsize = BN_bn2bin(xg, (void *)pparam->g.x.data); - pparam->g.y.dsize = BN_bn2bin(yg, (void *)pparam->g.y.data); - pparam->n.dsize = BN_bn2bin(order, (void *)pparam->n.data); + cv_param->order = EC_GROUP_get0_order(group); + if (!cv_param->order) + goto free_cv; + + fill_ecc_cv_param(pparam, cv_param, g_x, g_y); cv->type = WD_CV_CFG_PARAM; ret = 0; -err: - if (ctx) { - BN_CTX_end(ctx); - BN_CTX_free(ctx); - } + +free_cv: + OPENSSL_free(cv_param); +free_ctx: + BN_CTX_end(ctx); + BN_CTX_free(ctx);
return ret; } @@ -166,12 +213,13 @@ static handle_t ecc_alloc_sess(const EC_KEY *eckey, char *alg) sp.cv.cfg.pparam = ¶m; group = EC_KEY_get0_group(eckey); ret = set_sess_setup_cv(group, &sp.cv); - if (ret) { - free(dev); - return (handle_t)0; - } + if (ret) + goto free_dev;
order = EC_GROUP_get0_order(group); + if (!order) + goto free_dev; + key_bits = BN_num_bits(order); sp.alg = alg; sp.key_bits = get_smallest_hw_keybits(key_bits); @@ -184,8 +232,11 @@ static handle_t ecc_alloc_sess(const EC_KEY *eckey, char *alg) fprintf(stderr, "failed to alloc ecc sess\n");
free(dev); - return sess; + +free_dev: + free(dev); + return (handle_t)0; }
static int check_ecc_bit_useful(const int bits) @@ -506,6 +557,10 @@ static int ecdsa_do_verify_check(EC_KEY *eckey, const BIGNUM *order; int ret;
+ ret = eckey_check(eckey); + if (ret) + return ret; + if (!dgst) { fprintf(stderr, "dgst is NULL\n"); return -1; @@ -516,10 +571,6 @@ static int ecdsa_do_verify_check(EC_KEY *eckey, return -1; }
- ret = eckey_check(eckey); - if (ret) - return ret; - pub_key = EC_KEY_get0_public_key(eckey); if (!pub_key) { fprintf(stderr, "pub_key is NULL\n"); @@ -957,10 +1008,20 @@ static int ecdh_compkey_init_iot(handle_t sess, struct wd_ecc_req *req, ctx = BN_CTX_new(); if (!ctx) return -ENOMEM; + + BN_CTX_start(ctx); pkey_x = BN_CTX_get(ctx); + if (!pkey_x) + goto free_ctx; + pkey_y = BN_CTX_get(ctx); + if (!pkey_y) + goto free_ctx;
group = EC_KEY_get0_group(ecdh); + if (!group) + goto free_ctx; + uadk_get_affine_coordinates(group, pubkey, pkey_x, pkey_y, ctx); in_pkey.x.data = buf_x; in_pkey.y.data = buf_y; @@ -986,13 +1047,9 @@ static int ecdh_compkey_init_iot(handle_t sess, struct wd_ecc_req *req, ret = 1;
free_ctx: - if (ctx) { - if (pkey_x) - BN_clear(pkey_x); - if (pkey_y) - BN_clear(pkey_y); - BN_CTX_free(ctx); - } + BN_CTX_end(ctx); + BN_CTX_free(ctx); + return ret; }
Remove unused structure 'uadk_ecc_sess'.
Signed-off-by: Zhiqi Song songzhiqi1@huawei.com --- src/uadk_pkey.c | 9 --------- src/uadk_sm2.c | 9 --------- 2 files changed, 18 deletions(-)
diff --git a/src/uadk_pkey.c b/src/uadk_pkey.c index 019d4fd..9dfffac 100644 --- a/src/uadk_pkey.c +++ b/src/uadk_pkey.c @@ -44,15 +44,6 @@ struct ecc_res_config { int numa_id; };
-typedef struct uadk_ecc_sess { - handle_t sess; - struct wd_ecc_sess_setup setup; - struct wd_ecc_req req; - int is_pubkey_ready; - int is_privkey_ready; - int key_size; -} uadk_ecc_sess_t; - /* ecc global hardware resource is saved here */ struct ecc_res { struct wd_ctx_config *ctx_res; diff --git a/src/uadk_sm2.c b/src/uadk_sm2.c index 75d2b11..1ef6032 100644 --- a/src/uadk_sm2.c +++ b/src/uadk_sm2.c @@ -46,15 +46,6 @@ struct sm2_ctx { bool is_init; };
-typedef struct uadk_ecc_sess { - handle_t sess; - struct wd_ecc_sess_setup setup; - struct wd_ecc_req req; - int is_pubkey_ready; - int is_privkey_ready; - int key_size; -} uadk_ecc_sess_t; - typedef struct sm2_ciphertext { BIGNUM *C1x; BIGNUM *C1y;
The structure 'ECX_KEY' is defined in the libcrypto of OpenSSL, but OpenSSL does not put this definition in the header file in its 1.1.1x release version, so we can not use this structure directly. We should define a new structure that provides the same function to avoid conflict with the definition in OpenSSL when using static compilation.
Signed-off-by: Zhiqi Song songzhiqi1@huawei.com --- src/uadk_ecx.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/src/uadk_ecx.c b/src/uadk_ecx.c index 67f9350..73c41b9 100644 --- a/src/uadk_ecx.c +++ b/src/uadk_ecx.c @@ -31,14 +31,14 @@ #define X448_KEYLEN 56 #define X25519_KEYBITS 256 #define X448_KEYBITS 448 -#define MAX_KEYLEN 57 +#define ECX_MAX_KEYLEN 57 #define UADK_E_SUCCESS 1 #define UADK_E_FAIL 0
-typedef struct { - unsigned char pubkey[MAX_KEYLEN]; +struct ecx_key { + unsigned char pubkey[ECX_MAX_KEYLEN]; unsigned char *privkey; -} ECX_KEY; +};
struct ecx_ctx { handle_t sess; @@ -224,12 +224,12 @@ static int ecx_get_nid(EVP_PKEY_CTX *ctx) return nid; }
-static int ecx_create_privkey(ECX_KEY **ecx_key, int key_size) +static int ecx_create_privkey(struct ecx_key **ecx_key, int key_size) { unsigned char *privkey; int ret;
- *ecx_key = OPENSSL_zalloc(sizeof(ECX_KEY)); + *ecx_key = OPENSSL_zalloc(sizeof(struct ecx_key)); if (!(*ecx_key)) { fprintf(stderr, "failed to alloc ecx_key\n"); return UADK_E_FAIL; @@ -259,7 +259,8 @@ free_ecx_key: return UADK_E_FAIL; }
-static int ecx_keygen_set_private_key(struct ecx_ctx *ecx_ctx, ECX_KEY *ecx_key) +static int ecx_keygen_set_private_key(struct ecx_ctx *ecx_ctx, + struct ecx_key *ecx_key) { handle_t sess = ecx_ctx->sess; struct wd_ecc_key *ecc_key; @@ -280,14 +281,14 @@ static int ecx_keygen_set_private_key(struct ecx_ctx *ecx_ctx, ECX_KEY *ecx_key) }
static int ecx_keygen_set_pkey(EVP_PKEY *pkey, struct ecx_ctx *ecx_ctx, - struct wd_ecc_req *req, ECX_KEY *ecx_key) + struct wd_ecc_req *req, struct ecx_key *ecx_key) { struct wd_ecc_point *pubkey = NULL; int key_size = ecx_ctx->key_size; int ret;
wd_ecxdh_get_out_params(req->dst, &pubkey); - if (key_size > MAX_KEYLEN) { + if (key_size > ECX_MAX_KEYLEN) { fprintf(stderr, "invalid key size, key_size = %d\n", key_size); return UADK_E_FAIL; } @@ -368,8 +369,8 @@ static int openssl_do_ecx_genkey(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) static int x25519_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) { struct ecx_ctx *keygen_ctx = NULL; + struct ecx_key *ecx_key = NULL; struct wd_ecc_req req = {0}; - ECX_KEY *ecx_key = NULL; int ret;
ret = ecx_genkey_check(ctx, pkey); @@ -426,8 +427,8 @@ do_soft: static int x448_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) { struct ecx_ctx *keygen_ctx = NULL; + struct ecx_key *ecx_key = NULL; struct wd_ecc_req req = {0}; - ECX_KEY *ecx_key = NULL; int ret;
ret = ecx_genkey_check(ctx, pkey); @@ -482,12 +483,13 @@ do_soft: }
static int ecx_compkey_init_iot(struct ecx_ctx *ecx_ctx, struct wd_ecc_req *req, - ECX_KEY *peer_ecx_key, ECX_KEY *ecx_key) + struct ecx_key *peer_ecx_key, + struct ecx_key *ecx_key) { int key_size = ecx_ctx->key_size; + char buf_y[ECX_MAX_KEYLEN] = {0}; handle_t sess = ecx_ctx->sess; struct wd_ecc_point in_pubkey; - char buf_y[MAX_KEYLEN] = {0}; struct wd_ecc_out *ecx_out; struct wd_ecc_in *ecx_in; int ret; @@ -542,7 +544,8 @@ static void ecx_compkey_uninit_iot(handle_t sess, struct wd_ecc_req *req) wd_ecc_del_in(sess, req->src); }
-static int ecx_derive_set_private_key(struct ecx_ctx *ecx_ctx, ECX_KEY *ecx_key) +static int ecx_derive_set_private_key(struct ecx_ctx *ecx_ctx, + struct ecx_key *ecx_key) { int key_size = ecx_ctx->key_size; handle_t sess = ecx_ctx->sess; @@ -576,8 +579,8 @@ static int ecx_derive_set_private_key(struct ecx_ctx *ecx_ctx, ECX_KEY *ecx_key) return UADK_E_SUCCESS; }
-static int ecx_get_key(EVP_PKEY_CTX *ctx, ECX_KEY **ecx_key, - ECX_KEY **peer_ecx_key) +static int ecx_get_key(EVP_PKEY_CTX *ctx, struct ecx_key **ecx_key, + struct ecx_key **peer_ecx_key) { EVP_PKEY *pkey, *peer_key;
@@ -623,11 +626,11 @@ static void x25519_pad_out_key(unsigned char *dst_key, unsigned char *src_key, static int x25519_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen) { + struct ecx_key *peer_ecx_key = NULL; struct wd_ecc_point *s_key = NULL; struct ecx_ctx *derive_ctx = NULL; - ECX_KEY *peer_ecx_key = NULL; + struct ecx_key *ecx_key = NULL; struct wd_ecc_req req = {0}; - ECX_KEY *ecx_key = NULL; int ret;
ret = x25519_init(ctx); @@ -709,11 +712,11 @@ static void x448_pad_out_key(unsigned char *dst_key, unsigned char *src_key, static int x448_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen) { + struct ecx_key *peer_ecx_key = NULL; struct wd_ecc_point *s_key = NULL; struct ecx_ctx *derive_ctx = NULL; - ECX_KEY *peer_ecx_key = NULL; + struct ecx_key *ecx_key = NULL; struct wd_ecc_req req = {0}; - ECX_KEY *ecx_key = NULL; int ret;
ret = x448_init(ctx);
-
Zhiqi Song