From 6d4db1e3ad2c188bd584c2c731975dedfb4c6de3 Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 2 Jul 2023 13:37:09 +0000 Subject: [PATCH] Switch sign_sig() and sign_setup() to using BN_CTX Both these functions use a BN_CTX internally to deal with the EC API that usually requires one. However, they don't actually make use of it. Get the BIGNUMs from the BN_CTX instead, which simplifies the cleanup. Also defer allocation of the ECDSA_SIG to the very end. Instead of using its internal r and s, use two local r and s variables and transfer those to the ECDSA_SIG on success. ok beck jsing --- lib/libcrypto/ecdsa/ecs_ossl.c | 120 ++++++++++++++++++++------------- 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/lib/libcrypto/ecdsa/ecs_ossl.c b/lib/libcrypto/ecdsa/ecs_ossl.c index 728c07d8bbe..adbabb609be 100644 --- a/lib/libcrypto/ecdsa/ecs_ossl.c +++ b/lib/libcrypto/ecdsa/ecs_ossl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ecs_ossl.c,v 1.44 2023/07/02 13:26:36 tb Exp $ */ +/* $OpenBSD: ecs_ossl.c,v 1.45 2023/07/02 13:37:09 tb Exp $ */ /* * Written by Nils Larsch for the OpenSSL project */ @@ -124,11 +124,13 @@ int ossl_ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *in_ctx, BIGNUM **out_kinv, BIGNUM **out_r) { - BN_CTX *ctx = in_ctx; - BIGNUM *k = NULL, *r = NULL, *order = NULL, *x = NULL; - EC_POINT *point = NULL; const EC_GROUP *group; - int order_bits, ret = 0; + EC_POINT *point = NULL; + BN_CTX *ctx = NULL; + BIGNUM *k = NULL, *r = NULL; + BIGNUM *order, *x; + int order_bits; + int ret = 0; BN_free(*out_kinv); *out_kinv = NULL; @@ -138,21 +140,28 @@ ossl_ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *in_ctx, BIGNUM **out_kinv, if (eckey == NULL || (group = EC_KEY_get0_group(eckey)) == NULL) { ECDSAerror(ERR_R_PASSED_NULL_PARAMETER); - return 0; + goto err; } - if (ctx == NULL) { - if ((ctx = BN_CTX_new()) == NULL) { - ECDSAerror(ERR_R_MALLOC_FAILURE); - return 0; - } - } + if ((k = BN_new()) == NULL) + goto err; + if ((r = BN_new()) == NULL) + goto err; - if ((k = BN_new()) == NULL || (r = BN_new()) == NULL || - (order = BN_new()) == NULL || (x = BN_new()) == NULL) { + if ((ctx = in_ctx) == NULL) + ctx = BN_CTX_new(); + if (ctx == NULL) { ECDSAerror(ERR_R_MALLOC_FAILURE); goto err; } + + BN_CTX_start(ctx); + + if ((order = BN_CTX_get(ctx)) == NULL) + goto err; + if ((x = BN_CTX_get(ctx)) == NULL) + goto err; + if ((point = EC_POINT_new(group)) == NULL) { ECDSAerror(ERR_R_EC_LIB); goto err; @@ -236,14 +245,14 @@ ossl_ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *in_ctx, BIGNUM **out_kinv, ret = 1; err: - if (in_ctx == NULL) + BN_CTX_end(ctx); + if (ctx != in_ctx) BN_CTX_free(ctx); - BN_free(order); BN_free(k); BN_free(r); EC_POINT_free(point); - BN_free(x); - return (ret); + + return ret; } /* @@ -257,37 +266,51 @@ ECDSA_SIG * ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len, const BIGNUM *in_kinv, const BIGNUM *in_r, EC_KEY *eckey) { - BIGNUM *b = NULL, *binv = NULL, *bm = NULL, *bxr = NULL; - BIGNUM *kinv = NULL, *m = NULL, *order = NULL, *s; - const BIGNUM *ckinv, *priv_key; - BN_CTX *ctx = NULL; const EC_GROUP *group; - ECDSA_SIG *ret; + BN_CTX *ctx = NULL; + BIGNUM *kinv = NULL, *r = NULL, *s = NULL; + BIGNUM *b, *binv, *bm, *bxr, *m, *order; + const BIGNUM *ckinv, *priv_key; int attempts = 0; - int ok = 0; + ECDSA_SIG *sig = NULL; group = EC_KEY_get0_group(eckey); priv_key = EC_KEY_get0_private_key(eckey); if (group == NULL || priv_key == NULL) { ECDSAerror(ERR_R_PASSED_NULL_PARAMETER); - return NULL; + goto err; } - if ((ret = ECDSA_SIG_new()) == NULL) { + if ((r = BN_new()) == NULL) { ECDSAerror(ERR_R_MALLOC_FAILURE); - return NULL; + goto err; + } + if ((s = BN_new()) == NULL) { + ECDSAerror(ERR_R_MALLOC_FAILURE); + goto err; } - s = ret->s; - if ((ctx = BN_CTX_new()) == NULL || (order = BN_new()) == NULL || - (b = BN_new()) == NULL || - (binv = BN_new()) == NULL || (bm = BN_new()) == NULL || - (bxr = BN_new()) == NULL || (m = BN_new()) == NULL) { + if ((ctx = BN_CTX_new()) == NULL) { ECDSAerror(ERR_R_MALLOC_FAILURE); goto err; } + BN_CTX_start(ctx); + + if ((order = BN_CTX_get(ctx)) == NULL) + goto err; + if ((b = BN_CTX_get(ctx)) == NULL) + goto err; + if ((binv = BN_CTX_get(ctx)) == NULL) + goto err; + if ((bm = BN_CTX_get(ctx)) == NULL) + goto err; + if ((bxr = BN_CTX_get(ctx)) == NULL) + goto err; + if ((m = BN_CTX_get(ctx)) == NULL) + goto err; + if (!EC_GROUP_get_order(group, order, ctx)) { ECDSAerror(ERR_R_EC_LIB); goto err; @@ -298,14 +321,14 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len, do { if (in_kinv == NULL || in_r == NULL) { - if (!ECDSA_sign_setup(eckey, ctx, &kinv, &ret->r)) { + if (!ECDSA_sign_setup(eckey, ctx, &kinv, &r)) { ECDSAerror(ERR_R_ECDSA_LIB); goto err; } ckinv = kinv; } else { ckinv = in_kinv; - if (!bn_copy(ret->r, in_r)) { + if (!bn_copy(r, in_r)) { ECDSAerror(ERR_R_MALLOC_FAILURE); goto err; } @@ -338,7 +361,7 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len, ECDSAerror(ERR_R_BN_LIB); goto err; } - if (!BN_mod_mul(bxr, bxr, ret->r, order, ctx)) { /* bxr */ + if (!BN_mod_mul(bxr, bxr, r, order, ctx)) { /* bxr */ ECDSAerror(ERR_R_BN_LIB); goto err; } @@ -378,22 +401,25 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len, break; } while (1); - ok = 1; + if ((sig = ECDSA_SIG_new()) == NULL) { + ECDSAerror(ERR_R_MALLOC_FAILURE); + goto err; + } + if (!ECDSA_SIG_set0(sig, r, s)) { + ECDSA_SIG_free(sig); + goto err; + } + r = NULL; + s = NULL; err: - if (ok == 0) { - ECDSA_SIG_free(ret); - ret = NULL; - } + BN_CTX_end(ctx); BN_CTX_free(ctx); - BN_free(b); - BN_free(binv); - BN_free(bm); - BN_free(bxr); BN_free(kinv); - BN_free(m); - BN_free(order); - return ret; + BN_free(r); + BN_free(s); + + return sig; } int -- 2.20.1