Merge pull request #3100 from cortesi/block

Remove allowremote addon, add an improved take called block

5 files changed, 102 insertions, 94 deletions

diff --git a/mitmproxy/addons/__init__.py b/mitmproxy/addons/__init__.py
index 988bc904..838fba9b 100644
--- a/mitmproxy/addons/__init__.py
+++ b/mitmproxy/addons/__init__.py
@@ -1,6 +1,6 @@
-from mitmproxy.addons import allowremote
 from mitmproxy.addons import anticache
 from mitmproxy.addons import anticomp
+from mitmproxy.addons import block
 from mitmproxy.addons import browser
 from mitmproxy.addons import check_ca
 from mitmproxy.addons import clientplayback
@@ -25,7 +25,7 @@ def default_addons():
     return [
         core.Core(),
         browser.Browser(),
-        allowremote.AllowRemote(),
+        block.Block(),
         anticache.AntiCache(),
         anticomp.AntiComp(),
         check_ca.CheckCA(),
diff --git a/mitmproxy/addons/allowremote.py b/mitmproxy/addons/allowremote.py
deleted file mode 100644
index ad4c4940..00000000
--- a/mitmproxy/addons/allowremote.py
+++ /dev/null
@@ -1,31 +0,0 @@
-import ipaddress
-from mitmproxy import ctx
-
-
-class AllowRemote:
-    def load(self, loader):
-        loader.add_option(
-            "allow_remote", bool, False,
-            """
-            Allow remote clients to connect to proxy. If set to false,
-            client will not be able to connect to proxy unless it is on the same network
-            or the proxyauth option is set
-            """
-        )
-
-    def clientconnect(self, layer):
-        address = ipaddress.ip_address(layer.client_conn.address[0])
-        if isinstance(address, ipaddress.IPv6Address):
-            address = address.ipv4_mapped or address
-
-        accept_connection = (
-            ctx.options.allow_remote or
-            ipaddress.ip_address(address).is_private or
-            ctx.options.proxyauth is not None
-        )
-
-        if not accept_connection:
-            layer.reply.kill()
-            ctx.log.warn("Client connection was killed because allow_remote option is set to false, "
-                         "client IP was not a private IP and proxyauth was not set.\n"
-                         "To allow remote connections set allow_remote option to true or set proxyauth option.")
diff --git a/mitmproxy/addons/block.py b/mitmproxy/addons/block.py
new file mode 100644
index 00000000..a484f5c4
--- /dev/null
+++ b/mitmproxy/addons/block.py
@@ -0,0 +1,37 @@
+import ipaddress
+from mitmproxy import ctx
+
+
+class Block:
+    def load(self, loader):
+        loader.add_option(
+            "block_global", bool, True,
+            """
+            Block connections from globally reachable networks, as defined in
+            the IANA special purpose registries.
+            """
+        )
+        loader.add_option(
+            "block_private", bool, False,
+            """
+            Block connections from private networks, as defined in the IANA
+            special purpose registries. This option does not affect loopback
+            addresses.
+            """
+        )
+
+    def clientconnect(self, layer):
+        address = ipaddress.ip_address(layer.client_conn.address[0])
+        if isinstance(address, ipaddress.IPv6Address):
+            address = address.ipv4_mapped or address
+
+        ipa = ipaddress.ip_address(address)
+        if ipa.is_loopback:
+            return
+
+        if ctx.options.block_private and ipa.is_private:
+            ctx.log.warn("Client connection from %s killed by block_private" % address)
+            layer.reply.kill()
+        if ctx.options.block_global and ipa.is_global:
+            ctx.log.warn("Client connection from %s killed by block_global" % address)
+            layer.reply.kill()
\ No newline at end of file
diff --git a/test/mitmproxy/addons/test_allowremote.py b/test/mitmproxy/addons/test_allowremote.py
deleted file mode 100644
index c8e3eb9e..00000000
--- a/test/mitmproxy/addons/test_allowremote.py
+++ /dev/null
@@ -1,61 +0,0 @@
-from unittest import mock
-import pytest
-
-from mitmproxy.addons import allowremote, proxyauth
-from mitmproxy.test import taddons
-
-
-@pytest.mark.parametrize("allow_remote, should_be_killed, address", [
-    (True, False, ("10.0.0.1",)),
-    (True, False, ("172.20.0.1",)),
-    (True, False, ("192.168.1.1",)),
-    (True, False, ("1.1.1.1",)),
-    (True, False, ("8.8.8.8",)),
-    (True, False, ("216.58.207.174",)),
-    (True, False, ("::ffff:1.1.1.1",)),
-    (True, False, ("::ffff:8.8.8.8",)),
-    (True, False, ("::ffff:216.58.207.174",)),
-    (True, False, ("::ffff:10.0.0.1",)),
-    (True, False, ("::ffff:172.20.0.1",)),
-    (True, False, ("::ffff:192.168.1.1",)),
-    (True, False, ("fe80::",)),
-    (True, False, ("2001:4860:4860::8888",)),
-    (False, False, ("10.0.0.1",)),
-    (False, False, ("172.20.0.1",)),
-    (False, False, ("192.168.1.1",)),
-    (False, True, ("1.1.1.1",)),
-    (False, True, ("8.8.8.8",)),
-    (False, True, ("216.58.207.174",)),
-    (False, True, ("::ffff:1.1.1.1",)),
-    (False, True, ("::ffff:8.8.8.8",)),
-    (False, True, ("::ffff:216.58.207.174",)),
-    (False, False, ("::ffff:10.0.0.1",)),
-    (False, False, ("::ffff:172.20.0.1",)),
-    (False, False, ("::ffff:192.168.1.1",)),
-    (False, False, ("fe80::",)),
-    (False, True, ("2001:4860:4860::8888",)),
-])
-@pytest.mark.asyncio
-async def test_allowremote(allow_remote, should_be_killed, address):
-    if allow_remote:
-        # prevent faulty tests
-        assert not should_be_killed
-
-    ar = allowremote.AllowRemote()
-    up = proxyauth.ProxyAuth()
-    with taddons.context(ar, up) as tctx:
-        tctx.options.allow_remote = allow_remote
-
-        with mock.patch('mitmproxy.proxy.protocol.base.Layer') as layer:
-            layer.client_conn.address = address
-
-            ar.clientconnect(layer)
-            if should_be_killed:
-                assert await tctx.master.await_log("Client connection was killed", "warn")
-            else:
-                assert tctx.master.logs == []
-            tctx.master.clear()
-
-            tctx.options.proxyauth = "any"
-            ar.clientconnect(layer)
-            assert tctx.master.logs == []
diff --git a/test/mitmproxy/addons/test_block.py b/test/mitmproxy/addons/test_block.py
new file mode 100644
index 00000000..4446d89c
--- /dev/null
+++ b/test/mitmproxy/addons/test_block.py
@@ -0,0 +1,63 @@
+from unittest import mock
+import pytest
+
+from mitmproxy.addons import block
+from mitmproxy.test import taddons
+
+
+@pytest.mark.parametrize("block_global, block_private, should_be_killed, address", [
+    # block_global: loopback
+    (True, False, False, ("127.0.0.1",)),
+    (True, False, False, ("::1",)),
+    # block_global: private
+    (True, False, False, ("10.0.0.1",)),
+    (True, False, False, ("172.20.0.1",)),
+    (True, False, False, ("192.168.1.1",)),
+    (True, False, False, ("::ffff:10.0.0.1",)),
+    (True, False, False, ("::ffff:172.20.0.1",)),
+    (True, False, False, ("::ffff:192.168.1.1",)),
+    (True, False, False, ("fe80::",)),
+    # block_global: global
+    (True, False, True, ("1.1.1.1",)),
+    (True, False, True, ("8.8.8.8",)),
+    (True, False, True, ("216.58.207.174",)),
+    (True, False, True, ("::ffff:1.1.1.1",)),
+    (True, False, True, ("::ffff:8.8.8.8",)),
+    (True, False, True, ("::ffff:216.58.207.174",)),
+    (True, False, True, ("2001:4860:4860::8888",)),
+
+
+    # block_private: loopback
+    (False, True, False, ("127.0.0.1",)),
+    (False, True, False, ("::1",)),
+    # block_private: private
+    (False, True, True, ("10.0.0.1",)),
+    (False, True, True, ("172.20.0.1",)),
+    (False, True, True, ("192.168.1.1",)),
+    (False, True, True, ("::ffff:10.0.0.1",)),
+    (False, True, True, ("::ffff:172.20.0.1",)),
+    (False, True, True, ("::ffff:192.168.1.1",)),
+    (False, True, True, ("fe80::",)),
+    # block_private: global
+    (False, True, False, ("1.1.1.1",)),
+    (False, True, False, ("8.8.8.8",)),
+    (False, True, False, ("216.58.207.174",)),
+    (False, True, False, ("::ffff:1.1.1.1",)),
+    (False, True, False, ("::ffff:8.8.8.8",)),
+    (False, True, False, ("::ffff:216.58.207.174",)),
+    (False, True, False, ("2001:4860:4860::8888",)),
+])
+@pytest.mark.asyncio
+async def test_block_global(block_global, block_private, should_be_killed, address):
+    ar = block.Block()
+    with taddons.context(ar) as tctx:
+        tctx.options.block_global = block_global
+        tctx.options.block_private = block_private
+        with mock.patch('mitmproxy.proxy.protocol.base.Layer') as layer:
+            layer.client_conn.address = address
+            ar.clientconnect(layer)
+            if should_be_killed:
+                assert layer.reply.kill.called
+                assert await tctx.master.await_log("killed", "warn")
+            else:
+                assert not layer.reply.kill.called