From 856a35af6dee32090765ec0f5de0573ef63db4dd Mon Sep 17 00:00:00 2001 From: Denis Kasak Date: Tue, 9 Feb 2021 18:44:46 +0000 Subject: [PATCH] Use original flow host instead of IP when exporting to curl/httpie. (#4307) Use original flow host instead of IP when exporting to curl/httpie. Unless this is done, the SNI server name will not be sent, often making the curl/httpie command have different behaviour than the original request (most often in the form of failing to establish a TLS connection). With this change, we always use the original host, fixing this failure. However, if the original host is a domain, it may sometimes resolve to a different IP address later on. In curl, we solve this problem by forcing it to connect to the original IP using `--resolve`. For httpie there is currently no easy solution (see: https://github.com/httpie/httpie/issues/414). --- CHANGELOG.md | 2 ++ mitmproxy/addons/export.py | 44 ++++++++++++++++++++++++---- test/mitmproxy/addons/test_export.py | 33 ++++++++++++++++++++- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 633dd2670..516d1b2f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ If you depend on these features, please raise your voice in * Fix IDNA host 'Bad HTTP request line' error (@grahamrobbins) * Pressing `?` now exits console help view (@abitrolly) * `--modify-headers` now works correctly when modifying a header that is also part of the filter expression (@Prinzhorn) +* Fix SNI-related reproducibility issues when exporting to curl/httpie commands. (@dkasak) +* Add option `export_preserve_original_ip` to force exported command to connect to IP from original request. Only supports curl at the moment. (@dkasak) * --- TODO: add new PRs above this line --- * ... and various other fixes, documentation improvements, dependency version bumps, etc. diff --git a/mitmproxy/addons/export.py b/mitmproxy/addons/export.py index 6bcfa090e..d1be99c7e 100644 --- a/mitmproxy/addons/export.py +++ b/mitmproxy/addons/export.py @@ -55,10 +55,18 @@ def request_content_for_console(request: http.Request) -> str: ) -def curl_command(f: flow.Flow) -> str: +def curl_command(f: flow.Flow, preserve_ip: bool = False) -> str: request = cleanup_request(f) request = pop_headers(request) args = ["curl"] + + server_addr = f.server_conn.peername[0] if f.server_conn.peername else None + + if preserve_ip and server_addr and request.pretty_host != server_addr: + resolve = "{}:{}:[{}]".format(request.pretty_host, request.port, server_addr) + args.append("--resolve") + args.append(resolve) + for k, v in request.headers.items(multi=True): if k.lower() == "accept-encoding": args.append("--compressed") @@ -67,7 +75,9 @@ def curl_command(f: flow.Flow) -> str: if request.method != "GET": args += ["-X", request.method] - args.append(request.url) + + args.append(request.pretty_url) + if request.content: args += ["-d", request_content_for_console(request)] return ' '.join(shlex.quote(arg) for arg in args) @@ -76,7 +86,13 @@ def curl_command(f: flow.Flow) -> str: def httpie_command(f: flow.Flow) -> str: request = cleanup_request(f) request = pop_headers(request) - args = ["http", request.method, request.url] + + # TODO: Once https://github.com/httpie/httpie/issues/414 is implemented, we + # should ensure we always connect to the IP address specified in the flow, + # similar to how it's done in curl_command. + url = request.pretty_url + + args = ["http", request.method, url] for k, v in request.headers.items(multi=True): args.append(f"{k}: {v}") cmd = ' '.join(shlex.quote(arg) for arg in args) @@ -119,6 +135,18 @@ formats = dict( class Export(): + def load(self, loader): + loader.add_option( + "export_preserve_original_ip", bool, False, + """ + When exporting a request as an external command, make an effort to + connect to the same IP as in the original request. This helps with + reproducibility in cases where the behaviour depends on the + particular host we are connecting to. Currently this only affects + curl exports. + """ + ) + @command.command("export.formats") def formats(self) -> typing.Sequence[str]: """ @@ -134,7 +162,10 @@ class Export(): if format not in formats: raise exceptions.CommandError("No such export format: %s" % format) func: typing.Any = formats[format] - v = func(flow) + if format == "curl": + v = func(flow, preserve_ip=ctx.options.export_preserve_original_ip) + else: + v = func(flow) try: with open(path, "wb") as fp: if isinstance(v, bytes): @@ -152,7 +183,10 @@ class Export(): if format not in formats: raise exceptions.CommandError("No such export format: %s" % format) func: typing.Any = formats[format] - v = strutils.always_str(func(flow)) + if format == "curl": + v = strutils.always_str(func(flow, preserve_ip=ctx.options.export_preserve_original_ip)) + else: + v = strutils.always_str(func(flow)) try: pyperclip.copy(v) except pyperclip.PyperclipException as e: diff --git a/test/mitmproxy/addons/test_export.py b/test/mitmproxy/addons/test_export.py index 9d12535c1..56639c96a 100644 --- a/test/mitmproxy/addons/test_export.py +++ b/test/mitmproxy/addons/test_export.py @@ -98,6 +98,20 @@ class TestExportCurlCommand: result = """curl --compressed 'http://address:22/path?a=foo&a=bar&b=baz'""" assert export.curl_command(get_request) == result + # This tests that we always specify the original host in the URL, which is + # important for SNI. If option `export_preserve_original_ip` is true, we + # ensure that we still connect to the same IP by using curl's `--resolve` + # option. + def test_correct_host_used(self, get_request): + get_request.request.headers["host"] = "domain:22" + + result = """curl -H 'header: qvalue' -H 'host: domain:22' 'http://domain:22/path?a=foo&a=bar&b=baz'""" + assert export.curl_command(get_request) == result + + result = """curl --resolve 'domain:22:[192.168.0.1]' -H 'header: qvalue' -H 'host: domain:22' """ \ + """'http://domain:22/path?a=foo&a=bar&b=baz'""" + assert export.curl_command(get_request, preserve_ip=True) == result + class TestExportHttpieCommand: def test_get(self, get_request): @@ -136,6 +150,19 @@ class TestExportHttpieCommand: assert shlex.split(command)[-2] == '<<<' assert shlex.split(command)[-1] == "'&#" + # See comment in `TestExportCurlCommand.test_correct_host_used`. httpie + # currently doesn't have a way of forcing connection to a particular IP, so + # the command-line may not always reproduce the original request, in case + # the host is resolved to a different IP address. + # + # httpie tracking issue: https://github.com/httpie/httpie/issues/414 + def test_correct_host_used(self, get_request): + get_request.request.headers["host"] = "domain:22" + + result = """http GET 'http://domain:22/path?a=foo&a=bar&b=baz' """ \ + """'header: qvalue' 'host: domain:22'""" + assert export.httpie_command(get_request) == result + class TestRaw: def test_req_and_resp_present(self, get_flow): @@ -197,7 +224,9 @@ def qr(f): def test_export(tmpdir): f = str(tmpdir.join("path")) e = export.Export() - with taddons.context(): + with taddons.context() as tctx: + tctx.configure(e) + assert e.formats() == ["curl", "httpie", "raw", "raw_request", "raw_response"] with pytest.raises(exceptions.CommandError): e.file("nonexistent", tflow.tflow(resp=True), f) @@ -239,6 +268,8 @@ async def test_export_open(exception, log_message, tmpdir): async def test_clip(tmpdir): e = export.Export() with taddons.context() as tctx: + tctx.configure(e) + with pytest.raises(exceptions.CommandError): e.clip("nonexistent", tflow.tflow(resp=True))