You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
404 lines
14 KiB
404 lines
14 KiB
From 23985bac83fd50c8e29431009302b5442f985096 Mon Sep 17 00:00:00 2001 |
|
From: slontis <shane.lontis@oracle.com> |
|
Date: Wed, 11 Jan 2023 11:05:04 +1000 |
|
Subject: [PATCH 10/18] Fix NULL deference when validating FFC public key. |
|
|
|
Fixes CVE-2023-0217 |
|
|
|
When attempting to do a BN_Copy of params->p there was no NULL check. |
|
Since BN_copy does not check for NULL this is a NULL reference. |
|
|
|
As an aside BN_cmp() does do a NULL check, so there are other checks |
|
that fail because a NULL is passed. A more general check for NULL params |
|
has been added for both FFC public and private key validation instead. |
|
|
|
Reviewed-by: Matt Caswell <matt@openssl.org> |
|
Reviewed-by: Paul Dale <pauli@openssl.org> |
|
Reviewed-by: Tomas Mraz <tomas@openssl.org> |
|
--- |
|
crypto/ffc/ffc_key_validate.c | 9 +++++++++ |
|
include/internal/ffc.h | 1 + |
|
test/ffc_internal_test.c | 31 +++++++++++++++++++++++++++++++ |
|
3 files changed, 41 insertions(+) |
|
|
|
diff --git a/crypto/ffc/ffc_key_validate.c b/crypto/ffc/ffc_key_validate.c |
|
index 9f6525a2c8..442303e4b3 100644 |
|
--- a/crypto/ffc/ffc_key_validate.c |
|
+++ b/crypto/ffc/ffc_key_validate.c |
|
@@ -24,6 +24,11 @@ int ossl_ffc_validate_public_key_partial(const FFC_PARAMS *params, |
|
BN_CTX *ctx = NULL; |
|
|
|
*ret = 0; |
|
+ if (params == NULL || pub_key == NULL || params->p == NULL) { |
|
+ *ret = FFC_ERROR_PASSED_NULL_PARAM; |
|
+ return 0; |
|
+ } |
|
+ |
|
ctx = BN_CTX_new_ex(NULL); |
|
if (ctx == NULL) |
|
goto err; |
|
@@ -107,6 +112,10 @@ int ossl_ffc_validate_private_key(const BIGNUM *upper, const BIGNUM *priv, |
|
|
|
*ret = 0; |
|
|
|
+ if (priv == NULL || upper == NULL) { |
|
+ *ret = FFC_ERROR_PASSED_NULL_PARAM; |
|
+ goto err; |
|
+ } |
|
if (BN_cmp(priv, BN_value_one()) < 0) { |
|
*ret |= FFC_ERROR_PRIVKEY_TOO_SMALL; |
|
goto err; |
|
diff --git a/include/internal/ffc.h b/include/internal/ffc.h |
|
index 732514a6c2..b8b7140857 100644 |
|
--- a/include/internal/ffc.h |
|
+++ b/include/internal/ffc.h |
|
@@ -76,6 +76,7 @@ |
|
# define FFC_ERROR_NOT_SUITABLE_GENERATOR 0x08 |
|
# define FFC_ERROR_PRIVKEY_TOO_SMALL 0x10 |
|
# define FFC_ERROR_PRIVKEY_TOO_LARGE 0x20 |
|
+# define FFC_ERROR_PASSED_NULL_PARAM 0x40 |
|
|
|
/* |
|
* Finite field cryptography (FFC) domain parameters are used by DH and DSA. |
|
diff --git a/test/ffc_internal_test.c b/test/ffc_internal_test.c |
|
index 2c97293573..9f67bd29b9 100644 |
|
--- a/test/ffc_internal_test.c |
|
+++ b/test/ffc_internal_test.c |
|
@@ -510,6 +510,27 @@ static int ffc_public_validate_test(void) |
|
if (!TEST_true(ossl_ffc_validate_public_key(params, pub, &res))) |
|
goto err; |
|
|
|
+ /* Fail if params is NULL */ |
|
+ if (!TEST_false(ossl_ffc_validate_public_key(NULL, pub, &res))) |
|
+ goto err; |
|
+ if (!TEST_int_eq(FFC_ERROR_PASSED_NULL_PARAM, res)) |
|
+ goto err; |
|
+ res = -1; |
|
+ /* Fail if pubkey is NULL */ |
|
+ if (!TEST_false(ossl_ffc_validate_public_key(params, NULL, &res))) |
|
+ goto err; |
|
+ if (!TEST_int_eq(FFC_ERROR_PASSED_NULL_PARAM, res)) |
|
+ goto err; |
|
+ res = -1; |
|
+ |
|
+ BN_free(params->p); |
|
+ params->p = NULL; |
|
+ /* Fail if params->p is NULL */ |
|
+ if (!TEST_false(ossl_ffc_validate_public_key(params, pub, &res))) |
|
+ goto err; |
|
+ if (!TEST_int_eq(FFC_ERROR_PASSED_NULL_PARAM, res)) |
|
+ goto err; |
|
+ |
|
ret = 1; |
|
err: |
|
DH_free(dh); |
|
@@ -567,6 +588,16 @@ static int ffc_private_validate_test(void) |
|
if (!TEST_true(ossl_ffc_validate_private_key(params->q, priv, &res))) |
|
goto err; |
|
|
|
+ if (!TEST_false(ossl_ffc_validate_private_key(NULL, priv, &res))) |
|
+ goto err; |
|
+ if (!TEST_int_eq(FFC_ERROR_PASSED_NULL_PARAM, res)) |
|
+ goto err; |
|
+ res = -1; |
|
+ if (!TEST_false(ossl_ffc_validate_private_key(params->q, NULL, &res))) |
|
+ goto err; |
|
+ if (!TEST_int_eq(FFC_ERROR_PASSED_NULL_PARAM, res)) |
|
+ goto err; |
|
+ |
|
ret = 1; |
|
err: |
|
DH_free(dh); |
|
-- |
|
2.39.1 |
|
|
|
From c1b4467a7cc129a74fc5205b80a5c47556b99416 Mon Sep 17 00:00:00 2001 |
|
From: Tomas Mraz <tomas@openssl.org> |
|
Date: Fri, 13 Jan 2023 17:57:59 +0100 |
|
Subject: [PATCH 11/18] Prevent creating DSA and DH keys without parameters |
|
through import |
|
|
|
Reviewed-by: Matt Caswell <matt@openssl.org> |
|
Reviewed-by: Paul Dale <pauli@openssl.org> |
|
--- |
|
providers/implementations/keymgmt/dh_kmgmt.c | 4 ++-- |
|
providers/implementations/keymgmt/dsa_kmgmt.c | 5 +++-- |
|
2 files changed, 5 insertions(+), 4 deletions(-) |
|
|
|
diff --git a/providers/implementations/keymgmt/dh_kmgmt.c b/providers/implementations/keymgmt/dh_kmgmt.c |
|
index 58a5fd009f..c2d87b4a7f 100644 |
|
--- a/providers/implementations/keymgmt/dh_kmgmt.c |
|
+++ b/providers/implementations/keymgmt/dh_kmgmt.c |
|
@@ -198,8 +198,8 @@ static int dh_import(void *keydata, int selection, const OSSL_PARAM params[]) |
|
if ((selection & DH_POSSIBLE_SELECTIONS) == 0) |
|
return 0; |
|
|
|
- if ((selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) |
|
- ok = ok && ossl_dh_params_fromdata(dh, params); |
|
+ /* a key without parameters is meaningless */ |
|
+ ok = ok && ossl_dh_params_fromdata(dh, params); |
|
|
|
if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0) { |
|
int include_private = |
|
diff --git a/providers/implementations/keymgmt/dsa_kmgmt.c b/providers/implementations/keymgmt/dsa_kmgmt.c |
|
index 100e917167..881680c085 100644 |
|
--- a/providers/implementations/keymgmt/dsa_kmgmt.c |
|
+++ b/providers/implementations/keymgmt/dsa_kmgmt.c |
|
@@ -199,8 +199,9 @@ static int dsa_import(void *keydata, int selection, const OSSL_PARAM params[]) |
|
if ((selection & DSA_POSSIBLE_SELECTIONS) == 0) |
|
return 0; |
|
|
|
- if ((selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) |
|
- ok = ok && ossl_dsa_ffc_params_fromdata(dsa, params); |
|
+ /* a key without parameters is meaningless */ |
|
+ ok = ok && ossl_dsa_ffc_params_fromdata(dsa, params); |
|
+ |
|
if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0) { |
|
int include_private = |
|
selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY ? 1 : 0; |
|
-- |
|
2.39.1 |
|
|
|
From fab4973801bdc11c29c4c8ccf65cf39cbc63ce9b Mon Sep 17 00:00:00 2001 |
|
From: Tomas Mraz <tomas@openssl.org> |
|
Date: Fri, 13 Jan 2023 17:59:52 +0100 |
|
Subject: [PATCH 12/18] Do not create DSA keys without parameters by decoder |
|
|
|
Reviewed-by: Matt Caswell <matt@openssl.org> |
|
Reviewed-by: Paul Dale <pauli@openssl.org> |
|
--- |
|
crypto/x509/x_pubkey.c | 24 +++++++++++++++++++ |
|
include/crypto/x509.h | 3 +++ |
|
.../encode_decode/decode_der2key.c | 2 +- |
|
3 files changed, 28 insertions(+), 1 deletion(-) |
|
|
|
diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c |
|
index bc90ddd89b..77790faa1f 100644 |
|
--- a/crypto/x509/x_pubkey.c |
|
+++ b/crypto/x509/x_pubkey.c |
|
@@ -745,6 +745,30 @@ DSA *d2i_DSA_PUBKEY(DSA **a, const unsigned char **pp, long length) |
|
return key; |
|
} |
|
|
|
+/* Called from decoders; disallows provided DSA keys without parameters. */ |
|
+DSA *ossl_d2i_DSA_PUBKEY(DSA **a, const unsigned char **pp, long length) |
|
+{ |
|
+ DSA *key = NULL; |
|
+ const unsigned char *data; |
|
+ const BIGNUM *p, *q, *g; |
|
+ |
|
+ data = *pp; |
|
+ key = d2i_DSA_PUBKEY(NULL, &data, length); |
|
+ if (key == NULL) |
|
+ return NULL; |
|
+ DSA_get0_pqg(key, &p, &q, &g); |
|
+ if (p == NULL || q == NULL || g == NULL) { |
|
+ DSA_free(key); |
|
+ return NULL; |
|
+ } |
|
+ *pp = data; |
|
+ if (a != NULL) { |
|
+ DSA_free(*a); |
|
+ *a = key; |
|
+ } |
|
+ return key; |
|
+} |
|
+ |
|
int i2d_DSA_PUBKEY(const DSA *a, unsigned char **pp) |
|
{ |
|
EVP_PKEY *pktmp; |
|
diff --git a/include/crypto/x509.h b/include/crypto/x509.h |
|
index 1f00178e89..0c42730ee9 100644 |
|
--- a/include/crypto/x509.h |
|
+++ b/include/crypto/x509.h |
|
@@ -339,6 +339,9 @@ void ossl_X509_PUBKEY_INTERNAL_free(X509_PUBKEY *xpub); |
|
|
|
RSA *ossl_d2i_RSA_PSS_PUBKEY(RSA **a, const unsigned char **pp, long length); |
|
int ossl_i2d_RSA_PSS_PUBKEY(const RSA *a, unsigned char **pp); |
|
+# ifndef OPENSSL_NO_DSA |
|
+DSA *ossl_d2i_DSA_PUBKEY(DSA **a, const unsigned char **pp, long length); |
|
+# endif /* OPENSSL_NO_DSA */ |
|
# ifndef OPENSSL_NO_DH |
|
DH *ossl_d2i_DH_PUBKEY(DH **a, const unsigned char **pp, long length); |
|
int ossl_i2d_DH_PUBKEY(const DH *a, unsigned char **pp); |
|
diff --git a/providers/implementations/encode_decode/decode_der2key.c b/providers/implementations/encode_decode/decode_der2key.c |
|
index ebc2d24833..d6ad738ef3 100644 |
|
--- a/providers/implementations/encode_decode/decode_der2key.c |
|
+++ b/providers/implementations/encode_decode/decode_der2key.c |
|
@@ -374,7 +374,7 @@ static void *dsa_d2i_PKCS8(void **key, const unsigned char **der, long der_len, |
|
(key_from_pkcs8_t *)ossl_dsa_key_from_pkcs8); |
|
} |
|
|
|
-# define dsa_d2i_PUBKEY (d2i_of_void *)d2i_DSA_PUBKEY |
|
+# define dsa_d2i_PUBKEY (d2i_of_void *)ossl_d2i_DSA_PUBKEY |
|
# define dsa_free (free_key_fn *)DSA_free |
|
# define dsa_check NULL |
|
|
|
-- |
|
2.39.1 |
|
|
|
From 7e37185582995b35f885fec9dcc3670af9ffcbef Mon Sep 17 00:00:00 2001 |
|
From: Tomas Mraz <tomas@openssl.org> |
|
Date: Fri, 13 Jan 2023 18:46:15 +0100 |
|
Subject: [PATCH 13/18] Add test for DSA pubkey without param import and check |
|
|
|
Reviewed-by: Matt Caswell <matt@openssl.org> |
|
Reviewed-by: Paul Dale <pauli@openssl.org> |
|
--- |
|
test/recipes/91-test_pkey_check.t | 48 ++++++++++++++---- |
|
.../91-test_pkey_check_data/dsapub.pem | 12 +++++ |
|
.../dsapub_noparam.der | Bin 0 -> 108 bytes |
|
3 files changed, 49 insertions(+), 11 deletions(-) |
|
create mode 100644 test/recipes/91-test_pkey_check_data/dsapub.pem |
|
create mode 100644 test/recipes/91-test_pkey_check_data/dsapub_noparam.der |
|
|
|
diff --git a/test/recipes/91-test_pkey_check.t b/test/recipes/91-test_pkey_check.t |
|
index 612a3e3d6c..015d7805db 100644 |
|
--- a/test/recipes/91-test_pkey_check.t |
|
+++ b/test/recipes/91-test_pkey_check.t |
|
@@ -11,19 +11,24 @@ use strict; |
|
use warnings; |
|
|
|
use File::Spec; |
|
-use OpenSSL::Test qw/:DEFAULT data_file/; |
|
+use OpenSSL::Test qw/:DEFAULT data_file with/; |
|
use OpenSSL::Test::Utils; |
|
|
|
sub pkey_check { |
|
my $f = shift; |
|
+ my $pubcheck = shift; |
|
+ my @checkopt = ('-check'); |
|
|
|
- return run(app(['openssl', 'pkey', '-check', '-text', |
|
+ @checkopt = ('-pubcheck', '-pubin') if $pubcheck; |
|
+ |
|
+ return run(app(['openssl', 'pkey', @checkopt, '-text', |
|
'-in', $f])); |
|
} |
|
|
|
sub check_key { |
|
my $f = shift; |
|
my $should_fail = shift; |
|
+ my $pubcheck = shift; |
|
my $str; |
|
|
|
|
|
@@ -33,11 +38,10 @@ sub check_key { |
|
$f = data_file($f); |
|
|
|
if ( -s $f ) { |
|
- if ($should_fail) { |
|
- ok(!pkey_check($f), $str); |
|
- } else { |
|
- ok(pkey_check($f), $str); |
|
- } |
|
+ with({ exit_checker => sub { return shift == $should_fail; } }, |
|
+ sub { |
|
+ ok(pkey_check($f, $pubcheck), $str); |
|
+ }); |
|
} else { |
|
fail("Missing file $f"); |
|
} |
|
@@ -66,15 +70,37 @@ push(@positive_tests, ( |
|
"dhpkey.pem" |
|
)) unless disabled("dh"); |
|
|
|
+my @negative_pubtests = (); |
|
+ |
|
+push(@negative_pubtests, ( |
|
+ "dsapub_noparam.der" |
|
+ )) unless disabled("dsa"); |
|
+ |
|
+my @positive_pubtests = (); |
|
+ |
|
+push(@positive_pubtests, ( |
|
+ "dsapub.pem" |
|
+ )) unless disabled("dsa"); |
|
+ |
|
plan skip_all => "No tests within the current enabled feature set" |
|
- unless @negative_tests && @positive_tests; |
|
+ unless @negative_tests && @positive_tests |
|
+ && @negative_pubtests && @positive_pubtests; |
|
|
|
-plan tests => scalar(@negative_tests) + scalar(@positive_tests); |
|
+plan tests => scalar(@negative_tests) + scalar(@positive_tests) |
|
+ + scalar(@negative_pubtests) + scalar(@positive_pubtests); |
|
|
|
foreach my $t (@negative_tests) { |
|
- check_key($t, 1); |
|
+ check_key($t, 1, 0); |
|
} |
|
|
|
foreach my $t (@positive_tests) { |
|
- check_key($t, 0); |
|
+ check_key($t, 0, 0); |
|
+} |
|
+ |
|
+foreach my $t (@negative_pubtests) { |
|
+ check_key($t, 1, 1); |
|
+} |
|
+ |
|
+foreach my $t (@positive_pubtests) { |
|
+ check_key($t, 0, 1); |
|
} |
|
diff --git a/test/recipes/91-test_pkey_check_data/dsapub.pem b/test/recipes/91-test_pkey_check_data/dsapub.pem |
|
new file mode 100644 |
|
index 0000000000..0ff4bd83ed |
|
--- /dev/null |
|
+++ b/test/recipes/91-test_pkey_check_data/dsapub.pem |
|
@@ -0,0 +1,12 @@ |
|
+-----BEGIN PUBLIC KEY----- |
|
+MIIBvzCCATQGByqGSM44BAEwggEnAoGBAIjbXpOVVciVNuagg26annKkghIIZFI4 |
|
+4WdMomnV+I/oXyxHbZTBBBpW9xy/E1+yMjbp4GmX+VxyDj3WxUWxXllzL+miEkzD |
|
+9Xz638VzIBhjFbMvk1/N4kS4bKVUd9yk7HfvYzAdnRphk0WI+RoDiDrBNPPxSoQD |
|
+CEWgvwgsLIDhAh0A6dbz1IQpQwGF4+Ca28x6OO+UfJJv3ggeZ++fNwKBgQCA9XKV |
|
+lRrTY8ALBxS0KbZjpaIXuUj5nr3i1lIDyP3ISksDF0ekyLtn6eK9VijX6Pm65Np+ |
|
+4ic9Nr5WKLKhPaUSpLNRx1gDqo3sd92hYgiEUifzEuhLYfK/CsgFED+l2hDXtJUq |
|
+bISNSHVwI5lsyNXLu7HI1Fk8F5UO3LqsboFAngOBhAACgYATxFY89nEYcUhgHGgr |
|
+YDHhXBQfMKnTKYdvon4DN7WQ9ip+t4VUsLpTD1ZE9zrM2R/B04+8C6KGoViwyeER |
|
+kS4dxWOkX71x4X2DlNpYevcR53tNcTDqmMD7YKfDDmrb0lftMyfW8aESaiymVMys |
|
+DRjhKHBjdo0rZeSM8DAk3ctrXA== |
|
+-----END PUBLIC KEY----- |
|
diff --git a/test/recipes/91-test_pkey_check_data/dsapub_noparam.der b/test/recipes/91-test_pkey_check_data/dsapub_noparam.der |
|
new file mode 100644 |
|
index 0000000000000000000000000000000000000000..b8135f1ca94da914b6829421e0c13f6daa731862 |
|
GIT binary patch |
|
literal 108 |
|
zcmXpIGT>xm*J|@PXTieE%*wz71<Xv0AT}3_&&0^YB*etj0OvEYF$n`XLd*y;pgagL |
|
U3o&W4F|x9<gY>|F5F-Nv0Bz9(=Kufz |
|
|
|
literal 0 |
|
HcmV?d00001 |
|
|
|
-- |
|
2.39.1 |
|
|
|
From 2ad9928170768653d19d81881deabc5f9c1665c0 Mon Sep 17 00:00:00 2001 |
|
From: Tomas Mraz <tomas@openssl.org> |
|
Date: Fri, 3 Feb 2023 14:57:04 +0100 |
|
Subject: [PATCH 18/18] Internaly declare the DSA type for no-deprecated builds |
|
|
|
Reviewed-by: Hugo Landau <hlandau@openssl.org> |
|
Reviewed-by: Richard Levitte <levitte@openssl.org> |
|
(cherry picked from commit 7a21a1b5fa2dac438892cf3292d1f9c445d870d9) |
|
--- |
|
include/crypto/types.h | 3 +++ |
|
1 file changed, 3 insertions(+) |
|
|
|
diff --git a/include/crypto/types.h b/include/crypto/types.h |
|
index 0d81404091..0a75f03a3f 100644 |
|
--- a/include/crypto/types.h |
|
+++ b/include/crypto/types.h |
|
@@ -20,6 +20,9 @@ typedef struct rsa_meth_st RSA_METHOD; |
|
typedef struct ec_key_st EC_KEY; |
|
typedef struct ec_key_method_st EC_KEY_METHOD; |
|
# endif |
|
+# ifndef OPENSSL_NO_DSA |
|
+typedef struct dsa_st DSA; |
|
+# endif |
|
# endif |
|
|
|
# ifndef OPENSSL_NO_EC |
|
-- |
|
2.39.1 |
|
|
|
|