tls: add CRLDP data to transport failure reason#43679
tls: add CRLDP data to transport failure reason#43679guydc wants to merge 4 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Guy Daich <guy.daich@sap.com>
|
/retest |
Signed-off-by: Guy Daich <guy.daich@sap.com>
|
/retest |
ggreenway
left a comment
ggreenway
left a comment
There was a problem hiding this comment.
I like the idea of this, but I wonder if it would be better if instead of using the error text out of boringssl, you wrote text for each of the four specific errors
/wait-any
test/common/tls/utility_test.cc
Outdated
| Utility::getX509VerificationErrorInfo(store_ctx.get()), | ||
| "X509_verify_cert: certificate verification error at depth 0: unable to get certificate CRL, " | ||
| "certificate CRL distribution points: [http://crl.example.com/ca.crl, " | ||
| "http://backup-crl.example.com/ca.crl]"); |
There was a problem hiding this comment.
I'm worried that this error will be quite misleading. The text implies that envoy would be fetching the CRL from the distribution point, but the actual error is that the CRL isn't included in Envoy's configuration
There was a problem hiding this comment.
Hi @ggreenway, thanks for reviewing. Sure, I changed it to CRL for certificate was not provided instead. I think that other error CRL-related errors are sufficiently clear.
There was a problem hiding this comment.
actually, seems like ssl_socket_test expects to find the original BoringSSL error messages. I'll wrap them instead.
Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: Guy Daich <guy.daich@sap.com>
|
/retest |
2 participants
Commit Message: tls: add CRLDP data to transport failure reason
Additional Description: For errors such as
unable to get certificate CRL, the error message now includes the certificate's CRL distribution points and a clarification that the check is performed against user-defined CRLs (e.g.,X509_verify_cert: certificate verification error at depth 0: certificate revocation check against provided CRLs failed: unable to get certificate CRL, certificate CRL distribution points: [http://crl.example.com/ca.crl, http://backup-crl.example.com/ca.crl])Risk Level: Low
Testing: Added unit tests
Docs Changes: No
Release Notes: Added
Platform Specific Features: No
Fixes #43642