From 65b8b65ebc47ed9b610e0df765e273df6a27ce6a Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Tue, 27 Feb 2018 19:05:52 +0100 Subject: [PATCH 1/2] add missing client_certs directory expansion --- mitmproxy/connections.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mitmproxy/connections.py b/mitmproxy/connections.py index e8fc4fbf4..29ab6ab5e 100644 --- a/mitmproxy/connections.py +++ b/mitmproxy/connections.py @@ -281,6 +281,7 @@ class ServerConnection(tcp.TCPClient, stateobject.StateObject): raise ValueError("sni must be str, not " + type(sni).__name__) client_cert = None if client_certs: + client_certs = os.path.expanduser(client_certs) if os.path.isfile(client_certs): client_cert = client_certs else: From 944e81dcfcc5220c7853c64405a725f5c4039810 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Tue, 27 Feb 2018 19:05:59 +0100 Subject: [PATCH 2/2] clean up ProxyConfig some of these options weren't even used anymore, others only in one place where it makes sense to use options directly. --- mitmproxy/addons/core.py | 15 ++++++++++++++ mitmproxy/proxy/config.py | 31 +---------------------------- mitmproxy/proxy/protocol/tls.py | 5 +++-- test/mitmproxy/addons/test_core.py | 21 ++++++++++++++++++- test/mitmproxy/proxy/test_config.py | 18 ----------------- test/mitmproxy/test_proxy.py | 5 ----- 6 files changed, 39 insertions(+), 56 deletions(-) diff --git a/mitmproxy/addons/core.py b/mitmproxy/addons/core.py index 6edac6c3f..dbbbfd51d 100644 --- a/mitmproxy/addons/core.py +++ b/mitmproxy/addons/core.py @@ -1,5 +1,7 @@ import typing +import os + from mitmproxy.utils import human from mitmproxy import ctx from mitmproxy import exceptions @@ -42,6 +44,12 @@ class Core: "then the upstream certificate is not retrieved before generating " "the client certificate chain." ) + if opts.add_upstream_certs_to_client_chain and not opts.ssl_insecure: + raise exceptions.OptionsError( + "The verify-upstream-cert requires certificate verification to be disabled. " + "If upstream certificates are verified then extra upstream certificates are " + "not available for inclusion to the client chain." + ) if "body_size_limit" in updated: try: human.parse_size(opts.body_size_limit) @@ -66,6 +74,13 @@ class Core: raise exceptions.OptionsError( "Invalid mode specification: %s" % mode ) + if "client_certs" in updated: + if opts.client_certs: + client_certs = os.path.expanduser(opts.client_certs) + if not os.path.exists(client_certs): + raise exceptions.OptionsError( + "Client certificate path does not exist: {}".format(opts.client_certs) + ) @command.command("set") def set(self, *spec: str) -> None: diff --git a/mitmproxy/proxy/config.py b/mitmproxy/proxy/config.py index c15640d75..439beb3dd 100644 --- a/mitmproxy/proxy/config.py +++ b/mitmproxy/proxy/config.py @@ -2,12 +2,11 @@ import os import re import typing -from OpenSSL import SSL, crypto +from OpenSSL import crypto from mitmproxy import exceptions from mitmproxy import options as moptions from mitmproxy import certs -from mitmproxy.net import tls from mitmproxy.net import server_spec CONF_BASENAME = "mitmproxy" @@ -40,35 +39,16 @@ class ProxyConfig: self.check_ignore = None # type: HostMatcher self.check_tcp = None # type: HostMatcher self.certstore = None # type: certs.CertStore - self.client_certs = None # type: str - self.openssl_verification_mode_server = None # type: int self.upstream_server = None # type: typing.Optional[server_spec.ServerSpec] self.configure(options, set(options.keys())) options.changed.connect(self.configure) def configure(self, options: moptions.Options, updated: typing.Any) -> None: - if options.add_upstream_certs_to_client_chain and not options.ssl_insecure: - raise exceptions.OptionsError( - "The verify-upstream-cert requires certificate verification to be disabled. " - "If upstream certificates are verified then extra upstream certificates are " - "not available for inclusion to the client chain." - ) - - if options.ssl_insecure: - self.openssl_verification_mode_server = SSL.VERIFY_NONE - else: - self.openssl_verification_mode_server = SSL.VERIFY_PEER - if "ignore_hosts" in updated: self.check_ignore = HostMatcher(options.ignore_hosts) if "tcp_hosts" in updated: self.check_tcp = HostMatcher(options.tcp_hosts) - self.openssl_method_client, self.openssl_options_client = \ - tls.VERSION_CHOICES[options.ssl_version_client] - self.openssl_method_server, self.openssl_options_server = \ - tls.VERSION_CHOICES[options.ssl_version_server] - certstore_path = os.path.expanduser(options.cadir) if not os.path.exists(os.path.dirname(certstore_path)): raise exceptions.OptionsError( @@ -80,15 +60,6 @@ class ProxyConfig: CONF_BASENAME ) - if options.client_certs: - client_certs = os.path.expanduser(options.client_certs) - if not os.path.exists(client_certs): - raise exceptions.OptionsError( - "Client certificate path does not exist: %s" % - options.client_certs - ) - self.client_certs = client_certs - for c in options.certs: parts = c.split("=", 1) if len(parts) == 1: diff --git a/mitmproxy/proxy/protocol/tls.py b/mitmproxy/proxy/protocol/tls.py index 09ce87bad..ce3dc662c 100644 --- a/mitmproxy/proxy/protocol/tls.py +++ b/mitmproxy/proxy/protocol/tls.py @@ -374,10 +374,11 @@ class TlsLayer(base.Layer): extra_certs = None try: + tls_method, tls_options = net_tls.VERSION_CHOICES[self.config.options.ssl_version_client] self.client_conn.convert_to_tls( cert, key, - method=self.config.openssl_method_client, - options=self.config.openssl_options_client, + method=tls_method, + options=tls_options, cipher_list=self.config.options.ciphers_client or DEFAULT_CLIENT_CIPHERS, dhparams=self.config.certstore.dhparams, chain_file=chain_file, diff --git a/test/mitmproxy/addons/test_core.py b/test/mitmproxy/addons/test_core.py index b1b105d9d..3c674b3fc 100644 --- a/test/mitmproxy/addons/test_core.py +++ b/test/mitmproxy/addons/test_core.py @@ -3,6 +3,7 @@ from unittest import mock from mitmproxy.addons import core from mitmproxy.test import taddons from mitmproxy.test import tflow +from mitmproxy.test import tutils from mitmproxy import exceptions import pytest @@ -167,6 +168,12 @@ def test_validation_simple(): add_upstream_certs_to_client_chain = True, upstream_cert = False ) + with pytest.raises(exceptions.OptionsError, match="requires certificate verification to be disabled"): + tctx.configure( + sa, + add_upstream_certs_to_client_chain = True, + ssl_insecure = False + ) with pytest.raises(exceptions.OptionsError, match="Invalid mode"): tctx.configure( sa, @@ -188,4 +195,16 @@ def test_validation_modes(m): 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 + tctx.configure(sa, mode = "reverse:") + + +def test_client_certs(): + sa = core.Core() + with taddons.context() as tctx: + # Folders should work. + tctx.configure(sa, client_certs = tutils.test_data.path("mitmproxy/data/clientcert")) + # Files, too. + tctx.configure(sa, client_certs = tutils.test_data.path("mitmproxy/data/clientcert/client.pem")) + + with pytest.raises(exceptions.OptionsError, match="certificate path does not exist"): + tctx.configure(sa, client_certs = "invalid") diff --git a/test/mitmproxy/proxy/test_config.py b/test/mitmproxy/proxy/test_config.py index a7da980b3..60a0deb53 100644 --- a/test/mitmproxy/proxy/test_config.py +++ b/test/mitmproxy/proxy/test_config.py @@ -7,30 +7,12 @@ from mitmproxy.test import tutils class TestProxyConfig: - def test_upstream_cert_insecure(self): - opts = options.Options() - opts.add_upstream_certs_to_client_chain = True - with pytest.raises(exceptions.OptionsError, match="verify-upstream-cert"): - ProxyConfig(opts) - def test_invalid_cadir(self): opts = options.Options() opts.cadir = "foo" with pytest.raises(exceptions.OptionsError, match="parent directory does not exist"): ProxyConfig(opts) - def test_invalid_client_certs(self): - opts = options.Options() - opts.client_certs = "foo" - with pytest.raises(exceptions.OptionsError, match="certificate path does not exist"): - ProxyConfig(opts) - - def test_valid_client_certs(self): - opts = options.Options() - opts.client_certs = tutils.test_data.path("mitmproxy/data/clientcert/") - p = ProxyConfig(opts) - assert p.client_certs - def test_invalid_certificate(self): opts = options.Options() opts.certs = [tutils.test_data.path("mitmproxy/data/dumpfile-011")] diff --git a/test/mitmproxy/test_proxy.py b/test/mitmproxy/test_proxy.py index 299abab3c..75d4cdf03 100644 --- a/test/mitmproxy/test_proxy.py +++ b/test/mitmproxy/test_proxy.py @@ -1,6 +1,5 @@ import argparse from unittest import mock -from OpenSSL import SSL import pytest from mitmproxy.tools import cmdline @@ -50,10 +49,6 @@ class TestProcessProxyOptions: with pytest.raises(Exception, match="does not exist"): self.p("--cert", "nonexistent") - def test_insecure(self): - p = self.assert_noerr("--ssl-insecure") - assert p.openssl_verification_mode_server == SSL.VERIFY_NONE - class TestProxyServer: