lowercase user-added HTTP/2 headers, fix #4746 (#5186)

This commit is contained in:
Maximilian Hils 2022-03-16 10:59:30 +01:00 committed by GitHub
parent 6f0587734e
commit 148429c0b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 41 additions and 14 deletions

View File

@ -5,7 +5,7 @@
* Support proxy authentication for SOCKS v5 mode (@starplanet) * Support proxy authentication for SOCKS v5 mode (@starplanet)
* Make it possible to ignore connections in the tls_clienthello event hook (@mhils) * 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) * 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) * Trigger event hooks for flows with semantically invalid requests, for example invalid content-length headers (@mhils)
* Improve error message on TLS version mismatch (@mhils) * Improve error message on TLS version mismatch (@mhils)
* Windows: Switch to Python's default asyncio event loop, which increases the number of sockets * 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. * Change connection event hooks to be blocking.
Processing will only resume once the event hook has finished. (@Prinzhorn) Processing will only resume once the event hook has finished. (@Prinzhorn)
* Allow addon hooks to be async (@nneonneo, #4207) * 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) * Speculative fix for some rare HTTP/2 connection stalls (#5158, @EndUser509)
* Add ability to specify custom ports with LDAP authentication (#5068, @demonoidvk) * Add ability to specify custom ports with LDAP authentication (#5068, @demonoidvk)
* Console Improvements on Windows (@mhils) * Console Improvements on Windows (@mhils)
* Fix processing of `--set` options (#5067, @marwinxxii) * 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 ## 28 September 2021: mitmproxy 7.0.4

View File

@ -90,6 +90,14 @@ class Proxyserver:
"proxy_debug", bool, False, "proxy_debug", bool, False,
"Enable debug logs in the proxy core.", "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): def running(self):
self.master = ctx.master self.master = ctx.master

View File

@ -269,6 +269,13 @@ def normalize_h1_headers(headers: List[Tuple[bytes, bytes]], is_client: bool) ->
return headers 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): class Http2Server(Http2Connection):
h2_conf = h2.config.H2Configuration( h2_conf = h2.config.H2Configuration(
**Http2Connection.h2_conf_defaults, **Http2Connection.h2_conf_defaults,
@ -290,7 +297,10 @@ class Http2Server(Http2Connection):
(b":status", b"%d" % event.response.status_code), (b":status", b"%d" % event.response.status_code),
*event.response.headers.fields *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) headers = normalize_h1_headers(headers, False)
self.h2_conn.send_headers( self.h2_conn.send_headers(
@ -407,6 +417,8 @@ class Http2Client(Http2Connection):
if event.request.is_http2: if event.request.is_http2:
hdrs = list(event.request.headers.fields) hdrs = list(event.request.headers.fields)
if self.context.options.normalize_outbound_headers:
yield from normalize_h2_headers(hdrs)
else: else:
headers = event.request.headers headers = event.request.headers
if not event.request.authority and "host" in headers: if not event.request.authority and "host" in headers:

View File

@ -10,7 +10,7 @@ from mitmproxy.connection import ConnectionState, Server
from mitmproxy.flow import Error from mitmproxy.flow import Error
from mitmproxy.http import HTTPFlow, Headers, Request from mitmproxy.http import HTTPFlow, Headers, Request
from mitmproxy.net.http import status_codes 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.context import Context
from mitmproxy.proxy.events import ConnectionClosed, DataReceived from mitmproxy.proxy.events import ConnectionClosed, DataReceived
from mitmproxy.proxy.layers import http 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") @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.""" """Test that we don't normalize headers when we just pass them through."""
tctx.options.normalize_outbound_headers = normalize
server = Placeholder(Server) server = Placeholder(Server)
flow = Placeholder(HTTPFlow) flow = Placeholder(HTTPFlow)
playbook, cff = start_h2_client(tctx) playbook, cff = start_h2_client(tctx)
request_headers = example_request_headers + ( request_headers = list(example_request_headers) + [(b"Should-Not-Be-Capitalized! ", b" :) ")]
(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 = example_response_headers + ( response_headers_lower = [(k.lower(), v) for (k, v) in response_headers]
(b"Same", b"Here"),
)
initial = Placeholder(bytes) initial = Placeholder(bytes)
assert ( assert (
@ -385,18 +385,22 @@ def test_no_normalization(tctx):
hyperframe.frame.SettingsFrame, hyperframe.frame.SettingsFrame,
hyperframe.frame.HeadersFrame, 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() sff = FrameFactory()
assert ( (
playbook playbook
>> DataReceived(server, sff.build_headers_frame(response_headers, flags=["END_STREAM"]).serialize()) >> DataReceived(server, sff.build_headers_frame(response_headers, flags=["END_STREAM"]).serialize())
<< http.HttpResponseHeadersHook(flow) << http.HttpResponseHeadersHook(flow)
>> reply() >> reply()
<< http.HttpResponseHook(flow) << http.HttpResponseHook(flow)
>> reply() >> 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().request.headers.fields == ((b"Should-Not-Be-Capitalized! ", b" :) "),)
assert flow().response.headers.fields == ((b"Same", b"Here"),) assert flow().response.headers.fields == ((b"Same", b"Here"),)

View File

@ -34,6 +34,7 @@ export interface OptionsState {
mode: string mode: string
modify_body: string[] modify_body: string[]
modify_headers: string[] modify_headers: string[]
normalize_outbound_headers: boolean
onboarding: boolean onboarding: boolean
onboarding_host: string onboarding_host: string
onboarding_port: number onboarding_port: number
@ -120,6 +121,7 @@ export const defaultState: OptionsState = {
mode: "regular", mode: "regular",
modify_body: [], modify_body: [],
modify_headers: [], modify_headers: [],
normalize_outbound_headers: true,
onboarding: true, onboarding: true,
onboarding_host: "mitm.it", onboarding_host: "mitm.it",
onboarding_port: 80, onboarding_port: 80,