openssl compat: match openssl status cb behaviour for stapling #8036
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
rizlik
force-pushed
the
ocsp_ossl_status_cb
branch
8 times, most recently
from
45ca911 to
3ff26ca
Compare
rizlik
force-pushed
the
ocsp_ossl_status_cb
branch
3 times, most recently
from
dd22ee5 to
825fe1a
Compare
|
retest this please |
douzzer
requested changes
Oct 4, 2024
There was a problem hiding this comment.
One of our weirder test scenarios found something to complain about:
[quantum-safe-wolfssl-all-g++-latest-debug] [7 of 37] [0f9c9c4e18]
configure... real 0m24.537s user 0m11.589s sys 0m14.789s
build...In file included from ./wolfssl/wolfcrypt/logging.h:33,
from tests/api.c:39:
tests/api.c: In function ‘int test_ocsp_status_callback_cb(WOLFSSL*, void*)’:
9386a882b9 (<[email protected]> 2021-10-19 15:51:29 +0200 594) #define XMALLOC(s, h, t) ((void)(h), (void)(t), wolfSSL_Malloc((s)))
./wolfssl/wolfcrypt/types.h:594:67: error: invalid conversion from ‘void*’ to ‘byte*’ {aka ‘unsigned char*’} [-fpermissive]
594 | #define XMALLOC(s, h, t) ((void)(h), (void)(t), wolfSSL_Malloc((s)))
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
| |
| void*
0f9c9c4e18 (<[email protected]> 2024-10-02 11:35:11 +0000 96519) allocated = XMALLOC(_ctx->ocsp_resp_sz, NULL, 0);
tests/api.c:96519:17: note: in expansion of macro ‘XMALLOC’
96519 | allocated = XMALLOC(_ctx->ocsp_resp_sz, NULL, 0);
| ^~~~~~~
[...]
failed config: '--srcdir' '.' '--disable-jobserver' '--enable-option-checking=fatal' '--enable-all' '--enable-testcert' '--enable-debug' '--enable-debug-trace-errcodes' '--enable-sp-math-all' '--enable-experimental' '--enable-kyber' '--enable-lms' '--enable-xmss' '--enable-dilithium' '--disable-qt' 'CPPFLAGS=-DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK' 'CC=g++-15'
There may be more to come, but I wanted to stick a pin in it with this nit.
On the server side, OpenSSL offloads the creation and the verification of the ocsp response to staple to the user application. This commit moves the invocation point of the StatusCB so that wolfSSL doesn't try to verify the response and/or the server certificate.
rizlik
force-pushed
the
ocsp_ossl_status_cb
branch
from
825fe1a to
a2bf2ce
Compare
|
fix confirmed by the reporter |
This was referenced Oct 8, 2024
douzzer
requested changes
Oct 8, 2024
There was a problem hiding this comment.
cppcheck founds defects. FWIW, succeeded: quantum-safe-wolfssl-all-g++-latest-debug quantum-safe-wolfssl-all-clang-tidy quantum-safe-wolfssl-all-intelasm-sp-asm-sanitizer
reproduce with ../testing/git-hooks/wolfssl-multi-test.sh --enable-git-blame --no-result-cache --test-uncommitted quantum-safe-wolfssl-all-cppcheck
[quantum-safe-wolfssl-all-cppcheck] [2 of 4] [ea71e4b403]
configure... real 0m17.455s user 0m6.889s sys 0m12.361s
cppcheck...ec9d23a9c3 (<[email protected]> 2015-12-28 19:38:04 -0300 3514) if (ssl->status_request_v2)
src/tls.c:3514:13: warning: Either the condition 'ssl!=((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
if (ssl->status_request_v2)
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 3527) if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
src/tls.c:3527:21: note: Assuming that condition 'ssl!=((void*)0)' is not redundant
if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
^
ec9d23a9c3 (<[email protected]> 2015-12-28 19:38:04 -0300 3514) if (ssl->status_request_v2)
src/tls.c:3514:13: note: Null pointer dereference
if (ssl->status_request_v2)
^
0bf3a89992 (<[email protected]> 2018-07-02 16:59:23 +1000 3520) 0, ssl, ssl->heap, ssl->devId);
src/tls.c:3520:58: warning: Either the condition 'ssl!=((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
0, ssl, ssl->heap, ssl->devId);
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 3527) if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
src/tls.c:3527:21: note: Assuming that condition 'ssl!=((void*)0)' is not redundant
if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
^
0bf3a89992 (<[email protected]> 2018-07-02 16:59:23 +1000 3520) 0, ssl, ssl->heap, ssl->devId);
src/tls.c:3520:58: note: Null pointer dereference
0, ssl, ssl->heap, ssl->devId);
^
9af9941b90 (<[email protected]> 2019-07-16 12:10:58 +1000 3525) if (ssl->options.tls1_3) {
src/tls.c:3525:13: warning: Either the condition 'ssl!=((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
if (ssl->options.tls1_3) {
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 3527) if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
src/tls.c:3527:21: note: Assuming that condition 'ssl!=((void*)0)' is not redundant
if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
^
9af9941b90 (<[email protected]> 2019-07-16 12:10:58 +1000 3525) if (ssl->options.tls1_3) {
src/tls.c:3525:13: note: Null pointer dereference
if (ssl->options.tls1_3) {
^
0a928208f2 (<[email protected]> 2023-02-22 08:19:11 -0600 3532) if (ssl->buffers.certificate == NULL) {
src/tls.c:3532:17: warning: Either the condition 'ssl!=((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
if (ssl->buffers.certificate == NULL) {
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 3527) if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
src/tls.c:3527:21: note: Assuming that condition 'ssl!=((void*)0)' is not redundant
if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL
^
0a928208f2 (<[email protected]> 2023-02-22 08:19:11 -0600 3532) if (ssl->buffers.certificate == NULL) {
src/tls.c:3532:17: note: Null pointer dereference
if (ssl->buffers.certificate == NULL) {
^
4412496adb (<[email protected]> 2022-03-23 11:20:04 +0100 3991) if (ssl->status_request) {
src/tls.c:3991:17: warning: Either the condition 'ssl!=((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
if (ssl->status_request) {
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 3983) if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL &&
src/tls.c:3983:21: note: Assuming that condition 'ssl!=((void*)0)' is not redundant
if (ssl != NULL && SSL_CM(ssl) != NULL && SSL_CM(ssl)->ocsp_stapling != NULL &&
^
4412496adb (<[email protected]> 2022-03-23 11:20:04 +0100 3991) if (ssl->status_request) {
src/tls.c:3991:17: note: Null pointer dereference
if (ssl->status_request) {
^
c1640e8a3d (<[email protected]> 2017-04-07 15:46:32 -0700 24253) status_type = ssl->status_request;
src/internal.c:24253:19: warning: Either the condition 'ssl==((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
status_type = ssl->status_request;
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 24259) if (ssl == NULL || SSL_CM(ssl) == NULL) {
src/internal.c:24259:13: note: Assuming that condition 'ssl==((void*)0)' is not redundant
if (ssl == NULL || SSL_CM(ssl) == NULL) {
^
c1640e8a3d (<[email protected]> 2017-04-07 15:46:32 -0700 24253) status_type = ssl->status_request;
src/internal.c:24253:19: note: Null pointer dereference
status_type = ssl->status_request;
^
c1640e8a3d (<[email protected]> 2017-04-07 15:46:32 -0700 24257) status_type = status_type ? status_type : ssl->status_request_v2;
src/internal.c:24257:47: warning: Either the condition 'ssl==((void*)0)' is redundant or there is possible null pointer dereference: ssl. [nullPointerRedundantCheck]
status_type = status_type ? status_type : ssl->status_request_v2;
^
ea71e4b403 (<[email protected]> 2024-10-02 11:35:11 +0000 24259) if (ssl == NULL || SSL_CM(ssl) == NULL) {
src/internal.c:24259:13: note: Assuming that condition 'ssl==((void*)0)' is not redundant
if (ssl == NULL || SSL_CM(ssl) == NULL) {
^
c1640e8a3d (<[email protected]> 2017-04-07 15:46:32 -0700 24257) status_type = status_type ? status_type : ssl->status_request_v2;
src/internal.c:24257:47: note: Null pointer dereference
status_type = status_type ? status_type : ssl->status_request_v2;
^
real 2m51.220s user 23m37.972s sys 1m15.379s
quantum-safe-wolfssl-all-cppcheck fail_analysis
|
please wait before merging |
|
@rizlik please resolve merge conflicts. Assigning to you only as you said: "please wait before merging" |
|
indeed, we need some rework in OCSP_basic_verify as well before merging this. I'm closing it for now |
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.
Description
On the server side, OpenSSL offloads the creation and the verification of
the ocsp response to staple to the user application. This commit moves
the invocation point of the StatusCB so that wolfSSL doesn't try to
verify the response and/or the server certificate.
Fixes zd#18703