From 7bd63ee713650f2c22f3193f52a793f92eea75c4 Mon Sep 17 00:00:00 2001 From: Aldo Cortesi Date: Mon, 26 Feb 2018 11:26:32 +1300 Subject: [PATCH] Start consolidating core options This is a preparatory patch that paves the way to consolidating our core options in the core addon. It amalgamates the core_option_validation and core addons, prepares the test suite for a world where options live in core, and moves over two trivial options as a trial balloon. From here, things will get harder, but at the end of the process we'll have a core that's responsive to options. --- mitmproxy/addons/__init__.py | 2 - mitmproxy/addons/core.py | 57 +++++++++++++++++++ mitmproxy/addons/core_option_validation.py | 45 --------------- mitmproxy/addons/upstream_auth.py | 2 +- mitmproxy/options.py | 45 --------------- mitmproxy/test/taddons.py | 7 ++- mitmproxy/tools/cmdline.py | 4 +- test/mitmproxy/addons/test_core.py | 55 +++++++++++++++--- .../addons/test_core_option_validation.py | 42 -------------- test/mitmproxy/addons/test_proxyauth.py | 4 +- test/mitmproxy/addons/test_script.py | 4 +- test/mitmproxy/addons/test_upstream_auth.py | 2 +- test/mitmproxy/proxy/protocol/test_http2.py | 2 + .../proxy/protocol/test_websocket.py | 2 + test/mitmproxy/proxy/test_server.py | 41 ------------- test/mitmproxy/test_addonmanager.py | 2 +- test/mitmproxy/tservers.py | 3 + 17 files changed, 124 insertions(+), 195 deletions(-) delete mode 100644 mitmproxy/addons/core_option_validation.py delete mode 100644 test/mitmproxy/addons/test_core_option_validation.py diff --git a/mitmproxy/addons/__init__.py b/mitmproxy/addons/__init__.py index 8f84c20d9..988bc9049 100644 --- a/mitmproxy/addons/__init__.py +++ b/mitmproxy/addons/__init__.py @@ -4,7 +4,6 @@ from mitmproxy.addons import anticomp from mitmproxy.addons import browser from mitmproxy.addons import check_ca from mitmproxy.addons import clientplayback -from mitmproxy.addons import core_option_validation from mitmproxy.addons import core from mitmproxy.addons import cut from mitmproxy.addons import disable_h2c @@ -25,7 +24,6 @@ from mitmproxy.addons import upstream_auth def default_addons(): return [ core.Core(), - core_option_validation.CoreOptionValidation(), browser.Browser(), allowremote.AllowRemote(), anticache.AntiCache(), diff --git a/mitmproxy/addons/core.py b/mitmproxy/addons/core.py index ca21e1dc3..6edac6c3f 100644 --- a/mitmproxy/addons/core.py +++ b/mitmproxy/addons/core.py @@ -1,15 +1,72 @@ import typing +from mitmproxy.utils import human from mitmproxy import ctx from mitmproxy import exceptions from mitmproxy import command from mitmproxy import flow from mitmproxy import optmanager +from mitmproxy import platform +from mitmproxy.net import server_spec from mitmproxy.net.http import status_codes import mitmproxy.types +CA_DIR = "~/.mitmproxy" +LISTEN_PORT = 8080 + + class Core: + def load(self, loader): + loader.add_option( + "body_size_limit", typing.Optional[str], None, + """ + Byte size limit of HTTP request and response bodies. Understands + k/m/g suffixes, i.e. 3m for 3 megabytes. + """ + ) + loader.add_option( + "keep_host_header", bool, False, + """ + Reverse Proxy: Keep the original host header instead of rewriting it + to the reverse proxy target. + """ + ) + + def configure(self, updated): + opts = ctx.options + if opts.add_upstream_certs_to_client_chain and not opts.upstream_cert: + raise exceptions.OptionsError( + "The no-upstream-cert and add-upstream-certs-to-client-chain " + "options are mutually exclusive. If no-upstream-cert is enabled " + "then the upstream certificate is not retrieved before generating " + "the client certificate chain." + ) + if "body_size_limit" in updated: + try: + human.parse_size(opts.body_size_limit) + except ValueError as e: + raise exceptions.OptionsError( + "Invalid body size limit specification: %s" % + opts.body_size_limit + ) + if "mode" in updated: + mode = opts.mode + if mode.startswith("reverse:") or mode.startswith("upstream:"): + try: + server_spec.parse_with_mode(mode) + except ValueError as e: + raise exceptions.OptionsError(str(e)) from e + elif mode == "transparent": + if not platform.original_addr: + raise exceptions.OptionsError( + "Transparent mode not supported on this platform." + ) + elif mode not in ["regular", "socks5"]: + raise exceptions.OptionsError( + "Invalid mode specification: %s" % mode + ) + @command.command("set") def set(self, *spec: str) -> None: """ diff --git a/mitmproxy/addons/core_option_validation.py b/mitmproxy/addons/core_option_validation.py deleted file mode 100644 index 42da0b748..000000000 --- a/mitmproxy/addons/core_option_validation.py +++ /dev/null @@ -1,45 +0,0 @@ -""" - The core addon is responsible for verifying core settings that are not - checked by other addons. -""" -from mitmproxy import exceptions -from mitmproxy import platform -from mitmproxy import ctx -from mitmproxy.net import server_spec -from mitmproxy.utils import human - - -class CoreOptionValidation: - def configure(self, updated): - opts = ctx.options - if opts.add_upstream_certs_to_client_chain and not opts.upstream_cert: - raise exceptions.OptionsError( - "The no-upstream-cert and add-upstream-certs-to-client-chain " - "options are mutually exclusive. If no-upstream-cert is enabled " - "then the upstream certificate is not retrieved before generating " - "the client certificate chain." - ) - if "body_size_limit" in updated: - try: - human.parse_size(opts.body_size_limit) - except ValueError as e: - raise exceptions.OptionsError( - "Invalid body size limit specification: %s" % - opts.body_size_limit - ) - if "mode" in updated: - mode = opts.mode - if mode.startswith("reverse:") or mode.startswith("upstream:"): - try: - server_spec.parse_with_mode(mode) - except ValueError as e: - raise exceptions.OptionsError(str(e)) from e - elif mode == "transparent": - if not platform.original_addr: - raise exceptions.OptionsError( - "Transparent mode not supported on this platform." - ) - elif mode not in ["regular", "socks5"]: - raise exceptions.OptionsError( - "Invalid mode specification: %s" % mode - ) diff --git a/mitmproxy/addons/upstream_auth.py b/mitmproxy/addons/upstream_auth.py index 0b90b48f5..c0581e27c 100644 --- a/mitmproxy/addons/upstream_auth.py +++ b/mitmproxy/addons/upstream_auth.py @@ -57,5 +57,5 @@ class UpstreamAuth(): if self.auth: if f.mode == "upstream" and not f.server_conn.via: f.request.headers["Proxy-Authorization"] = self.auth - elif ctx.options.mode == "reverse": + elif ctx.options.mode.startswith("reverse"): f.request.headers["Proxy-Authorization"] = self.auth diff --git a/mitmproxy/options.py b/mitmproxy/options.py index b6f3398b4..ce7597a84 100644 --- a/mitmproxy/options.py +++ b/mitmproxy/options.py @@ -10,37 +10,6 @@ LISTEN_PORT = 8080 class Options(optmanager.OptManager): - if False: - # This provides type hints for IDEs (e.g. PyCharm) and mypy. - # Autogenerated using test/helper_tools/typehints_for_options.py - add_upstream_certs_to_client_chain = None # type: bool - body_size_limit = None # type: Optional[str] - cadir = None # type: str - certs = None # type: Sequence[str] - ciphers_client = None # type: Optional[str] - ciphers_server = None # type: Optional[str] - client_certs = None # type: Optional[str] - http2 = None # type: bool - http2_priority = None # type: bool - ignore_hosts = None # type: Sequence[str] - keep_host_header = None # type: bool - listen_host = None # type: str - listen_port = None # type: int - mode = None # type: str - rawtcp = None # type: bool - server = None # type: bool - showhost = None # type: bool - spoof_source_address = None # type: bool - ssl_insecure = None # type: bool - ssl_verify_upstream_trusted_ca = None # type: Optional[str] - ssl_verify_upstream_trusted_cadir = None # type: Optional[str] - ssl_version_client = None # type: str - ssl_version_server = None # type: str - tcp_hosts = None # type: Sequence[str] - upstream_bind_address = None # type: str - upstream_cert = None # type: bool - websocket = None # type: bool - def __init__(self, **kwargs) -> None: super().__init__() self.add_option( @@ -60,13 +29,6 @@ class Options(optmanager.OptManager): that will be served to the proxy client, as extras. """ ) - self.add_option( - "body_size_limit", Optional[str], None, - """ - Byte size limit of HTTP request and response bodies. Understands - k/m/g suffixes, i.e. 3m for 3 megabytes. - """ - ) self.add_option( "cadir", str, CA_DIR, "Location of the default mitmproxy CA files." @@ -128,13 +90,6 @@ class Options(optmanager.OptManager): "upstream_cert", bool, True, "Connect to upstream server to look up certificate details." ) - self.add_option( - "keep_host_header", bool, False, - """ - Reverse Proxy: Keep the original host header instead of rewriting it - to the reverse proxy target. - """ - ) self.add_option( "http2", bool, True, diff --git a/mitmproxy/test/taddons.py b/mitmproxy/test/taddons.py index 1ca8ba8da..82a935d21 100644 --- a/mitmproxy/test/taddons.py +++ b/mitmproxy/test/taddons.py @@ -6,7 +6,7 @@ import mitmproxy.options from mitmproxy import addonmanager from mitmproxy import command from mitmproxy import eventsequence -from mitmproxy.addons import script +from mitmproxy.addons import script, core class TestAddons(addonmanager.AddonManager): @@ -59,7 +59,7 @@ class context: provides a number of helper methods for common testing scenarios. """ - def __init__(self, *addons, options=None): + def __init__(self, *addons, options=None, loadcore=True): options = options or mitmproxy.options.Options() self.master = RecordingMaster( options @@ -67,6 +67,9 @@ class context: self.options = self.master.options self.wrapped = None + if loadcore: + self.master.addons.add(core.Core()) + for a in addons: self.master.addons.add(a) diff --git a/mitmproxy/tools/cmdline.py b/mitmproxy/tools/cmdline.py index d413ff282..4b7598cf9 100644 --- a/mitmproxy/tools/cmdline.py +++ b/mitmproxy/tools/cmdline.py @@ -1,10 +1,10 @@ import argparse import os -from mitmproxy import options +from mitmproxy.addons import core -CONFIG_PATH = os.path.join(options.CA_DIR, "config.yaml") +CONFIG_PATH = os.path.join(core.CA_DIR, "config.yaml") def common_options(parser, opts): diff --git a/test/mitmproxy/addons/test_core.py b/test/mitmproxy/addons/test_core.py index 3f95bed4e..b1b105d9d 100644 --- a/test/mitmproxy/addons/test_core.py +++ b/test/mitmproxy/addons/test_core.py @@ -1,3 +1,5 @@ +from unittest import mock + from mitmproxy.addons import core from mitmproxy.test import taddons from mitmproxy.test import tflow @@ -7,9 +9,7 @@ import pytest def test_set(): sa = core.Core() - with taddons.context() as tctx: - tctx.master.addons.add(sa) - + with taddons.context(loadcore=False) as tctx: assert tctx.master.options.server tctx.command(sa.set, "server=false") assert not tctx.master.options.server @@ -20,7 +20,7 @@ def test_set(): def test_resume(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow() assert not sa.resume([f]) f.intercept() @@ -30,7 +30,7 @@ def test_resume(): def test_mark(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow() assert not f.marked sa.mark([f], True) @@ -44,7 +44,7 @@ def test_mark(): def test_kill(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow() f.intercept() assert f.killable @@ -54,7 +54,7 @@ def test_kill(): def test_revert(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow() f.backup() f.request.content = b"bar" @@ -65,7 +65,7 @@ def test_revert(): def test_flow_set(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow(resp=True) assert sa.flow_set_options() @@ -101,7 +101,7 @@ def test_flow_set(): def test_encoding(): sa = core.Core() - with taddons.context(): + with taddons.context(loadcore=False): f = tflow.tflow() assert sa.encode_options() sa.encode([f], "request", "deflate") @@ -152,3 +152,40 @@ def test_options(tmpdir): f.write("'''") with pytest.raises(exceptions.CommandError): sa.options_load(p) + + +def test_validation_simple(): + sa = core.Core() + with taddons.context() as tctx: + with pytest.raises(exceptions.OptionsError): + tctx.configure(sa, body_size_limit = "invalid") + tctx.configure(sa, body_size_limit = "1m") + + with pytest.raises(exceptions.OptionsError, match="mutually exclusive"): + tctx.configure( + sa, + add_upstream_certs_to_client_chain = True, + upstream_cert = False + ) + with pytest.raises(exceptions.OptionsError, match="Invalid mode"): + tctx.configure( + sa, + mode = "Flibble" + ) + + +@mock.patch("mitmproxy.platform.original_addr", None) +def test_validation_no_transparent(): + sa = core.Core() + with taddons.context() as tctx: + with pytest.raises(Exception, match="Transparent mode not supported"): + tctx.configure(sa, mode = "transparent") + + +@mock.patch("mitmproxy.platform.original_addr") +def test_validation_modes(m): + sa = core.Core() + with taddons.context() as tctx: + tctx.configure(sa, mode = "reverse:http://localhost") + with pytest.raises(Exception, match="Invalid server specification"): + tctx.configure(sa, mode = "reverse:") \ No newline at end of file diff --git a/test/mitmproxy/addons/test_core_option_validation.py b/test/mitmproxy/addons/test_core_option_validation.py deleted file mode 100644 index cd5d4dfa7..000000000 --- a/test/mitmproxy/addons/test_core_option_validation.py +++ /dev/null @@ -1,42 +0,0 @@ -from mitmproxy import exceptions -from mitmproxy.addons import core_option_validation -from mitmproxy.test import taddons -import pytest -from unittest import mock - - -def test_simple(): - sa = core_option_validation.CoreOptionValidation() - with taddons.context() as tctx: - with pytest.raises(exceptions.OptionsError): - tctx.configure(sa, body_size_limit = "invalid") - tctx.configure(sa, body_size_limit = "1m") - - with pytest.raises(exceptions.OptionsError, match="mutually exclusive"): - tctx.configure( - sa, - add_upstream_certs_to_client_chain = True, - upstream_cert = False - ) - with pytest.raises(exceptions.OptionsError, match="Invalid mode"): - tctx.configure( - sa, - mode = "Flibble" - ) - - -@mock.patch("mitmproxy.platform.original_addr", None) -def test_no_transparent(): - sa = core_option_validation.CoreOptionValidation() - with taddons.context() as tctx: - with pytest.raises(Exception, match="Transparent mode not supported"): - tctx.configure(sa, mode = "transparent") - - -@mock.patch("mitmproxy.platform.original_addr") -def test_modes(m): - sa = core_option_validation.CoreOptionValidation() - with taddons.context() as tctx: - tctx.configure(sa, mode = "reverse:http://localhost") - with pytest.raises(Exception, match="Invalid server specification"): - tctx.configure(sa, mode = "reverse:") diff --git a/test/mitmproxy/addons/test_proxyauth.py b/test/mitmproxy/addons/test_proxyauth.py index 9e2365cfc..7816dd188 100644 --- a/test/mitmproxy/addons/test_proxyauth.py +++ b/test/mitmproxy/addons/test_proxyauth.py @@ -49,7 +49,7 @@ class TestProxyAuth: ]) def test_is_proxy_auth(self, mode, expected): up = proxyauth.ProxyAuth() - with taddons.context(up) as ctx: + with taddons.context(up, loadcore=False) as ctx: ctx.options.mode = mode assert up.is_proxy_auth() is expected @@ -133,7 +133,7 @@ class TestProxyAuth: def test_authenticate(self): up = proxyauth.ProxyAuth() - with taddons.context(up) as ctx: + with taddons.context(up, loadcore=False) as ctx: ctx.configure(up, proxyauth="any", mode="regular") f = tflow.tflow() diff --git a/test/mitmproxy/addons/test_script.py b/test/mitmproxy/addons/test_script.py index 73641d24a..dc21e6fd2 100644 --- a/test/mitmproxy/addons/test_script.py +++ b/test/mitmproxy/addons/test_script.py @@ -189,7 +189,7 @@ class TestScriptLoader: def test_simple(self): sc = script.ScriptLoader() - with taddons.context() as tctx: + with taddons.context(loadcore=False) as tctx: tctx.master.addons.add(sc) sc.running() assert len(tctx.master.addons) == 1 @@ -231,7 +231,7 @@ class TestScriptLoader: def test_load_err(self): sc = script.ScriptLoader() - with taddons.context(sc) as tctx: + with taddons.context(sc, loadcore=False) as tctx: tctx.configure(sc, scripts=[ tutils.test_data.path("mitmproxy/data/addonscripts/load_error.py") ]) diff --git a/test/mitmproxy/addons/test_upstream_auth.py b/test/mitmproxy/addons/test_upstream_auth.py index 53b342a27..ae037693c 100644 --- a/test/mitmproxy/addons/test_upstream_auth.py +++ b/test/mitmproxy/addons/test_upstream_auth.py @@ -41,7 +41,7 @@ def test_simple(): up.requestheaders(f) assert "proxy-authorization" not in f.request.headers - tctx.configure(up, mode="reverse") + tctx.configure(up, mode="reverse:127.0.0.1") f = tflow.tflow() f.mode = "transparent" up.requestheaders(f) diff --git a/test/mitmproxy/proxy/protocol/test_http2.py b/test/mitmproxy/proxy/protocol/test_http2.py index 194a57c90..d9aa03b41 100644 --- a/test/mitmproxy/proxy/protocol/test_http2.py +++ b/test/mitmproxy/proxy/protocol/test_http2.py @@ -10,6 +10,7 @@ import h2 from mitmproxy import options import mitmproxy.net +from mitmproxy.addons import core from ...net import tservers as net_tservers from mitmproxy import exceptions from mitmproxy.net.http import http1, http2 @@ -90,6 +91,7 @@ class _Http2TestBase: def setup_class(cls): cls.options = cls.get_options() tmaster = tservers.TestMaster(cls.options) + tmaster.addons.add(core.Core()) cls.proxy = tservers.ProxyThread(tmaster) cls.proxy.start() diff --git a/test/mitmproxy/proxy/protocol/test_websocket.py b/test/mitmproxy/proxy/protocol/test_websocket.py index 5cd9601cb..661605b7b 100644 --- a/test/mitmproxy/proxy/protocol/test_websocket.py +++ b/test/mitmproxy/proxy/protocol/test_websocket.py @@ -6,6 +6,7 @@ import traceback from mitmproxy import options from mitmproxy import exceptions +from mitmproxy.addons import core from mitmproxy.http import HTTPFlow from mitmproxy.websocket import WebSocketFlow @@ -52,6 +53,7 @@ class _WebSocketTestBase: def setup_class(cls): cls.options = cls.get_options() tmaster = tservers.TestMaster(cls.options) + tmaster.addons.add(core.Core()) cls.proxy = tservers.ProxyThread(tmaster) cls.proxy.start() diff --git a/test/mitmproxy/proxy/test_server.py b/test/mitmproxy/proxy/test_server.py index 87ec443ac..986dfb39c 100644 --- a/test/mitmproxy/proxy/test_server.py +++ b/test/mitmproxy/proxy/test_server.py @@ -10,7 +10,6 @@ from mitmproxy import certs from mitmproxy import exceptions from mitmproxy import http from mitmproxy import options -from mitmproxy.addons import proxyauth from mitmproxy.addons import script from mitmproxy.net import socks from mitmproxy.net import tcp @@ -306,46 +305,6 @@ class TestHTTP(tservers.HTTPProxyTest, CommonMixin): assert self.server.last_log()["request"]["first_line_format"] == "relative" -class TestHTTPAuth(tservers.HTTPProxyTest): - def test_auth(self): - self.master.addons.add(proxyauth.ProxyAuth()) - self.master.addons.trigger( - "configure", self.master.options.keys() - ) - self.master.options.proxyauth = "test:test" - assert self.pathod("202").status_code == 407 - p = self.pathoc() - with p.connect(): - ret = p.request(""" - get - 'http://localhost:%s/p/202' - h'%s'='%s' - """ % ( - self.server.port, - "Proxy-Authorization", - proxyauth.mkauth("test", "test") - )) - assert ret.status_code == 202 - - -class TestHTTPReverseAuth(tservers.ReverseProxyTest): - def test_auth(self): - self.master.addons.add(proxyauth.ProxyAuth()) - self.master.options.proxyauth = "test:test" - assert self.pathod("202").status_code == 401 - p = self.pathoc() - with p.connect(): - ret = p.request(""" - get - '/p/202' - h'%s'='%s' - """ % ( - "Authorization", - proxyauth.mkauth("test", "test") - )) - assert ret.status_code == 202 - - class TestHTTPS(tservers.HTTPProxyTest, CommonMixin, TcpMixin): ssl = True ssloptions = pathod.SSLOptions(request_client_cert=True) diff --git a/test/mitmproxy/test_addonmanager.py b/test/mitmproxy/test_addonmanager.py index 274d2d90c..ad56cb225 100644 --- a/test/mitmproxy/test_addonmanager.py +++ b/test/mitmproxy/test_addonmanager.py @@ -107,7 +107,7 @@ def test_loader(): def test_simple(): - with taddons.context() as tctx: + with taddons.context(loadcore=False) as tctx: a = tctx.master.addons assert len(a) == 0 diff --git a/test/mitmproxy/tservers.py b/test/mitmproxy/tservers.py index 4363931f7..0040b0235 100644 --- a/test/mitmproxy/tservers.py +++ b/test/mitmproxy/tservers.py @@ -5,6 +5,7 @@ import sys from unittest import mock import mitmproxy.platform +from mitmproxy.addons import core from mitmproxy.proxy.config import ProxyConfig from mitmproxy.proxy.server import ProxyServer from mitmproxy import controller @@ -132,6 +133,7 @@ class ProxyTestBase: cls.options = cls.get_options() tmaster = cls.masterclass(cls.options) + tmaster.addons.add(core.Core()) cls.proxy = ProxyThread(tmaster) cls.proxy.start() @@ -343,6 +345,7 @@ class ChainProxyTest(ProxyTestBase): for _ in range(cls.n): opts = cls.get_options() tmaster = cls.masterclass(opts) + tmaster.addons.add(core.Core()) proxy = ProxyThread(tmaster) proxy.start() cls.chain.insert(0, proxy)