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

ocsp: propagate ocsp cb return error #8055

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

rizlik
Copy link
Contributor

@rizlik rizlik commented Oct 9, 2024

Description

The error returned from OCSP cb is not propagated.
Propagating the error allows the verify callback to adopt a custom fallback policy regarding the error.

This is unfortunately a breaking change in this scenario:

  • custom cb return a negative value different than OCSP_INVALID_STATUS
  • verify cb relies on OCSP_INVALID_STATUS ret error.

@Dor-kpf
Copy link

Dor-kpf commented Oct 10, 2024

Hi @rizlik,
I tested the PR and it looks like the returned error from the OCSP callback is indeed propagated to the verify callback now. But it looks like the incorrect error code is set in the verifyCallback.
When my OCSP callback returns WOLFSSL_CBIO_ERR_TIMEOUT (-6) it invokes the VerifyCallback with the error: WOLFSSL_ERROR_ZERO_RETURN_E (-6). This is a different error but with the same value of -6. The casting to the right wolfSSL_ErrorCodes enum value is missing.
Looking at the code, I can see that the CheckOcspRequest function casts WOLFSSL_CBIO_ERR_WANT_READ (-2) to OCSP_WANT_READ (-408) right when returning from the call to ocspiocb. Maybe in our case we can cast to HTTP_TIMEOUT (-417)?
Thanks!

@rizlik rizlik force-pushed the ocsp-cb-ret-propagate branch from 174657a to 1572e4a Compare October 14, 2024 15:49
@rizlik
Copy link
Contributor Author

rizlik commented Oct 14, 2024

Hi @Dor-kpf ,
You are right! Now the PR should map to the right error.
Thanks

@rizlik
Copy link
Contributor Author

rizlik commented Oct 15, 2024

retest this please

@Dor-kpf
Copy link

Dor-kpf commented Oct 15, 2024

Hi @rizlik,
I have tested it and it looks good!
In my OCSP callback I have returned WOLFSSL_CBIO_ERR_TIMEOUT (-6) and then my verify callback was later called and the error was propagated correctly (the reported error is HTTP_TIMEOUT (-417) as expected). In my verify callback I check if I use the soft/hard fail approach and then continue/abort the TLS handshake accordingly.

@rizlik rizlik force-pushed the ocsp-cb-ret-propagate branch from 1572e4a to 724fdae Compare October 15, 2024 10:03
@rizlik rizlik marked this pull request as ready for review October 15, 2024 10:48
@rizlik rizlik assigned wolfSSL-Bot and unassigned rizlik Oct 15, 2024
@douzzer
Copy link
Contributor

douzzer commented Oct 15, 2024

retest this please

@douzzer douzzer merged commit c714664 into wolfSSL:master Oct 15, 2024
139 of 140 checks passed
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.

4 participants