{{ message }}
Repository navigation
Fail the connection when a fatal TLS alert arrives after the handshake - #8490
Open
choksi2212 wants to merge 3 commits into
Open
choksi2212 wants to merge 3 commits into
choksi2212 wants to merge 3 commits into
Conversation
TLS 1.3 servers may reject a client certificate only after the handshake has already completed. The fatal alert then surfaces from recv() as SSL.Error, which the TLS layer logged without failing the connection, so the HTTP layer was left waiting on a connection that would never deliver a response (a TLS 1.2 peer that rejects during the handshake yields a prompt 502 instead). We now record the error on the connection and dispatch ConnectionClosed to the child layer, mirroring the QUIC stream layer's existing error handling. The HTTP layer surfaces conn.error in the 502 it synthesizes, so the response names the actual TLS failure instead of a generic "server closed connection". Fixes mitmproxy#8486 Signed-off-by: choksi2212 <choksi2212@users.noreply.github.com>
# Conflicts: # CHANGELOG.md
Author
This branch has not been deployed
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.

Description
Fixes #8486.
A TLS 1.3 server that rejects the client certificate only after the handshake has completed sends a fatal alert that surfaces from
SSL.Connection.recv()asSSL.Error.TLSLayer.receive_datalogged that alert but never failed the connection, so the child layer (e.g. the HTTP layer waiting for a response) was left waiting on a connection that would never deliver a response — the client hangs. A TLS 1.2 peer that rejects the certificate during the handshake instead fails the connection promptly and yields a502 Bad Gateway, so the TLS 1.3 behavior is an observable regression in failure handling.With this change,
TLSLayer.receive_datarecords the error on the connection (conn.error, the same convention the QUIC stream layer already uses) and dispatchesConnectionClosedto the child layer, so the HTTP layer synthesizes its 502 promptly.Http1Clientnow surfacesconn.errorin theResponseProtocolErrorit emits when the server connection dies with a recorded error, so the 502 names the actual TLS failure instead of the generic "server closed connection".WantReadError(need more data) andZeroReturnError(close_notify) handling is unchanged; the new close path only fires when OpenSSL has permanently failed the record layer.Reproduction
Reporter's exact scenario, TLS 1.3 server requesting a client cert and rejecting it after the handshake:
mitmdumplogsTLS Error: [('SSL routines', '', 'tlsv13 alert certificate required')]and then nothing — the client hangs indefinitely.<< OpenSSL Error([('SSL routines', '', 'tlsv13 alert certificate required')])— same prompt-failure behavior as TLS 1.2, with the actual reason named.Testing
test/mitmproxy/proxy/layers/test_tls.py::TestServerTLS::test_fatal_alert_after_handshake— drives a genuine server-sent fatal alert (undecryptable record → OpenSSL fatal alert) throughServerTLSLayerpost-handshake and asserts the connection is failed (CloseConnection+ConnectionClosedto the child +CAN_READcleared +conn.errorrecorded). Fails onmainwith a playbook mismatch: noCloseConnectionis ever emitted.test/mitmproxy/proxy/layers/http/test_http1.py::TestClient::test_server_closed_with_error— assertsHttp1Clientsurfaces the recorded connection error in theResponseProtocolErrorwhen the server connection dies. Fails onmain, which emits the genericserver closed connectionmessage instead.test_memory_usage_*tests pass standalone on this machine and only hit a missing_cursesmodule under the xdist run — Python curses is not available on Windows).ruff check/ruff formatclean on all changed files;mypyreports no errors for the changed modules.