diff options
author | Neil Horman <nhorman@openssl.org> | 2023-11-30 14:28:09 -0500 |
---|---|---|
committer | Neil Horman <nhorman@openssl.org> | 2023-12-21 13:38:31 -0500 |
commit | 682fd21afb5428b5716e62eaefb09a7419f9cfd7 (patch) | |
tree | 71fb0a05bcde1b208ae8c47ddb2ded95e9bc284c /test | |
parent | 506ff20662a228b17840f0b49865a927a45c2908 (diff) | |
download | openssl-682fd21afb5428b5716e62eaefb09a7419f9cfd7.zip openssl-682fd21afb5428b5716e62eaefb09a7419f9cfd7.tar.gz openssl-682fd21afb5428b5716e62eaefb09a7419f9cfd7.tar.bz2 |
Detect and prevent recursive config parsing
If a malformed config file is provided such as the following:
openssl_conf = openssl_init
[openssl_init]
providers = provider_sect
[provider_sect]
= provider_sect
The config parsing library will crash overflowing the stack, as it
recursively parses the same provider_sect ad nauseum.
Prevent this by maintaing a list of visited nodes as we recurse through
referenced sections, and erroring out in the event we visit any given
section node more than once.
Note, adding the test for this revealed that our diagnostic code
inadvertently pops recorded errors off the error stack because
provider_conf_load returns success even in the event that a
configuration parse failed. The call path to provider_conf_load has been
updated in this commit to address that shortcoming, allowing recorded
errors to be visibile to calling applications.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22898)
Diffstat (limited to 'test')
-rw-r--r-- | test/build.info | 8 | ||||
-rw-r--r-- | test/prov_config_test.c | 30 | ||||
-rw-r--r-- | test/recipes/30-test_prov_config.t | 7 | ||||
-rw-r--r-- | test/recursive.cnf | 8 |
4 files changed, 47 insertions, 6 deletions
diff --git a/test/build.info b/test/build.info index af43407..88620b8 100644 --- a/test/build.info +++ b/test/build.info @@ -60,10 +60,10 @@ IF[{- !$disabled{tests} -}] context_internal_test aesgcmtest params_test evp_pkey_dparams_test \ keymgmt_internal_test hexstr_test provider_status_test defltfips_test \ bio_readbuffer_test user_property_test pkcs7_test upcallstest \ - provfetchtest prov_config_test rand_test ca_internals_test \ - bio_tfo_test membio_test bio_dgram_test list_test fips_version_test \ - x509_test hpke_test pairwise_fail_test nodefltctxtest \ - evp_xof_test x509_load_cert_file_test + provfetchtest prov_config_test rand_test \ + ca_internals_test bio_tfo_test membio_test bio_dgram_test list_test \ + fips_version_test x509_test hpke_test pairwise_fail_test \ + nodefltctxtest evp_xof_test x509_load_cert_file_test IF[{- !$disabled{'rpk'} -}] PROGRAMS{noinst}=rpktest diff --git a/test/prov_config_test.c b/test/prov_config_test.c index 4b04211..b44ec78 100644 --- a/test/prov_config_test.c +++ b/test/prov_config_test.c @@ -8,9 +8,11 @@ */ #include <openssl/evp.h> +#include <openssl/conf.h> #include "testutil.h" static char *configfile = NULL; +static char *recurseconfigfile = NULL; /* * Test to make sure there are no leaks or failures from loading the config @@ -44,6 +46,30 @@ static int test_double_config(void) return testresult; } +static int test_recursive_config(void) +{ + OSSL_LIB_CTX *ctx = OSSL_LIB_CTX_new(); + int testresult = 0; + unsigned long err; + + if (!TEST_ptr(recurseconfigfile)) + goto err; + + if (!TEST_ptr(ctx)) + goto err; + + if (!TEST_false(OSSL_LIB_CTX_load_config(ctx, recurseconfigfile))) + goto err; + + err = ERR_peek_error(); + /* We expect to get a recursion error here */ + if (ERR_GET_REASON(err) == CONF_R_RECURSIVE_SECTION_REFERENCE) + testresult = 1; + err: + OSSL_LIB_CTX_free(ctx); + return testresult; +} + OPT_TEST_DECLARE_USAGE("configfile\n") int setup_tests(void) @@ -56,6 +82,10 @@ int setup_tests(void) if (!TEST_ptr(configfile = test_get_argument(0))) return 0; + if (!TEST_ptr(recurseconfigfile = test_get_argument(1))) + return 0; + + ADD_TEST(test_recursive_config); ADD_TEST(test_double_config); return 1; } diff --git a/test/recipes/30-test_prov_config.t b/test/recipes/30-test_prov_config.t index f97a26d..7f6350f 100644 --- a/test/recipes/30-test_prov_config.t +++ b/test/recipes/30-test_prov_config.t @@ -22,11 +22,14 @@ my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0); plan tests => 2; -ok(run(test(["prov_config_test", srctop_file("test", "default.cnf")])), +ok(run(test(["prov_config_test", srctop_file("test", "default.cnf"), + srctop_file("test", "recursive.cnf")])), "running prov_config_test default.cnf"); + SKIP: { skip "Skipping FIPS test in this build", 1 if $no_fips; - ok(run(test(["prov_config_test", srctop_file("test", "fips.cnf")])), + ok(run(test(["prov_config_test", srctop_file("test", "fips.cnf"), + srctop_file("test", "recursive.cnf")])), "running prov_config_test fips.cnf"); } diff --git a/test/recursive.cnf b/test/recursive.cnf new file mode 100644 index 0000000..505733a --- /dev/null +++ b/test/recursive.cnf @@ -0,0 +1,8 @@ +openssl_conf = openssl_init +config_diagnostics = yes + +[openssl_init] +providers = provider_sect + +[provider_sect] + = provider_sect |