From 93de96a720a60365117ef41f0801e8cff7ac06bd Mon Sep 17 00:00:00 2001
From: Maximilian Hils <git@maximilianhils.com>
Date: Sun, 7 Mar 2021 16:00:33 +0100
Subject: [PATCH] don't set IP addresses as SNI (#4480)

---
 mitmproxy/addons/tlsconfig.py  | 13 +++++++++++--
 mitmproxy/net/tls.py           | 10 +++++-----
 test/mitmproxy/net/test_tls.py |  2 +-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/mitmproxy/addons/tlsconfig.py b/mitmproxy/addons/tlsconfig.py
index 5d0412b3a..2df6cf0c3 100644
--- a/mitmproxy/addons/tlsconfig.py
+++ b/mitmproxy/addons/tlsconfig.py
@@ -1,3 +1,4 @@
+import ipaddress
 import os
 from pathlib import Path
 from typing import List, Optional, TypedDict, Any
@@ -198,7 +199,7 @@ class TlsConfig:
             max_version=net_tls.Version[ctx.options.tls_version_client_max],
             cipher_list=cipher_list,
             verify=verify,
-            sni=server.sni,
+            hostname=server.sni,
             ca_path=ctx.options.ssl_verify_upstream_trusted_confdir,
             ca_pemfile=ctx.options.ssl_verify_upstream_trusted_ca,
             client_cert=client_cert,
@@ -207,7 +208,15 @@ class TlsConfig:
 
         tls_start.ssl_conn = SSL.Connection(ssl_ctx)
         if server.sni:
-            tls_start.ssl_conn.set_tlsext_host_name(server.sni.encode())
+            try:
+                ipaddress.ip_address(server.sni)
+            except ValueError:
+                tls_start.ssl_conn.set_tlsext_host_name(server.sni.encode())
+            else:
+                # RFC 6066: Literal IPv4 and IPv6 addresses are not permitted in "HostName".
+                # It's not really ideal that we only enforce that here, but otherwise we need to add checks everywhere
+                # where we assign .sni, which is much less robust.
+                pass
         tls_start.ssl_conn.set_connect_state()
 
     def running(self):
diff --git a/mitmproxy/net/tls.py b/mitmproxy/net/tls.py
index c2ee14bb6..a47f204f8 100644
--- a/mitmproxy/net/tls.py
+++ b/mitmproxy/net/tls.py
@@ -130,7 +130,7 @@ def create_proxy_server_context(
         max_version: Version,
         cipher_list: Optional[Iterable[str]],
         verify: Verify,
-        sni: Optional[str],
+        hostname: Optional[str],
         ca_path: Optional[str],
         ca_pemfile: Optional[str],
         client_cert: Optional[str],
@@ -143,12 +143,12 @@ def create_proxy_server_context(
         cipher_list=cipher_list,
     )
 
-    if verify is not Verify.VERIFY_NONE and sni is None:
+    if verify is not Verify.VERIFY_NONE and hostname is None:
         raise ValueError("Cannot validate certificate hostname without SNI")
 
     context.set_verify(verify.value, None)
-    if sni is not None:
-        assert isinstance(sni, str)
+    if hostname is not None:
+        assert isinstance(hostname, str)
         # Manually enable hostname verification on the context object.
         # https://wiki.openssl.org/index.php/Hostname_validation
         param = SSL._lib.SSL_CTX_get0_param(context._context)
@@ -159,7 +159,7 @@ def create_proxy_server_context(
             SSL._lib.X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS | SSL._lib.X509_CHECK_FLAG_NEVER_CHECK_SUBJECT
         )
         SSL._openssl_assert(
-            SSL._lib.X509_VERIFY_PARAM_set1_host(param, sni.encode(), 0) == 1
+            SSL._lib.X509_VERIFY_PARAM_set1_host(param, hostname.encode(), 0) == 1
         )
 
     if ca_path is None and ca_pemfile is None:
diff --git a/test/mitmproxy/net/test_tls.py b/test/mitmproxy/net/test_tls.py
index 8833743db..9fe6f0f82 100644
--- a/test/mitmproxy/net/test_tls.py
+++ b/test/mitmproxy/net/test_tls.py
@@ -36,7 +36,7 @@ def test_sslkeylogfile(tdata, monkeypatch):
         max_version=tls.DEFAULT_MAX_VERSION,
         cipher_list=None,
         verify=tls.Verify.VERIFY_NONE,
-        sni=None,
+        hostname=None,
         ca_path=None,
         ca_pemfile=None,
         client_cert=None,