Merge pull request #3692 from mhils/tls13

Update cryptography, enable TLS 1.3

3 files changed, 60 insertions, 37 deletions

diff --git a/mitmproxy/__init__.py b/mitmproxy/__init__.py
index e69de29b..e930cf1e 100644
--- a/mitmproxy/__init__.py
+++ b/mitmproxy/__init__.py
@@ -0,0 +1,8 @@
+import asyncio
+import sys
+
+if sys.platform == 'win32':
+    # workaround for
+    # https://github.com/tornadoweb/tornado/issues/2751
+    # https://www.tornadoweb.org/en/stable/index.html#installation
+    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
diff --git a/mitmproxy/net/tls.py b/mitmproxy/net/tls.py
index d68a008f..d8e943d3 100644
--- a/mitmproxy/net/tls.py
+++ b/mitmproxy/net/tls.py
@@ -7,14 +7,13 @@ import os
 import struct
 import threading
 import typing
-from ssl import match_hostname, CertificateError
 
 import certifi
 from OpenSSL import SSL
 from kaitaistruct import KaitaiStream
 
-import mitmproxy.options  # noqa
-from mitmproxy import exceptions, certs
+import mitmproxy.options
+from mitmproxy import certs, exceptions
 from mitmproxy.contrib.kaitaistruct import tls_client_hello
 from mitmproxy.net import check
 
@@ -88,7 +87,18 @@ class MasterSecretLogger:
     __name__ = "MasterSecretLogger"
 
     def __call__(self, connection, where, ret):
-        if where == SSL.SSL_CB_HANDSHAKE_DONE and ret == 1:
+        done_now = (
+            where == SSL.SSL_CB_HANDSHAKE_DONE and ret == 1
+        )
+        # this is a horrendous workaround for https://github.com/mitmproxy/mitmproxy/pull/3692#issuecomment-608454530:
+        # OpenSSL 1.1.1f decided to not make connection.master_key() fail in the SSL_CB_HANDSHAKE_DONE callback.
+        # To support various OpenSSL versions and still log master secrets, we now mark connections where this has
+        # happened and then try again on the next event. This is ugly and shouldn't be done, but eventually we
+        # replace this with context.set_keylog_callback anyways.
+        done_previously_but_not_logged_yet = (
+            hasattr(connection, "_still_needs_masterkey")
+        )
+        if done_now or done_previously_but_not_logged_yet:
             with self.lock:
                 if not self.f:
                     d = os.path.dirname(self.filename)
@@ -96,10 +106,16 @@ class MasterSecretLogger:
                         os.makedirs(d)
                     self.f = open(self.filename, "ab")
                     self.f.write(b"\r\n")
-                client_random = binascii.hexlify(connection.client_random())
-                masterkey = binascii.hexlify(connection.master_key())
-                self.f.write(b"CLIENT_RANDOM %s %s\r\n" % (client_random, masterkey))
-                self.f.flush()
+                try:
+                    client_random = binascii.hexlify(connection.client_random())
+                    masterkey = binascii.hexlify(connection.master_key())
+                except (AssertionError, SSL.Error):  # careful: exception type changes between pyOpenSSL versions
+                    connection._still_needs_masterkey = True
+                else:
+                    self.f.write(b"CLIENT_RANDOM %s %s\r\n" % (client_random, masterkey))
+                    self.f.flush()
+                    if hasattr(connection, "_still_needs_masterkey"):
+                        delattr(connection, "_still_needs_masterkey")
 
     def close(self):
         with self.lock:
@@ -237,33 +253,11 @@ def create_client_context(
             depth: int,
             is_cert_verified: bool
     ) -> bool:
-        if is_cert_verified and depth == 0:
-            # Verify hostname of leaf certificate.
-            cert = certs.Cert(x509)
-            try:
-                crt: typing.Dict[str, typing.Any] = dict(
-                    subjectAltName=[("DNS", x.decode("ascii", "strict")) for x in cert.altnames]
-                )
-                if cert.cn:
-                    crt["subject"] = [[["commonName", cert.cn.decode("ascii", "strict")]]]
-                if sni:
-                    # SNI hostnames allow support of IDN by using ASCII-Compatible Encoding
-                    # Conversion algorithm is in RFC 3490 which is implemented by idna codec
-                    # https://docs.python.org/3/library/codecs.html#text-encodings
-                    # https://tools.ietf.org/html/rfc6066#section-3
-                    # https://tools.ietf.org/html/rfc4985#section-3
-                    hostname = sni.encode("idna").decode("ascii")
-                else:
-                    hostname = "no-hostname"
-                match_hostname(crt, hostname)
-            except (ValueError, CertificateError) as e:
-                conn.cert_error = exceptions.InvalidCertificateException(
-                    "Certificate verification error for {}: {}".format(
-                        sni or repr(address),
-                        str(e)
-                    )
-                )
-                is_cert_verified = False
+        if is_cert_verified and depth == 0 and not sni:
+            conn.cert_error = exceptions.InvalidCertificateException(
+                f"Certificate verification error for {address}: Cannot validate hostname, SNI missing."
+            )
+            is_cert_verified = False
         elif is_cert_verified:
             pass
         else:
@@ -285,6 +279,20 @@ def create_client_context(
         **sslctx_kwargs,
     )
 
+    if sni:
+        # 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)
+        # Matching on the CN is disabled in both Chrome and Firefox, so we disable it, too.
+        # https://www.chromestatus.com/feature/4981025180483584
+        SSL._lib.X509_VERIFY_PARAM_set_hostflags(
+            param,
+            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("idna"), 0) == 1
+        )
+
     # Client Certs
     if cert:
         try:
diff --git a/mitmproxy/proxy/protocol/rawtcp.py b/mitmproxy/proxy/protocol/rawtcp.py
index 0ec50594..00bba04c 100644
--- a/mitmproxy/proxy/protocol/rawtcp.py
+++ b/mitmproxy/proxy/protocol/rawtcp.py
@@ -29,13 +29,20 @@ class RawTCPLayer(base.Layer):
         server = self.server_conn.connection
         conns = [client, server]
 
+        # https://github.com/openssl/openssl/issues/6234
+        for conn in conns:
+            if isinstance(conn, SSL.Connection) and hasattr(SSL._lib, "SSL_clear_mode"):
+                SSL._lib.SSL_clear_mode(conn._ssl, SSL._lib.SSL_MODE_AUTO_RETRY)
+
         try:
             while not self.channel.should_exit.is_set():
                 r = mitmproxy.net.tcp.ssl_read_select(conns, 10)
                 for conn in r:
                     dst = server if conn == client else client
-
-                    size = conn.recv_into(buf, self.chunk_size)
+                    try:
+                        size = conn.recv_into(buf, self.chunk_size)
+                    except (SSL.WantReadError, SSL.WantWriteError):
+                        continue
                     if not size:
                         conns.remove(conn)
                         # Shutdown connection to the other peer