From c964b656380bdf4be4d5c24eba03759a0b906a93 Mon Sep 17 00:00:00 2001 From: K1 Date: Tue, 25 Jul 2023 15:22:48 +0200 Subject: [PATCH 1/3] DH_check(): Do not try checking q properties if it is obviously invalid 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 Reviewed-by: Paul Dale Reviewed-by: Tom Cosgrove Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/21550) --- crypto/dh/dh_check.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c index c22eba5e1..f9bf9b6ff 100644 --- a/crypto/dh/dh_check.c +++ b/crypto/dh/dh_check.c @@ -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); @@ -171,6 +171,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) From 9a84fcba76117355adf88894d9fe7847bbb13e98 Mon Sep 17 00:00:00 2001 From: K1 Date: Tue, 25 Jul 2023 15:23:43 +0200 Subject: [PATCH 2/3] dhtest.c: Add test of DH_check() with q = p + 1 This must fail with DH_CHECK_INVALID_Q_VALUE and with DH_CHECK_Q_NOT_PRIME unset. Reviewed-by: Matt Caswell Reviewed-by: Paul Dale Reviewed-by: Tom Cosgrove Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/21550) --- test/dhtest.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/dhtest.c b/test/dhtest.c index f8dd8f3aa..14b00f0d3 100644 --- a/test/dhtest.c +++ b/test/dhtest.c @@ -124,6 +124,15 @@ static int dh_test(void) /* We'll have a stale error on the queue from the above test so clear it */ ERR_clear_error(); + if (!TEST_ptr(BN_copy(q, p)) || !TEST_true(BN_add(q, q, BN_value_one()))) + goto err3; + + if (!TEST_true(DH_check(dh, &i))) + goto err3; + if (!TEST_true(i & DH_CHECK_INVALID_Q_VALUE) + || !TEST_false(i & DH_CHECK_Q_NOT_PRIME)) + goto err3; + /* Modulus of size: dh check max modulus bits + 1 */ if (!TEST_true(BN_set_word(p, 1)) || !TEST_true(BN_lshift(p, p, OPENSSL_DH_CHECK_MAX_MODULUS_BITS))) @@ -135,6 +144,9 @@ static int dh_test(void) if (!TEST_false(DH_check(dh, &i))) goto err3; + /* We'll have a stale error on the queue from the above test so clear it */ + ERR_clear_error(); + /* * II) key generation */ @@ -588,7 +600,7 @@ static int rfc5114_test(void) if (!TEST_ptr(priv_key = BN_bin2bn(td->xB, td->xB_len, NULL)) || !TEST_ptr(pub_key = BN_bin2bn(td->yB, td->yB_len, NULL)) - || !TEST_true( DH_set0_key(dhB, pub_key, priv_key))) + || !TEST_true(DH_set0_key(dhB, pub_key, priv_key))) goto bad_err; priv_key = pub_key = NULL; From fa43b4941561b7320113c8b7f3013f7dec2a3dd3 Mon Sep 17 00:00:00 2001 From: K1 Date: Tue, 15 Aug 2023 16:43:31 +0800 Subject: [PATCH 3/3] Update CHANGES for CVE-2023-3817 --- CHANGES | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 6bbb7e9ec..a6c03dc1b 100644 --- a/CHANGES +++ b/CHANGES @@ -4,9 +4,11 @@ Changes between 8.4.0-pre1 and 8.4.0-pre2 [xx XXX xxxx] - *) 修复CVE-2023-3446 + *) 修复CVE-2023-3817 - *) 修复CVE-2023-2975 + *) 修复CVE-2023-3446 + + *) 修复CVE-2023-2975 *) 实现基于64位平台架构的SM2算法性能优化