From: tb <tb@openbsd.org>
Date: Sun, 2 Jul 2023 13:37:09 +0000 (+0000)
Subject: Switch sign_sig() and sign_setup() to using BN_CTX
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=6d4db1e3ad2c188bd584c2c731975dedfb4c6de3;p=openbsd

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
---

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