From 60acbd79b9b81ff0e733628dc3ee538011a37778 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Sun, 6 May 2018 12:43:25 +1200 Subject: [PATCH] Remove allowremote addon, add an improved take called block We now have two options: block_global blocks global networks, block_private blocks private networks. The block_global option is true by default, and block_private is false by default. The addon name is "block" so the options are correctly prefixed. Also make option documentation precise, reduce verbosity of logs. --- mitmproxy/addons/__init__.py | 4 +- mitmproxy/addons/allowremote.py | 31 ----------- mitmproxy/addons/block.py | 37 +++++++++++++ test/mitmproxy/addons/test_allowremote.py | 61 ---------------------- test/mitmproxy/addons/test_block.py | 63 +++++++++++++++++++++++ 5 files changed, 102 insertions(+), 94 deletions(-) delete mode 100644 mitmproxy/addons/allowremote.py create mode 100644 mitmproxy/addons/block.py delete mode 100644 test/mitmproxy/addons/test_allowremote.py create mode 100644 test/mitmproxy/addons/test_block.py diff --git a/mitmproxy/addons/__init__.py b/mitmproxy/addons/__init__.py index 988bc9049..838fba9b2 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 ad4c49408..000000000 --- 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 000000000..a484f5c43 --- /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 c8e3eb9e8..000000000 --- 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 000000000..4446d89c5 --- /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