Merge pull request #5542 from mhils/os-proxy

Simplify transparent mode, fix listening on port 0

mitmproxy/proxy/commands.py

@@ -119,15 +119,6 @@ class StartHook(Command, mitmproxy.hooks.Hook):
         return super().__new__(cls, *args, **kwargs)
 
 
-class GetSocket(ConnectionCommand):
-    """
-    Get the underlying socket.
-    This should really never be used, but is required to implement transparent mode.
-    """
-
-    blocking = True
-
-
 class Log(Command):
     message: str
     level: str

mitmproxy/proxy/events.py

@@ -3,7 +3,6 @@ When IO actions occur at the proxy server, they are passed down to layers as eve
 Events represent the only way for layers to receive new data from sockets.
 The counterpart to events are commands.
 """
-import socket
 import warnings
 from dataclasses import dataclass, is_dataclass
 from typing import Any, Generic, Optional, TypeVar
@@ -112,12 +111,6 @@ class HookCompleted(CommandCompleted):
     reply: None = None
 
 
-@dataclass(repr=False)
-class GetSocketCompleted(CommandCompleted):
-    command: commands.GetSocket
-    reply: socket.socket
-
-
 T = TypeVar("T")
 
 

mitmproxy/proxy/layers/modes.py

@@ -4,7 +4,7 @@ from abc import ABCMeta
 from dataclasses import dataclass
 from typing import Optional
 
-from mitmproxy import connection, platform
+from mitmproxy import connection
 from mitmproxy.proxy import commands, events, layer
 from mitmproxy.proxy.commands import StartHook
 from mitmproxy.proxy.layers import tls
@@ -74,15 +74,8 @@ class ReverseProxy(DestinationKnown):
 class TransparentProxy(DestinationKnown):
     @expect(events.Start)
     def _handle_event(self, event: events.Event) -> layer.CommandGenerator[None]:
-        assert platform.original_addr is not None
-        socket = yield commands.GetSocket(self.context.client)
-        try:
-            self.context.server.address = platform.original_addr(socket)
-        except Exception as e:
-            yield commands.Log(f"Transparent mode failure: {e!r}")
-
+        assert self.context.server.address
         self.child_layer = layer.NextLayer(self.context)
-
         err = yield from self.finish_start()
         if err:
             yield commands.CloseConnection(self.context.client)

mitmproxy/proxy/mode_servers.py

@@ -13,13 +13,14 @@ from __future__ import annotations
 
 import asyncio
 import errno
+import socket
 import struct
 import typing
 from abc import ABCMeta, abstractmethod
 from contextlib import contextmanager
 from typing import ClassVar, Generic, TypeVar, cast, get_args
 
-from mitmproxy import ctx, flow, log
+from mitmproxy import ctx, flow, log, platform
 from mitmproxy.connection import Address
 from mitmproxy.master import Master
 from mitmproxy.net import udp
@@ -37,6 +38,19 @@ class ProxyConnectionHandler(server.LiveConnectionHandler):
         super().__init__(r, w, options, mode)
         self.log_prefix = f"{human.format_address(self.client.peername)}: "
 
+    async def handle_client(self) -> None:
+        if self.client.proxy_mode.type == "transparent":
+            writer = self.transports[self.client].writer
+            assert writer
+            socket = writer.get_extra_info("socket")
+            try:
+                assert platform.original_addr
+                self.layer.context.server.address = platform.original_addr(socket)
+            except Exception as e:
+                self.log(f"Transparent mode failure: {e!r}")
+                return
+        return await super().handle_client()
+
     async def handle_hook(self, hook: commands.StartHook) -> None:
         with self.timeout_watchdog.disarm():
             # We currently only support single-argument hooks.
@@ -117,11 +131,20 @@ class AsyncioServerInstance(ServerInstance[M], metaclass=ABCMeta):
     def is_running(self) -> bool:
         return self._server is not None
 
-    async def start(self):
+    async def start(self) -> None:
         assert self._server is None
         host = self.mode.listen_host(ctx.options.listen_host)
         port = self.mode.listen_port(ctx.options.listen_port)
         try:
+            # workaround for https://github.com/python/cpython/issues/89856:
+            # We want both IPv4 and IPv6 sockets to bind to the same port.
+            if port == 0:
+                s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+                s.bind((host, 0))
+                port = s.getsockname()[1]
+                s.close()
+                # there is a slight race condition here where the port is reused by another application between the
+                # close above and the listen below. We ignore this until it we have an actual bug report for it.
             self._server = await self.listen(host, port)
             self._listen_addrs = tuple(s.getsockname() for s in self._server.sockets)
         except OSError as e:
@@ -138,7 +161,7 @@ class AsyncioServerInstance(ServerInstance[M], metaclass=ABCMeta):
             self.last_exception = None
         ctx.log.info(f"{self.log_desc} listening at {' and '.join(map(human.format_address, self._listen_addrs))}.")
 
-    async def stop(self):
+    async def stop(self) -> None:
         assert self._server is not None
         # we always reset _server and _listen_addrs and ignore failures
         server = self._server
@@ -215,7 +238,7 @@ class UpstreamInstance(TcpServerInstance[mode_specs.UpstreamMode]):
 
 
 class TransparentInstance(TcpServerInstance[mode_specs.TransparentMode]):
-    log_desc = "Transparent proxy"
+    log_desc = "transparent proxy"
 
     def make_top_layer(self, context: Context) -> Layer:
         return layers.modes.TransparentProxy(context)
@@ -224,7 +247,7 @@ class TransparentInstance(TcpServerInstance[mode_specs.TransparentMode]):
 class ReverseInstance(TcpServerInstance[mode_specs.ReverseMode]):
     @property
     def log_desc(self) -> str:
-        return f"Reverse proxy to {self.mode.data}"
+        return f"reverse proxy to {self.mode.data}"
 
     def make_top_layer(self, context: Context) -> Layer:
         return layers.modes.ReverseProxy(context)

mitmproxy/proxy/server.py

@@ -357,11 +357,6 @@ class ConnectionHandler(metaclass=abc.ABCMeta):
                     writer.write(command.data)
                 elif isinstance(command, commands.CloseConnection):
                     self.close_connection(command.connection, command.half_close)
-                elif isinstance(command, commands.GetSocket):
-                    writer = self.transports[command.connection].writer
-                    assert writer
-                    socket = writer.get_extra_info("socket")
-                    self.server_event(events.GetSocketCompleted(command, socket))
                 elif isinstance(command, commands.StartHook):
                     asyncio_utils.create_task(
                         self.hook_task(command),

test/mitmproxy/proxy/layers/test_modes.py

@@ -2,12 +2,10 @@ import copy
 
 import pytest
 
-from mitmproxy import platform
 from mitmproxy.addons.proxyauth import ProxyAuth
 from mitmproxy.connection import Client, Server
 from mitmproxy.proxy.commands import (
     CloseConnection,
-    GetSocket,
     Log,
     OpenConnection,
     SendData,
@@ -218,16 +216,12 @@ def test_reverse_proxy_tcp_over_tls(
 
 
 @pytest.mark.parametrize("connection_strategy", ["eager", "lazy"])
-def test_transparent_tcp(tctx: Context, monkeypatch, connection_strategy):
-    monkeypatch.setattr(platform, "original_addr", lambda sock: ("address", 22))
-
+def test_transparent_tcp(tctx: Context, connection_strategy):
     flow = Placeholder(TCPFlow)
     tctx.options.connection_strategy = connection_strategy
+    tctx.server.address = ("address", 22)
 
-    sock = object()
     playbook = Playbook(modes.TransparentProxy(tctx))
-    playbook << GetSocket(tctx.client)
-    playbook >> reply(sock)
     if connection_strategy == "lazy":
         assert playbook
     else:
@@ -250,23 +244,6 @@ def test_transparent_tcp(tctx: Context, monkeypatch, connection_strategy):
     assert tctx.server.address == ("address", 22)
 
 
-def test_transparent_failure(tctx: Context, monkeypatch):
-    """Test that we recover from a transparent mode resolve error."""
-
-    def raise_err(sock):
-        raise RuntimeError("platform-specific error")
-
-    monkeypatch.setattr(platform, "original_addr", raise_err)
-    assert (
-        Playbook(modes.TransparentProxy(tctx), logs=True)
-        << GetSocket(tctx.client)
-        >> reply(object())
-        << Log(
-            "Transparent mode failure: RuntimeError('platform-specific error')", "info"
-        )
-    )
-
-
 def test_reverse_eager_connect_failure(tctx: Context):
     """
     Test
@@ -286,15 +263,13 @@ def test_reverse_eager_connect_failure(tctx: Context):
     )
 
 
-def test_transparent_eager_connect_failure(tctx: Context, monkeypatch):
-    """Test that we recover from a transparent mode resolve error."""
+def test_transparent_eager_connect_failure(tctx: Context):
+    """Test that we recover from a transparent mode connect error."""
     tctx.options.connection_strategy = "eager"
-    monkeypatch.setattr(platform, "original_addr", lambda sock: ("address", 22))
+    tctx.server.address = ("address", 22)
 
     assert (
         Playbook(modes.TransparentProxy(tctx), logs=True)
-        << GetSocket(tctx.client)
-        >> reply(object())
         << OpenConnection(tctx.server)
         >> reply("something something")
         << CloseConnection(tctx.client)

test/mitmproxy/proxy/test_commands.py

@@ -17,7 +17,6 @@ def test_dataclasses(tconn):
     assert repr(commands.SendData(tconn, b"foo"))
     assert repr(commands.OpenConnection(tconn))
     assert repr(commands.CloseConnection(tconn))
-    assert repr(commands.GetSocket(tconn))
     assert repr(commands.Log("hello", "info"))
 
 

test/mitmproxy/proxy/test_mode_servers.py

@@ -4,6 +4,8 @@ from unittest.mock import AsyncMock, MagicMock, Mock
 
 import pytest
 
+from mitmproxy import platform
+from mitmproxy.addons.proxyserver import Proxyserver
 from mitmproxy.net import udp
 from mitmproxy.proxy.mode_servers import DnsInstance, ServerInstance, DtlsInstance
 from mitmproxy.test import taddons
@@ -70,6 +72,38 @@ async def test_tcp_start_stop():
         assert await tctx.master.await_log("stopped HTTP(S) proxy")
 
 
+@pytest.mark.parametrize("failure", [True, False])
+async def test_transparent(failure, monkeypatch):
+    manager = MagicMock()
+
+    if failure:
+        monkeypatch.setattr(platform, "original_addr", None)
+    else:
+        monkeypatch.setattr(platform, "original_addr", lambda s: ("address", 42))
+
+    with taddons.context(Proxyserver()) as tctx:
+        tctx.options.connection_strategy = "lazy"
+        inst = ServerInstance.make("transparent@127.0.0.1:0", manager)
+        await inst.start()
+        await tctx.master.await_log("proxy listening")
+
+        host, port, *_ = inst.listen_addrs[0]
+        reader, writer = await asyncio.open_connection(host, port)
+
+        if failure:
+            assert await tctx.master.await_log("Transparent mode failure")
+            writer.close()
+            await writer.wait_closed()
+        else:
+            assert await tctx.master.await_log("client connect")
+            writer.close()
+            await writer.wait_closed()
+            assert await tctx.master.await_log("client disconnect")
+
+        await inst.stop()
+        assert await tctx.master.await_log("stopped transparent proxy")
+
+
 async def test_tcp_start_error():
     manager = MagicMock()