From 88086825e5c8654b89259d4f2d71292c1f45be64 Mon Sep 17 00:00:00 2001
From: Maximilian Hils <git@maximilianhils.com>
Date: Thu, 15 Jul 2021 11:54:03 +0200
Subject: [PATCH] only warn about failed TLS handshakes if we sent a
 ServerHello, fix #4678

---
 mitmproxy/proxy/layers/tls.py           |  9 ++++-
 test/mitmproxy/proxy/layers/test_tls.py | 52 ++++++++++++++++++-------
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/mitmproxy/proxy/layers/tls.py b/mitmproxy/proxy/layers/tls.py
index f29587300..7a4605c6f 100644
--- a/mitmproxy/proxy/layers/tls.py
+++ b/mitmproxy/proxy/layers/tls.py
@@ -161,6 +161,8 @@ class _TLSLayer(tunnel.TunnelLayer):
 
     def start_tls(self) -> layer.CommandGenerator[None]:
         assert not self.tls
+        if not self.conn.connected:
+            return
 
         tls_start = TlsStartData(self.conn, self.context)
         if tls_start.conn == tls_start.context.client:
@@ -379,6 +381,8 @@ class ClientTLSLayer(_TLSLayer):
                                    f"Trying to establish TLS with client anyway.")
 
         yield from self.start_tls()
+        if not self.conn.connected:
+            return False, "connection closed early"
 
         ret = yield from super().receive_handshake_data(bytes(self.recv_buffer))
         self.recv_buffer.clear()
@@ -410,9 +414,12 @@ class ClientTLSLayer(_TLSLayer):
                 f"this may indicate that the client does not trust the proxy's certificate."
             )
             level = "info"
+        elif err == "connection closed early":
+            pass
         else:
             err = f"The client may not trust the proxy's certificate for {dest} ({err})"
-        yield commands.Log(f"Client TLS handshake failed. {err}", level=level)
+        if err != "connection closed early":
+            yield commands.Log(f"Client TLS handshake failed. {err}", level=level)
         yield from super().on_handshake_error(err)
         self.event_to_child = self.errored  # type: ignore
 
diff --git a/test/mitmproxy/proxy/layers/test_tls.py b/test/mitmproxy/proxy/layers/test_tls.py
index d343a329a..4ffb66ac4 100644
--- a/test/mitmproxy/proxy/layers/test_tls.py
+++ b/test/mitmproxy/proxy/layers/test_tls.py
@@ -520,21 +520,45 @@ class TestClientTLS:
         )
         assert not tctx.client.tls_established
 
-    def test_mitmproxy_ca_is_untrusted_immediate_disconnect(self, tctx: context.Context):
-        """Test the scenario where the client doesn't trust the mitmproxy CA."""
+    @pytest.mark.parametrize("close_at", ["tls_clienthello", "tls_start_client", "handshake"])
+    def test_immediate_disconnect(self, tctx: context.Context, close_at):
+        """Test the scenario where the client is disconnecting during the handshake.
+        This may happen because they are not interested in the connection anymore, or because they do not like
+        the proxy certificate."""
         playbook, client_layer, tssl_client = make_client_tls_layer(tctx, sni=b"wrong.host.mitmproxy.org")
+        playbook.logs = True
+
+        playbook >> events.DataReceived(tctx.client, tssl_client.bio_read())
+        playbook << tls.TlsClienthelloHook(tutils.Placeholder())
+
+        if close_at == "tls_clienthello":
+            assert (
+                playbook
+                >> events.ConnectionClosed(tctx.client)
+                >> tutils.reply(to=-2)
+                << commands.CloseConnection(tctx.client)
+            )
+            return
+
+        playbook >> tutils.reply()
+        playbook << tls.TlsStartClientHook(tutils.Placeholder())
+
+        if close_at == "tls_start_client":
+            assert (
+                playbook
+                >> events.ConnectionClosed(tctx.client)
+                >> reply_tls_start_client(to=-2)
+                << commands.CloseConnection(tctx.client)
+            )
+            return
 
         assert (
-                playbook
-                >> events.DataReceived(tctx.client, tssl_client.bio_read())
-                << tls.TlsClienthelloHook(tutils.Placeholder())
-                >> tutils.reply()
-                << tls.TlsStartClientHook(tutils.Placeholder())
-                >> reply_tls_start_client()
-                << commands.SendData(tctx.client, tutils.Placeholder())
-                >> events.ConnectionClosed(tctx.client)
-                << commands.Log("Client TLS handshake failed. The client disconnected during the handshake. "
-                                "If this happens consistently for wrong.host.mitmproxy.org, this may indicate that the "
-                                "client does not trust the proxy's certificate.", "info")
-                << commands.CloseConnection(tctx.client)
+            playbook
+            >> reply_tls_start_client()
+            << commands.SendData(tctx.client, tutils.Placeholder())
+            >> events.ConnectionClosed(tctx.client)
+            << commands.Log("Client TLS handshake failed. The client disconnected during the handshake. "
+                            "If this happens consistently for wrong.host.mitmproxy.org, this may indicate that the "
+                            "client does not trust the proxy's certificate.", "info")
+            << commands.CloseConnection(tctx.client)
         )