improve TLS version mismatch error, fix #4758 (#4772)

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * 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
+* Improve error message on TLS version mismatch.
 * Windows: Switch to Python's default asyncio event loop, which increases the number of sockets
   that can be processed simultaneously.
 

mitmproxy/proxy/context.py

@@ -34,15 +34,10 @@ class Context:
         return ret
 
     def __repr__(self):
-        layers = "\n    ".join(repr(l) for l in self.layers)
-        if layers:
-            layers = f"[\n    {layers}\n  ]"
-        else:
-            layers = "[]"
         return (
             f"Context(\n"
             f"  {self.client!r},\n"
             f"  {self.server!r},\n"
-            f"  layers={layers}\n"
+            f"  layers=[{self.layers!r}]\n"
             f")"
         )

mitmproxy/proxy/layers/tls.py

@@ -213,8 +213,12 @@ class _TLSLayer(tunnel.TunnelLayer):
                 err = last_err[2]
             elif last_err == ('SSL routines', 'ssl3_get_record', 'wrong version number') and data[:4].isascii():
                 err = f"The remote server does not speak TLS."
-            else:  # pragma: no cover
+            elif last_err == ('SSL routines', 'ssl3_read_bytes', 'tlsv1 alert protocol version'):
-                # TODO: Add test case once we find one.
+                err = (
+                    f"The remote server and mitmproxy cannot agree on a TLS version to use. "
+                    f"You may need to adjust mitmproxy's tls_version_server_min option."
+                )
+            else:
                 err = f"OpenSSL {e!r}"
             self.conn.error = err
             return False, err
@@ -428,6 +432,11 @@ class ClientTLSLayer(_TLSLayer):
         level: Literal["warn", "info"] = "warn"
         if err.startswith("Cannot parse ClientHello"):
             pass
+        elif "('SSL routines', 'tls_early_post_process_client_hello', 'unsupported protocol')" in err:
+            err = (
+                f"Client and mitmproxy cannot agree on a TLS version to use. "
+                f"You may need to adjust mitmproxy's tls_version_client_min option."
+            )
         elif "unknown ca" in err or "bad certificate" in err:
             err = f"The client does not trust the proxy's certificate for {dest} ({err})"
         elif err == "connection closed":

test/mitmproxy/proxy/layers/test_tls.py

@@ -86,8 +86,13 @@ def test_parse_client_hello():
 class SSLTest:
     """Helper container for Python's builtin SSL object."""
 
-    def __init__(self, server_side: bool = False, alpn: typing.Optional[typing.List[str]] = None,
+    def __init__(
-                 sni: typing.Optional[bytes] = b"example.mitmproxy.org"):
+        self,
+        server_side: bool = False,
+        alpn: typing.Optional[typing.List[str]] = None,
+        sni: typing.Optional[bytes] = b"example.mitmproxy.org",
+        max_ver: typing.Optional[ssl.TLSVersion] = None,
+    ):
         self.inc = ssl.MemoryBIO()
         self.out = ssl.MemoryBIO()
         self.ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER if server_side else ssl.PROTOCOL_TLS_CLIENT)
@@ -104,6 +109,8 @@ class SSLTest:
                 certfile=tlsdata.path("../../net/data/verificationcerts/trusted-leaf.crt"),
                 keyfile=tlsdata.path("../../net/data/verificationcerts/trusted-leaf.key"),
             )
+        if max_ver:
+            self.ctx.maximum_version = max_ver
 
         self.obj = self.ctx.wrap_bio(
             self.inc,
@@ -162,7 +169,10 @@ def reply_tls_start_client(alpn: typing.Optional[bytes] = None, *args, **kwargs)
     """
 
     def make_client_conn(tls_start: tls.TlsStartData) -> None:
+        # ssl_context = SSL.Context(Method.TLS_METHOD)
+        # ssl_context.set_min_proto_version(SSL.TLS1_3_VERSION)
         ssl_context = SSL.Context(SSL.SSLv23_METHOD)
+        ssl_context.set_options(SSL.OP_NO_SSLv3 | SSL.OP_NO_TLSv1 | SSL.OP_NO_TLSv1_1 | SSL.OP_NO_TLSv1_2)
         ssl_context.use_privatekey_file(
             tlsdata.path("../../net/data/verificationcerts/trusted-leaf.key")
         )
@@ -184,7 +194,10 @@ def reply_tls_start_server(alpn: typing.Optional[bytes] = None, *args, **kwargs)
     """
 
     def make_server_conn(tls_start: tls.TlsStartData) -> None:
+        # ssl_context = SSL.Context(Method.TLS_METHOD)
+        # ssl_context.set_min_proto_version(SSL.TLS1_3_VERSION)
         ssl_context = SSL.Context(SSL.SSLv23_METHOD)
+        ssl_context.set_options(SSL.OP_NO_SSLv3 | SSL.OP_NO_TLSv1 | SSL.OP_NO_TLSv1_1 | SSL.OP_NO_TLSv1_2)
         ssl_context.load_verify_locations(
             cafile=tlsdata.path("../../net/data/verificationcerts/trusted-root.crt")
         )
@@ -337,6 +350,39 @@ class TestServerTLS:
                 << commands.CloseConnection(tctx.server)
         )
 
+    def test_unsupported_protocol(self, tctx: context.Context):
+        """Test the scenario where the server only supports an outdated TLS version by default."""
+        playbook = tutils.Playbook(tls.ServerTLSLayer(tctx))
+        tctx.server.address = ("example.mitmproxy.org", 443)
+        tctx.server.state = ConnectionState.OPEN
+        tctx.server.sni = "example.mitmproxy.org"
+
+        # noinspection PyTypeChecker
+        tssl = SSLTest(server_side=True, max_ver=ssl.TLSVersion.TLSv1_2)
+
+        # send ClientHello
+        data = tutils.Placeholder(bytes)
+        assert (
+            playbook
+            << tls.TlsStartServerHook(tutils.Placeholder())
+            >> reply_tls_start_server()
+            << commands.SendData(tctx.server, data)
+        )
+
+        # receive ServerHello
+        tssl.bio_write(data())
+        with pytest.raises(ssl.SSLError):
+            tssl.do_handshake()
+
+        # send back error
+        assert (
+            playbook
+            >> 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")
+            << commands.CloseConnection(tctx.server)
+        )
+
 
 def make_client_tls_layer(
         tctx: context.Context,
@@ -564,3 +610,20 @@ class TestClientTLS:
                             "client does not trust the proxy's certificate.", "info")
             << commands.CloseConnection(tctx.client)
         )
+
+    def test_unsupported_protocol(self, tctx: context.Context):
+        """Test the scenario where the client only supports an outdated TLS version by default."""
+        playbook, client_layer, tssl_client = make_client_tls_layer(tctx, max_ver=ssl.TLSVersion.TLSv1_2)
+        playbook.logs = True
+
+        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.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")
+            << commands.CloseConnection(tctx.client)
+        )

test/mitmproxy/proxy/tutils.py

@@ -2,6 +2,7 @@ import collections.abc
 import difflib
 import itertools
 import re
+import textwrap
 import traceback
 import typing
 
@@ -65,6 +66,7 @@ def _fmt_entry(x: PlaybookEntry):
     x = str(x)
     x = re.sub('Placeholder:None', '<unset placeholder>', x, flags=re.IGNORECASE)
     x = re.sub('Placeholder:', '', x, flags=re.IGNORECASE)
+    x = textwrap.indent(x, "     ")[5:]
     return f"{arrow} {x}"