From 8b3e3e7b40830aa8a6e9acbd3e190356f2511049 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 14:34:24 +0200 Subject: [PATCH 1/6] remove unused basethread implementation --- mitmproxy/coretypes/basethread.py | 11 ----------- test/mitmproxy/coretypes/test_basethread.py | 7 ------- 2 files changed, 18 deletions(-) delete mode 100644 mitmproxy/coretypes/basethread.py delete mode 100644 test/mitmproxy/coretypes/test_basethread.py diff --git a/mitmproxy/coretypes/basethread.py b/mitmproxy/coretypes/basethread.py deleted file mode 100644 index 9a3c64bdc..000000000 --- a/mitmproxy/coretypes/basethread.py +++ /dev/null @@ -1,11 +0,0 @@ -import time -import threading - - -class BaseThread(threading.Thread): - def __init__(self, name, *args, **kwargs): - super().__init__(name=name, *args, **kwargs) - self._thread_started = time.time() - - def _threadinfo(self): - return "%s - age: %is" % (self.name, int(time.time() - self._thread_started)) diff --git a/test/mitmproxy/coretypes/test_basethread.py b/test/mitmproxy/coretypes/test_basethread.py deleted file mode 100644 index 59e28bb0e..000000000 --- a/test/mitmproxy/coretypes/test_basethread.py +++ /dev/null @@ -1,7 +0,0 @@ -import re -from mitmproxy.coretypes import basethread - - -def test_basethread(): - t = basethread.BaseThread("foobar") - assert re.match(r"foobar - age: \d+s", t._threadinfo()) From b5834a604b80441e23a07e368e60d0e073d11d1f Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 16:39:38 +0200 Subject: [PATCH 2/6] wireguard: specify dns server without this Android clients will do DNS lookups outside of WireGuard, which a) is leaking and b) may or may not work at all. --- mitmproxy/addons/dns_resolver.py | 8 +++++++- mitmproxy/proxy/mode_servers.py | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mitmproxy/addons/dns_resolver.py b/mitmproxy/addons/dns_resolver.py index f4b5ac59e..92624de46 100644 --- a/mitmproxy/addons/dns_resolver.py +++ b/mitmproxy/addons/dns_resolver.py @@ -139,7 +139,13 @@ async def resolve_message( class DnsResolver: async def dns_request(self, flow: dns.DNSFlow) -> None: should_resolve = ( - isinstance(flow.client_conn.proxy_mode, mode_specs.DnsMode) + ( + isinstance(flow.client_conn.proxy_mode, mode_specs.DnsMode) + or ( + isinstance(flow.client_conn.proxy_mode, mode_specs.WireGuardMode) + and flow.server_conn.address == ("10.0.0.53", 53) + ) + ) and flow.live and not flow.response and not flow.error diff --git a/mitmproxy/proxy/mode_servers.py b/mitmproxy/proxy/mode_servers.py index 3bc515f27..60762626b 100644 --- a/mitmproxy/proxy/mode_servers.py +++ b/mitmproxy/proxy/mode_servers.py @@ -352,6 +352,7 @@ class WireGuardServerInstance(ServerInstance[mode_specs.WireGuardMode]): [Interface] PrivateKey = {self.client_key} Address = 10.0.0.1/32 + DNS = 10.0.0.53 [Peer] PublicKey = {wg.pubkey(self.server_key)} From 539dff7c2af8767f645c7c1a00f7c074cd89539e Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 16:42:18 +0200 Subject: [PATCH 3/6] h2: use proper logging --- mitmproxy/proxy/layers/http/_http2.py | 4 ++-- mitmproxy/proxy/layers/http/_http_h2.py | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/mitmproxy/proxy/layers/http/_http2.py b/mitmproxy/proxy/layers/http/_http2.py index 2c7a20b03..540606fb1 100644 --- a/mitmproxy/proxy/layers/http/_http2.py +++ b/mitmproxy/proxy/layers/http/_http2.py @@ -70,8 +70,8 @@ class Http2Connection(HttpConnection): super().__init__(context, conn) if self.debug: self.h2_conf.logger = H2ConnectionLogger( - f"{human.format_address(self.context.client.peername)}: " - f"{self.__class__.__name__}" + self.context.client.peername, + self.__class__.__name__ ) self.h2_conf.validate_inbound_headers = ( self.context.options.validate_inbound_headers diff --git a/mitmproxy/proxy/layers/http/_http_h2.py b/mitmproxy/proxy/layers/http/_http_h2.py index abcce6579..8533b7afb 100644 --- a/mitmproxy/proxy/layers/http/_http_h2.py +++ b/mitmproxy/proxy/layers/http/_http_h2.py @@ -1,4 +1,5 @@ import collections +import logging from typing import Dict, List, NamedTuple, Tuple import h2.config @@ -9,16 +10,29 @@ import h2.settings import h2.stream +logger = logging.getLogger(__name__) + + class H2ConnectionLogger(h2.config.DummyLogger): - def __init__(self, name: str): + def __init__(self, peername: tuple, conn_type: str): super().__init__() - self.name = name + self.peername = peername + self.conn_type = conn_type def debug(self, fmtstr, *args): - print(f"{self.name} h2 (debug): {fmtstr % args}") + logger.debug( + f"{self.conn_type} {fmtstr}", + *args, + extra={"client": self.peername} + ) def trace(self, fmtstr, *args): - print(f"{self.name} h2 (trace): {fmtstr % args}") + logger.log( + logging.DEBUG - 1, + f"{self.conn_type} {fmtstr}", + *args, + extra={"client": self.peername} + ) class SendH2Data(NamedTuple): From 46c3e5f374f16bcb65e046efb2f51ff7e038660f Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 16:42:38 +0200 Subject: [PATCH 4/6] don't dim debug logs that makes them too hard to read. --- mitmproxy/log.py | 2 +- test/mitmproxy/addons/test_termlog.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mitmproxy/log.py b/mitmproxy/log.py index 2e4597372..05b609c7d 100644 --- a/mitmproxy/log.py +++ b/mitmproxy/log.py @@ -51,7 +51,7 @@ class MitmFormatter(logging.Formatter): message = miniclick.style( message, fg=LOG_COLORS.get(record.levelno), - dim=(record.levelno <= logging.DEBUG) + # dim=(record.levelno <= logging.DEBUG) ) if client := getattr(record, "client", None): client = human.format_address(client) diff --git a/test/mitmproxy/addons/test_termlog.py b/test/mitmproxy/addons/test_termlog.py index c742c80e3..209f8eb89 100644 --- a/test/mitmproxy/addons/test_termlog.py +++ b/test/mitmproxy/addons/test_termlog.py @@ -53,5 +53,5 @@ async def test_styling(monkeypatch) -> None: tctx.configure(t) logging.warning("hello") - assert "\x1b[33m\x1b[22mhello\x1b[0m" in f.getvalue() + assert "\x1b[33mhello\x1b[0m" in f.getvalue() t.done() From 52f303c63680b27d36d3a6d98c4c7011c9674afd Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 16:43:18 +0200 Subject: [PATCH 5/6] fix nits --- mitmproxy/proxy/layers/modes.py | 1 + mitmproxy/tools/main.py | 1 + mitmproxy/tools/web/app.py | 3 ++- web/src/css/capture-setup.less | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/mitmproxy/proxy/layers/modes.py b/mitmproxy/proxy/layers/modes.py index b80d5104a..d320b513f 100644 --- a/mitmproxy/proxy/layers/modes.py +++ b/mitmproxy/proxy/layers/modes.py @@ -37,6 +37,7 @@ class DestinationKnown(layer.Layer, metaclass=ABCMeta): if ( self.context.options.connection_strategy == "eager" and self.context.server.address + and self.context.server.transport_protocol == "tcp" ): err = yield commands.OpenConnection(self.context.server) if err: diff --git a/mitmproxy/tools/main.py b/mitmproxy/tools/main.py index c640326aa..749356a63 100644 --- a/mitmproxy/tools/main.py +++ b/mitmproxy/tools/main.py @@ -53,6 +53,7 @@ def run( logging.getLogger().setLevel(logging.DEBUG) logging.getLogger("tornado").setLevel(logging.WARNING) logging.getLogger("asyncio").setLevel(logging.WARNING) + logging.getLogger("hpack").setLevel(logging.WARNING) debug.register_info_dumpers() opts = options.Options() diff --git a/mitmproxy/tools/web/app.py b/mitmproxy/tools/web/app.py index 65d1e06c2..66868abb6 100644 --- a/mitmproxy/tools/web/app.py +++ b/mitmproxy/tools/web/app.py @@ -290,7 +290,8 @@ class WebSocketEventBroadcaster(tornado.websocket.WebSocketHandler): for conn in cls.connections: try: - conn.write_message(message) + if not conn.ws_connection.is_closing(): + conn.write_message(message) except Exception: # pragma: no cover logging.error("Error sending message", exc_info=True) diff --git a/web/src/css/capture-setup.less b/web/src/css/capture-setup.less index d81145320..290dab444 100644 --- a/web/src/css/capture-setup.less +++ b/web/src/css/capture-setup.less @@ -1,4 +1,7 @@ .wireguard-config { + > * { + margin: 0; + } margin: 1rem 0; display: flex; flex-wrap: wrap; From 3fcb3f0773259caf94a5d2708183de8273bf1257 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 19 Sep 2022 17:04:40 +0200 Subject: [PATCH 6/6] set `client.sockname` to original destination for transparent modes If we don't do this we have non-unique connection tuples, which would be bad. --- mitmproxy/addons/proxyserver.py | 2 +- mitmproxy/proxy/mode_servers.py | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/mitmproxy/addons/proxyserver.py b/mitmproxy/addons/proxyserver.py index ec11cea3a..9d592ccd7 100644 --- a/mitmproxy/addons/proxyserver.py +++ b/mitmproxy/addons/proxyserver.py @@ -270,7 +270,7 @@ class Proxyserver(ServerManager): def inject_event(self, event: events.MessageInjected): connection_id = ( - "tcp", + event.flow.client_conn.transport_protocol, event.flow.client_conn.peername, event.flow.client_conn.sockname, ) diff --git a/mitmproxy/proxy/mode_servers.py b/mitmproxy/proxy/mode_servers.py index 60762626b..1baebea34 100644 --- a/mitmproxy/proxy/mode_servers.py +++ b/mitmproxy/proxy/mode_servers.py @@ -139,11 +139,6 @@ class ServerInstance(Generic[M], metaclass=ABCMeta): reader: asyncio.StreamReader | wg.TcpStream, writer: asyncio.StreamWriter | wg.TcpStream, ) -> None: - connection_id = ( - "tcp", - writer.get_extra_info("peername"), - writer.get_extra_info("sockname"), - ) handler = ProxyConnectionHandler( ctx.master, reader, writer, ctx.options, self.mode ) @@ -152,12 +147,23 @@ class ServerInstance(Generic[M], metaclass=ABCMeta): socket = writer.get_extra_info("socket") try: assert platform.original_addr - handler.layer.context.server.address = platform.original_addr(socket) + original_dst = platform.original_addr(socket) except Exception as e: logger.error(f"Transparent mode failure: {e!r}") return + else: + handler.layer.context.client.sockname = original_dst + handler.layer.context.server.address = original_dst elif isinstance(self.mode, mode_specs.WireGuardMode): - handler.layer.context.server.address = writer.get_extra_info("original_dst") + original_dst = writer.get_extra_info("original_dst") + handler.layer.context.client.sockname = original_dst + handler.layer.context.server.address = original_dst + + connection_id = ( + handler.layer.context.client.transport_protocol, + handler.layer.context.client.peername, + handler.layer.context.client.sockname, + ) with self.manager.register_connection(connection_id, handler): await handler.handle_client()