Have OSSL_PARAM_allocate_from_text() fail on odd number of hex digits #23374
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. Fixes openssl#23373
github-actions
bot
added
the
severity: fips change
levitte
added
branch: master
|
Test coming up |
… digits This adds a test that I hope is comprehensive enough
|
With this PR, the added test succeeds. Applied alone on master, it fails like this: $ make test TESTS=test_params
make depend && make _tests
make[1]: Entering directory '/home/levitte/gitwrk/openssl.net/official/_build'
make[1]: Leaving directory '/home/levitte/gitwrk/openssl.net/official/_build'
make[1]: Entering directory '/home/levitte/gitwrk/openssl.net/official/_build'
make run_tests
make[2]: Entering directory '/home/levitte/gitwrk/openssl.net/official/_build'
( SRCTOP=../master \
BLDTOP=. \
PERL="/usr/bin/perl" \
FIPSKEY="f4556650ac31d35461610bac4ed81b1a181b2d8a43ea2854cbae22ca74560813" \
EXE_EXT= \
/usr/bin/perl ../master/test/run_tests.pl test_params )
00-prep_fipsmodule_cnf.t .. ok
All tests successful.
Files=1, Tests=1, 1 wallclock secs ( 0.00 usr 0.01 sys + 0.26 cusr 0.02 csys = 0.29 CPU)
Result: PASS
04-test_params.t ..
# ERROR: (bool) 'OSSL_PARAM_allocate_from_text(¶m, params_from_text, "hexoctets", values[i], 0, NULL) == false' failed @ ../master/test/params_test.c:682
# true
# ERROR: @ ../master/test/params_test.c:687
# unexpected OSSL_PARAM_allocate_from_text() success for 'octets' "F"
#
# OPENSSL_TEST_RAND_SEED=1706016382
not ok 3 - test_more_allocate_from_text
# ------------------------------------------------------------------------------
../../util/wrap.pl ../../test/params_test => 1
not ok 1 - running params_test
04-test_params.t .. 1/? --------------------------------------------------------
# Failed test 'running params_test'
# at /home/levitte/gitwrk/openssl.net/official/_build/../master/util/perl/OpenSSL/Test/Simple.pm line 77.
04-test_params.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests
Test Summary Report
-------------------
04-test_params.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.00 sys + 0.19 cusr 0.00 csys = 0.20 CPU)
Result: FAIL
make[2]: *** [Makefile:3858: run_tests] Error 1
make[2]: Leaving directory '/home/levitte/gitwrk/openssl.net/official/_build'
make[1]: *** [Makefile:3855: _tests] Error 2
make[1]: Leaving directory '/home/levitte/gitwrk/openssl.net/official/_build'
make: *** [Makefile:3853: tests] Fel 2 |
t8m
requested changes
Jan 23, 2024
crypto/params_from_text.c
Outdated
| *buf_n = strlen(value) >> 1; | ||
| size_t hexdigits = strlen(value); | ||
| if ((hexdigits % 2) != 0) { | ||
| /* We don't accept an off number of hex digits */ |
t8m
approved these changes
Jan 24, 2024
tom-cosgrove-arm
approved these changes
Jan 24, 2024
tom-cosgrove-arm
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
openssl-machine
pushed a commit
that referenced
this pull request
Jan 25, 2024
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. Fixes #23373 Reviewed-by: Tom Cosgrove <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #23374)
openssl-machine
pushed a commit
that referenced
this pull request
Jan 25, 2024
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. Fixes #23373 Reviewed-by: Tom Cosgrove <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #23374) (cherry picked from commit ea6268c)
|
Merged to the master, 3.2, 3.1 and 3.0 branches. Thank you. |
openssl-machine
pushed a commit
that referenced
this pull request
Jan 25, 2024
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. Fixes #23373 Reviewed-by: Tom Cosgrove <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #23374) (cherry picked from commit ea6268c)
openssl-machine
pushed a commit
that referenced
this pull request
Jan 25, 2024
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. Fixes #23373 Reviewed-by: Tom Cosgrove <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #23374) (cherry picked from commit ea6268c)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The failure would be caught later on, so this went unnoticed, until someone
tried with just one hex digit, which was simply ignored.
Fixes #23373