Conversation
| // TODO: Implement OCSP revocation check of OCSP signer certificate | ||
| // Note: This needs special care to prevent endless loops on specifically | ||
| // forged chains of OCSP responses referring to each other. |
There was a problem hiding this comment.
I figure this is worth discussing in more detail and might go beyond the scope of this refactoring. Nevertheless, please bear with me:
OCSP responder certificates can again offer an OCSP responder in their AIA extension. Clients compliant with RFC 6960 4.2.2.2.1 (and TR-02103 8.5.5 (in German) for that matter) are obliged to recursively validate OCSP responses.
Currently, Botan's API cannot really support this for applications where check_ocsp_online() is not an option -- i.e. that are forced to provide all relevant OCSP responses up-front. x509_path_validate() takes vectors of X509_Certificate and OCSP::Response objects. The contents of those vectors must be in "lock step", i.e. the i-th OCSP::Response refers to the i-th X509_Certificate. Hence, there's no way an application could insert OCSP responses for "higher order" revocation checks of OCSP responder certificate as they don't actually show up in the invocation of x509_path_validate() in the first place.
Assuming that these "higher order" OCSP responses are quite a corner case anyway, I guess it should be fine to just not support them for now.
Though, I believe the API shortcoming is something we might want to consider. Even without the context of "higher order OCSP responses".
Applications that cannot rely on check_ocsp_online() and use the X.509 validation independently from TLS currently need to duplicate a fair share of the certificate path-building code. They must figure out themselves which certificates and OCSP responses are needed for x509_path_validate() to succeed. When working on Bdrive, we used to maintain quite some machinery to make this work (even without the "higher order" OCSP responses).
It would be great if the X.509 path validation would provide hooks to query an application for missing bits and pieces (mainly intermediate certs or OCSP responses).
There was a problem hiding this comment.
Thanks for providing the context on this. I agree this seems a deficiency in the API. Can you open an issue so we can track this independently of this PR? Just copying above text is sufficient.
It would be easy enough to allow an application to provide a std::function<std::optional<X509_Certificate> (Some_Context)> that is called when we are missing a certificate. And likewise OCSP responses. However what contextual information an application might need in order to find the desired data is unclear to me at this time.
reneme
force-pushed
the
feature/overhaul_ocsp_validation
branch
from
a2cd00d to
9cfc118
Compare
* introduce `trusted_ocsp_responders` in `Path_Validation_Restrictions` allowing applications to specify inherently trusted OCSP responder certificates * Introduce `OCSP::Response::find_signing_certificate()` to centralize the search logic for the various possible OCSP responder certs * Remove `OCSP::Response::check_signature()` obsoleted by explicitly calling `::find_signing_certificate()` followed by `::verify_signature()` * Perform path validation on authorized OCSP responder certificates see `verify_ocsp_signing_cert()` in x509path.cpp
reneme
force-pushed
the
feature/overhaul_ocsp_validation
branch
from
9cfc118 to
395e597
Compare
Codecov ReportBase: 92.56% // Head: 92.61% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3067 +/- ##
==========================================
+ Coverage 92.56% 92.61% +0.04%
==========================================
Files 602 602
Lines 70650 70805 +155
Branches 6662 6656 -6
==========================================
+ Hits 65398 65575 +177
+ Misses 5222 5200 -22
Partials 30 30
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
From my perspective this is ready to go. This PR was just moved out of the former security advisory. Edit: I know I promised you to look into moving the test data generation scripts over to Botan CLI. But unfortunately I don't think I'll have the time to do that. :( Do you think, its okay to leave the OpenSSL-based scripts in the repo for future reference? |
|
Re the script, please move it to |
randombit
left a comment
randombit
left a comment
There was a problem hiding this comment.
Sorry for the delay. Ship it.
| // TODO: Implement OCSP revocation check of OCSP signer certificate | ||
| // Note: This needs special care to prevent endless loops on specifically | ||
| // forged chains of OCSP responses referring to each other. |
There was a problem hiding this comment.
Thanks for providing the context on this. I agree this seems a deficiency in the API. Can you open an issue so we can track this independently of this PR? Just copying above text is sufficient.
It would be easy enough to allow an application to provide a std::function<std::optional<X509_Certificate> (Some_Context)> that is called when we are missing a certificate. And likewise OCSP responses. However what contextual information an application might need in order to find the desired data is unclear to me at this time.
3 participants
This refactors the OCSP validation code to allow for easier validation of authorized OCSP responder certificates and improves on the mitigation for CVE-2022-43705 in Botan 2.19.3.
Also, this introduces the "OCSP NoCheck" extension (RFC 6960 4.2.2.2.1) that may be used in an authorized responder certificate to signal that no further revocation checks need to be performed on the authorized responder.
The new method
OCSP::Response::find_signing_certificate()requires the respective issuer CA certificate of the certificate in question and an optionaltrusted_ocsp_responders[*1] certificate store and determines the signer certificate of the OCSP response.OCSP::Response::verify_signature()takes the signer certificate and simply validates the OCSP response's signature. Previously, there used to be an additonalOCSP:Response::check_signature()that was meant to combine the above functionality but is removed entirely.It is now up to the caller to:
...::find_signing_certificate()to obtain the OCSP responder's certificate...::verify_signature()to ensure the OCSP response is authentic...::status_for()to look up the revocation of the certificate in questionNote that in most cases "the caller" won't be the using application but
PKIX::check_ocsp()which now implements all of the steps above. Particularly, it callsx509_path_validate()for authorized responder certificates. If OCSP responses are directly signed by the issuing CA of the certificate in question, it assumes that the CA certificate is trustworthy (as it is checked in the running chain validation anyway).Caveat: Authorized responder certificates might in turn rely on another OCSP responder for their revocation information. This "higher order" OCSP request is currently not performed, even if the responder did not feature a "NoCheck" extension. It would require even more intrusive public API changes and is therefore left for future work.
[*1] The API now explicitly allows an application to define "trusted OCSP responders". Before, this use case would have required to put such responder certificates into the trusted root chain.