From 1a36efbb6a2d689aaf05c6d7dd614072fd7d6c1c Mon Sep 17 00:00:00 2001 From: Thomas Kriechbaumer Date: Thu, 1 Dec 2016 10:36:18 +0100 Subject: [PATCH] simplify ALPN and OpenSSL on macOS --- .travis.yml | 34 +++++----- mitmproxy/net/tcp.py | 5 +- test/conftest.py | 14 +++++ test/mitmproxy/net/test_tcp.py | 54 +++++++++------- test/mitmproxy/protocol/test_http2.py | 7 +-- test/mitmproxy/test_dump.py | 8 +-- test/pathod/test_pathoc.py | 89 +++++++++++++++------------ test/pathod/test_pathod.py | 12 +++- test/pathod/test_protocols_http2.py | 34 +++++----- tox.ini | 2 +- 10 files changed, 147 insertions(+), 112 deletions(-) create mode 100644 test/conftest.py diff --git a/.travis.yml b/.travis.yml index 0df328996..c078e30ac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,15 +1,6 @@ sudo: false language: python -addons: - apt: - sources: - # Debian sid currently holds OpenSSL 1.0.2 - # change this with future releases! - - debian-sid - packages: - - libssl-dev - env: global: - CI_DEPS=codecov>=2.0.5 @@ -25,9 +16,21 @@ matrix: language: generic env: TOXENV=py35 BDIST=1 - python: 3.5 - env: TOXENV=py35 BDIST=1 + env: TOXENV=py35 OPENSSL_OLD + addons: + apt: + packages: + - libssl-dev - python: 3.5 - env: TOXENV=py35 NO_ALPN=1 + env: TOXENV=py35 BDIST=1 OPENSSL_ALPN + addons: + apt: + sources: + # Debian sid currently holds OpenSSL 1.1.0 + # change this with future releases! + - debian-sid + packages: + - libssl-dev - python: 3.5 env: TOXENV=docs git: @@ -39,10 +42,8 @@ install: - | if [[ $TRAVIS_OS_NAME == "osx" ]] then - brew update || brew update # try again if it fails - brew upgrade - brew reinstall openssl - brew reinstall pyenv + brew update || brew update + brew outdated pyenv || brew upgrade pyenv eval "$(pyenv init -)" env PYTHON_CONFIGURE_OPTS="--enable-framework" pyenv install --skip-existing 3.5.2 pyenv global 3.5.2 @@ -52,8 +53,8 @@ install: - pip install tox script: + - tox -- --cov mitmproxy --cov pathod -v - | - tox -- --cov mitmproxy --cov pathod -v if [[ $BDIST == "1" ]] then git fetch --unshallow --tags @@ -80,3 +81,4 @@ cache: directories: - $HOME/.pyenv - $HOME/.cache/pip + # - $HOME/build/mitmproxy/mitmproxy/.tox diff --git a/mitmproxy/net/tcp.py b/mitmproxy/net/tcp.py index ac78e70d4..11cabf07e 100644 --- a/mitmproxy/net/tcp.py +++ b/mitmproxy/net/tcp.py @@ -30,10 +30,7 @@ version_check.check_pyopenssl_version() socket_fileobject = socket.SocketIO EINTR = 4 -if os.environ.get("NO_ALPN"): - HAS_ALPN = False -else: - HAS_ALPN = SSL._lib.Cryptography_HAS_ALPN +HAS_ALPN = SSL._lib.Cryptography_HAS_ALPN # To enable all SSL methods use: SSLv23 # then add options to disable certain methods diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 000000000..3d129ecf7 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,14 @@ +import pytest +import OpenSSL +import mitmproxy.net.tcp + + +requires_alpn = pytest.mark.skipif( + not mitmproxy.net.tcp.HAS_ALPN, + reason='requires OpenSSL with ALPN support') + + +@pytest.fixture() +def disable_alpn(monkeypatch): + monkeypatch.setattr(mitmproxy.net.tcp, 'HAS_ALPN', False) + monkeypatch.setattr(OpenSSL.SSL._lib, 'Cryptography_HAS_ALPN', False) diff --git a/test/mitmproxy/net/test_tcp.py b/test/mitmproxy/net/test_tcp.py index cf3d30f7c..fe44973bd 100644 --- a/test/mitmproxy/net/test_tcp.py +++ b/test/mitmproxy/net/test_tcp.py @@ -6,7 +6,7 @@ import random import os import threading import mock - +import pytest from OpenSSL import SSL from mitmproxy import certs @@ -15,6 +15,7 @@ from mitmproxy.test import tutils from mitmproxy import exceptions from . import tservers +from ...conftest import requires_alpn class EchoHandler(tcp.BaseHandler): @@ -526,40 +527,47 @@ class TestTimeOut(tservers.ServerTestBase): tutils.raises(exceptions.TcpTimeout, c.rfile.read, 10) +class TestCryptographyALPN: + + def test_has_alpn(self): + if 'OPENSSL_ALPN' in os.environ: + assert tcp.HAS_ALPN + assert SSL._lib.Cryptography_HAS_ALPN + elif 'OPENSSL_OLD' in os.environ: + assert not tcp.HAS_ALPN + assert not SSL._lib.Cryptography_HAS_ALPN + + class TestALPNClient(tservers.ServerTestBase): handler = ALPNHandler ssl = dict( alpn_select=b"bar" ) - if tcp.HAS_ALPN: - def test_alpn(self): - c = tcp.TCPClient(("127.0.0.1", self.port)) - with c.connect(): - c.convert_to_ssl(alpn_protos=[b"foo", b"bar", b"fasel"]) - assert c.get_alpn_proto_negotiated() == b"bar" - assert c.rfile.readline().strip() == b"bar" + @requires_alpn + @pytest.mark.parametrize('has_alpn,alpn_protos, expected_negotiated, expected_response', [ + (True, [b"foo", b"bar", b"fasel"], b'bar', b'bar'), + (True, [], b'', b'NONE'), + (True, None, b'', b'NONE'), + (False, [b"foo", b"bar", b"fasel"], b'', b'NONE'), + (False, [], b'', b'NONE'), + (False, None, b'', b'NONE'), + ]) + def test_alpn(self, monkeypatch, has_alpn, alpn_protos, expected_negotiated, expected_response): + monkeypatch.setattr(tcp, 'HAS_ALPN', has_alpn) + monkeypatch.setattr(SSL._lib, 'Cryptography_HAS_ALPN', has_alpn) - def test_no_alpn(self): - c = tcp.TCPClient(("127.0.0.1", self.port)) - with c.connect(): - c.convert_to_ssl() - assert c.get_alpn_proto_negotiated() == b"" - assert c.rfile.readline().strip() == b"NONE" - - else: - def test_none_alpn(self): - c = tcp.TCPClient(("127.0.0.1", self.port)) - with c.connect(): - c.convert_to_ssl(alpn_protos=[b"foo", b"bar", b"fasel"]) - assert c.get_alpn_proto_negotiated() == b"" - assert c.rfile.readline() == b"NONE" + c = tcp.TCPClient(("127.0.0.1", self.port)) + with c.connect(): + c.convert_to_ssl(alpn_protos=alpn_protos) + assert c.get_alpn_proto_negotiated() == expected_negotiated + assert c.rfile.readline().strip() == expected_response class TestNoSSLNoALPNClient(tservers.ServerTestBase): handler = ALPNHandler - def test_no_ssl_no_alpn(self): + def test_no_ssl_no_alpn(self, disable_alpn): c = tcp.TCPClient(("127.0.0.1", self.port)) with c.connect(): assert c.get_alpn_proto_negotiated() == b"" diff --git a/test/mitmproxy/protocol/test_http2.py b/test/mitmproxy/protocol/test_http2.py index d135cf087..8e8ba6448 100644 --- a/test/mitmproxy/protocol/test_http2.py +++ b/test/mitmproxy/protocol/test_http2.py @@ -1,7 +1,6 @@ # coding=utf-8 -import pytest import os import tempfile import traceback @@ -17,6 +16,7 @@ from mitmproxy import exceptions from mitmproxy.net.http import http1, http2 from .. import tservers +from ...conftest import requires_alpn import logging logging.getLogger("hyper.packages.hpack.hpack").setLevel(logging.WARNING) @@ -27,11 +27,6 @@ logging.getLogger("PIL.Image").setLevel(logging.WARNING) logging.getLogger("PIL.PngImagePlugin").setLevel(logging.WARNING) -requires_alpn = pytest.mark.skipif( - not mitmproxy.net.tcp.HAS_ALPN, - reason='requires OpenSSL with ALPN support') - - # inspect the log: # for msg in self.proxy.tmaster.tlog: # print(msg) diff --git a/test/mitmproxy/test_dump.py b/test/mitmproxy/test_dump.py index e331637d9..c6b15c845 100644 --- a/test/mitmproxy/test_dump.py +++ b/test/mitmproxy/test_dump.py @@ -51,14 +51,14 @@ class TestDumpMaster(mastertest.MasterTest): assert "error" in o.tfile.getvalue() def test_replay(self): - o = dump.Options(server_replay=["nonexistent"], replay_kill_extra=True) + o = dump.Options(http2=False, server_replay=["nonexistent"], replay_kill_extra=True) tutils.raises(exceptions.OptionsError, dump.DumpMaster, o, proxy.DummyServer()) with tutils.tmpdir() as t: p = os.path.join(t, "rep") self.flowfile(p) - o = dump.Options(server_replay=[p], replay_kill_extra=True) + o = dump.Options(http2=False, server_replay=[p], replay_kill_extra=True) o.verbosity = 0 o.flow_detail = 0 m = dump.DumpMaster(o, proxy.DummyServer()) @@ -66,13 +66,13 @@ class TestDumpMaster(mastertest.MasterTest): self.cycle(m, b"content") self.cycle(m, b"content") - o = dump.Options(server_replay=[p], replay_kill_extra=False) + o = dump.Options(http2=False, server_replay=[p], replay_kill_extra=False) o.verbosity = 0 o.flow_detail = 0 m = dump.DumpMaster(o, proxy.DummyServer()) self.cycle(m, b"nonexistent") - o = dump.Options(client_replay=[p], replay_kill_extra=False) + o = dump.Options(http2=False, client_replay=[p], replay_kill_extra=False) o.verbosity = 0 o.flow_detail = 0 m = dump.DumpMaster(o, proxy.DummyServer()) diff --git a/test/pathod/test_pathoc.py b/test/pathod/test_pathoc.py index 69baae545..274e2be7f 100644 --- a/test/pathod/test_pathoc.py +++ b/test/pathod/test_pathoc.py @@ -1,8 +1,8 @@ import io from mock import Mock +import pytest from mitmproxy.net import http -from mitmproxy.net import tcp from mitmproxy.net.http import http1 from mitmproxy import exceptions @@ -11,6 +11,7 @@ from pathod.protocols.http2 import HTTP2StateProtocol from mitmproxy.test import tutils from . import tservers +from ..conftest import requires_alpn def test_response(): @@ -211,45 +212,57 @@ class TestDaemonHTTP2(PathocTestDaemon): ssl = True explain = False - if tcp.HAS_ALPN: + @requires_alpn + def test_http2(self): + c = pathoc.Pathoc( + ("127.0.0.1", self.d.port), + fp=None, + ssl=True, + use_http2=True, + ) + assert isinstance(c.protocol, HTTP2StateProtocol) - def test_http2(self): - c = pathoc.Pathoc( - ("127.0.0.1", self.d.port), - fp=None, - ssl=True, - use_http2=True, - ) - assert isinstance(c.protocol, HTTP2StateProtocol) + c = pathoc.Pathoc( + ("127.0.0.1", self.d.port), + ) + assert c.protocol == http1 - c = pathoc.Pathoc( - ("127.0.0.1", self.d.port), - ) - assert c.protocol == http1 + @requires_alpn + def test_http2_alpn(self): + c = pathoc.Pathoc( + ("127.0.0.1", self.d.port), + fp=None, + ssl=True, + use_http2=True, + http2_skip_connection_preface=True, + ) - def test_http2_alpn(self): - c = pathoc.Pathoc( - ("127.0.0.1", self.d.port), - fp=None, - ssl=True, - use_http2=True, - http2_skip_connection_preface=True, - ) + tmp_convert_to_ssl = c.convert_to_ssl + c.convert_to_ssl = Mock() + c.convert_to_ssl.side_effect = tmp_convert_to_ssl + with c.connect(): + _, kwargs = c.convert_to_ssl.call_args + assert set(kwargs['alpn_protos']) == set([b'http/1.1', b'h2']) - tmp_convert_to_ssl = c.convert_to_ssl - c.convert_to_ssl = Mock() - c.convert_to_ssl.side_effect = tmp_convert_to_ssl + @requires_alpn + def test_request(self): + c = pathoc.Pathoc( + ("127.0.0.1", self.d.port), + fp=None, + ssl=True, + use_http2=True, + ) + with c.connect(): + resp = c.request("get:/p/200") + assert resp.status_code == 200 + + def test_failing_request(self, disable_alpn): + c = pathoc.Pathoc( + ("127.0.0.1", self.d.port), + fp=None, + ssl=True, + use_http2=True, + ) + with pytest.raises(NotImplementedError): with c.connect(): - _, kwargs = c.convert_to_ssl.call_args - assert set(kwargs['alpn_protos']) == set([b'http/1.1', b'h2']) - - def test_request(self): - c = pathoc.Pathoc( - ("127.0.0.1", self.d.port), - fp=None, - ssl=True, - use_http2=True, - ) - with c.connect(): - resp = c.request("get:/p/200") - assert resp.status_code == 200 + c.request("get:/p/200") diff --git a/test/pathod/test_pathod.py b/test/pathod/test_pathod.py index 6a4e1c623..1e34af23d 100644 --- a/test/pathod/test_pathod.py +++ b/test/pathod/test_pathod.py @@ -1,11 +1,14 @@ import io +import pytest + from pathod import pathod from mitmproxy.net import tcp from mitmproxy import exceptions from mitmproxy.test import tutils from . import tservers +from ..conftest import requires_alpn class TestPathod: @@ -257,8 +260,11 @@ class TestHTTP2(tservers.DaemonTests): ssl = True nohang = True - if tcp.HAS_ALPN: + @requires_alpn + def test_http2(self): + r, _ = self.pathoc(["GET:/"], ssl=True, use_http2=True) + assert r[0].status_code == 800 - def test_http2(self): + def test_no_http2(self, disable_alpn): + with pytest.raises(NotImplementedError): r, _ = self.pathoc(["GET:/"], ssl=True, use_http2=True) - assert r[0].status_code == 800 diff --git a/test/pathod/test_protocols_http2.py b/test/pathod/test_protocols_http2.py index d77702a3a..8531887b1 100644 --- a/test/pathod/test_protocols_http2.py +++ b/test/pathod/test_protocols_http2.py @@ -11,6 +11,8 @@ from ..mitmproxy.net import tservers as net_tservers from pathod.protocols.http2 import HTTP2StateProtocol, TCPHandler +from ..conftest import requires_alpn + class TestTCPHandlerWrapper: def test_wrapped(self): @@ -66,37 +68,35 @@ class TestProtocol: assert mock_server_method.called +@requires_alpn class TestCheckALPNMatch(net_tservers.ServerTestBase): handler = EchoHandler ssl = dict( alpn_select=b'h2', ) - if tcp.HAS_ALPN: - - def test_check_alpn(self): - c = tcp.TCPClient(("127.0.0.1", self.port)) - with c.connect(): - c.convert_to_ssl(alpn_protos=[b'h2']) - protocol = HTTP2StateProtocol(c) - assert protocol.check_alpn() + def test_check_alpn(self): + c = tcp.TCPClient(("127.0.0.1", self.port)) + with c.connect(): + c.convert_to_ssl(alpn_protos=[b'h2']) + protocol = HTTP2StateProtocol(c) + assert protocol.check_alpn() +@requires_alpn class TestCheckALPNMismatch(net_tservers.ServerTestBase): handler = EchoHandler ssl = dict( alpn_select=None, ) - if tcp.HAS_ALPN: - - def test_check_alpn(self): - c = tcp.TCPClient(("127.0.0.1", self.port)) - with c.connect(): - c.convert_to_ssl(alpn_protos=[b'h2']) - protocol = HTTP2StateProtocol(c) - with raises(NotImplementedError): - protocol.check_alpn() + def test_check_alpn(self): + c = tcp.TCPClient(("127.0.0.1", self.port)) + with c.connect(): + c.convert_to_ssl(alpn_protos=[b'h2']) + protocol = HTTP2StateProtocol(c) + with raises(NotImplementedError): + protocol.check_alpn() class TestPerformServerConnectionPreface(net_tservers.ServerTestBase): diff --git a/tox.ini b/tox.ini index 3f8040d73..dc76cb704 100644 --- a/tox.ini +++ b/tox.ini @@ -8,7 +8,7 @@ basepython = python3.5 deps = {env:CI_DEPS:} -rrequirements.txt -passenv = CODECOV_TOKEN CI CI_* TRAVIS TRAVIS_* APPVEYOR APPVEYOR_* SNAPSHOT_* +passenv = CODECOV_TOKEN CI CI_* TRAVIS TRAVIS_* APPVEYOR APPVEYOR_* SNAPSHOT_* OPENSSL_* setenv = HOME = {envtmpdir} commands = mitmdump --sysinfo