Skip to content
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

Fix CompareOcspReqResp. #3990

Merged
merged 1 commit into from Apr 28, 2021
Merged

Fix CompareOcspReqResp. #3990

merged 1 commit into from Apr 28, 2021

Conversation

haydenroche5
Copy link
Contributor

@haydenroche5 haydenroche5 commented Apr 27, 2021

There was a bug in this function that could cause a match to be reported even
when the OCSP request and response in fact had a mismatch.
ZD 12148

There was a bug in this function that could cause a match to be reported even
when the OCSP request and response in fact had a mismatch.
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on the fix making sure cmp is set and also adding a test case. Only optional feedback is the test could make the array's like response const to reduce stack use.

tests/api.c Show resolved Hide resolved
@dgarske dgarske merged commit 385e0be into wolfSSL:master Apr 28, 2021
24 checks passed
@dgarske
Copy link
Contributor

dgarske commented May 3, 2021

@haydenroche5 , this PR now appears to be causing failures in master with --enable-all?

./configure --enable-all && make

./scripts/unit.test
starting unit tests...

-----------------Porting tests------------------
 Begin API Tests
   wolfSSL_Init(): passed
   wolfSSL_CTX_use_certificate_buffer(): passed
   wolfSSL_CertManagerCheckOCSPResponse():
ERROR - tests/api.c line 1254 failed with:
    expected: wolfSSL_CertManagerCheckOCSPResponse(cm, response, sizeof(response), ((void *)0), status, entry, request) == WOLFSSL_SUCCESS
    result:   -367 != 1

Guessing it has to do with the date in the captured OCSP response? Can you look into this? Thanks

@kaleb-himes
Copy link
Contributor

This also appears to be the same issue without having to do enable all:

./configure --enable-jni && make

./tests/unit.test
ERROR - tests/api.c line 1254 failed with:
    expected: wolfSSL_CertManagerCheckOCSPResponse(cm, response, sizeof(response), ((void *)0), status, entry, request) == WOLFSSL_SUCCESS
    result:   -367 != 1
zsh: abort      ./scripts/unit.test

@risicle
Copy link

risicle commented Aug 8, 2021

For anyone else late to the party it seems the test failure got addressed in 822aa92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants