From: tb <tb@openbsd.org>
Date: Sat, 19 Nov 2022 11:48:24 +0000 (+0000)
Subject: Rework DSA_sign() and DSA_verify()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0c6c9df49e0705b9b151b8603834dcfc013ea7e7;p=openbsd

Rework DSA_sign() and DSA_verify()

Change DSA_sign() to single exit and check the signed i2d_DSA_SIG() return
value before assigning it to an unsigned int.

In DSA_verify() let d2i_DSA_SIG() handle the allocation, split error check
of i2d_DSA_SIG() from signature check and change an unnecessary freezero()
to free.

ok jsing
---

diff --git a/lib/libcrypto/dsa/dsa_asn1.c b/lib/libcrypto/dsa/dsa_asn1.c
index b6482e50649..87b930d5ad6 100644
--- a/lib/libcrypto/dsa/dsa_asn1.c
+++ b/lib/libcrypto/dsa/dsa_asn1.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_asn1.c,v 1.26 2022/11/19 06:33:00 tb Exp $ */
+/* $OpenBSD: dsa_asn1.c,v 1.27 2022/11/19 11:48:24 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2000.
  */
@@ -399,18 +399,27 @@ DSAparams_dup(DSA *dsa)
 
 int
 DSA_sign(int type, const unsigned char *dgst, int dlen, unsigned char *sig,
-    unsigned int *siglen, DSA *dsa)
+    unsigned int *out_siglen, DSA *dsa)
 {
 	DSA_SIG *s;
+	int siglen;
+	int ret = 0;
 
-	s = DSA_do_sign(dgst, dlen, dsa);
-	if (s == NULL) {
-		*siglen = 0;
-		return 0;
-	}
-	*siglen = i2d_DSA_SIG(s,&sig);
+	*out_siglen = 0;
+
+	if ((s = DSA_do_sign(dgst, dlen, dsa)) == NULL)
+		goto err;
+
+	if ((siglen = i2d_DSA_SIG(s, &sig)) < 0)
+		goto err;
+
+	*out_siglen = siglen;
+
+	ret = 1;
+ err:
 	DSA_SIG_free(s);
-	return 1;
+
+	return ret;
 }
 
 /*
@@ -424,24 +433,26 @@ int
 DSA_verify(int type, const unsigned char *dgst, int dgst_len,
     const unsigned char *sigbuf, int siglen, DSA *dsa)
 {
-	DSA_SIG *s;
+	DSA_SIG *s = NULL;
 	unsigned char *der = NULL;
-	const unsigned char *p = sigbuf;
-	int derlen = -1;
+	const unsigned char *p;
 	int ret = -1;
 
-	s = DSA_SIG_new();
-	if (s == NULL)
-		return ret;
-	if (d2i_DSA_SIG(&s, &p, siglen) == NULL)
+	p = sigbuf;
+	if ((s = d2i_DSA_SIG(NULL, &p, siglen)) == NULL)
 		goto err;
+
 	/* Ensure signature uses DER and doesn't have trailing garbage */
-	derlen = i2d_DSA_SIG(s, &der);
-	if (derlen != siglen || memcmp(sigbuf, der, derlen))
+	if (i2d_DSA_SIG(s, &der) != siglen)
 		goto err;
+
+	if (memcmp(der, sigbuf, siglen) != 0)
+		goto err;
+
 	ret = DSA_do_verify(dgst, dgst_len, s, dsa);
-err:
-	freezero(der, derlen);
+ err:
+	free(der);
 	DSA_SIG_free(s);
+
 	return ret;
 }