The branch master has been updated via 96d6a4e4464c3dec9cdf2c6fb59d737c8cb1be49 (commit) via ca7cac886b0f1084acfe2e07135acd212415e2bd (commit) via 589fbc18aa5e72b2574a71d69c09b4f63f0ae943 (commit) via 123ed334337e874acb1f55b36dc671de7e306824 (commit) via 09f38299ccc006e0ce7e94897250e995ec2fc337 (commit) from 2a7855fb2596048e5038afa5e49a02853297df6d (commit)
- Log ----------------------------------------------------------------- commit 96d6a4e4464c3dec9cdf2c6fb59d737c8cb1be49 Author: Pauli <pa...@openssl.org> Date: Wed Jul 28 09:52:23 2021 +1000 test: add a comment indication that a bad MAC is intentional This permits negative testing of FIPS module load failure. Also changed the MAC to all zeros to make it even clearer. Reviewed-by: Tim Hudson <t...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16168) commit ca7cac886b0f1084acfe2e07135acd212415e2bd Author: Matt Caswell <m...@openssl.org> Date: Tue Jul 27 16:36:41 2021 +0100 Add some testing for the case where the FIPS provider fails to load Ensure we get correct behaviour in the event that an attempt is made to load the fips provider but it fails to load. Reviewed-by: Tim Hudson <t...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16168) commit 589fbc18aa5e72b2574a71d69c09b4f63f0ae943 Author: Matt Caswell <m...@openssl.org> Date: Tue Jul 27 16:59:59 2021 +0100 Don't try and load the config file while already loading the config file Calls to the API function EVP_default_properties_enable_fips() will automatically attempt to load the default config file if it is not already loaded. Therefore this function should not be called from inside code to process the config file. Fixes #16165 Reviewed-by: Tim Hudson <t...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16168) commit 123ed334337e874acb1f55b36dc671de7e306824 Author: Matt Caswell <m...@openssl.org> Date: Tue Jul 27 16:36:24 2021 +0100 Ensure any default_properties still apply even in the event of a provider load failure We don't treat a failure to load a provider as a fatal error. If it is fatal then we give up attempting to load the config file - including reading any default properties. Additionally if an attempt has been made to load a provider then we disable fallback loading. Fixes #16166 Reviewed-by: Tim Hudson <t...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16168) commit 09f38299ccc006e0ce7e94897250e995ec2fc337 Author: Matt Caswell <m...@openssl.org> Date: Tue Jul 27 16:31:20 2021 +0100 Don't leak the OSSL_LIB_CTX in the event of a failure to load the FIPS module Ensure we free the OSSL_LIB_CTX on the error path. Fixes #16163 Reviewed-by: Tim Hudson <t...@openssl.org> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16168) ----------------------------------------------------------------------- Summary of changes: crypto/evp/evp_cnf.c | 4 +-- crypto/evp/evp_fetch.c | 17 ++++++++---- crypto/provider_conf.c | 16 ++++++++++- include/crypto/evp.h | 2 ++ providers/fips/fipsprov.c | 1 + test/defltfips_test.c | 39 +++++++++++++++++++-------- test/{fips.cnf => fips-alt.cnf} | 2 +- test/recipes/30-test_defltfips.t | 19 ++++++++++--- test/recipes/30-test_defltfips/fipsmodule.cnf | 7 +++++ 9 files changed, 84 insertions(+), 23 deletions(-) copy test/{fips.cnf => fips-alt.cnf} (91%) create mode 100644 test/recipes/30-test_defltfips/fipsmodule.cnf diff --git a/crypto/evp/evp_cnf.c b/crypto/evp/evp_cnf.c index 415712dffa..0e7fe64cf9 100644 --- a/crypto/evp/evp_cnf.c +++ b/crypto/evp/evp_cnf.c @@ -46,8 +46,8 @@ static int alg_module_init(CONF_IMODULE *md, const CONF *cnf) * fips_mode is deprecated and should not be used in new * configurations. */ - if (!EVP_default_properties_enable_fips(NCONF_get0_libctx((CONF *)cnf), - m > 0)) { + if (!evp_default_properties_enable_fips_int( + NCONF_get0_libctx((CONF *)cnf), m > 0, 0)) { ERR_raise(ERR_LIB_EVP, EVP_R_SET_DEFAULT_PROPERTY_FAILURE); return 0; } diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c index 3067928030..5303cf8859 100644 --- a/crypto/evp/evp_fetch.c +++ b/crypto/evp/evp_fetch.c @@ -479,15 +479,16 @@ int EVP_set_default_properties(OSSL_LIB_CTX *libctx, const char *propq) return evp_set_default_properties_int(libctx, propq, 1, 0); } -static int evp_default_properties_merge(OSSL_LIB_CTX *libctx, const char *propq) +static int evp_default_properties_merge(OSSL_LIB_CTX *libctx, const char *propq, + int loadconfig) { - OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(libctx, 1); + OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(libctx, loadconfig); OSSL_PROPERTY_LIST *pl1, *pl2; if (propq == NULL) return 1; if (plp == NULL || *plp == NULL) - return EVP_set_default_properties(libctx, propq); + return evp_set_default_properties_int(libctx, propq, 0, 0); if ((pl1 = ossl_parse_query(libctx, propq, 1)) == NULL) { ERR_raise(ERR_LIB_EVP, EVP_R_DEFAULT_QUERY_PARSE_ERROR); return 0; @@ -518,11 +519,17 @@ int EVP_default_properties_is_fips_enabled(OSSL_LIB_CTX *libctx) return evp_default_property_is_enabled(libctx, "fips"); } -int EVP_default_properties_enable_fips(OSSL_LIB_CTX *libctx, int enable) +int evp_default_properties_enable_fips_int(OSSL_LIB_CTX *libctx, int enable, + int loadconfig) { const char *query = (enable != 0) ? "fips=yes" : "-fips"; - return evp_default_properties_merge(libctx, query); + return evp_default_properties_merge(libctx, query, loadconfig); +} + +int EVP_default_properties_enable_fips(OSSL_LIB_CTX *libctx, int enable) +{ + return evp_default_properties_enable_fips_int(libctx, enable, 1); } char *evp_get_global_properties_str(OSSL_LIB_CTX *libctx, int loadconfig) diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c index 1d4e695fb8..fe66e1158e 100644 --- a/crypto/provider_conf.c +++ b/crypto/provider_conf.c @@ -156,6 +156,16 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name, } if (activate) { + /* + * There is an attempt to activate a provider, so we should disable + * loading of fallbacks. Otherwise a misconfiguration could mean the + * intended provider does not get loaded. Subsequent fetches could then + * fallback to the default provider - which may be the wrong thing. + */ + if (!ossl_provider_disable_fallback_loading(libctx)) { + ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR); + return 0; + } prov = ossl_provider_find(libctx, name, 1); if (prov == NULL) prov = ossl_provider_new(libctx, name, NULL, 1); @@ -215,7 +225,11 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name, } - return ok; + /* + * Even if ok is 0, we still return success. Failure to load a provider is + * not fatal. We want to continue to load the rest of the config file. + */ + return 1; } static int provider_conf_init(CONF_IMODULE *md, const CONF *cnf) diff --git a/include/crypto/evp.h b/include/crypto/evp.h index 68aab33cae..41ac80ed9d 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -891,6 +891,8 @@ int evp_pkey_ctx_use_cached_data(EVP_PKEY_CTX *ctx); # endif /* !defined(FIPS_MODULE) */ int evp_method_store_flush(OSSL_LIB_CTX *libctx); +int evp_default_properties_enable_fips_int(OSSL_LIB_CTX *libctx, int enable, + int loadconfig); int evp_set_default_properties_int(OSSL_LIB_CTX *libctx, const char *propq, int loadconfig, int mirrored); char *evp_get_global_properties_str(OSSL_LIB_CTX *libctx, int loadconfig); diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c index 4155b64197..b69299e046 100644 --- a/providers/fips/fipsprov.c +++ b/providers/fips/fipsprov.c @@ -712,6 +712,7 @@ int OSSL_provider_init_int(const OSSL_CORE_HANDLE *handle, return 1; err: fips_teardown(*provctx); + OSSL_LIB_CTX_free(libctx); *provctx = NULL; return 0; } diff --git a/test/defltfips_test.c b/test/defltfips_test.c index 21c5e1524d..8b6dc0d6f1 100644 --- a/test/defltfips_test.c +++ b/test/defltfips_test.c @@ -4,6 +4,7 @@ #include "testutil.h" static int is_fips; +static int bad_fips; static int test_is_fips_enabled(void) { @@ -24,8 +25,8 @@ static int test_is_fips_enabled(void) * on the default properties. However we only set those properties if also * loading the FIPS provider. */ - if (!TEST_int_eq(is_fips, is_fips_enabled) - || !TEST_int_eq(is_fips, is_fips_loaded)) + if (!TEST_int_eq(is_fips || bad_fips, is_fips_enabled) + || !TEST_int_eq(is_fips && !bad_fips, is_fips_loaded)) return 0; /* @@ -33,19 +34,26 @@ static int test_is_fips_enabled(void) * expected provider. */ sha256 = EVP_MD_fetch(NULL, "SHA2-256", NULL); - if (!TEST_ptr(sha256)) - return 0; - if (is_fips - && !TEST_str_eq(OSSL_PROVIDER_get0_name(EVP_MD_get0_provider(sha256)), - "fips")) { + if (bad_fips) { + if (!TEST_ptr_null(sha256)) { + EVP_MD_free(sha256); + return 0; + } + } else { + if (!TEST_ptr(sha256)) + return 0; + if (is_fips + && !TEST_str_eq(OSSL_PROVIDER_get0_name(EVP_MD_get0_provider(sha256)), + "fips")) { + EVP_MD_free(sha256); + return 0; + } EVP_MD_free(sha256); - return 0; } - EVP_MD_free(sha256); /* State should still be consistent */ is_fips_enabled = EVP_default_properties_is_fips_enabled(NULL); - if (!TEST_int_eq(is_fips, is_fips_enabled)) + if (!TEST_int_eq(is_fips || bad_fips, is_fips_enabled)) return 0; return 1; @@ -54,6 +62,7 @@ static int test_is_fips_enabled(void) int setup_tests(void) { size_t argc; + char *arg1; if (!test_skip_common_options()) { TEST_error("Error parsing test options\n"); @@ -64,10 +73,18 @@ int setup_tests(void) switch(argc) { case 0: is_fips = 0; + bad_fips = 0; break; case 1: - if (strcmp(test_get_argument(0), "fips") == 0) { + arg1 = test_get_argument(0); + if (strcmp(arg1, "fips") == 0) { is_fips = 1; + bad_fips = 0; + break; + } else if (strcmp(arg1, "badfips") == 0) { + /* Configured for FIPS, but the module fails to load */ + is_fips = 0; + bad_fips = 1; break; } /* fall through */ diff --git a/test/fips.cnf b/test/fips-alt.cnf similarity index 91% copy from test/fips.cnf copy to test/fips-alt.cnf index fa131a8bf6..17889372c7 100644 --- a/test/fips.cnf +++ b/test/fips-alt.cnf @@ -10,7 +10,7 @@ alg_section = evp_properties # Ensure FIPS non-approved algorithms in the FIPS module are suppressed (e.g. # TEST-RAND). This also means that EVP_default_properties_is_fips_enabled() # returns the expected value -default_properties = "fips=yes" +fips_mode = true [provider_sect] fips = fips_sect diff --git a/test/recipes/30-test_defltfips.t b/test/recipes/30-test_defltfips.t index 73bb4bce9c..f0338bb3e0 100644 --- a/test/recipes/30-test_defltfips.t +++ b/test/recipes/30-test_defltfips.t @@ -10,12 +10,12 @@ use strict; use warnings; -use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_file bldtop_dir/; +use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_file bldtop_dir data_dir/; use OpenSSL::Test::Utils; use Cwd qw(abs_path); BEGIN { - setup("test_evp"); + setup("test_defltfips"); } use lib srctop_dir('Configurations'); @@ -24,11 +24,24 @@ use lib bldtop_dir('.'); my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0); plan tests => - ($no_fips ? 1 : 2); + ($no_fips ? 1 : 5); unless ($no_fips) { $ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "fips.cnf")); ok(run(test(["defltfips_test", "fips"])), "running defltfips_test fips"); + + #Test an alternative way of configuring fips + $ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "fips-alt.cnf")); + ok(run(test(["defltfips_test", "fips"])), "running defltfips_test fips"); + + #Configured to run FIPS but the module-mac is bad + $ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "fips.cnf")); + $ENV{OPENSSL_CONF_INCLUDE} = srctop_file("test", "recipes", "30-test_defltfips"); + ok(run(test(["defltfips_test", "badfips"])), "running defltfips_test badfips"); + + #Test an alternative way of configuring fips (but still with bad module-mac) + $ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "fips-alt.cnf")); + ok(run(test(["defltfips_test", "badfips"])), "running defltfips_test badfips"); } $ENV{OPENSSL_CONF} = abs_path(srctop_file("test", "default.cnf")); diff --git a/test/recipes/30-test_defltfips/fipsmodule.cnf b/test/recipes/30-test_defltfips/fipsmodule.cnf new file mode 100644 index 0000000000..14d26d8e56 --- /dev/null +++ b/test/recipes/30-test_defltfips/fipsmodule.cnf @@ -0,0 +1,7 @@ +#The MAC here is meant to be incorrect, do not modify it + +[fips_sect] +activate = 1 +conditional-errors = 1 +security-checks = 1 +module-mac = 00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00