From: tb <tb@openbsd.org>
Date: Sat, 4 Mar 2023 21:42:49 +0000 (+0000)
Subject: Simplify the consistency checks in old_dsa_priv_decode()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=94bb0884bd30b5ae207124046e01a81de80b714a;p=openbsd

Simplify the consistency checks in old_dsa_priv_decode()

We have long had expensive checks for DSA domain parameters in
old_dsa_priv_decode(). These were implemented in a more complicated
way than necesary.

ok beck jsing
---

diff --git a/lib/libcrypto/dsa/dsa_ameth.c b/lib/libcrypto/dsa/dsa_ameth.c
index 495c32cbae6..f282caae06b 100644
--- a/lib/libcrypto/dsa/dsa_ameth.c
+++ b/lib/libcrypto/dsa/dsa_ameth.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_ameth.c,v 1.41 2023/03/04 21:08:14 tb Exp $ */
+/* $OpenBSD: dsa_ameth.c,v 1.42 2023/03/04 21:42:49 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2006.
  */
@@ -504,7 +504,7 @@ old_dsa_priv_decode(EVP_PKEY *pkey, const unsigned char **pder, int derlen)
 {
 	DSA *dsa;
 	BN_CTX *ctx = NULL;
-	BIGNUM *j, *p1, *newp1, *powg;
+	BIGNUM *result;
 
 	if ((dsa = d2i_DSAPrivateKey(NULL, pder, derlen)) == NULL) {
 		DSAerror(ERR_R_DSA_LIB);
@@ -519,30 +519,19 @@ old_dsa_priv_decode(EVP_PKEY *pkey, const unsigned char **pder, int derlen)
 
 	BN_CTX_start(ctx);
 
-	/*
-	 * Check that p and q are consistent with each other.
-	 */
-	if ((j = BN_CTX_get(ctx)) == NULL)
-		goto err;
-	if ((p1 = BN_CTX_get(ctx)) == NULL)
-		goto err;
-	if ((newp1 = BN_CTX_get(ctx)) == NULL)
-		goto err;
-	if ((powg = BN_CTX_get(ctx)) == NULL)
+	if ((result = BN_CTX_get(ctx)) == NULL)
 		goto err;
 
-	/* p1 = p - 1 */
-	if (BN_sub(p1, dsa->p, BN_value_one()) == 0)
-		goto err;
+	/*
+	 * Check that p and q are consistent with each other. dsa_check_key()
+	 * ensures that 1 < q < p. Now check that q divides p - 1.
+	 */
 
-	/* j = (p - 1) / q */
-	if (BN_div_ct(j, NULL, p1, dsa->q, ctx) == 0)
+	if (!BN_sub(result, dsa->p, BN_value_one()))
 		goto err;
-
-	/* q * j should == p - 1 */
-	if (BN_mul(newp1, dsa->q, j, ctx) == 0)
+	if (!BN_mod_ct(result, result, dsa->q, ctx))
 		goto err;
-	if (BN_cmp(newp1, p1) != 0) {
+	if (!BN_is_zero(result)) {
 		DSAerror(DSA_R_BAD_Q_VALUE);
 		goto err;
 	}
@@ -553,10 +542,10 @@ old_dsa_priv_decode(EVP_PKEY *pkey, const unsigned char **pder, int derlen)
 	 * Once we know that q is prime, this is enough.
 	 */
 
-	if (!BN_mod_exp_ct(powg, dsa->g, dsa->q, dsa->p, ctx))
+	if (!BN_mod_exp_ct(result, dsa->g, dsa->q, dsa->p, ctx))
 		goto err;
-	if (BN_cmp(powg, BN_value_one()) != 0) {
-		DSAerror(DSA_R_PARAMETER_ENCODING_ERROR); /* XXX */
+	if (BN_cmp(result, BN_value_one()) != 0) {
+		DSAerror(DSA_R_INVALID_PARAMETERS);
 		goto err;
 	}