From 83c2de88490a3596ab98b55c5a497f8c2bb71563 Mon Sep 17 00:00:00 2001 From: Thomas Kriechbaumer Date: Sat, 18 Feb 2017 14:30:08 +0100 Subject: [PATCH] http2: disable priority forwarding --- mitmproxy/options.py | 15 +- mitmproxy/proxy/protocol/http2.py | 15 +- mitmproxy/tools/cmdline.py | 27 +++- test/mitmproxy/proxy/protocol/test_http2.py | 167 +++++--------------- 4 files changed, 79 insertions(+), 145 deletions(-) diff --git a/mitmproxy/options.py b/mitmproxy/options.py index 630d29645..2467b9dda 100644 --- a/mitmproxy/options.py +++ b/mitmproxy/options.py @@ -54,6 +54,7 @@ class Options(optmanager.OptManager): server_replay_ignore_params: Sequence[str] = [], server_replay_ignore_payload_params: Sequence[str] = [], server_replay_ignore_host: bool = False, + # Proxy options auth_nonanonymous: bool = False, auth_singleuser: Optional[str] = None, @@ -65,15 +66,18 @@ class Options(optmanager.OptManager): ciphers_client: str=DEFAULT_CLIENT_CIPHERS, ciphers_server: Optional[str]=None, clientcerts: Optional[str] = None, - http2: bool = True, ignore_hosts: Sequence[str] = [], listen_host: str = "", listen_port: int = LISTEN_PORT, upstream_bind_address: str = "", mode: str = "regular", no_upstream_cert: bool = False, - rawtcp: bool = False, + + http2: bool = True, + http2_priority: bool = False, websocket: bool = True, + rawtcp: bool = False, + spoof_source_address: bool = False, upstream_server: Optional[str] = None, upstream_auth: Optional[str] = None, @@ -152,15 +156,18 @@ class Options(optmanager.OptManager): self.ciphers_client = ciphers_client self.ciphers_server = ciphers_server self.clientcerts = clientcerts - self.http2 = http2 self.ignore_hosts = ignore_hosts self.listen_host = listen_host self.listen_port = listen_port self.upstream_bind_address = upstream_bind_address self.mode = mode self.no_upstream_cert = no_upstream_cert - self.rawtcp = rawtcp + + self.http2 = http2 + self.http2_priority = http2_priority self.websocket = websocket + self.rawtcp = rawtcp + self.spoof_source_address = spoof_source_address self.upstream_server = upstream_server self.upstream_auth = upstream_auth diff --git a/mitmproxy/proxy/protocol/http2.py b/mitmproxy/proxy/protocol/http2.py index b7548221c..cdce24b38 100644 --- a/mitmproxy/proxy/protocol/http2.py +++ b/mitmproxy/proxy/protocol/http2.py @@ -268,6 +268,10 @@ class Http2Layer(base.Layer): return True def _handle_priority_updated(self, eid, event): + if not self.config.options.http2_priority: + self.log("HTTP/2 PRIORITY frame surpressed. Use --http2-priority to enable forwarding.", "debug") + return True + if eid in self.streams and self.streams[eid].handled_priority_event is event: # this event was already handled during stream creation # HeadersFrame + Priority information as RequestReceived @@ -527,9 +531,12 @@ class Http2SingleStreamLayer(httpbase._HttpTransmissionLayer, basethread.BaseThr if self.handled_priority_event: # only send priority information if they actually came with the original HeadersFrame # and not if they got updated before/after with a PriorityFrame - priority_exclusive = self.priority_exclusive - priority_depends_on = self._map_depends_on_stream_id(self.server_stream_id, self.priority_depends_on) - priority_weight = self.priority_weight + if not self.config.options.http2_priority: + self.log("HTTP/2 PRIORITY information in HEADERS frame surpressed. Use --http2-priority to enable forwarding.", "debug") + else: + priority_exclusive = self.priority_exclusive + priority_depends_on = self._map_depends_on_stream_id(self.server_stream_id, self.priority_depends_on) + priority_weight = self.priority_weight try: self.connections[self.server_conn].safe_send_headers( @@ -610,7 +617,7 @@ class Http2SingleStreamLayer(httpbase._HttpTransmissionLayer, basethread.BaseThr chunks ) - def __call__(self): + def __call__(self): # pragma: no cover raise EnvironmentError('Http2SingleStreamLayer must be run as thread') def run(self): diff --git a/mitmproxy/tools/cmdline.py b/mitmproxy/tools/cmdline.py index 1c620fd60..bb0bb17a1 100644 --- a/mitmproxy/tools/cmdline.py +++ b/mitmproxy/tools/cmdline.py @@ -137,7 +137,6 @@ def get_common_options(args): ciphers_client = args.ciphers_client, ciphers_server = args.ciphers_server, clientcerts = args.clientcerts, - http2 = args.http2, ignore_hosts = args.ignore_hosts, listen_host = args.addr, listen_port = args.port, @@ -145,8 +144,12 @@ def get_common_options(args): mode = mode, no_upstream_cert = args.no_upstream_cert, spoof_source_address = args.spoof_source_address, - rawtcp = args.rawtcp, + + http2 = args.http2, + http2_priority = args.http2_priority, websocket = args.websocket, + rawtcp = args.rawtcp, + upstream_server = upstream_server, upstream_auth = args.upstream_auth, ssl_version_client = args.ssl_version_client, @@ -334,18 +337,26 @@ def proxy_options(parser): ) http2 = group.add_mutually_exclusive_group() - http2.add_argument("--no-http2", action="store_false", dest="http2", + http2.add_argument("--no-http2", action="store_false", dest="http2") + http2.add_argument("--http2", action="store_true", dest="http2", help="Explicitly enable/disable HTTP/2 support. " - "Enabled by default." + "HTTP/2 support is enabled by default.", ) - http2.add_argument("--http2", action="store_true", dest="http2") + + http2_priority = group.add_mutually_exclusive_group() + http2_priority.add_argument("--http2-priority", action="store_true", dest="http2_priority") + http2_priority.add_argument("--no-http2-priority", action="store_false", dest="http2_priority", + help="Explicitly enable/disable PRIORITY forwarding for HTTP/2 connections. " + "PRIORITY forwarding is disabled by default, " + "because some webservers fail at implementing the RFC properly.", + ) websocket = group.add_mutually_exclusive_group() - websocket.add_argument("--no-websocket", action="store_false", dest="websocket", + websocket.add_argument("--no-websocket", action="store_false", dest="websocket") + websocket.add_argument("--websocket", action="store_true", dest="websocket", help="Explicitly enable/disable WebSocket support. " - "Enabled by default." + "WebSocket support is enabled by default.", ) - websocket.add_argument("--websocket", action="store_true", dest="websocket") parser.add_argument( "--upstream-auth", diff --git a/test/mitmproxy/proxy/protocol/test_http2.py b/test/mitmproxy/proxy/protocol/test_http2.py index eec7af895..cb9c0474c 100644 --- a/test/mitmproxy/proxy/protocol/test_http2.py +++ b/test/mitmproxy/proxy/protocol/test_http2.py @@ -4,7 +4,7 @@ import os import tempfile import traceback - +import pytest import h2 from mitmproxy import options @@ -369,7 +369,15 @@ class TestRequestWithPriority(_Http2Test): wfile.flush() return True - def test_request_with_priority(self): + @pytest.mark.parametrize("http2_priority_enabled, priority, expected_priority", [ + (True, (True, 42424242, 42), ('True', '42424242', '42')), + (False, (True, 42424242, 42), (None, None, None)), + (True, (None, None, None), (None, None, None)), + (False, (None, None, None), (None, None, None)), + ]) + def test_request_with_priority(self, http2_priority_enabled, priority, expected_priority): + self.config.options.http2_priority = http2_priority_enabled + client, h2_conn = self._setup_connection() self._send_request( @@ -381,9 +389,9 @@ class TestRequestWithPriority(_Http2Test): (':scheme', 'https'), (':path', '/'), ], - priority_exclusive=True, - priority_depends_on=42424242, - priority_weight=42, + priority_exclusive=priority[0], + priority_depends_on=priority[1], + priority_weight=priority[2], ) done = False @@ -407,123 +415,15 @@ class TestRequestWithPriority(_Http2Test): client.wfile.flush() assert len(self.master.state.flows) == 1 - assert self.master.state.flows[0].response.headers['priority_exclusive'] == 'True' - assert self.master.state.flows[0].response.headers['priority_depends_on'] == '42424242' - assert self.master.state.flows[0].response.headers['priority_weight'] == '42' - def test_request_without_priority(self): - client, h2_conn = self._setup_connection() - - self._send_request( - client.wfile, - h2_conn, - headers=[ - (':authority', "127.0.0.1:{}".format(self.server.server.address.port)), - (':method', 'GET'), - (':scheme', 'https'), - (':path', '/'), - ], - ) - - done = False - while not done: - try: - raw = b''.join(http2.read_raw_frame(client.rfile)) - events = h2_conn.receive_data(raw) - except exceptions.HttpException: - print(traceback.format_exc()) - assert False - - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() - - for event in events: - if isinstance(event, h2.events.StreamEnded): - done = True - - h2_conn.close_connection() - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() - - assert len(self.master.state.flows) == 1 - assert 'priority_exclusive' not in self.master.state.flows[0].response.headers - assert 'priority_depends_on' not in self.master.state.flows[0].response.headers - assert 'priority_weight' not in self.master.state.flows[0].response.headers + resp = self.master.state.flows[0].response + assert resp.headers.get('priority_exclusive', None) == expected_priority[0] + assert resp.headers.get('priority_depends_on', None) == expected_priority[1] + assert resp.headers.get('priority_weight', None) == expected_priority[2] @requires_alpn class TestPriority(_Http2Test): - priority_data = None - - @classmethod - def handle_server_event(cls, event, h2_conn, rfile, wfile): - if isinstance(event, h2.events.ConnectionTerminated): - return False - elif isinstance(event, h2.events.PriorityUpdated): - cls.priority_data = (event.exclusive, event.depends_on, event.weight) - elif isinstance(event, h2.events.RequestReceived): - import warnings - with warnings.catch_warnings(): - # Ignore UnicodeWarning: - # h2/utilities.py:64: UnicodeWarning: Unicode equal comparison - # failed to convert both arguments to Unicode - interpreting - # them as being unequal. - # elif header[0] in (b'cookie', u'cookie') and len(header[1]) < 20: - - warnings.simplefilter("ignore") - - headers = [(':status', '200')] - h2_conn.send_headers(event.stream_id, headers) - h2_conn.end_stream(event.stream_id) - wfile.write(h2_conn.data_to_send()) - wfile.flush() - return True - - def test_priority(self): - client, h2_conn = self._setup_connection() - - h2_conn.prioritize(1, exclusive=True, depends_on=0, weight=42) - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() - - self._send_request( - client.wfile, - h2_conn, - headers=[ - (':authority', "127.0.0.1:{}".format(self.server.server.address.port)), - (':method', 'GET'), - (':scheme', 'https'), - (':path', '/'), - ], - ) - - done = False - while not done: - try: - raw = b''.join(http2.read_raw_frame(client.rfile)) - events = h2_conn.receive_data(raw) - except exceptions.HttpException: - print(traceback.format_exc()) - assert False - - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() - - for event in events: - if isinstance(event, h2.events.StreamEnded): - done = True - - h2_conn.close_connection() - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() - - assert len(self.master.state.flows) == 1 - assert self.priority_data == (True, 0, 42) - - -@requires_alpn -class TestPriorityWithExistingStream(_Http2Test): - priority_data = [] @classmethod def handle_server_event(cls, event, h2_conn, rfile, wfile): @@ -532,8 +432,6 @@ class TestPriorityWithExistingStream(_Http2Test): elif isinstance(event, h2.events.PriorityUpdated): cls.priority_data.append((event.exclusive, event.depends_on, event.weight)) elif isinstance(event, h2.events.RequestReceived): - assert not event.priority_updated - import warnings with warnings.catch_warnings(): # Ignore UnicodeWarning: @@ -546,17 +444,27 @@ class TestPriorityWithExistingStream(_Http2Test): headers = [(':status', '200')] h2_conn.send_headers(event.stream_id, headers) - wfile.write(h2_conn.data_to_send()) - wfile.flush() - elif isinstance(event, h2.events.StreamEnded): h2_conn.end_stream(event.stream_id) wfile.write(h2_conn.data_to_send()) wfile.flush() return True - def test_priority_with_existing_stream(self): + @pytest.mark.parametrize("prioritize_before", [True, False]) + @pytest.mark.parametrize("http2_priority_enabled, priority, expected_priority", [ + (True, (True, 42424242, 42), [(True, 42424242, 42)]), + (False, (True, 42424242, 42), []), + ]) + def test_priority(self, prioritize_before, http2_priority_enabled, priority, expected_priority): + self.config.options.http2_priority = http2_priority_enabled + self.__class__.priority_data = [] + client, h2_conn = self._setup_connection() + if prioritize_before: + h2_conn.prioritize(1, exclusive=priority[0], depends_on=priority[1], weight=priority[2]) + client.wfile.write(h2_conn.data_to_send()) + client.wfile.flush() + self._send_request( client.wfile, h2_conn, @@ -566,13 +474,14 @@ class TestPriorityWithExistingStream(_Http2Test): (':scheme', 'https'), (':path', '/'), ], - end_stream=False, + end_stream=prioritize_before, ) - h2_conn.prioritize(1, exclusive=True, depends_on=0, weight=42) - h2_conn.end_stream(1) - client.wfile.write(h2_conn.data_to_send()) - client.wfile.flush() + if not prioritize_before: + h2_conn.prioritize(1, exclusive=priority[0], depends_on=priority[1], weight=priority[2]) + h2_conn.end_stream(1) + client.wfile.write(h2_conn.data_to_send()) + client.wfile.flush() done = False while not done: @@ -595,7 +504,7 @@ class TestPriorityWithExistingStream(_Http2Test): client.wfile.flush() assert len(self.master.state.flows) == 1 - assert self.priority_data == [(True, 0, 42)] + assert self.priority_data == expected_priority @requires_alpn