From d3f3725479c78a4b0a5c15042b836576d0aea044 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 29 Mar 2021 13:24:46 +0200 Subject: [PATCH] wait for TLS ClientHello when connection_strategy=eager --- mitmproxy/addons/next_layer.py | 16 ++++++------ mitmproxy/proxy/layers/tls.py | 31 +++++++++++++++++++++--- test/mitmproxy/addons/test_next_layer.py | 4 +++ test/mitmproxy/proxy/layers/test_tls.py | 20 +++++++++++---- 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/mitmproxy/addons/next_layer.py b/mitmproxy/addons/next_layer.py index a640cb4c6..622bb93a0 100644 --- a/mitmproxy/addons/next_layer.py +++ b/mitmproxy/addons/next_layer.py @@ -105,8 +105,6 @@ class NextLayer: def s(*layers): return stack_match(context, layers) - top_layer = context.layers[-1] - # 1. check for --ignore/--allow ignore = self.ignore_connection(context.server.address, data_client) if ignore is True: @@ -116,13 +114,17 @@ class NextLayer: # 2. Check for TLS if client_tls: - # client tls requires a server tls layer as parent layer - # reverse proxy mode manages this itself. - # a secure web proxy doesn't have a server part. - if isinstance(top_layer, layers.ServerTLSLayer) or s(modes.ReverseProxy) or s(modes.HttpProxy): + # client tls usually requires a server tls layer as parent layer, except: + # - reverse proxy mode manages this itself. + # - a secure web proxy doesn't have a server part. + if s(modes.ReverseProxy) or s(modes.HttpProxy): return layers.ClientTLSLayer(context) else: - return layers.ServerTLSLayer(context) + # We already assign the next layer here os that ServerTLSLayer + # knows that it can safely wait for a ClientHello. + ret = layers.ServerTLSLayer(context) + ret.child_layer = layers.ClientTLSLayer(context) + return ret # 3. Setup the HTTP layer for a regular HTTP proxy or an upstream proxy. if any([ diff --git a/mitmproxy/proxy/layers/tls.py b/mitmproxy/proxy/layers/tls.py index cdc3bdec0..0dea00820 100644 --- a/mitmproxy/proxy/layers/tls.py +++ b/mitmproxy/proxy/layers/tls.py @@ -270,14 +270,39 @@ class ServerTLSLayer(_TLSLayer): """ This layer establishes TLS for a single server connection. """ - command_to_reply_to: Optional[commands.OpenConnection] = None + wait_for_clienthello: bool = False def __init__(self, context: context.Context, conn: Optional[connection.Server] = None): super().__init__(context, conn or context.server) def start_handshake(self) -> layer.CommandGenerator[None]: - yield from self.start_tls() - yield from self.receive_handshake_data(b"") + wait_for_clienthello = ( + # if command_to_reply_to is set, we've been instructed to open the connection from the child layer. + # in that case any potential ClientHello is already parsed (by the ClientTLS child layer). + not self.command_to_reply_to + # if command_to_reply_to is not set, the connection was already open when this layer received its Start + # event (eager connection strategy). We now want to establish TLS right away, _unless_ we already know + # that there's TLS on the client side as well (we check if our immediate child layer is set to be ClientTLS) + # In this case want to wait for ClientHello to be parsed, so that we can incorporate SNI/ALPN from there. + and isinstance(self.child_layer, ClientTLSLayer) + ) + if wait_for_clienthello: + self.wait_for_clienthello = True + self.tunnel_state = tunnel.TunnelState.CLOSED + else: + yield from self.start_tls() + yield from self.receive_handshake_data(b"") + + def event_to_child(self, event: events.Event) -> layer.CommandGenerator[None]: + if self.wait_for_clienthello: + for command in super().event_to_child(event): + if isinstance(command, commands.OpenConnection) and command.connection == self.conn: + self.wait_for_clienthello = False + # swallow OpenConnection here by not re-yielding it. + else: + yield command + else: + yield from super().event_to_child(event) def on_handshake_error(self, err: str) -> layer.CommandGenerator[None]: yield commands.Log(f"Server TLS handshake failed. {err}", level="warn") diff --git a/test/mitmproxy/addons/test_next_layer.py b/test/mitmproxy/addons/test_next_layer.py index a1efea87a..614fbe063 100644 --- a/test/mitmproxy/addons/test_next_layer.py +++ b/test/mitmproxy/addons/test_next_layer.py @@ -97,6 +97,10 @@ class TestNextLayer: tctx.configure(nl, ignore_hosts=[]) assert isinstance(nl._next_layer(ctx, client_hello_no_extensions, b""), layers.ServerTLSLayer) + assert isinstance(ctx.layers[-1], layers.ClientTLSLayer) + + ctx.layers = [] + assert isinstance(nl._next_layer(ctx, b"", b""), layers.modes.HttpProxy) assert isinstance(nl._next_layer(ctx, client_hello_no_extensions, b""), layers.ClientTLSLayer) ctx.layers = [] diff --git a/test/mitmproxy/proxy/layers/test_tls.py b/test/mitmproxy/proxy/layers/test_tls.py index 28b8625c6..63edc9e00 100644 --- a/test/mitmproxy/proxy/layers/test_tls.py +++ b/test/mitmproxy/proxy/layers/test_tls.py @@ -391,11 +391,14 @@ class TestClientTLS: << commands.SendData(other_server, b"plaintext") ) - def test_server_required(self, tctx): + @pytest.mark.parametrize("eager", ["eager", ""]) + def test_server_required(self, tctx, eager): """ Test the scenario where a server connection is required (for example, because of an unknown ALPN) to establish TLS with the client. """ + if eager: + tctx.server.state = ConnectionState.OPEN tssl_server = SSLTest(server_side=True, alpn=["quux"]) playbook, client_layer, tssl_client = make_client_tls_layer(tctx, alpn=["quux"]) @@ -405,16 +408,23 @@ class TestClientTLS: def require_server_conn(client_hello: tls.ClientHelloData) -> None: client_hello.establish_server_tls_first = True - assert ( + ( playbook >> events.DataReceived(tctx.client, tssl_client.bio_read()) << tls.TlsClienthelloHook(tutils.Placeholder()) >> tutils.reply(side_effect=require_server_conn) + ) + if not eager: + ( + playbook << commands.OpenConnection(tctx.server) >> tutils.reply(None) - << tls.TlsStartHook(tutils.Placeholder()) - >> reply_tls_start(alpn=b"quux") - << commands.SendData(tctx.server, data) + ) + assert ( + playbook + << tls.TlsStartHook(tutils.Placeholder()) + >> reply_tls_start(alpn=b"quux") + << commands.SendData(tctx.server, data) ) # Establish TLS with the server...