Repository navigation
httpclient: a refused server certificate says which check refused it - #358
Merged
Merged
Conversation
A TLS client that refused a certificate reported only OpenSSL's "certificate verify failed", whatever the reason. SSL_get_verify_result, which holds it, was read only after a handshake that succeeded, where it can only be X509_V_OK. And the error text itself was read after the alert's flush, an await during which other connections on the reactor run OpenSSL calls that clear the thread's error queue - the race #319 fixed on the server. The failure is now captured as soon as SSL_connect fails, before the flush: the queued error, and when verification is on and refused the certificate, X509_verify_cert_error_string of the verify result ("certificate has expired", "self-signed certificate", "unable to get local issuer certificate", ...). The pending test "tls client: a verification failure says which check failed" is now a Test.
This was referenced Oct 12, 2026
Merged
This file contains hidden or 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
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.
Fixes #343.
When the TLS client refused a server certificate, the error was the same sentence whatever the reason:
certificate verify failed.SSL_get_verify_result, which holds the reason, was read only after a handshake that succeeded, where it can only beX509_V_OK. The error text was also read afterawait FlushCiphertextAsync()(the alert's flush). During that await other connections on the reactor run OpenSSL calls that clear the thread's error queue, which is the race #319 fixed on the server.The failure is now captured as soon as
SSL_connectfails, before the flush, in the same shape as #319's server fix. It holds the queued error and, when verification is on and the verify result is notX509_V_OK,X509_verify_cert_error_stringof that result. Verification has to be on because withSSL_VERIFY_NONEOpenSSL still records a verify result, and appending it to an unrelated failure would mislead. Nothing is added per request: oneSSL_get_verify_resulton a failed handshake.TLS handshake to 'origin.example.com' failed: error:0A000086:SSL routines::certificate verify failed... certificate verify failed: certificate has expired... certificate verify failed: self-signed certificate... certificate verify failed: unable to get local issuer certificateProof
The pending test
tls client: a verification failure says which check failedis now aTest().src/change revertedFAIL ... three different verification failures reported the same thing: TLS handshake to 'origin.example.com' failed: error:0A000086:SSL routines::certificate verify failed: 83 passed, 1 failed, 3 pendingPASS: 84 passed, 0 failed, 3 pendingThe 3 pending are the tests for #342, #340 and #344, as on main (83 passed, 4 pending there). Every existing
certificate verify failedassertion still passes, since the reason is appended after that text.The error-queue half (reading before the flush) has no test of its own. Two client handshakes refused in one reactor turn, for reasons with different queue text, would pin it the way #319's server test does. The verify-result reason is read from the SSL, not from the queue, so it is right either way.
With its siblings
This batch is #356 (#342), #358 (#343), #366 (#344) and #370 (#340). They merge with each other without conflicts, checked pairwise and all four together. #358 and #366 both edit
RunHandshakeAsync, in separate hunks. With all four merged, the Http suite is 88 passed, 0 failed, 0 pending.