-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make conf_diagnostics apply also to the SSL conf errors #24275
Conversation
This is a draft PR - the CI failures demonstrate that conf_diagnostics work and make SSL_CTX_new to fail. |
Looks good to me in terms of improving what we currently have ... |
I'd like to have some syntax check for the config file as a part of the openssl command line util but it can be a separate PR |
793a8ee
to
0882a91
Compare
The CI failure looks relevant |
The question is whether the failing test really makes sense. It basically loads a config into a single libctx concurrently from multiple threads. What is the semantics of such call? If the same file is being loaded then it could somewhat work, although I would not be surprised if there are some strange circumstances where it would not. However if a different file would be loaded concurrently into the same libctx, what the application can expect then? Random results. I am very much inclined to drop the testcase. |
This pull request is ready to merge |
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
…g file Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
…bctx The semantics of such concurrent call is not defined. Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #24275)
@t8m the fixups appear to be causing some conflicts during merge. I can merge them if you like, but if you can I would prefer you take a look at them |
I've merged this yesterday. Thank you for the reviews. |
make test TESTS=test_sysdefault
...
make[2]: Entering directory '/home/xnox/upstream/openssl'
( SRCTOP=. \
BLDTOP=. \
PERL="/usr/bin/perl" \
FIPSKEY="f4556650ac31d35461610bac4ed81b1a181b2d8a43ea2854cbae22ca74560813" \
EXE_EXT= \
/usr/bin/perl ./test/run_tests.pl test_sysdefault )
00-prep_fipsmodule_cnf.t .. ok
All tests successful.
Files=1, Tests=1, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.27 cusr 0.05 csys = 0.33 CPU)
Result: PASS
90-test_sysdefault.t ..
# ERROR: (ptr) 'ctx = SSL_CTX_new(TLS_method()) != NULL' failed @ test/sysdefaulttest.c:25
# 0x0
# 80CB81B9097E0000:error:0A0001A3:SSL routines:SSL_CTX_new_ex:error in system default config:ssl/ssl_lib.c:4100:
# OPENSSL_TEST_RAND_SEED=1715381166
not ok 1 - test_func
# ------------------------------------------------------------------------------
../../util/wrap.pl ../../test/sysdefaulttest => 1
not ok 1 - sysdefaulttest
90-test_sysdefault.t .. 1/? ----------------------------------------------------
# Failed test 'sysdefaulttest'
# at test/recipes/90-test_sysdefault.t line 23.
90-test_sysdefault.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests
Test Summary Report
-------------------
90-test_sysdefault.t (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
Failed test: 1
Non-zero exit status: 1
Files=1, Tests=1, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.17 cusr 0.04 csys = 0.23 CPU)
Result: FAIL
make[2]: *** [Makefile:3949: run_tests] Error 1
make[2]: Leaving directory '/home/xnox/upstream/openssl'
make[1]: *** [Makefile:3946: _tests] Error 2
make[1]: Leaving directory '/home/xnox/upstream/openssl'
make: *** [Makefile:3944: tests] Error 2 Fails for me, and git bisect blames:
This is on Ubuntu 24.04 host x86-64 with gcc-13. Is my host system config somehow bad? or is the test case looking for something incorrect? or is Ubuntu 24.04 missbuilding things? |
@xnox please open a new issue for this. Is this a clean build without any patches on top? |
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
…g file Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
…bctx The semantics of such concurrent call is not defined. Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl#24275)
Checklist