Skip to content

Commit

Permalink
DH_check(): Do not try checking q properties if it is obviously invalid
Browse files Browse the repository at this point in the history
If  |q| >= |p| then the q value is obviously wrong as q
is supposed to be a prime divisor of p-1.

We check if p is overly large so this added test implies that
q is not large either when performing subsequent tests using that
q value.

Otherwise if it is too large these additional checks of the q value
such as the primality test can then trigger DoS by doing overly long
computations.

Fixes CVE-2023-3817

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from #21550)

(cherry picked from commit 1c16253)
(cherry picked from commit 6a1eb62)
  • Loading branch information
t8m authored and tmshort committed Jul 27, 2023
1 parent e648db5 commit 9002fd0
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crypto/dh/dh_check.c
Expand Up @@ -143,7 +143,7 @@ int DH_check(const DH *dh, int *ret)
#ifdef FIPS_MODULE
return DH_check_params(dh, ret);
#else
int ok = 0, r;
int ok = 0, r, q_good = 0;
BN_CTX *ctx = NULL;
BIGNUM *t1 = NULL, *t2 = NULL;
int nid = DH_get_nid((DH *)dh);
Expand Down Expand Up @@ -172,6 +172,13 @@ int DH_check(const DH *dh, int *ret)
goto err;

if (dh->params.q != NULL) {
if (BN_ucmp(dh->params.p, dh->params.q) > 0)
q_good = 1;
else
*ret |= DH_CHECK_INVALID_Q_VALUE;
}

if (q_good) {
if (BN_cmp(dh->params.g, BN_value_one()) <= 0)
*ret |= DH_NOT_SUITABLE_GENERATOR;
else if (BN_cmp(dh->params.g, dh->params.p) >= 0)
Expand Down

0 comments on commit 9002fd0

Please sign in to comment.