From fea833cb5621a90fe876a42b4eff063e0bc3fb95 Mon Sep 17 00:00:00 2001 From: Dmitry Belyavskiy Date: Wed, 22 Jun 2022 12:52:57 +0200 Subject: [PATCH] Strict certificates validation shouldn't allow explicit EC parameters Related: rhbz#2058663 --- 0015-FIPS-decoded-from-explicit.patch | 140 ++++++++++++++++++ ...clevel-2-if-rh-allow-sha1-signatures.patch | 4 +- openssl.spec | 4 + 3 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 0015-FIPS-decoded-from-explicit.patch diff --git a/0015-FIPS-decoded-from-explicit.patch b/0015-FIPS-decoded-from-explicit.patch new file mode 100644 index 0000000..19d19a3 --- /dev/null +++ b/0015-FIPS-decoded-from-explicit.patch @@ -0,0 +1,140 @@ +diff --git a/crypto/ec/ec_backend.c b/crypto/ec/ec_backend.c +index bea01fb38f66..48721369ae8f 100644 +--- a/crypto/ec/ec_backend.c ++++ b/crypto/ec/ec_backend.c +@@ -318,6 +318,11 @@ int ossl_ec_group_todata(const EC_GROUP *group, OSSL_PARAM_BLD *tmpl, + return 0; + } + ++ if (!ossl_param_build_set_int(tmpl, params, ++ OSSL_PKEY_PARAM_EC_DECODED_FROM_EXPLICIT_PARAMS, ++ group->decoded_from_explicit_params)) ++ return 0; ++ + curve_nid = EC_GROUP_get_curve_name(group); + + /* +diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c +index 6b0591c6c8c7..b1696d93bd6d 100644 +--- a/crypto/ec/ec_lib.c ++++ b/crypto/ec/ec_lib.c +@@ -1556,13 +1556,23 @@ EC_GROUP *EC_GROUP_new_from_params(const OSSL_PARAM params[], + /* This is the simple named group case */ + ptmp = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_GROUP_NAME); + if (ptmp != NULL) { +- group = group_new_from_name(ptmp, libctx, propq); +- if (group != NULL) { +- if (!ossl_ec_group_set_params(group, params)) { +- EC_GROUP_free(group); +- group = NULL; +- } ++ int decoded = 0; ++ ++ if ((group = group_new_from_name(ptmp, libctx, propq)) == NULL) ++ return NULL; ++ if (!ossl_ec_group_set_params(group, params)) { ++ EC_GROUP_free(group); ++ return NULL; ++ } ++ ++ ptmp = OSSL_PARAM_locate_const(params, ++ OSSL_PKEY_PARAM_EC_DECODED_FROM_EXPLICIT_PARAMS); ++ if (ptmp != NULL && !OSSL_PARAM_get_int(ptmp, &decoded)) { ++ ERR_raise(ERR_LIB_EC, EC_R_WRONG_CURVE_PARAMETERS); ++ EC_GROUP_free(group); ++ return NULL; + } ++ group->decoded_from_explicit_params = decoded > 0; + return group; + } + #ifdef FIPS_MODULE +@@ -1733,6 +1743,8 @@ EC_GROUP *EC_GROUP_new_from_params(const OSSL_PARAM params[], + EC_GROUP_free(group); + group = named_group; + } ++ /* We've imported the group from explicit parameters, set it so. */ ++ group->decoded_from_explicit_params = 1; + ok = 1; + err: + if (!ok) { +diff --git a/doc/man7/EVP_PKEY-EC.pod b/doc/man7/EVP_PKEY-EC.pod +index eed83237c3b2..ee66a074f889 100644 +--- a/doc/man7/EVP_PKEY-EC.pod ++++ b/doc/man7/EVP_PKEY-EC.pod +@@ -70,8 +70,8 @@ I multiplied by the I gives the number of points on the curve. + + =item "decoded-from-explicit" (B) + +-Gets a flag indicating wether the key or parameters were decoded from explicit +-curve parameters. Set to 1 if so or 0 if a named curve was used. ++Sets or gets a flag indicating whether the key or parameters were decoded from ++explicit curve parameters. Set to 1 if so or 0 if a named curve was used. + + =item "use-cofactor-flag" (B) + +diff --git a/providers/implementations/keymgmt/ec_kmgmt.c b/providers/implementations/keymgmt/ec_kmgmt.c +index 9260d4bf3635..7aed057cac89 100644 +--- a/providers/implementations/keymgmt/ec_kmgmt.c ++++ b/providers/implementations/keymgmt/ec_kmgmt.c +@@ -525,7 +525,8 @@ int ec_export(void *keydata, int selection, OSSL_CALLBACK *param_cb, + OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_EC_GENERATOR, NULL, 0), \ + OSSL_PARAM_BN(OSSL_PKEY_PARAM_EC_ORDER, NULL, 0), \ + OSSL_PARAM_BN(OSSL_PKEY_PARAM_EC_COFACTOR, NULL, 0), \ +- OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_EC_SEED, NULL, 0) ++ OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_EC_SEED, NULL, 0), \ ++ OSSL_PARAM_int(OSSL_PKEY_PARAM_EC_DECODED_FROM_EXPLICIT_PARAMS, NULL) + + # define EC_IMEXPORTABLE_PUBLIC_KEY \ + OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_PUB_KEY, NULL, 0) +diff --git a/test/recipes/25-test_verify.t b/test/recipes/25-test_verify.t +index 700bbd849c95..ede14864d5ac 100644 +--- a/test/recipes/25-test_verify.t ++++ b/test/recipes/25-test_verify.t +@@ -12,7 +12,7 @@ use warnings; + + use File::Spec::Functions qw/canonpath/; + use File::Copy; +-use OpenSSL::Test qw/:DEFAULT srctop_file ok_nofips with/; ++use OpenSSL::Test qw/:DEFAULT srctop_file bldtop_dir ok_nofips with/; + use OpenSSL::Test::Utils; + + setup("test_verify"); +@@ -29,7 +29,7 @@ sub verify { + run(app([@args])); + } + +-plan tests => 160; ++plan tests => 163; + + # Canonical success + ok(verify("ee-cert", "sslserver", ["root-cert"], ["ca-cert"]), +@@ -309,6 +309,29 @@ SKIP: { + ["ca-cert-ec-named"]), + "accept named curve leaf with named curve intermediate"); + } ++# Same as above but with base provider used for decoding ++SKIP: { ++ my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0); ++ skip "EC is not supported or FIPS is disabled", 3 ++ if disabled("ec") || $no_fips; ++ ++ my $provconf = srctop_file("test", "fips-and-base.cnf"); ++ my $provpath = bldtop_dir("providers"); ++ my @prov = ("-provider-path", $provpath); ++ $ENV{OPENSSL_CONF} = $provconf; ++ ++ ok(!verify("ee-cert-ec-explicit", "", ["root-cert"], ++ ["ca-cert-ec-named"], @prov), ++ "reject explicit curve leaf with named curve intermediate w/fips"); ++ ok(!verify("ee-cert-ec-named-explicit", "", ["root-cert"], ++ ["ca-cert-ec-explicit"], @prov), ++ "reject named curve leaf with explicit curve intermediate w/fips"); ++ ok(verify("ee-cert-ec-named-named", "", ["root-cert"], ++ ["ca-cert-ec-named"], @prov), ++ "accept named curve leaf with named curve intermediate w/fips"); ++ ++ delete $ENV{OPENSSL_CONF}; ++} + + # Depth tests, note the depth limit bounds the number of CA certificates + # between the trust-anchor and the leaf, so, for example, with a root->ca->leaf diff --git a/0052-Allow-SHA1-in-seclevel-2-if-rh-allow-sha1-signatures.patch b/0052-Allow-SHA1-in-seclevel-2-if-rh-allow-sha1-signatures.patch index c7cb9b7..5208f40 100644 --- a/0052-Allow-SHA1-in-seclevel-2-if-rh-allow-sha1-signatures.patch +++ b/0052-Allow-SHA1-in-seclevel-2-if-rh-allow-sha1-signatures.patch @@ -184,8 +184,8 @@ index 700bbd849c..2de1d76b5e 100644 run(app([@args])); } --plan tests => 160; -+plan tests => 159; +-plan tests => 163; ++plan tests => 162; # Canonical success ok(verify("ee-cert", "sslserver", ["root-cert"], ["ca-cert"]), diff --git a/openssl.spec b/openssl.spec index 64ccc85..b8dd873 100644 --- a/openssl.spec +++ b/openssl.spec @@ -74,6 +74,8 @@ Patch12: 0012-Disable-explicit-ec.patch Patch13: 0013-FIPS-provider-explicit-ec.patch # https://github.com/openssl/openssl/pull/17998 Patch14: 0014-FIPS-disable-explicit-ec.patch +# https://github.com/openssl/openssl/pull/18609 +Patch15: 0015-FIPS-decoded-from-explicit.patch # Instructions to load legacy provider in openssl.cnf Patch24: 0024-load-legacy-prov.patch # Tmp: test name change @@ -470,6 +472,8 @@ install -m644 %{SOURCE9} \ - Related: rhbz#2070197 - Fix PPC64 Montgomery multiplication bug - Related: rhbz#2098199 +- Strict certificates validation shouldn't allow explicit EC parameters +- Related: rhbz#2058663 * Wed Jun 08 2022 Clemens Lang - 1:3.0.1-35 - Add explicit indicators for signatures in FIPS mode and mark signature