simplify ALPN and OpenSSL on macOS

10 files changed, 151 insertions, 116 deletions

diff --git a/.travis.yml b/.travis.yml
index 0df32899..c078e30a 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 ac78e70d..11cabf07 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 00000000..3d129ecf
--- /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 cf3d30f7..fe44973b 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"
-
-        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"
+    @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)
 
-    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 d135cf08..8e8ba644 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 e331637d..c6b15c84 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 69baae54..274e2be7 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:
-
-        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
-
-        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'])
-
-        def test_request(self):
-            c = pathoc.Pathoc(
-                ("127.0.0.1", self.d.port),
-                fp=None,
-                ssl=True,
-                use_http2=True,
-            )
+    @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)
+
+        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,
+        )
+
+        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'])
+
+    @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():
-                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 6a4e1c62..1e34af23 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 d77702a3..8531887b 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 3f8040d7..dc76cb70 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