[sans-io] tls: handle untrusted mitmproxy certs

mitmproxy/proxy2/layers/tls.py

@@ -92,6 +92,9 @@ HTTP_ALPNS = (b"h2", b"http/1.1", b"http/1.0", b"http/0.9")
 
 
 class EstablishServerTLS(commands.ConnectionCommand):
+    """Establish TLS on the given connection.
+
+    If TLS establishment fails, the connection will automatically be closed by the TLS layer."""
     connection: context.Server
     blocking = True
 
@@ -177,15 +180,20 @@ class _TLSLayer(layer.Layer):
             return False, None
         except SSL.Error as e:
             # provide more detailed information for some errors.
-            last_err = e.args[0][-1]
+            last_err = e.args and isinstance(e.args[0], list) and e.args[0] and e.args[0][-1]
             if last_err == ('SSL routines', 'tls_process_server_certificate', 'certificate verify failed'):
                 verify_result = SSL._lib.SSL_get_verify_result(self.tls[conn]._ssl)
                 error = SSL._ffi.string(SSL._lib.X509_verify_cert_error_string(verify_result)).decode()
-                return False, f"Certificate verify failed: {error}"
+                err = f"Certificate verify failed: {error}"
-            elif last_err == ('SSL routines', 'ssl3_read_bytes', 'tlsv1 alert unknown ca'):
+            elif last_err in [
-                return False, "TLS Alert: Unknown CA"
+                ('SSL routines', 'ssl3_read_bytes', 'tlsv1 alert unknown ca'),
+                ('SSL routines', 'ssl3_read_bytes', 'sslv3 alert bad certificate')
+            ]:
+                err = last_err[2]
             else:
-                return False, repr(e)
+                err = repr(e)
+            yield from self.on_handshake_error(conn, err)
+            return False, err
         else:
             # Get all peer certificates.
             # https://www.openssl.org/docs/man1.1.1/man3/SSL_get_peer_cert_chain.html
@@ -197,7 +205,6 @@ class _TLSLayer(layer.Layer):
                 if cert:
                     all_certs.insert(0, cert)
 
-
             conn.tls_established = True
             conn.sni = self.tls[conn].get_servername()
             conn.alpn = self.tls[conn].get_alpn_proto_negotiated()
@@ -250,15 +257,20 @@ class _TLSLayer(layer.Layer):
                 yield from self.negotiate(event.connection, event.data)
             else:
                 yield from self.receive(event.connection, event.data)
-        elif (
+        elif isinstance(event, events.ConnectionClosed) and event.connection in self.tls:
-                isinstance(event, events.ConnectionClosed) and
+            if event.connection.tls_established:
-                event.connection in self.tls and
+                if self.tls[event.connection].get_shutdown() & SSL.RECEIVED_SHUTDOWN:
-                self.tls[event.connection].get_shutdown() & SSL.RECEIVED_SHUTDOWN
+                    pass  # We have already dispatched a ConnectionClosed to the child layer.
-        ):
+                else:
-            pass  # We have already dispatched a ConnectionClosed to the child layer.
+                    yield from self.event_to_child(event)
+            else:
+                yield from self.on_handshake_error(event.connection, "connection closed without notice")
         else:
             yield from self.event_to_child(event)
 
+    def on_handshake_error(self, conn: context.Connection, err: str) -> commands.TCommandGenerator:
+        yield commands.CloseConnection(conn)
+
 
 class ServerTLSLayer(_TLSLayer):
     """
@@ -287,6 +299,13 @@ class ServerTLSLayer(_TLSLayer):
             else:
                 yield command
 
+    def on_handshake_error(self, conn: context.Connection, err: str) -> commands.TCommandGenerator:
+        yield commands.Log(
+            f"Server TLS handshake failed. {err}",
+            level="warn"
+        )
+        yield from super().on_handshake_error(conn, err)
+
 
 class ClientTLSLayer(_TLSLayer):
     """
@@ -320,13 +339,13 @@ class ClientTLSLayer(_TLSLayer):
         self._handle_event = self.state_wait_for_clienthello
         yield from ()
 
-    def state_wait_for_clienthello(self, event: events.Event):
+    def state_wait_for_clienthello(self, event: events.Event) -> commands.TCommandGenerator:
         client = self.context.client
         if isinstance(event, events.DataReceived) and event.connection == client:
             self.recv_buffer.extend(event.data)
             try:
                 client_hello = parse_client_hello(self.recv_buffer)
-            except ValueError as e:
+            except ValueError:
                 yield commands.Log(f"Cannot parse ClientHello: {self.recv_buffer.hex()}")
                 yield commands.CloseConnection(client)
                 return
@@ -373,21 +392,18 @@ class ClientTLSLayer(_TLSLayer):
             )
             return err
 
-    def negotiate(self, conn: context.Connection, data: bytes) -> Generator[commands.Command, Any, bool]:
+    def on_handshake_error(self, conn: context.Connection, err: str) -> commands.TCommandGenerator:
-        done, err = yield from super().negotiate(conn, data)
+        if conn.sni:
-        if err:
+            dest = conn.sni.decode("idna")
-            if self.context.client.sni:
+        else:
-                dest = self.context.client.sni.decode("idna")
+            dest = human.format_address(self.context.server.address)
-            else:
+        if "unknown ca" in err or "bad certificate" in err:
-                dest = human.format_address(self.context.server.address)
+            keyword = "does not"
-            if "Unknown CA" in err:
+        else:
-                keyword = "does not"
+            keyword = "may not"
-            else:
+        yield commands.Log(
-                keyword = "may not"
+            f"Client TLS handshake failed. "
-            yield commands.Log(
+            f"The client {keyword} trust the proxy's certificate for {dest} ({err})",
-                f"Client TLS Handshake failed. "
+            level="warn"
-                f"The client {keyword} trust the proxy's certificate for {dest} ({err}).",
+        )
-                level="warn"
+        yield from super().on_handshake_error(conn, err)
-            )
-            yield commands.CloseConnection(self.context.client)
-        return done

test/mitmproxy/proxy2/layers/test_tls.py

@@ -276,12 +276,17 @@ class TestServerTLS:
         assert (
                 playbook
                 >> events.DataReceived(tctx.server, tssl.out.read())
+                << commands.Log("Server TLS handshake failed. Certificate verify failed: Hostname mismatch","warn")
+                << commands.CloseConnection(tctx.server)
                 << commands.SendData(tctx.client, b"server-tls-failed: Certificate verify failed: Hostname mismatch")
         )
         assert not tctx.server.tls_established
 
 
-def _make_client_tls_layer(tctx: context.Context) -> typing.Tuple[tutils.Playbook, tls.ClientTLSLayer]:
+def make_client_tls_layer(
+        tctx: context.Context,
+        **kwargs
+) -> typing.Tuple[tutils.Playbook, tls.ClientTLSLayer, SSLTest]:
     # This is a bit contrived as the client layer expects a server layer as parent.
     # We also set child layers manually to avoid NextLayer noise.
     server_layer = tls.ServerTLSLayer(tctx)
@@ -289,18 +294,13 @@ def _make_client_tls_layer(tctx: context.Context) -> typing.Tuple[tutils.Playboo
     server_layer.child_layer = client_layer
     client_layer.child_layer = TlsEchoLayer(tctx)
     playbook = tutils.Playbook(server_layer)
-    return playbook, client_layer
 
-
+    # Add some server config, this is needed anyways.
-def _test_tls_client_server(tctx: context.Context, **kwargs) -> typing.Tuple[
-    tutils.Playbook, tls.ClientTLSLayer, SSLTest]:
-    playbook, client_layer = _make_client_tls_layer(tctx)
-    tctx.server.tls = True
     tctx.server.address = ("example.mitmproxy.org", 443)
     tctx.server.sni = b"example.mitmproxy.org"
-    tssl_client = SSLTest(**kwargs)
 
-    # Send ClientHello
+    tssl_client = SSLTest(**kwargs)
+    # Start handshake.
     with pytest.raises(ssl.SSLWantReadError):
         tssl_client.obj.do_handshake()
 
@@ -310,69 +310,43 @@ def _test_tls_client_server(tctx: context.Context, **kwargs) -> typing.Tuple[
 class TestClientTLS:
     def test_client_only(self, tctx: context.Context):
         """Test TLS with client only"""
-        playbook, client_layer = _make_client_tls_layer(tctx)
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx)
-        tssl = SSLTest()
         assert not tctx.client.tls_established
 
-        # Start Handshake, send ClientHello and ServerHello
+        # Send ClientHello, receive ServerHello
-        with pytest.raises(ssl.SSLWantReadError):
-            tssl.obj.do_handshake()
         data = tutils.Placeholder()
         assert (
                 playbook
-                >> events.DataReceived(tctx.client, tssl.out.read())
+                >> events.DataReceived(tctx.client, tssl_client.out.read())
                 << commands.Hook("tls_clienthello", tutils.Placeholder())
                 >> tutils.reply()
                 << commands.Hook("tls_start", tutils.Placeholder())
                 >> reply_tls_start()
                 << commands.SendData(tctx.client, data)
         )
-        tssl.inc.write(data())
+        tssl_client.inc.write(data())
-        tssl.obj.do_handshake()
+        tssl_client.obj.do_handshake()
         # Finish Handshake
-        interact(playbook, tctx.client, tssl)
+        interact(playbook, tctx.client, tssl_client)
 
-        assert tssl.obj.getpeercert(True)
+        assert tssl_client.obj.getpeercert(True)
         assert tctx.client.tls_established
 
         # Echo
-        _test_echo(playbook, tssl, tctx.client)
+        _test_echo(playbook, tssl_client, tctx.client)
         assert (
                 playbook
                 >> events.DataReceived(tctx.server, b"Plaintext")
                 << commands.SendData(tctx.server, b"plaintext")
         )
 
-    def test_server_not_required(self, tctx):
-        """
-        Here we test the scenario where a server connection is _not_ required
-        to establish TLS with the client. After determining this when parsing the ClientHello,
-        we only establish a connection with the client. The server connection may ultimately
-        be established when OpenConnection is called.
-        """
-        playbook, client_layer, tssl = _test_tls_client_server(tctx)
-        data = tutils.Placeholder()
-        assert (
-                playbook
-                >> events.DataReceived(tctx.client, tssl.out.read())
-                << commands.Hook("tls_clienthello", tutils.Placeholder())
-                >> tutils.reply()
-                << commands.Hook("tls_start", tutils.Placeholder())
-                >> reply_tls_start()
-                << commands.SendData(tctx.client, data)
-        )
-        tssl.inc.write(data())
-        tssl.obj.do_handshake()
-        interact(playbook, tctx.client, tssl)
-        assert tctx.client.tls_established
-
     def test_server_required(self, tctx):
         """
-        Here we test the scenario where a server connection is required (for example, because SNI is missing)
+        Test the scenario where a server connection is required (for example, because of an unknown ALPN)
         to establish TLS with the client.
         """
         tssl_server = SSLTest(server_side=True, alpn=["quux"])
-        playbook, client_layer, tssl_client = _test_tls_client_server(tctx, alpn=["quux"])
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx, alpn=["quux"])
 
         # We should now get instructed to open a server connection.
         data = tutils.Placeholder()
@@ -430,8 +404,8 @@ class TestClientTLS:
         _test_echo(playbook, tssl_client, tctx.client)
 
     def test_cannot_parse_clienthello(self, tctx: context.Context):
-        """We have a client layer, but we only receive garbage."""
+        """Test the scenario where we cannot parse the ClientHello"""
-        playbook, client_layer = _make_client_tls_layer(tctx)
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx)
 
         invalid = b"\x16\x03\x01\x00\x00"
 
@@ -442,3 +416,49 @@ class TestClientTLS:
                 << commands.CloseConnection(tctx.client)
         )
         assert not tctx.client.tls_established
+
+    def test_mitmproxy_ca_is_untrusted(self, tctx: context.Context):
+        """Test the scenario where the client doesn't trust the mitmproxy CA."""
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx, sni=b"wrong.host.mitmproxy.org")
+
+        data = tutils.Placeholder()
+        assert (
+                playbook
+                >> events.DataReceived(tctx.client, tssl_client.out.read())
+                << commands.Hook("tls_clienthello", tutils.Placeholder())
+                >> tutils.reply()
+                << commands.Hook("tls_start", tutils.Placeholder())
+                >> reply_tls_start()
+                << commands.SendData(tctx.client, data)
+        )
+        tssl_client.inc.write(data())
+        with pytest.raises(ssl.SSLCertVerificationError):
+            tssl_client.obj.do_handshake()
+        # Finish Handshake
+        assert (
+                playbook
+                >> events.DataReceived(tctx.client, tssl_client.out.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")
+                << commands.CloseConnection(tctx.client)
+        )
+        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."""
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx, sni=b"wrong.host.mitmproxy.org")
+
+        data = tutils.Placeholder()
+        assert (
+                playbook
+                >> events.DataReceived(tctx.client, tssl_client.out.read())
+                << commands.Hook("tls_clienthello", tutils.Placeholder())
+                >> tutils.reply()
+                << commands.Hook("tls_start", tutils.Placeholder())
+                >> reply_tls_start()
+                << commands.SendData(tctx.client, data)
+                >> events.ConnectionClosed(tctx.client)
+                << commands.Log("Client TLS handshake failed. The client may not trust the proxy's certificate "
+                                "for wrong.host.mitmproxy.org (connection closed without notice)", "warn")
+                << commands.CloseConnection(tctx.client)
+        )