From 148429c0b3e8ddafac7011c533bac3ca8442d370 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Wed, 16 Mar 2022 10:59:30 +0100 Subject: [PATCH] lowercase user-added HTTP/2 headers, fix #4746 (#5186) --- CHANGELOG.md | 5 ++-- mitmproxy/addons/proxyserver.py | 8 ++++++ mitmproxy/proxy/layers/http/_http2.py | 14 +++++++++- .../mitmproxy/proxy/layers/http/test_http2.py | 26 +++++++++++-------- web/src/js/ducks/_options_gen.ts | 2 ++ 5 files changed, 41 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3369b1015..0fe21aadd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * Support proxy authentication for SOCKS v5 mode (@starplanet) * Make it possible to ignore connections in the tls_clienthello event hook (@mhils) * Add `tls_established/failed_client/server` event hooks to record negotiation success/failure (@mhils) -* fix some responses not being decoded properly if the encoding was uppercase #4735 (@Mattwmaster58) +* fix some responses not being decoded properly if the encoding was uppercase (#4735, @Mattwmaster58) * Trigger event hooks for flows with semantically invalid requests, for example invalid content-length headers (@mhils) * Improve error message on TLS version mismatch (@mhils) * Windows: Switch to Python's default asyncio event loop, which increases the number of sockets @@ -29,11 +29,12 @@ * Change connection event hooks to be blocking. Processing will only resume once the event hook has finished. (@Prinzhorn) * Allow addon hooks to be async (@nneonneo, #4207) -* Reintroduce `Flow.live`, which signals if a flow belongs to a currently active connection. (@mhils, #4207) +* Reintroduce `Flow.live`, which signals if a flow belongs to a currently active connection. (#4207, @mhils) * Speculative fix for some rare HTTP/2 connection stalls (#5158, @EndUser509) * Add ability to specify custom ports with LDAP authentication (#5068, @demonoidvk) * Console Improvements on Windows (@mhils) * Fix processing of `--set` options (#5067, @marwinxxii) +* Lowercase user-added header names and emit a log message to notify the user when using HTTP/2 (#4746, @mhils) ## 28 September 2021: mitmproxy 7.0.4 diff --git a/mitmproxy/addons/proxyserver.py b/mitmproxy/addons/proxyserver.py index a05fd69c9..06efaf974 100644 --- a/mitmproxy/addons/proxyserver.py +++ b/mitmproxy/addons/proxyserver.py @@ -90,6 +90,14 @@ class Proxyserver: "proxy_debug", bool, False, "Enable debug logs in the proxy core.", ) + loader.add_option( + "normalize_outbound_headers", bool, True, + """ + Normalize outgoing HTTP/2 header names, but emit a warning when doing so. + HTTP/2 does not allow uppercase header names. This option makes sure that HTTP/2 headers set + in custom scripts are lowercased before they are sent. + """, + ) def running(self): self.master = ctx.master diff --git a/mitmproxy/proxy/layers/http/_http2.py b/mitmproxy/proxy/layers/http/_http2.py index 8e477c34f..cf71e8425 100644 --- a/mitmproxy/proxy/layers/http/_http2.py +++ b/mitmproxy/proxy/layers/http/_http2.py @@ -269,6 +269,13 @@ def normalize_h1_headers(headers: List[Tuple[bytes, bytes]], is_client: bool) -> return headers +def normalize_h2_headers(headers: List[Tuple[bytes, bytes]]) -> CommandGenerator[None]: + for i in range(len(headers)): + if not headers[i][0].islower(): + yield Log(f"Lowercased {repr(headers[i][0]).lstrip('b')} header as uppercase is not allowed with HTTP/2.") + headers[i] = (headers[i][0].lower(), headers[i][1]) + + class Http2Server(Http2Connection): h2_conf = h2.config.H2Configuration( **Http2Connection.h2_conf_defaults, @@ -290,7 +297,10 @@ class Http2Server(Http2Connection): (b":status", b"%d" % event.response.status_code), *event.response.headers.fields ] - if not event.response.is_http2: + if event.response.is_http2: + if self.context.options.normalize_outbound_headers: + yield from normalize_h2_headers(headers) + else: headers = normalize_h1_headers(headers, False) self.h2_conn.send_headers( @@ -407,6 +417,8 @@ class Http2Client(Http2Connection): if event.request.is_http2: hdrs = list(event.request.headers.fields) + if self.context.options.normalize_outbound_headers: + yield from normalize_h2_headers(hdrs) else: headers = event.request.headers if not event.request.authority and "host" in headers: diff --git a/test/mitmproxy/proxy/layers/http/test_http2.py b/test/mitmproxy/proxy/layers/http/test_http2.py index 5d76324cc..60ed36369 100644 --- a/test/mitmproxy/proxy/layers/http/test_http2.py +++ b/test/mitmproxy/proxy/layers/http/test_http2.py @@ -10,7 +10,7 @@ from mitmproxy.connection import ConnectionState, Server from mitmproxy.flow import Error from mitmproxy.http import HTTPFlow, Headers, Request from mitmproxy.net.http import status_codes -from mitmproxy.proxy.commands import CloseConnection, OpenConnection, SendData +from mitmproxy.proxy.commands import CloseConnection, Log, OpenConnection, SendData from mitmproxy.proxy.context import Context from mitmproxy.proxy.events import ConnectionClosed, DataReceived from mitmproxy.proxy.layers import http @@ -353,19 +353,19 @@ def test_http2_client_aborts(tctx, stream, when, how): @pytest.mark.xfail(reason="inbound validation turned on to protect against request smuggling") -def test_no_normalization(tctx): +@pytest.mark.parametrize("normalize", [True, False]) +def test_no_normalization(tctx, normalize): """Test that we don't normalize headers when we just pass them through.""" + tctx.options.normalize_outbound_headers = normalize server = Placeholder(Server) flow = Placeholder(HTTPFlow) playbook, cff = start_h2_client(tctx) - request_headers = example_request_headers + ( - (b"Should-Not-Be-Capitalized! ", b" :) "), - ) - response_headers = example_response_headers + ( - (b"Same", b"Here"), - ) + request_headers = list(example_request_headers) + [(b"Should-Not-Be-Capitalized! ", b" :) ")] + request_headers_lower = [(k.lower(), v) for (k, v) in request_headers] + response_headers = list(example_response_headers) + [(b"Same", b"Here")] + response_headers_lower = [(k.lower(), v) for (k, v) in response_headers] initial = Placeholder(bytes) assert ( @@ -385,18 +385,22 @@ def test_no_normalization(tctx): hyperframe.frame.SettingsFrame, hyperframe.frame.HeadersFrame, ] - assert hpack.hpack.Decoder().decode(frames[1].data, True) == list(request_headers) + assert hpack.hpack.Decoder().decode(frames[1].data, True) == request_headers_lower if normalize else request_headers sff = FrameFactory() - assert ( + ( playbook >> DataReceived(server, sff.build_headers_frame(response_headers, flags=["END_STREAM"]).serialize()) << http.HttpResponseHeadersHook(flow) >> reply() << http.HttpResponseHook(flow) >> reply() - << SendData(tctx.client, cff.build_headers_frame(response_headers, flags=["END_STREAM"]).serialize()) ) + if normalize: + playbook << Log("Lowercased 'Same' header as uppercase is not allowed with HTTP/2.") + hdrs = response_headers_lower if normalize else response_headers + assert playbook << SendData(tctx.client, cff.build_headers_frame(hdrs, flags=["END_STREAM"]).serialize()) + assert flow().request.headers.fields == ((b"Should-Not-Be-Capitalized! ", b" :) "),) assert flow().response.headers.fields == ((b"Same", b"Here"),) diff --git a/web/src/js/ducks/_options_gen.ts b/web/src/js/ducks/_options_gen.ts index f077cafc7..56ee7bcaf 100644 --- a/web/src/js/ducks/_options_gen.ts +++ b/web/src/js/ducks/_options_gen.ts @@ -34,6 +34,7 @@ export interface OptionsState { mode: string modify_body: string[] modify_headers: string[] + normalize_outbound_headers: boolean onboarding: boolean onboarding_host: string onboarding_port: number @@ -120,6 +121,7 @@ export const defaultState: OptionsState = { mode: "regular", modify_body: [], modify_headers: [], + normalize_outbound_headers: true, onboarding: true, onboarding_host: "mitm.it", onboarding_port: 80,