From 0df1a5ef3fb1ec6a932b5ebc0a4f2a25ff2b10eb Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Mon, 3 Jul 2023 13:53:54 +0000
Subject: [PATCH] Rework the logic in ECDSA sign_sig()

If the caller supplied both kinv and r, we don't loop but rather throw
an undocumented error code that no one uses, which is intended to tell
the caller to run ECDSA_sign_setup() and try again.

Use a boolean that indicates this situation so that the logic becomes
a bit more transparent.

ok jsing
---
 lib/libcrypto/ecdsa/ecs_ossl.c | 54 +++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/lib/libcrypto/ecdsa/ecs_ossl.c b/lib/libcrypto/ecdsa/ecs_ossl.c
index 1dce05c35f9..d935d237bab 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.59 2023/07/03 11:10:28 tb Exp $ */
+/* $OpenBSD: ecs_ossl.c,v 1.60 2023/07/03 13:53:54 tb Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project
  */
@@ -275,6 +275,7 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
 	BIGNUM *kinv = NULL, *r = NULL, *s = NULL;
 	BIGNUM *b, *binv, *bm, *bxr, *m;
 	const BIGNUM *ckinv, *order, *priv_key;
+	int caller_supplied_values = 0;
 	int attempts = 0;
 	ECDSA_SIG *sig = NULL;
 
@@ -322,19 +323,28 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
 	if (!ecdsa_prepare_digest(dgst, dgst_len, order, m))
 		goto err;
 
+	if (in_kinv != NULL && in_r != NULL) {
+		/*
+		 * Use the caller's kinv and r. Don't call ECDSA_sign_setup().
+		 * If we're unable to compute a valid signature, the caller
+		 * must provide new values.
+		 */
+		caller_supplied_values = 1;
+
+		ckinv = in_kinv;
+		if (!bn_copy(r, in_r)) {
+			ECDSAerror(ERR_R_MALLOC_FAILURE);
+			goto err;
+		}
+	}
+
 	do {
-		if (in_kinv == NULL || in_r == NULL) {
+		if (!caller_supplied_values) {
 			if (!ECDSA_sign_setup(eckey, ctx, &kinv, &r)) {
 				ECDSAerror(ERR_R_ECDSA_LIB);
 				goto err;
 			}
 			ckinv = kinv;
-		} else {
-			ckinv = in_kinv;
-			if (!bn_copy(r, in_r)) {
-				ECDSAerror(ERR_R_MALLOC_FAILURE);
-				goto err;
-			}
 		}
 
 		/*
@@ -385,23 +395,19 @@ ossl_ecdsa_sign_sig(const unsigned char *dgst, int dgst_len,
 			goto err;
 		}
 
-		if (BN_is_zero(s)) {
-			/*
-			 * If kinv and r have been supplied by the caller,
-			 * don't generate new kinv and r values
-			 */
-			if (in_kinv != NULL && in_r != NULL) {
-				ECDSAerror(ECDSA_R_NEED_NEW_SETUP_VALUES);
-				goto err;
-			}
-
-			if (++attempts > ECDSA_MAX_SIGN_ITERATIONS) {
-				ECDSAerror(EC_R_WRONG_CURVE_PARAMETERS);
-				goto err;
-			}
-		} else
-			/* s != 0 => we have a valid signature */
+		/* If s is non-zero, we have a valid signature. */
+		if (!BN_is_zero(s))
 			break;
+
+		if (caller_supplied_values) {
+			ECDSAerror(ECDSA_R_NEED_NEW_SETUP_VALUES);
+			goto err;
+		}
+
+		if (++attempts > ECDSA_MAX_SIGN_ITERATIONS) {
+			ECDSAerror(EC_R_WRONG_CURVE_PARAMETERS);
+			goto err;
+		}
 	} while (1);
 
 	if ((sig = ECDSA_SIG_new()) == NULL) {
-- 
2.20.1