https://github.com/openssl/openssl/commit/1fa20cf2f506113c761777127a38bce5068740eb https://github.com/openssl/openssl/commit/8a62fd996cb1c22383ec75b4155d54dec4a1b0ee From 1fa20cf2f506113c761777127a38bce5068740eb Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 6 Jul 2023 16:36:35 +0100 Subject: [PATCH] Fix DH_check() excessive time with over sized modulus The DH_check() function checks numerous aspects of the key or parameters that have been supplied. Some of those checks use the supplied modulus value even if it is excessively large. There is already a maximum DH modulus size (10,000 bits) over which OpenSSL will not generate or derive keys. DH_check() will however still perform various tests for validity on such a large modulus. We introduce a new maximum (32,768) over which DH_check() will just fail. An application that calls DH_check() and supplies a key or parameters obtained from an untrusted source could be vulnerable to a Denial of Service attack. The function DH_check() is itself called by a number of other OpenSSL functions. An application calling any of those other functions may similarly be affected. The other functions affected by this are DH_check_ex() and EVP_PKEY_param_check(). CVE-2023-3446 Reviewed-by: Paul Dale Reviewed-by: Tom Cosgrove Reviewed-by: Bernd Edlinger Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/21451) (cherry picked from commit 9e0094e2aa1b3428a12d5095132f133c078d3c3d) --- a/crypto/dh/dh_check.c +++ b/crypto/dh/dh_check.c @@ -152,6 +152,12 @@ int DH_check(const DH *dh, int *ret) if (nid != NID_undef) return 1; + /* Don't do any checks at all with an excessively large modulus */ + if (BN_num_bits(dh->params.p) > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) { + ERR_raise(ERR_LIB_DH, DH_R_MODULUS_TOO_LARGE); + return 0; + } + if (!DH_check_params(dh, ret)) return 0; --- a/include/openssl/dh.h +++ b/include/openssl/dh.h @@ -89,7 +89,11 @@ int EVP_PKEY_CTX_get0_dh_kdf_ukm(EVP_PKEY_CTX *ctx, unsigned char **ukm); # include # ifndef OPENSSL_DH_MAX_MODULUS_BITS -# define OPENSSL_DH_MAX_MODULUS_BITS 10000 +# define OPENSSL_DH_MAX_MODULUS_BITS 10000 +# endif + +# ifndef OPENSSL_DH_CHECK_MAX_MODULUS_BITS +# define OPENSSL_DH_CHECK_MAX_MODULUS_BITS 32768 # endif # define OPENSSL_DH_FIPS_MIN_MODULUS_BITS 1024 From 8a62fd996cb1c22383ec75b4155d54dec4a1b0ee Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 7 Jul 2023 14:39:48 +0100 Subject: [PATCH] Add a test for CVE-2023-3446 Confirm that the only errors DH_check() finds with DH parameters with an excessively long modulus is that the modulus is too large. We should not be performing time consuming checks using that modulus. Reviewed-by: Paul Dale Reviewed-by: Tom Cosgrove Reviewed-by: Bernd Edlinger Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/21451) (cherry picked from commit ede782b4c8868d1f09c9cd237f82b6f35b7dba8b) --- a/test/dhtest.c +++ b/test/dhtest.c @@ -73,7 +73,7 @@ static int dh_test(void) goto err1; /* check fails, because p is way too small */ - if (!DH_check(dh, &i)) + if (!TEST_true(DH_check(dh, &i))) goto err2; i ^= DH_MODULUS_TOO_SMALL; if (!TEST_false(i & DH_CHECK_P_NOT_PRIME) @@ -124,6 +124,17 @@ static int dh_test(void) /* We'll have a stale error on the queue from the above test so clear it */ ERR_clear_error(); + /* 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))) + goto err3; + + /* + * We expect no checks at all for an excessively large modulus + */ + if (!TEST_false(DH_check(dh, &i))) + goto err3; + /* * II) key generation */ @@ -138,7 +149,7 @@ static int dh_test(void) goto err3; /* ... and check whether it is valid */ - if (!DH_check(a, &i)) + if (!TEST_true(DH_check(a, &i))) goto err3; if (!TEST_false(i & DH_CHECK_P_NOT_PRIME) || !TEST_false(i & DH_CHECK_P_NOT_SAFE_PRIME)