parent
57be30a986
commit
8de9f2d836
@ -0,0 +1,127 @@
|
|||||||
|
From 8780a896543a654e757db1b9396383f9d8095528 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Matt Caswell <matt@openssl.org>
|
||||||
|
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 <pauli@openssl.org>
|
||||||
|
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
|
||||||
|
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
|
||||||
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
||||||
|
(Merged from https://github.com/openssl/openssl/pull/21452)
|
||||||
|
|
||||||
|
Upstream-Status: Backport [8780a896543a654e757db1b9396383f9d8095528]
|
||||||
|
---
|
||||||
|
crypto/dh/dh_check.c | 6 ++++++
|
||||||
|
crypto/dh/dh_err.c | 3 ++-
|
||||||
|
crypto/err/openssl.txt | 3 ++-
|
||||||
|
include/openssl/dh.h | 3 +++
|
||||||
|
include/openssl/dherr.h | 3 ++-
|
||||||
|
5 files changed, 15 insertions(+), 3 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
|
||||||
|
index 4ac169e75c..e5f9dd5030 100644
|
||||||
|
--- a/crypto/dh/dh_check.c
|
||||||
|
+++ b/crypto/dh/dh_check.c
|
||||||
|
@@ -101,6 +101,12 @@ int DH_check(const DH *dh, int *ret)
|
||||||
|
BN_CTX *ctx = NULL;
|
||||||
|
BIGNUM *t1 = NULL, *t2 = NULL;
|
||||||
|
|
||||||
|
+ /* Don't do any checks at all with an excessively large modulus */
|
||||||
|
+ if (BN_num_bits(dh->p) > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) {
|
||||||
|
+ DHerr(DH_F_DH_CHECK, DH_R_MODULUS_TOO_LARGE);
|
||||||
|
+ return 0;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
if (!DH_check_params(dh, ret))
|
||||||
|
return 0;
|
||||||
|
|
||||||
|
diff --git a/crypto/dh/dh_err.c b/crypto/dh/dh_err.c
|
||||||
|
index 7285587b4a..92800d3fcc 100644
|
||||||
|
--- a/crypto/dh/dh_err.c
|
||||||
|
+++ b/crypto/dh/dh_err.c
|
||||||
|
@@ -1,6 +1,6 @@
|
||||||
|
/*
|
||||||
|
* Generated by util/mkerr.pl DO NOT EDIT
|
||||||
|
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
|
||||||
|
+ * Copyright 1995-2023 The OpenSSL Project Authors. All Rights Reserved.
|
||||||
|
*
|
||||||
|
* Licensed under the OpenSSL license (the "License"). You may not use
|
||||||
|
* this file except in compliance with the License. You can obtain a copy
|
||||||
|
@@ -18,6 +18,7 @@ static const ERR_STRING_DATA DH_str_functs[] = {
|
||||||
|
{ERR_PACK(ERR_LIB_DH, DH_F_DHPARAMS_PRINT_FP, 0), "DHparams_print_fp"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, DH_F_DH_BUILTIN_GENPARAMS, 0),
|
||||||
|
"dh_builtin_genparams"},
|
||||||
|
+ {ERR_PACK(ERR_LIB_DH, DH_F_DH_CHECK, 0), "DH_check"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, DH_F_DH_CHECK_EX, 0), "DH_check_ex"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, DH_F_DH_CHECK_PARAMS_EX, 0), "DH_check_params_ex"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, DH_F_DH_CHECK_PUB_KEY_EX, 0), "DH_check_pub_key_ex"},
|
||||||
|
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
|
||||||
|
index 9f91a4a811..c0a3cd720b 100644
|
||||||
|
--- a/crypto/err/openssl.txt
|
||||||
|
+++ b/crypto/err/openssl.txt
|
||||||
|
@@ -402,6 +402,7 @@ CT_F_SCT_SET_VERSION:104:SCT_set_version
|
||||||
|
DH_F_COMPUTE_KEY:102:compute_key
|
||||||
|
DH_F_DHPARAMS_PRINT_FP:101:DHparams_print_fp
|
||||||
|
DH_F_DH_BUILTIN_GENPARAMS:106:dh_builtin_genparams
|
||||||
|
+DH_F_DH_CHECK:126:DH_check
|
||||||
|
DH_F_DH_CHECK_EX:121:DH_check_ex
|
||||||
|
DH_F_DH_CHECK_PARAMS_EX:122:DH_check_params_ex
|
||||||
|
DH_F_DH_CHECK_PUB_KEY_EX:123:DH_check_pub_key_ex
|
||||||
|
diff --git a/include/openssl/dh.h b/include/openssl/dh.h
|
||||||
|
index 3527540cdd..892e31559d 100644
|
||||||
|
--- a/include/openssl/dh.h
|
||||||
|
+++ b/include/openssl/dh.h
|
||||||
|
@@ -29,6 +29,9 @@ extern "C" {
|
||||||
|
# ifndef OPENSSL_DH_MAX_MODULUS_BITS
|
||||||
|
# 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
|
||||||
|
# define OPENSSL_DH_FIPS_MIN_MODULUS_BITS_GEN 2048
|
||||||
|
|
||||||
|
diff --git a/include/openssl/dherr.h b/include/openssl/dherr.h
|
||||||
|
index 916b3bed0b..528c819856 100644
|
||||||
|
--- a/include/openssl/dherr.h
|
||||||
|
+++ b/include/openssl/dherr.h
|
||||||
|
@@ -1,6 +1,6 @@
|
||||||
|
/*
|
||||||
|
* Generated by util/mkerr.pl DO NOT EDIT
|
||||||
|
- * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
|
||||||
|
+ * Copyright 1995-2023 The OpenSSL Project Authors. All Rights Reserved.
|
||||||
|
*
|
||||||
|
* Licensed under the OpenSSL license (the "License"). You may not use
|
||||||
|
* this file except in compliance with the License. You can obtain a copy
|
||||||
|
@@ -30,6 +30,7 @@ int ERR_load_DH_strings(void);
|
||||||
|
# define DH_F_COMPUTE_KEY 102
|
||||||
|
# define DH_F_DHPARAMS_PRINT_FP 101
|
||||||
|
# define DH_F_DH_BUILTIN_GENPARAMS 106
|
||||||
|
+# define DH_F_DH_CHECK 126
|
||||||
|
# define DH_F_DH_CHECK_EX 121
|
||||||
|
# define DH_F_DH_CHECK_PARAMS_EX 122
|
||||||
|
# define DH_F_DH_CHECK_PUB_KEY_EX 123
|
||||||
|
--
|
||||||
|
2.41.0
|
||||||
|
|
@ -0,0 +1,60 @@
|
|||||||
|
From 91ddeba0f2269b017dc06c46c993a788974b1aa5 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tomas Mraz <tomas@openssl.org>
|
||||||
|
Date: Fri, 21 Jul 2023 11:39:41 +0200
|
||||||
|
Subject: [PATCH] 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: Paul Dale <pauli@openssl.org>
|
||||||
|
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||||
|
(Merged from https://github.com/openssl/openssl/pull/21551)
|
||||||
|
|
||||||
|
Upstream-Status: Backport [91ddeba0f2269b017dc06c46c993a788974b1aa5]
|
||||||
|
---
|
||||||
|
crypto/dh/dh_check.c | 11 +++++++++--
|
||||||
|
1 file changed, 9 insertions(+), 2 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
|
||||||
|
index 2001d2e7cb..9ae96991eb 100644
|
||||||
|
--- a/crypto/dh/dh_check.c
|
||||||
|
+++ b/crypto/dh/dh_check.c
|
||||||
|
@@ -105,7 +105,7 @@ int DH_check_ex(const DH *dh)
|
||||||
|
/* Note: according to documentation - this only checks the params */
|
||||||
|
int DH_check(const DH *dh, int *ret)
|
||||||
|
{
|
||||||
|
- int ok = 0, r;
|
||||||
|
+ int ok = 0, r, q_good = 0;
|
||||||
|
BN_CTX *ctx = NULL;
|
||||||
|
BIGNUM *t1 = NULL, *t2 = NULL;
|
||||||
|
|
||||||
|
@@ -130,7 +130,14 @@ int DH_check(const DH *dh, int *ret)
|
||||||
|
if (t2 == NULL)
|
||||||
|
goto err;
|
||||||
|
|
||||||
|
- if (dh->q) {
|
||||||
|
+ if (dh->q != NULL) {
|
||||||
|
+ if (BN_ucmp(dh->p, dh->q) > 0)
|
||||||
|
+ q_good = 1;
|
||||||
|
+ else
|
||||||
|
+ *ret |= DH_CHECK_INVALID_Q_VALUE;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if (q_good) {
|
||||||
|
if (BN_cmp(dh->g, BN_value_one()) <= 0)
|
||||||
|
*ret |= DH_NOT_SUITABLE_GENERATOR;
|
||||||
|
else if (BN_cmp(dh->g, dh->p) >= 0)
|
||||||
|
--
|
||||||
|
2.41.0
|
||||||
|
|
@ -0,0 +1,154 @@
|
|||||||
|
From 0814467cc1b6a2839877277d3efa69cdd4582dd7 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Richard Levitte <levitte@openssl.org>
|
||||||
|
Date: Fri, 20 Oct 2023 09:18:19 +0200
|
||||||
|
Subject: [PATCH] Make DH_check_pub_key() and DH_generate_key() safer yet
|
||||||
|
|
||||||
|
We already check for an excessively large P in DH_generate_key(), but not in
|
||||||
|
DH_check_pub_key(), and none of them check for an excessively large Q.
|
||||||
|
|
||||||
|
This change adds all the missing excessive size checks of P and Q.
|
||||||
|
|
||||||
|
It's to be noted that behaviours surrounding excessively sized P and Q
|
||||||
|
differ. DH_check() raises an error on the excessively sized P, but only
|
||||||
|
sets a flag for the excessively sized Q. This behaviour is mimicked in
|
||||||
|
DH_check_pub_key().
|
||||||
|
|
||||||
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
||||||
|
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||||
|
Reviewed-by: Hugo Landau <hlandau@openssl.org>
|
||||||
|
(Merged from https://github.com/openssl/openssl/pull/22518)
|
||||||
|
|
||||||
|
(cherry picked from commit ddeb4b6c6d527e54ce9a99cba785c0f7776e54b6)
|
||||||
|
Backported-by: Clemens Lang <cllang@redhat.com>
|
||||||
|
---
|
||||||
|
crypto/dh/dh_check.c | 17 +++++++++++++++++
|
||||||
|
crypto/dh/dh_err.c | 1 +
|
||||||
|
crypto/dh/dh_key.c | 10 ++++++++++
|
||||||
|
crypto/err/openssl.txt | 1 +
|
||||||
|
include/openssl/dh.h | 6 ++++--
|
||||||
|
include/openssl/dherr.h | 1 +
|
||||||
|
6 files changed, 34 insertions(+), 2 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
|
||||||
|
index ae1b03bc92..424a3bb4cd 100644
|
||||||
|
--- a/crypto/dh/dh_check.c
|
||||||
|
+++ b/crypto/dh/dh_check.c
|
||||||
|
@@ -198,10 +198,27 @@ int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret)
|
||||||
|
BN_CTX *ctx = NULL;
|
||||||
|
|
||||||
|
*ret = 0;
|
||||||
|
+
|
||||||
|
ctx = BN_CTX_new();
|
||||||
|
if (ctx == NULL)
|
||||||
|
goto err;
|
||||||
|
BN_CTX_start(ctx);
|
||||||
|
+
|
||||||
|
+ /* Don't do any checks at all with an excessively large modulus */
|
||||||
|
+ if (BN_num_bits(dh->p) > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) {
|
||||||
|
+ DHerr(DH_F_DH_CHECK, DH_R_MODULUS_TOO_LARGE);
|
||||||
|
+ *ret = DH_MODULUS_TOO_LARGE | DH_CHECK_PUBKEY_INVALID;
|
||||||
|
+ goto err;
|
||||||
|
+ }
|
||||||
|
+ if (dh->q != NULL && BN_ucmp(dh->p, dh->q) < 0) {
|
||||||
|
+ *ret |= DH_CHECK_INVALID_Q_VALUE | DH_CHECK_PUBKEY_INVALID;
|
||||||
|
+ /* This may look strange here, but returning 1 after setting ret is
|
||||||
|
+ * correct. See also the behavior of the pub_key^q == 1 mod p check
|
||||||
|
+ * further down, which behaves in the same way. */
|
||||||
|
+ ok = 1;
|
||||||
|
+ goto err;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
tmp = BN_CTX_get(ctx);
|
||||||
|
if (tmp == NULL || !BN_set_word(tmp, 1))
|
||||||
|
goto err;
|
||||||
|
diff --git a/crypto/dh/dh_err.c b/crypto/dh/dh_err.c
|
||||||
|
index 92800d3fcc..b3b1e7a706 100644
|
||||||
|
--- a/crypto/dh/dh_err.c
|
||||||
|
+++ b/crypto/dh/dh_err.c
|
||||||
|
@@ -87,6 +87,7 @@ static const ERR_STRING_DATA DH_str_reasons[] = {
|
||||||
|
{ERR_PACK(ERR_LIB_DH, 0, DH_R_PARAMETER_ENCODING_ERROR),
|
||||||
|
"parameter encoding error"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, 0, DH_R_PEER_KEY_ERROR), "peer key error"},
|
||||||
|
+ {ERR_PACK(ERR_LIB_DH, 0, DH_R_Q_TOO_LARGE), "q too large"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, 0, DH_R_SHARED_INFO_ERROR), "shared info error"},
|
||||||
|
{ERR_PACK(ERR_LIB_DH, 0, DH_R_UNABLE_TO_CHECK_GENERATOR),
|
||||||
|
"unable to check generator"},
|
||||||
|
diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
|
||||||
|
index 117f2fa883..9f5e6f6d4c 100644
|
||||||
|
--- a/crypto/dh/dh_key.c
|
||||||
|
+++ b/crypto/dh/dh_key.c
|
||||||
|
@@ -140,6 +140,11 @@ static int generate_key(DH *dh)
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
+ if (dh->q != NULL && BN_num_bits(dh->q) > OPENSSL_DH_MAX_MODULUS_BITS) {
|
||||||
|
+ DHerr(DH_F_GENERATE_KEY, DH_R_Q_TOO_LARGE);
|
||||||
|
+ return 0;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
ctx = BN_CTX_new();
|
||||||
|
if (ctx == NULL)
|
||||||
|
goto err;
|
||||||
|
@@ -250,6 +255,12 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
|
||||||
|
DHerr(DH_F_COMPUTE_KEY, DH_R_MODULUS_TOO_LARGE);
|
||||||
|
goto err;
|
||||||
|
}
|
||||||
|
+
|
||||||
|
+ if (dh->q != NULL && BN_num_bits(dh->q) > OPENSSL_DH_MAX_MODULUS_BITS) {
|
||||||
|
+ DHerr(DH_F_COMPUTE_KEY, DH_R_Q_TOO_LARGE);
|
||||||
|
+ goto err;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
#ifdef OPENSSL_FIPS
|
||||||
|
if (FIPS_mode()
|
||||||
|
&& (BN_num_bits(dh->p) < OPENSSL_DH_FIPS_MIN_MODULUS_BITS)) {
|
||||||
|
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
|
||||||
|
index c0a3cd720b..5e0ff47516 100644
|
||||||
|
--- a/crypto/err/openssl.txt
|
||||||
|
+++ b/crypto/err/openssl.txt
|
||||||
|
@@ -2151,6 +2151,7 @@DH_R_NO_PARAMETERS_SET:107:no parameters set
|
||||||
|
DH_R_NO_PRIVATE_VALUE:100:no private value
|
||||||
|
DH_R_PARAMETER_ENCODING_ERROR:105:parameter encoding error
|
||||||
|
DH_R_PEER_KEY_ERROR:111:peer key error
|
||||||
|
+DH_R_Q_TOO_LARGE:130:q too large
|
||||||
|
DH_R_SHARED_INFO_ERROR:113:shared info error
|
||||||
|
DH_R_UNABLE_TO_CHECK_GENERATOR:121:unable to check generator
|
||||||
|
DSA_R_BAD_Q_VALUE:102:bad q value
|
||||||
|
diff --git a/include/openssl/dh.h b/include/openssl/dh.h
|
||||||
|
index 6c6ff3636a..b7df43b44f 100644
|
||||||
|
--- a/include/openssl/dh.h
|
||||||
|
+++ b/include/openssl/dh.h
|
||||||
|
@@ -72,14 +72,16 @@ DECLARE_ASN1_ITEM(DHparams)
|
||||||
|
/* #define DH_GENERATOR_3 3 */
|
||||||
|
# define DH_GENERATOR_5 5
|
||||||
|
|
||||||
|
-/* DH_check error codes */
|
||||||
|
+/* DH_check error codes, some of them shared with DH_check_pub_key */
|
||||||
|
# define DH_CHECK_P_NOT_PRIME 0x01
|
||||||
|
# define DH_CHECK_P_NOT_SAFE_PRIME 0x02
|
||||||
|
# define DH_UNABLE_TO_CHECK_GENERATOR 0x04
|
||||||
|
# define DH_NOT_SUITABLE_GENERATOR 0x08
|
||||||
|
# define DH_CHECK_Q_NOT_PRIME 0x10
|
||||||
|
-# define DH_CHECK_INVALID_Q_VALUE 0x20
|
||||||
|
+# define DH_CHECK_INVALID_Q_VALUE 0x20 /* +DH_check_pub_key */
|
||||||
|
# define DH_CHECK_INVALID_J_VALUE 0x40
|
||||||
|
+/* DH_MODULUS_TOO_SMALL is 0x80 upstream */
|
||||||
|
+# define DH_MODULUS_TOO_LARGE 0x100 /* +DH_check_pub_key */
|
||||||
|
|
||||||
|
/* DH_check_pub_key error codes */
|
||||||
|
# define DH_CHECK_PUBKEY_TOO_SMALL 0x01
|
||||||
|
diff --git a/include/openssl/dherr.h b/include/openssl/dherr.h
|
||||||
|
index 528c819856..d66c35aa8e 100644
|
||||||
|
--- a/include/openssl/dherr.h
|
||||||
|
+++ b/include/openssl/dherr.h
|
||||||
|
@@ -87,6 +87,7 @@ int ERR_load_DH_strings(void);
|
||||||
|
# define DH_R_NON_FIPS_METHOD 202
|
||||||
|
# define DH_R_PARAMETER_ENCODING_ERROR 105
|
||||||
|
# define DH_R_PEER_KEY_ERROR 111
|
||||||
|
+# define DH_R_Q_TOO_LARGE 130
|
||||||
|
# define DH_R_SHARED_INFO_ERROR 113
|
||||||
|
# define DH_R_UNABLE_TO_CHECK_GENERATOR 121
|
||||||
|
|
||||||
|
--
|
||||||
|
2.41.0
|
||||||
|
|
File diff suppressed because it is too large
Load Diff
Loading…
Reference in new issue