From 0bb380a89f735847b3d4c715660f8a5e3ea565b3 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 2 May 2016 16:51:19 -0700 Subject: [PATCH] improve tls handling, separate `set_server` and `set_server_tls` --- mitmproxy/protocol/base.py | 11 +--- mitmproxy/protocol/http.py | 12 ++-- mitmproxy/protocol/tls.py | 130 +++++++++++++++++++++---------------- 3 files changed, 78 insertions(+), 75 deletions(-) diff --git a/mitmproxy/protocol/base.py b/mitmproxy/protocol/base.py index 536f2753c8..c8e58d1b33 100644 --- a/mitmproxy/protocol/base.py +++ b/mitmproxy/protocol/base.py @@ -133,24 +133,15 @@ def __check_self_connect(self): "The proxy shall not connect to itself.".format(repr(address)) ) - def set_server(self, address, server_tls=None, sni=None): + def set_server(self, address): """ Sets a new server address. If there is an existing connection, it will be closed. - - Raises: - ~mitmproxy.exceptions.ProtocolException: - if ``server_tls`` is ``True``, but there was no TLS layer on the - protocol stack which could have processed this. """ if self.server_conn: self.disconnect() self.log("Set new server address: " + repr(address), "debug") self.server_conn.address = address self.__check_self_connect() - if server_tls: - raise ProtocolException( - "Cannot upgrade to TLS, no TLS layer on the protocol stack." - ) def disconnect(self): """ diff --git a/mitmproxy/protocol/http.py b/mitmproxy/protocol/http.py index fc181b5dc6..922008f0e0 100644 --- a/mitmproxy/protocol/http.py +++ b/mitmproxy/protocol/http.py @@ -120,7 +120,7 @@ def change_upstream_proxy_server(self, address): if address != self.server_conn.via.address: self.ctx.set_server(address) - def set_server(self, address, server_tls=None, sni=None): + def set_server(self, address): if self.ctx.server_conn: self.ctx.disconnect() address = tcp.Address.wrap(address) @@ -128,11 +128,6 @@ def set_server(self, address, server_tls=None, sni=None): self.connect_request.port = address.port self.server_conn.address = address - if server_tls: - raise ProtocolException( - "Cannot upgrade to TLS, no TLS layer on the protocol stack." - ) - class HttpLayer(Layer): @@ -355,8 +350,9 @@ def establish_server_connection(self, flow): if self.mode == "regular" or self.mode == "transparent": # If there's an existing connection that doesn't match our expectations, kill it. - if address != self.server_conn.address or tls != self.server_conn.tls_established: - self.set_server(address, tls, address.host) + if address != self.server_conn.address or tls != self.server_tls: + self.set_server(address) + self.set_server_tls(tls, address.host) # Establish connection is neccessary. if not self.server_conn: self.connect() diff --git a/mitmproxy/protocol/tls.py b/mitmproxy/protocol/tls.py index 229f0db171..80b33bfd9a 100644 --- a/mitmproxy/protocol/tls.py +++ b/mitmproxy/protocol/tls.py @@ -308,6 +308,15 @@ def __repr__(self): class TlsLayer(Layer): + """ + The TLS layer implements transparent TLS connections. + + It exposes the following API to child layers: + + - :py:meth:`set_server_tls` to modify TLS settings for the server connection. + - :py:attr:`server_tls`, :py:attr:`server_sni` as read-only attributes describing the current TLS settings for + the server connection. + """ def __init__(self, ctx, client_tls, server_tls): self.client_sni = None @@ -318,40 +327,46 @@ def __init__(self, ctx, client_tls, server_tls): self._client_tls = client_tls self._server_tls = server_tls - self._sni_from_server_change = None + self._custom_server_sni = None def __call__(self): """ - The strategy for establishing SSL is as follows: + The strategy for establishing TLS is as follows: First, we determine whether we need the server cert to establish ssl with the client. If so, we first connect to the server and then to the client. - If not, we only connect to the client and do the server_ssl lazily on a Connect message. - - An additional complexity is that establish ssl with the server may require a SNI value from - the client. In an ideal world, we'd do the following: - 1. Start the SSL handshake with the client - 2. Check if the client sends a SNI. - 3. Pause the client handshake, establish SSL with the server. - 4. Finish the client handshake with the certificate from the server. - There's just one issue: We cannot get a callback from OpenSSL if the client doesn't send a SNI. :( - Thus, we manually peek into the connection and parse the ClientHello message to obtain both SNI and ALPN values. - - Further notes: - - OpenSSL 1.0.2 introduces a callback that would help here: - https://www.openssl.org/docs/ssl/SSL_CTX_set_cert_cb.html - - The original mitmproxy issue is https://github.com/mitmproxy/mitmproxy/issues/427 + If not, we only connect to the client and do the server handshake lazily. + + An additional complexity is that we need to mirror SNI and ALPN from the client when connecting to the server. + We manually peek into the connection and parse the ClientHello message to obtain these values. """ if self._client_tls: - self._parse_client_hello() - + # Peek into the connection, read the initial client hello and parse it to obtain SNI and ALPN values. + try: + parsed = TlsClientHello.from_client_conn(self.client_conn) + self.client_sni = parsed.client_sni + self.client_alpn_protocols = parsed.client_alpn_protocols + self.client_ciphers = parsed.client_cipher_suites + except TlsProtocolException as e: + self.log("Cannot parse Client Hello: %s" % repr(e), "error") + + # Do we need the server certificate to establish TLS with the client? # First, this requires that we have TLS on both the client and the server connection. # Second, this must be disabled if the user specified --no-upstream-cert - # Third, if the client sends a SNI value, we can be reasonably sure that this is the actual target host. - client_tls_requires_server_cert = ( - self._client_tls and self._server_tls and not self.config.no_upstream_cert and not self.client_sni + # Third, we need to connect if add_upstream_certs_to_client_chain is on. + # Fourth, we need to connect if the client wants to negotiate an alternate protocol using ALPN. + # Fifth, we need to connect if the client did not send a SNI value. + client_tls_requires_server_connection = ( + self._client_tls and self._server_tls + and not self.config.no_upstream_cert + and + ( + self.config.add_upstream_certs_to_client_chain + or self.client_alpn_protocols + or not self.client_sni + ) ) - if client_tls_requires_server_cert: + if client_tls_requires_server_connection: self._establish_tls_with_client_and_server() elif self._client_tls: self._establish_tls_with_client() @@ -369,47 +384,48 @@ def __repr__(self): # pragma: no cover else: return "TlsLayer(inactive)" - def _parse_client_hello(self): - """ - Peek into the connection, read the initial client hello and parse it to obtain ALPN values. - """ - try: - parsed = TlsClientHello.from_client_conn(self.client_conn) - self.client_sni = parsed.client_sni - self.client_alpn_protocols = parsed.client_alpn_protocols - self.client_ciphers = parsed.client_cipher_suites - except TlsProtocolException as e: - self.log("Cannot parse Client Hello: %s" % repr(e), "error") - def connect(self): if not self.server_conn: self.ctx.connect() if self._server_tls and not self.server_conn.tls_established: self._establish_tls_with_server() - def set_server(self, address, server_tls=None, sni=None): - if server_tls is not None: - self._sni_from_server_change = sni - self._server_tls = server_tls - self.ctx.set_server(address, None, None) + def set_server_tls(self, server_tls, sni=None): + """ + Set the TLS settings for the next server connection that will be established. + This function will not alter an existing connection. + + Args: + server_tls: Shall we establish TLS with the server? + sni: ``bytes`` for a custom SNI value, + ``None`` for the client SNI value, + ``False`` if no SNI value should be sent. + """ + self._server_tls = server_tls + self._custom_server_sni = sni + + @property + def server_tls(self): + """ + ``True``, if the next server connection that will be established should be upgraded to TLS. + """ + return self._server_tls @property - def sni_for_server_connection(self): - if self._sni_from_server_change is False: + def server_sni(self): + """ + The Server Name Indication we want to send with the next server TLS handshake. + """ + if self._custom_server_sni is False: return None else: - return self._sni_from_server_change or self.client_sni + return self._custom_server_sni or self.client_sni @property def alpn_for_client_connection(self): return self.server_conn.get_alpn_proto_negotiated() def __alpn_select_callback(self, conn_, options): - """ - Once the client signals the alternate protocols it supports, - we reconnect upstream with the same list and pass the server's choice down to the client. - """ - # This gets triggered if we haven't established an upstream connection yet. default_alpn = b'http/1.1' # alpn_preference = b'h2' @@ -424,12 +440,12 @@ def __alpn_select_callback(self, conn_, options): return choice def _establish_tls_with_client_and_server(self): - # If establishing TLS with the server fails, we try to establish TLS with the client nonetheless - # to send an error message over TLS. try: self.ctx.connect() self._establish_tls_with_server() except Exception: + # If establishing TLS with the server fails, we try to establish TLS with the client nonetheless + # to send an error message over TLS. try: self._establish_tls_with_client() except: @@ -499,7 +515,7 @@ def _establish_tls_with_server(self): self.server_conn.establish_ssl( self.config.clientcerts, - self.sni_for_server_connection, + self.server_sni, method=self.config.openssl_method_server, options=self.config.openssl_options_server, verify_options=self.config.openssl_verification_mode_server, @@ -526,7 +542,7 @@ def _establish_tls_with_server(self): TlsProtocolException, TlsProtocolException("Cannot establish TLS with {address} (sni: {sni}): {e}".format( address=repr(self.server_conn.address), - sni=self.sni_for_server_connection, + sni=self.server_sni, e=repr(e), )), sys.exc_info()[2] @@ -536,7 +552,7 @@ def _establish_tls_with_server(self): TlsProtocolException, TlsProtocolException("Cannot establish TLS with {address} (sni: {sni}): {e}".format( address=repr(self.server_conn.address), - sni=self.sni_for_server_connection, + sni=self.server_sni, e=repr(e), )), sys.exc_info()[2] @@ -573,11 +589,11 @@ def _find_cert(self): # Also add SNI values. if self.client_sni: sans.add(self.client_sni) - if self._sni_from_server_change: - sans.add(self._sni_from_server_change) + if self._custom_server_sni: + sans.add(self._custom_server_sni) - # Some applications don't consider the CN and expect the hostname to be in the SANs. - # For example, Thunderbird 38 will display a warning if the remote host is only the CN. + # RFC 2818: If a subjectAltName extension of type dNSName is present, that MUST be used as the identity. + # In other words, the Common Name is irrelevant then. if host: sans.add(host) return self.config.certstore.get_cert(host, list(sans))