From: tb <tb@openbsd.org>
Date: Sat, 4 Mar 2023 21:37:37 +0000 (+0000)
Subject: Cap the number of iterations in ECDSA signing
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0e40a4a32e25cab9ecc20364f76d0fbbeef213ce;p=openbsd

Cap the number of iterations in ECDSA signing

ECDSA is essentially the same thing as DSA, except that it is slightly
less stupid. Signing specifies an infinite loop, which is only possible
with arbitrary ECDSA domain parameters. Fortunately, most use of ECDSA
in the wild is based on well-known groups, so it is known a priori that
the loop is not infinite. Still, infinite loops are bad. A retry is
unlikely, 32 retries have a probability of ~2^-8000. So it's pretty
safe to error out.

ok beck jsing
---

diff --git a/lib/libcrypto/ecdsa/ecs_ossl.c b/lib/libcrypto/ecdsa/ecs_ossl.c
index eec757ef7c5..6f45e173b8f 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.26 2022/11/26 16:08:52 tb Exp $ */
+/* $OpenBSD: ecs_ossl.c,v 1.27 2023/03/04 21:37:37 tb Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project
  */
@@ -255,6 +255,14 @@ ossl_ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp
 	return ecdsa->meth->ecdsa_sign_setup(eckey, ctx_in, kinvp, rp);
 }
 
+
+/*
+ * It is too expensive to check curve parameters on every sign operation.
+ * Instead, cap the number of retries. A single retry is very unlikely, so
+ * allowing 32 retries is amply enough.
+ */
+#define ECDSA_MAX_SIGN_ITERATIONS		32
+
 static ECDSA_SIG *
 ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
     const BIGNUM *in_kinv, const BIGNUM *in_r, EC_KEY *eckey)
@@ -266,6 +274,7 @@ ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
 	const EC_GROUP *group;
 	ECDSA_SIG  *ret;
 	ECDSA_DATA *ecdsa;
+	int attempts = 0;
 	int ok = 0;
 
 	ecdsa = ecdsa_check(eckey);
@@ -380,6 +389,11 @@ ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
 				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 */
 			break;