From 516e64a8fae54d284565199258118d549f955532 Mon Sep 17 00:00:00 2001 From: Thomas Kriechbaumer Date: Tue, 16 Aug 2016 10:39:07 +0200 Subject: [PATCH 1/3] fix #1476 --- mitmproxy/protocol/tls.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/mitmproxy/protocol/tls.py b/mitmproxy/protocol/tls.py index d08e2e329..e41a9af08 100644 --- a/mitmproxy/protocol/tls.py +++ b/mitmproxy/protocol/tls.py @@ -369,8 +369,10 @@ class TlsLayer(base.Layer): not self.config.options.no_upstream_cert and ( self.config.options.add_upstream_certs_to_client_chain or - self._client_hello.alpn_protocols or - not self._client_hello.sni + self._client_tls and ( + self._client_hello.alpn_protocols or + not self._client_hello.sni + ) ) ) establish_server_tls_now = ( @@ -434,7 +436,7 @@ class TlsLayer(base.Layer): if self._custom_server_sni is False: return None else: - return self._custom_server_sni or self._client_hello.sni + return self._custom_server_sni or self._client_hello and self._client_hello.sni @property def alpn_for_client_connection(self): @@ -509,21 +511,18 @@ class TlsLayer(base.Layer): def _establish_tls_with_server(self): self.log("Establish TLS with server", "debug") try: - # We only support http/1.1 and h2. - # If the server only supports spdy (next to http/1.1), it may select that - # and mitmproxy would enter TCP passthrough mode, which we want to avoid. - def deprecated_http2_variant(x): - return x.startswith(b"h2-") or x.startswith(b"spdy") - - if self._client_hello.alpn_protocols: - alpn = [x for x in self._client_hello.alpn_protocols if not deprecated_http2_variant(x)] - else: - alpn = None - if alpn and b"h2" in alpn and not self.config.options.http2: - alpn.remove(b"h2") + alpn = None + if self._client_tls: + if self._client_hello.alpn_protocols: + # We only support http/1.1 and h2. + # If the server only supports spdy (next to http/1.1), it may select that + # and mitmproxy would enter TCP passthrough mode, which we want to avoid. + alpn = [x for x in self._client_hello.alpn_protocols if not (x.startswith(b"h2-") or x.startswith(b"spdy"))] + if alpn and b"h2" in alpn and not self.config.options.http2: + alpn.remove(b"h2") ciphers_server = self.config.options.ciphers_server - if not ciphers_server: + if not ciphers_server and self._client_tls: ciphers_server = [] for id in self._client_hello.cipher_suites: if id in CIPHER_ID_NAME_MAP.keys(): @@ -562,7 +561,8 @@ class TlsLayer(base.Layer): sys.exc_info()[2] ) - self.log("ALPN selected by server: %s" % self.alpn_for_client_connection, "debug") + proto = self.alpn_for_client_connection.decode() if self.alpn_for_client_connection else '-' + self.log("ALPN selected by server: {}".format(proto), "debug") def _find_cert(self): """ From cd35a6ff47f48da4479cbe6d2cbba34d31735d97 Mon Sep 17 00:00:00 2001 From: Thomas Kriechbaumer Date: Tue, 16 Aug 2016 11:00:10 +0200 Subject: [PATCH 2/3] add regression test for #1476 --- test/mitmproxy/test_server.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/mitmproxy/test_server.py b/test/mitmproxy/test_server.py index 78a97dc34..e0a8da471 100644 --- a/test/mitmproxy/test_server.py +++ b/test/mitmproxy/test_server.py @@ -472,6 +472,11 @@ class TestReverse(tservers.ReverseProxyTest, CommonMixin, TcpMixin): reverse = True +class TestReverseSSL(tservers.ReverseProxyTest, CommonMixin, TcpMixin): + reverse = True + ssl = True + + class TestSocks5(tservers.SocksModeTest): def test_simple(self): From eb83107e1c0ac8ddb0d198d71feac76939463f08 Mon Sep 17 00:00:00 2001 From: Thomas Kriechbaumer Date: Tue, 16 Aug 2016 11:00:21 +0200 Subject: [PATCH 3/3] reenable options test --- test/mitmproxy/test_proxy.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/mitmproxy/test_proxy.py b/test/mitmproxy/test_proxy.py index 848380189..f7c64e506 100644 --- a/test/mitmproxy/test_proxy.py +++ b/test/mitmproxy/test_proxy.py @@ -85,22 +85,22 @@ class TestProcessProxyOptions: @mock.patch("mitmproxy.platform.resolver") def test_modes(self, _): - # self.assert_noerr("-R", "http://localhost") - # self.assert_err("expected one argument", "-R") - # self.assert_err("Invalid server specification", "-R", "reverse") - # - # self.assert_noerr("-T") - # - # self.assert_noerr("-U", "http://localhost") - # self.assert_err("expected one argument", "-U") - # self.assert_err("Invalid server specification", "-U", "upstream") - # - # self.assert_noerr("--upstream-auth", "test:test") - # self.assert_err("expected one argument", "--upstream-auth") + self.assert_noerr("-R", "http://localhost") + self.assert_err("expected one argument", "-R") + self.assert_err("Invalid server specification", "-R", "reverse") + + self.assert_noerr("-T") + + self.assert_noerr("-U", "http://localhost") + self.assert_err("expected one argument", "-U") + self.assert_err("Invalid server specification", "-U", "upstream") + + self.assert_noerr("--upstream-auth", "test:test") + self.assert_err("expected one argument", "--upstream-auth") self.assert_err( "Invalid upstream auth specification", "--upstream-auth", "test" ) - # self.assert_err("mutually exclusive", "-R", "http://localhost", "-T") + self.assert_err("mutually exclusive", "-R", "http://localhost", "-T") def test_socks_auth(self): self.assert_err(