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.
431 lines
15 KiB
431 lines
15 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)
|
|
ok = ok && ossl_dh_key_fromdata(dh, params);
|
|
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)
|
|
ok = ok && ossl_dsa_key_fromdata(dsa, params);
|
|
|
|
--
|
|
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
|
|
|
|
--- openssl-3.0.1/test/recipes/91-test_pkey_check.t 2023-02-08 13:43:56.228487948 +0100
|
|
+++ openssl-3.0.7/test/recipes/91-test_pkey_check.t 2023-02-08 12:47:13.531027540 +0100
|
|
@@ -1,5 +1,5 @@
|
|
#! /usr/bin/env perl
|
|
-# Copyright 2017-2021 The OpenSSL Project Authors. All Rights Reserved.
|
|
+# Copyright 2017-2022 The OpenSSL Project Authors. All Rights Reserved.
|
|
#
|
|
# Licensed under the Apache License 2.0 (the "License"). You may not use
|
|
# this file except in compliance with the License. You can obtain a copy
|
|
@@ -11,24 +11,37 @@
|
|
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 check_key {
|
|
+sub pkey_check {
|
|
my $f = shift;
|
|
+ my $pubcheck = shift;
|
|
+ my @checkopt = ('-check');
|
|
+
|
|
+ @checkopt = ('-pubcheck', '-pubin') if $pubcheck;
|
|
|
|
- return run(app(['openssl', 'pkey', '-check', '-text',
|
|
+ return run(app(['openssl', 'pkey', @checkopt, '-text',
|
|
'-in', $f]));
|
|
}
|
|
|
|
-sub check_key_notok {
|
|
+sub check_key {
|
|
my $f = shift;
|
|
- my $str = "$f should fail validation";
|
|
+ my $should_fail = shift;
|
|
+ my $pubcheck = shift;
|
|
+ my $str;
|
|
+
|
|
+
|
|
+ $str = "$f should fail validation" if $should_fail;
|
|
+ $str = "$f should pass validation" unless $should_fail;
|
|
|
|
$f = data_file($f);
|
|
|
|
if ( -s $f ) {
|
|
- ok(!check_key($f), $str);
|
|
+ with({ exit_checker => sub { return shift == $should_fail; } },
|
|
+ sub {
|
|
+ ok(pkey_check($f, $pubcheck), $str);
|
|
+ });
|
|
} else {
|
|
fail("Missing file $f");
|
|
}
|
|
@@ -36,26 +49,54 @@
|
|
|
|
setup("test_pkey_check");
|
|
|
|
-my @tests = ();
|
|
+my @negative_tests = ();
|
|
|
|
-push(@tests, (
|
|
+push(@negative_tests, (
|
|
# For EC keys the range for the secret scalar `k` is `1 <= k <= n-1`
|
|
"ec_p256_bad_0.pem", # `k` set to `n` (equivalent to `0 mod n`, invalid)
|
|
"ec_p256_bad_1.pem", # `k` set to `n+1` (equivalent to `1 mod n`, invalid)
|
|
)) unless disabled("ec");
|
|
|
|
-push(@tests, (
|
|
+push(@negative_tests, (
|
|
# For SM2 keys the range for the secret scalar `k` is `1 <= k < n-1`
|
|
"sm2_bad_neg1.pem", # `k` set to `n-1` (invalid, because SM2 range)
|
|
"sm2_bad_0.pem", # `k` set to `n` (equivalent to `0 mod n`, invalid)
|
|
"sm2_bad_1.pem", # `k` set to `n+1` (equivalent to `1 mod n`, invalid)
|
|
)) unless disabled("sm2");
|
|
|
|
+my @positive_tests = ();
|
|
+
|
|
+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 @tests;
|
|
+ unless @negative_tests && @positive_tests
|
|
+ && @negative_pubtests && @positive_pubtests;
|
|
+
|
|
+plan tests => scalar(@negative_tests) + scalar(@positive_tests)
|
|
+ + scalar(@negative_pubtests) + scalar(@positive_pubtests);
|
|
+
|
|
+foreach my $t (@negative_tests) {
|
|
+ check_key($t, 1, 0);
|
|
+}
|
|
+
|
|
+foreach my $t (@positive_tests) {
|
|
+ check_key($t, 0, 0);
|
|
+}
|
|
|
|
-plan tests => scalar(@tests);
|
|
+foreach my $t (@negative_pubtests) {
|
|
+ check_key($t, 1, 1);
|
|
+}
|
|
|
|
-foreach my $t (@tests) {
|
|
- check_key_notok($t);
|
|
+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
|
|
|