From 3cb87f5a2f00790535e9ffa3c7ccf3566291a055 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 22 Nov 2021 10:10:14 +0100 Subject: [PATCH] split `tls_handshake` hook into client/server and success/fail variants --- CHANGELOG.md | 6 +--- docs/scripts/api-events.py | 5 +++- examples/contrib/tls_passthrough.py | 17 +++++------ mitmproxy/proxy/layers/tls.py | 40 +++++++++++++++++++++---- test/mitmproxy/proxy/layers/test_tls.py | 26 +++++++++------- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7c07c3a9..cee75fc2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,8 @@ * Support proxy authentication for SOCKS v5 mode (@starplanet) * Make it possible to ignore connections in the tls_clienthello event hook (@mhils) -* Add `tls_handshake` event hook to record negotiation success/failure (@mhils) +* Add `tls_established/failed_client/server` event hooks to record negotiation success/failure (@mhils) * fix some responses not being decoded properly if the encoding was uppercase #4735 (@Mattwmaster58) -* Expose TLS 1.0 as possible minimum version on older pyOpenSSL releases (@mhils) -* Improve error message on TLS version mismatch. (@mhils) -* Windows: Switch to Python's default asyncio event loop, which increases the number of sockets - that can be processed simultaneously (@mhils) * Trigger event hooks for flows with semantically invalid requests, for example invalid content-length headers (@mhils) * Improve error message on TLS version mismatch (@mhils) * Windows: Switch to Python's default asyncio event loop, which increases the number of sockets diff --git a/docs/scripts/api-events.py b/docs/scripts/api-events.py index 462c02531..eca751a79 100644 --- a/docs/scripts/api-events.py +++ b/docs/scripts/api-events.py @@ -124,7 +124,10 @@ with outfile.open("w") as f, contextlib.redirect_stdout(f): tls.TlsClienthelloHook, tls.TlsStartClientHook, tls.TlsStartServerHook, - tls.TlsHandshakeHook, + tls.TlsEstablishedClientHook, + tls.TlsEstablishedServerHook, + tls.TlsFailedClientHook, + tls.TlsFailedServerHook, ] ) diff --git a/examples/contrib/tls_passthrough.py b/examples/contrib/tls_passthrough.py index 182f26289..811d56abf 100644 --- a/examples/contrib/tls_passthrough.py +++ b/examples/contrib/tls_passthrough.py @@ -94,16 +94,15 @@ class MaybeTls: data.ignore_connection = True self.strategy.record_skipped(server_address) - def tls_handshake(self, data: tls.TlsData): - if isinstance(data.conn, connection.Server): - return # we are only interested in failing client connections here. + def tls_established_client(self, data: tls.TlsData): server_address = data.context.server.peername - if data.conn.error is None: - ctx.log(f"TLS handshake successful: {human.format_address(server_address)}") - self.strategy.record_success(server_address) - else: - ctx.log(f"TLS handshake failed: {human.format_address(server_address)}") - self.strategy.record_failure(server_address) + ctx.log(f"TLS handshake successful: {human.format_address(server_address)}") + self.strategy.record_success(server_address) + + def tls_failed_client(self, data: tls.TlsData): + server_address = data.context.server.peername + ctx.log(f"TLS handshake failed: {human.format_address(server_address)}") + self.strategy.record_failure(server_address) addons = [MaybeTls()] diff --git a/mitmproxy/proxy/layers/tls.py b/mitmproxy/proxy/layers/tls.py index 99eb14681..924a0fff7 100644 --- a/mitmproxy/proxy/layers/tls.py +++ b/mitmproxy/proxy/layers/tls.py @@ -133,11 +133,33 @@ class TlsStartServerHook(StartHook): @dataclass -class TlsHandshakeHook(StartHook): +class TlsEstablishedClientHook(StartHook): """ - A TLS handshake has been completed. + The TLS handshake with the client has been completed successfully. + """ + data: TlsData - If `data.conn.error` is `None`, negotiation was successful. + +@dataclass +class TlsEstablishedServerHook(StartHook): + """ + The TLS handshake with the server has been completed successfully. + """ + data: TlsData + + +@dataclass +class TlsFailedClientHook(StartHook): + """ + The TLS handshake with the client has failed. + """ + data: TlsData + + +@dataclass +class TlsFailedServerHook(StartHook): + """ + The TLS handshake with the server has failed. """ data: TlsData @@ -162,7 +184,7 @@ class _TLSLayer(tunnel.TunnelLayer): assert not self.tls tls_start = TlsData(self.conn, self.context) - if tls_start.conn == tls_start.context.client: + if self.conn == self.context.client: yield TlsStartClientHook(tls_start) else: yield TlsStartServerHook(tls_start) @@ -234,13 +256,19 @@ class _TLSLayer(tunnel.TunnelLayer): self.conn.tls_version = self.tls.get_protocol_version_name() if self.debug: yield commands.Log(f"{self.debug}[tls] tls established: {self.conn}", "debug") - yield TlsHandshakeHook(TlsData(self.conn, self.context, self.tls)) + if self.conn == self.context.client: + yield TlsEstablishedClientHook(TlsData(self.conn, self.context, self.tls)) + else: + yield TlsEstablishedServerHook(TlsData(self.conn, self.context, self.tls)) yield from self.receive_data(b"") return True, None def on_handshake_error(self, err: str) -> layer.CommandGenerator[None]: self.conn.error = err - yield TlsHandshakeHook(TlsData(self.conn, self.context, self.tls)) + if self.conn == self.context.client: + yield TlsFailedClientHook(TlsData(self.conn, self.context, self.tls)) + else: + yield TlsFailedServerHook(TlsData(self.conn, self.context, self.tls)) yield from super().on_handshake_error(err) def receive_data(self, data: bytes) -> layer.CommandGenerator[None]: diff --git a/test/mitmproxy/proxy/layers/test_tls.py b/test/mitmproxy/proxy/layers/test_tls.py index e20537c34..651e2c87e 100644 --- a/test/mitmproxy/proxy/layers/test_tls.py +++ b/test/mitmproxy/proxy/layers/test_tls.py @@ -158,10 +158,14 @@ class TlsEchoLayer(tutils.EchoLayer): def finish_handshake(playbook: tutils.Playbook, conn: connection.Connection, tssl: SSLTest): data = tutils.Placeholder(bytes) tls_hook_data = tutils.Placeholder(TlsData) + if isinstance(conn, connection.Client): + established_hook = tls.TlsEstablishedClientHook(tls_hook_data) + else: + established_hook = tls.TlsEstablishedServerHook(tls_hook_data) assert ( playbook >> events.DataReceived(conn, tssl.bio_read()) - << tls.TlsHandshakeHook(tls_hook_data) + << established_hook >> tutils.reply() << commands.SendData(conn, data) ) @@ -334,7 +338,7 @@ class TestServerTLS: playbook >> events.DataReceived(tctx.server, tssl.bio_read()) << commands.Log("Server TLS handshake failed. Certificate verify failed: Hostname mismatch", "warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedServerHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.server) << commands.SendData(tctx.client, @@ -358,7 +362,7 @@ class TestServerTLS: << commands.SendData(tctx.server, data) >> events.DataReceived(tctx.server, b"HTTP/1.1 404 Not Found\r\n") << commands.Log("Server TLS handshake failed. The remote server does not speak TLS.", "warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedServerHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.server) ) @@ -395,7 +399,7 @@ class TestServerTLS: >> events.DataReceived(tctx.server, tssl.bio_read()) << commands.Log("Server TLS handshake failed. The remote server and mitmproxy cannot agree on a TLS version" " to use. You may need to adjust mitmproxy's tls_version_server_min option.", "warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedServerHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.server) ) @@ -506,7 +510,7 @@ class TestClientTLS: assert ( playbook >> events.DataReceived(tctx.server, tssl_server.bio_read()) - << tls.TlsHandshakeHook(tutils.Placeholder()) + << tls.TlsEstablishedServerHook(tutils.Placeholder()) >> tutils.reply() << commands.SendData(tctx.server, data) << tls.TlsStartClientHook(tutils.Placeholder()) @@ -579,7 +583,7 @@ class TestClientTLS: playbook >> events.DataReceived(tctx.client, invalid) << commands.Log(f"Client TLS handshake failed. Cannot parse ClientHello: {invalid.hex()}", level="warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) ) @@ -620,7 +624,7 @@ class TestClientTLS: >> events.DataReceived(tctx.client, tssl_client.bio_read()) << commands.Log("Client TLS handshake failed. The client does not trust the proxy's certificate " "for wrong.host.mitmproxy.org (sslv3 alert bad certificate)", "warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) >> events.ConnectionClosed(tctx.client) @@ -647,7 +651,7 @@ class TestClientTLS: >> tutils.reply(to=-2) << tls.TlsStartClientHook(tutils.Placeholder()) >> reply_tls_start_client() - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) ) @@ -662,7 +666,7 @@ class TestClientTLS: playbook >> events.ConnectionClosed(tctx.client) >> reply_tls_start_client(to=-2) - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) ) @@ -677,7 +681,7 @@ class TestClientTLS: << 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") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) ) @@ -698,7 +702,7 @@ class TestClientTLS: >> reply_tls_start_client() << commands.Log("Client TLS handshake failed. Client and mitmproxy cannot agree on a TLS version to " "use. You may need to adjust mitmproxy's tls_version_client_min option.", "warn") - << tls.TlsHandshakeHook(tls_hook_data) + << tls.TlsFailedClientHook(tls_hook_data) >> tutils.reply() << commands.CloseConnection(tctx.client) )