wait for TLS ClientHello when connection_strategy=eager

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([

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")

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 = []

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...