From 3f42ce0a890cb77e6aed67407cafb72c2ae359c5 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Sat, 4 Mar 2023 20:54:52 +0000
Subject: [PATCH] Provide dsa_check_key()

This is a cheap check that ensures basid parameter consistency per
FIPS 186-4: 1 < g < q, that q has the allowed bit sizes 160, 224, 256
and that p is neither too small nor too large. Unfortunately, enforcing
the three allowed sizes for p is not possible since the default dsa key
generation has not respected this limitation.

Instead of checking that p and q are prime, we only check that they
are odd. Check that public and private keys, if set, are in the proper
range. In particular, disallow zero values.

Various versions of these checks have been added to the dsa code
over time. This consolidates and extends them and in a subsequent
commit wewill replace the incomplete checks. BoringSSL has a similar
function of the same name, thanks to David Benjamin for pointing it
out.

ok beck jsing
---
 lib/libcrypto/dsa/dsa_lib.c   | 75 ++++++++++++++++++++++++++++++++++-
 lib/libcrypto/dsa/dsa_local.h |  4 +-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/lib/libcrypto/dsa/dsa_lib.c b/lib/libcrypto/dsa/dsa_lib.c
index af8f33980a7..38f0b071ec5 100644
--- a/lib/libcrypto/dsa/dsa_lib.c
+++ b/lib/libcrypto/dsa/dsa_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_lib.c,v 1.39 2022/11/26 16:08:52 tb Exp $ */
+/* $OpenBSD: dsa_lib.c,v 1.40 2023/03/04 20:54:52 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -423,3 +423,76 @@ DSA_bits(const DSA *dsa)
 {
 	return BN_num_bits(dsa->p);
 }
+
+int
+dsa_check_key(const DSA *dsa)
+{
+	int p_bits, q_bits;
+
+	if (dsa->p == NULL || dsa->q == NULL || dsa->g == NULL) {
+		DSAerror(DSA_R_MISSING_PARAMETERS);
+		return 0;
+	}
+
+	/* Checking that p and q are primes is expensive. Check they are odd. */
+	if (!BN_is_odd(dsa->p) || !BN_is_odd(dsa->q)) {
+		DSAerror(DSA_R_INVALID_PARAMETERS);
+		return 0;
+	}
+
+	/* FIPS 186-4: 1 < g < p. */
+	if (BN_cmp(dsa->g, BN_value_one()) <= 0 ||
+	    BN_cmp(dsa->g, dsa->p) >= 0) {
+		DSAerror(DSA_R_INVALID_PARAMETERS);
+		return 0;
+	}
+
+	/* We know p and g are positive. The next two checks imply q > 0. */
+	if (BN_is_negative(dsa->q)) {
+		DSAerror(DSA_R_BAD_Q_VALUE);
+		return 0;
+	}
+
+	/* FIPS 186-4 only allows three sizes for q. */
+	q_bits = BN_num_bits(dsa->q);
+	if (q_bits != 160 && q_bits != 224 && q_bits != 256) {
+		DSAerror(DSA_R_BAD_Q_VALUE);
+		return 0;
+	}
+
+	/*
+	 * XXX - FIPS 186-4 only allows 1024, 2048, and 3072 bits for p.
+	 * Cap the size to reduce DoS risks. Poor defaults make keys with
+	 * incorrect p sizes >= 512 bits common, so only enforce a weak
+	 * lower bound.
+	 */
+	p_bits = BN_num_bits(dsa->p);
+	if (p_bits > OPENSSL_DSA_MAX_MODULUS_BITS) {
+		DSAerror(DSA_R_MODULUS_TOO_LARGE);
+		return 0;
+	}
+	if (p_bits < 512) {
+		DSAerror(DSA_R_INVALID_PARAMETERS);
+		return 0;
+	}
+
+	/* The public key must be in the multiplicative group (mod p). */
+	if (dsa->pub_key != NULL) {
+		if (BN_cmp(dsa->pub_key, BN_value_one()) <= 0 ||
+		    BN_cmp(dsa->pub_key, dsa->p) >= 0) {
+			DSAerror(DSA_R_INVALID_PARAMETERS);
+			return 0;
+		}
+	}
+
+	/* The private key must be nonzero and in GF(q). */
+	if (dsa->priv_key != NULL) {
+		if (BN_cmp(dsa->priv_key, BN_value_one()) <= 0 ||
+		    BN_cmp(dsa->priv_key, dsa->q) >= 0) {
+			DSAerror(DSA_R_INVALID_PARAMETERS);
+			return 0;
+		}
+	}
+
+	return 1;
+}
diff --git a/lib/libcrypto/dsa/dsa_local.h b/lib/libcrypto/dsa/dsa_local.h
index 0487bd9f418..a413db97479 100644
--- a/lib/libcrypto/dsa/dsa_local.h
+++ b/lib/libcrypto/dsa/dsa_local.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_local.h,v 1.1 2022/11/26 16:08:52 tb Exp $ */
+/* $OpenBSD: dsa_local.h,v 1.2 2023/03/04 20:54:52 tb Exp $ */
 /* ====================================================================
  * Copyright (c) 2007 The OpenSSL Project.  All rights reserved.
  *
@@ -115,4 +115,6 @@ int dsa_builtin_paramgen(DSA *ret, size_t bits, size_t qbits,
 	unsigned char *seed_out,
 	int *counter_ret, unsigned long *h_ret, BN_GENCB *cb);
 
+int dsa_check_key(const DSA *dsa);
+
 __END_HIDDEN_DECLS
-- 
2.20.1