From f59e6b3e7de5b6f875e76a99aee6791d12e506a6 Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Wed, 5 Dec 2018 20:45:34 -0600 Subject: [PATCH 01/33] Add tests for specific TLS/SSL versions --- dummyserver/testcase.py | 37 ++++++++- src/urllib3/contrib/pyopenssl.py | 3 + src/urllib3/contrib/securetransport.py | 25 ++++++- test/contrib/test_pyopenssl.py | 6 +- test/contrib/test_securetransport.py | 5 +- test/with_dummyserver/test_https.py | 91 ++++++++++++++++------- test/with_dummyserver/test_socketlevel.py | 2 +- 7 files changed, 137 insertions(+), 32 deletions(-) diff --git a/dummyserver/testcase.py b/dummyserver/testcase.py index ebd0bcb841..a9ee783550 100644 --- a/dummyserver/testcase.py +++ b/dummyserver/testcase.py @@ -1,6 +1,7 @@ import threading import unittest +import ssl import pytest from tornado import ioloop, web @@ -20,6 +21,22 @@ def consume_socket(sock, chunks=65536): pass +def create_TLSv1_3_context(): + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.check_hostname = True + context.options |= ssl.OP_NO_SSLv2 + context.options |= ssl.OP_NO_SSLv3 + context.options |= ssl.OP_NO_TLSv1 + context.options |= ssl.OP_NO_TLSv1_1 + context.options |= ssl.OP_NO_TLSv1_2 + + context.load_cert_chain( + DEFAULT_CERTS["certfile"], + DEFAULT_CERTS["keyfile"] + ) + return context + + class SocketDummyServerTestCase(unittest.TestCase): """ A simple socket-based server is created for this class that is good for @@ -115,13 +132,16 @@ class HTTPDummyServerTestCase(unittest.TestCase): scheme = 'http' host = 'localhost' host_alt = '127.0.0.1' # Some tests need two hosts - certs = DEFAULT_CERTS + + @classmethod + def certs(cls): + return DEFAULT_CERTS @classmethod def _start_server(cls): cls.io_loop = ioloop.IOLoop.current() app = web.Application([(r".*", TestingApp)]) - cls.server, cls.port = run_tornado_app(app, cls.io_loop, cls.certs, + cls.server, cls.port = run_tornado_app(app, cls.io_loop, cls.certs(), cls.scheme, cls.host) cls.server_thread = run_loop_in_thread(cls.io_loop) @@ -143,7 +163,10 @@ def tearDownClass(cls): class HTTPSDummyServerTestCase(HTTPDummyServerTestCase): scheme = 'https' host = 'localhost' - certs = DEFAULT_CERTS + + @classmethod + def certs(cls): + return DEFAULT_CERTS @pytest.mark.skipif(not HAS_IPV6, reason='IPv6 not available') @@ -207,3 +230,11 @@ class IPv6HTTPDummyProxyTestCase(HTTPDummyProxyTestCase): proxy_host = '::1' proxy_host_alt = '127.0.0.1' + + +@pytest.mark.skipif(not hasattr(ssl, "OP_NO_TLSv1_3"), reason='TLS 1.3 not available') +class TLS1_3HTTPSDummyServerTestCase(HTTPSDummyServerTestCase): + + @classmethod + def certs(cls): + ctx = super().certs() diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index 539bf8a8e1..be028aaf26 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -360,6 +360,9 @@ def getpeercert(self, binary_form=False): 'subjectAltName': get_subj_alt_name(x509) } + def version(self): + return self.connection.get_protocol_version_name() + def _reuse(self): self._makefile_refs += 1 diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 77cb59ed71..a41152a044 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -124,7 +124,7 @@ # Basically this is simple: for PROTOCOL_SSLv23 we turn it into a low of # TLSv1 and a high of TLSv1.2. For everything else, we pin to that version. _protocol_to_min_max = { - ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocol12), + ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocol13), } if hasattr(ssl, "PROTOCOL_SSLv2"): @@ -147,6 +147,10 @@ _protocol_to_min_max[ssl.PROTOCOL_TLSv1_2] = ( SecurityConst.kTLSProtocol12, SecurityConst.kTLSProtocol12 ) +if hasattr(ssl, "PROTOCOL_TLSv1_3"): + _protocol_to_min_max[ssl.PROTOCOL_TLSv1_3] = ( + SecurityConst.kTLSProtocol13, SecurityConst.kTLSProtocol13 + ) if hasattr(ssl, "PROTOCOL_TLS"): _protocol_to_min_max[ssl.PROTOCOL_TLS] = _protocol_to_min_max[ssl.PROTOCOL_SSLv23] @@ -667,6 +671,25 @@ def getpeercert(self, binary_form=False): return der_bytes + def version(self): + protocol = Security.SSLProtocol() + result = Security.SSLGetNegotiatedProtocolVersion(self.context, ctypes.byref(protocol)) + _assert_no_error(result) + if protocol == SecurityConst.kTLSProtocol13: + return 'TLSv1.3' + elif protocol == SecurityConst.kTLSProtocol12: + return 'TLSv1.2' + elif protocol == SecurityConst.kTLSProtocol11: + return 'TLSv1.1' + elif protocol == SecurityConst.kTLSProtocol1: + return 'TLSv1' + elif protocol == SecurityConst.kSSLProtocol3: + return 'SSLv3' + elif protocol == SecurityConst.kSSLProtocol2: + return 'SSLv2' + else: + raise ssl.SSLError('Unknown TLS version: %r' % protocol) + def _reuse(self): self._makefile_refs += 1 diff --git a/test/contrib/test_pyopenssl.py b/test/contrib/test_pyopenssl.py index 21cde45a03..237d229bc2 100644 --- a/test/contrib/test_pyopenssl.py +++ b/test/contrib/test_pyopenssl.py @@ -31,7 +31,11 @@ def teardown_module(): pass -from ..with_dummyserver.test_https import TestHTTPS, TestHTTPS_TLSv1 # noqa: F401 +from ..with_dummyserver.test_https import ( # noqa: F401 + TestHTTPS, TestHTTPS_TLSv1, TestHTTPS_TLSv1_1, + TestHTTPS_TLSv1_2, TestHTTPS_TLSv1_3, TestHTTPS_IPSAN, + TestHTTPS_IPv6Addr, TestHTTPS_NoSAN +) from ..with_dummyserver.test_socketlevel import ( # noqa: F401 TestSNI, TestSocketClosing, TestClientCerts ) diff --git a/test/contrib/test_securetransport.py b/test/contrib/test_securetransport.py index 0a2c2a8cac..623b5ef46d 100644 --- a/test/contrib/test_securetransport.py +++ b/test/contrib/test_securetransport.py @@ -27,7 +27,10 @@ def teardown_module(): pass -from ..with_dummyserver.test_https import TestHTTPS, TestHTTPS_TLSv1 # noqa: F401 +from ..with_dummyserver.test_https import ( # noqa: F401 + TestHTTPS, TestHTTPS_TLSv1, TestHTTPS_TLSv1_1, + TestHTTPS_TLSv1_2, TestHTTPS_TLSv1_3 +) from ..with_dummyserver.test_socketlevel import ( # noqa: F401 TestSNI, TestSocketClosing, TestClientCerts ) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 66834c72ce..c9019690e5 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -72,11 +72,6 @@ def test_dotted_fqdn(self): r = pool.request('GET', '/') self.assertEqual(r.status, 200, r.data) - def test_set_ssl_version_to_tlsv1(self): - self._pool.ssl_version = ssl.PROTOCOL_TLSv1 - r = self._pool.request('GET', '/') - self.assertEqual(r.status, 200, r.data) - def test_client_intermediate(self): client_cert, client_key = ( DEFAULT_CLIENT_CERTS['certfile'], @@ -559,30 +554,72 @@ def _request_without_resource_warnings(self, method, url): return [x for x in w if not isinstance(x.message, ResourceWarning)] -class TestHTTPS_TLSv1(HTTPSDummyServerTestCase): - certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLSv1 +class TestHTTPS_TLSVersion(TestHTTPS): + tls_protocol_name = None - def setUp(self): - self._pool = HTTPSConnectionPool(self.host, self.port) - self.addCleanup(self._pool.close) + @classmethod + def certs(cls): + return pytest.skip('Don\'t run parent version.') - def test_discards_connection_on_sslerror(self): - self._pool.cert_reqs = 'CERT_REQUIRED' - with self.assertRaises(MaxRetryError) as cm: - self._pool.request('GET', '/', retries=0) - self.assertIsInstance(cm.exception.reason, SSLError) - self._pool.ca_certs = DEFAULT_CA - self._pool.request('GET', '/') + def test_set_ssl_version_to_tls_version(self): + self._pool.ssl_version = self.certs()['ssl_version'] + r = self._pool.request('GET', '/') + self.assertEqual(r.status, 200, r.data) - def test_set_cert_default_cert_required(self): - conn = VerifiedHTTPSConnection(self.host, self.port) - conn.set_cert(ca_certs=DEFAULT_CA) - self.assertEqual(conn.cert_reqs, 'CERT_REQUIRED') + def test_tls_protocol_name_of_socket(self): + conn = self._pool._get_conn() + conn.connect() + self.assertEqual(conn.sock.version(), self.tls_protocol_name) + + +@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1"), reason="Requires TLSv1 support") +class TestHTTPS_TLSv1(TestHTTPS_TLSVersion): + tls_protocol_name = 'TLSv1' + + @classmethod + def certs(cls): + certs = DEFAULT_CERTS.copy() + certs['ssl_version'] = ssl.PROTOCOL_TLSv1 + return certs + + +@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_1"), reason="Requires TLSv1.1 support") +class TestHTTPS_TLSv1_1(TestHTTPS_TLSVersion): + tls_protocol_name = 'TLSv1.1' + + @classmethod + def certs(cls): + certs = DEFAULT_CERTS.copy() + certs['ssl_version'] = ssl.PROTOCOL_TLSv1_1 + return certs + + +@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_2"), reason="Requires TLSv1.2 support") +class TestHTTPS_TLSv1_2(TestHTTPS_TLSVersion): + tls_protocol_name = 'TLSv1.2' + + @classmethod + def certs(cls): + certs = DEFAULT_CERTS.copy() + certs['ssl_version'] = ssl.PROTOCOL_TLSv1_2 + return certs + + +@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_3"), reason="Requires TLSv1.3 support") +class TestHTTPS_TLSv1_3(TestHTTPS_TLSVersion): + tls_protocol_name = 'TLSv1.3' + + @classmethod + def certs(cls): + certs = DEFAULT_CERTS.copy() + certs['ssl_version'] = ssl.PROTOCOL_TLSv1_3 + return certs class TestHTTPS_NoSAN(HTTPSDummyServerTestCase): - certs = NO_SAN_CERTS + @classmethod + def certs(cls): + return NO_SAN_CERTS def test_warning_for_certs_without_a_san(self): """Ensure that a warning is raised when the cert from the server has @@ -598,7 +635,9 @@ def test_warning_for_certs_without_a_san(self): class TestHTTPS_IPSAN(HTTPSDummyServerTestCase): - certs = IP_SAN_CERTS + @classmethod + def certs(cls): + return IP_SAN_CERTS def test_can_validate_ip_san(self): """Ensure that urllib3 can validate SANs with IP addresses in them.""" @@ -616,7 +655,9 @@ def test_can_validate_ip_san(self): class TestHTTPS_IPv6Addr(IPV6HTTPSDummyServerTestCase): - certs = IPV6_ADDR_CERTS + @classmethod + def certs(cls): + return IPV6_ADDR_CERTS @pytest.mark.skipif(not HAS_IPV6, reason='Only runs on IPv6 systems') def test_strip_square_brackets_before_validating(self): diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py index 778b7c5886..7c1c466e2d 100644 --- a/test/with_dummyserver/test_socketlevel.py +++ b/test/with_dummyserver/test_socketlevel.py @@ -626,7 +626,7 @@ def socket_handler(listener): # First request should fail. response = pool.urlopen('GET', '/', retries=0, preload_content=False, - timeout=Timeout(connect=1, read=0.001)) + timeout=Timeout(connect=1, read=0.1)) try: self.assertRaises(ReadTimeoutError, response.read) finally: From 417c33373d184a58969f4821f9ac5a3bc220daa2 Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Thu, 6 Dec 2018 22:20:37 -0600 Subject: [PATCH 02/33] Add change and update bindings --- CHANGES.rst | 2 ++ dummyserver/testcase.py | 25 ------------------- .../contrib/_securetransport/bindings.py | 1 + 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index a65bb5bc96..394061aa38 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,8 @@ dev (master) * Implemented a more efficient ``HTTPResponse.__iter__()`` method (Issue #1483) +* Added support for TLSv1.3 support for all ``HTTPSConnection`` implementations. (PR #1496) + * ... [Short description of non-trivial change.] (Issue #) diff --git a/dummyserver/testcase.py b/dummyserver/testcase.py index a9ee783550..14216791be 100644 --- a/dummyserver/testcase.py +++ b/dummyserver/testcase.py @@ -1,7 +1,6 @@ import threading import unittest -import ssl import pytest from tornado import ioloop, web @@ -21,22 +20,6 @@ def consume_socket(sock, chunks=65536): pass -def create_TLSv1_3_context(): - context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) - context.check_hostname = True - context.options |= ssl.OP_NO_SSLv2 - context.options |= ssl.OP_NO_SSLv3 - context.options |= ssl.OP_NO_TLSv1 - context.options |= ssl.OP_NO_TLSv1_1 - context.options |= ssl.OP_NO_TLSv1_2 - - context.load_cert_chain( - DEFAULT_CERTS["certfile"], - DEFAULT_CERTS["keyfile"] - ) - return context - - class SocketDummyServerTestCase(unittest.TestCase): """ A simple socket-based server is created for this class that is good for @@ -230,11 +213,3 @@ class IPv6HTTPDummyProxyTestCase(HTTPDummyProxyTestCase): proxy_host = '::1' proxy_host_alt = '127.0.0.1' - - -@pytest.mark.skipif(not hasattr(ssl, "OP_NO_TLSv1_3"), reason='TLS 1.3 not available') -class TLS1_3HTTPSDummyServerTestCase(HTTPSDummyServerTestCase): - - @classmethod - def certs(cls): - ctx = super().certs() diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index bcf41c02b2..84069698a2 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -516,6 +516,7 @@ class SecurityConst(object): kTLSProtocol1 = 4 kTLSProtocol11 = 7 kTLSProtocol12 = 8 + kTLSProtocol13 = 10 kSSLClientSide = 1 kSSLStreamType = 0 From eaa6f38c7b719d5d99f8309a5c09522668035124 Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Thu, 6 Dec 2018 22:31:44 -0600 Subject: [PATCH 03/33] SSLSocket.version() not available sometimes --- test/with_dummyserver/test_https.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index c9019690e5..129c94adbe 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -569,6 +569,10 @@ def test_set_ssl_version_to_tls_version(self): def test_tls_protocol_name_of_socket(self): conn = self._pool._get_conn() conn.connect() + + if not hasattr(conn.sock, 'version'): + pytest.skip('SSLSocket.version() not available') + self.assertEqual(conn.sock.version(), self.tls_protocol_name) From 20b5eb56ce522f527cba7447067b383e57374c55 Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Thu, 6 Dec 2018 22:47:11 -0600 Subject: [PATCH 04/33] Add support for kTLSProtocolMaxSupported --- src/urllib3/contrib/_securetransport/bindings.py | 1 + src/urllib3/contrib/securetransport.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index 84069698a2..dbb050f44a 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -517,6 +517,7 @@ class SecurityConst(object): kTLSProtocol11 = 7 kTLSProtocol12 = 8 kTLSProtocol13 = 10 + kTLSProtocolMaxSupported = 999 kSSLClientSide = 1 kSSLStreamType = 0 diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index a41152a044..942b15ba7e 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -124,7 +124,7 @@ # Basically this is simple: for PROTOCOL_SSLv23 we turn it into a low of # TLSv1 and a high of TLSv1.2. For everything else, we pin to that version. _protocol_to_min_max = { - ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocol13), + ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocolMaxSupported), } if hasattr(ssl, "PROTOCOL_SSLv2"): From ab1e014fccdcb971eb2417013a3877a4712f7c1d Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Thu, 6 Dec 2018 23:13:13 -0600 Subject: [PATCH 05/33] Try setProtocolVersionMax again if error --- src/urllib3/contrib/securetransport.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 942b15ba7e..bbf27fd8a1 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -44,6 +44,7 @@ _assert_no_error, _cert_array_from_pem, _temporary_keychain, _load_client_cert_chain ) +from ._securetransport.bindings import version_info try: # Platform-specific: Python 2 from socket import _fileobject @@ -147,7 +148,7 @@ _protocol_to_min_max[ssl.PROTOCOL_TLSv1_2] = ( SecurityConst.kTLSProtocol12, SecurityConst.kTLSProtocol12 ) -if hasattr(ssl, "PROTOCOL_TLSv1_3"): +if hasattr(ssl, "PROTOCOL_TLSv1_3") and version_info >= (10, 13): _protocol_to_min_max[ssl.PROTOCOL_TLSv1_3] = ( SecurityConst.kTLSProtocol13, SecurityConst.kTLSProtocol13 ) @@ -462,7 +463,14 @@ def handshake(self, # Set the minimum and maximum TLS versions. result = Security.SSLSetProtocolVersionMin(self.context, min_version) _assert_no_error(result) + + # TLS 1.3 isn't necessarily enabled by the OS + # so we have to detect when we error out and try + # setting TLS 1.3 if it's allowed. result = Security.SSLSetProtocolVersionMax(self.context, max_version) + print("err", result) + if result and max_version == SecurityConst.kTLSProtocolMaxSupported: + result = Security.SSLSetProtocolVersionMax(self.context, SecurityConst.kTLSProtocol12) _assert_no_error(result) # If there's a trust DB, we need to use it. We do that by telling From fefc4030a59eaf30c5d5820fd2f7501e83bb017f Mon Sep 17 00:00:00 2001 From: SethMichaelLarson Date: Thu, 6 Dec 2018 23:21:41 -0600 Subject: [PATCH 06/33] Get ctypes.c_uint.value for SSLSocket.version() --- src/urllib3/contrib/securetransport.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index bbf27fd8a1..4e698968b4 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -468,8 +468,7 @@ def handshake(self, # so we have to detect when we error out and try # setting TLS 1.3 if it's allowed. result = Security.SSLSetProtocolVersionMax(self.context, max_version) - print("err", result) - if result and max_version == SecurityConst.kTLSProtocolMaxSupported: + if result != 0 and max_version == SecurityConst.kTLSProtocolMaxSupported: result = Security.SSLSetProtocolVersionMax(self.context, SecurityConst.kTLSProtocol12) _assert_no_error(result) @@ -683,17 +682,17 @@ def version(self): protocol = Security.SSLProtocol() result = Security.SSLGetNegotiatedProtocolVersion(self.context, ctypes.byref(protocol)) _assert_no_error(result) - if protocol == SecurityConst.kTLSProtocol13: + if protocol.value == SecurityConst.kTLSProtocol13: return 'TLSv1.3' - elif protocol == SecurityConst.kTLSProtocol12: + elif protocol.value == SecurityConst.kTLSProtocol12: return 'TLSv1.2' - elif protocol == SecurityConst.kTLSProtocol11: + elif protocol.value == SecurityConst.kTLSProtocol11: return 'TLSv1.1' - elif protocol == SecurityConst.kTLSProtocol1: + elif protocol.value == SecurityConst.kTLSProtocol1: return 'TLSv1' - elif protocol == SecurityConst.kSSLProtocol3: + elif protocol.value == SecurityConst.kSSLProtocol3: return 'SSLv3' - elif protocol == SecurityConst.kSSLProtocol2: + elif protocol.value == SecurityConst.kSSLProtocol2: return 'SSLv2' else: raise ssl.SSLError('Unknown TLS version: %r' % protocol) From 32de3a59b1b6a07b742554ffba8529a7e0bf212b Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Fri, 7 Dec 2018 15:05:42 -0600 Subject: [PATCH 07/33] Opt-in TLS 1.3 on macOS 10.13 --- _travis/install.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/_travis/install.sh b/_travis/install.sh index eab76c1a2f..447a592bbe 100755 --- a/_travis/install.sh +++ b/_travis/install.sh @@ -21,6 +21,9 @@ if [[ "$(uname -s)" == 'Darwin' ]]; then # The pip in older MacPython releases doesn't support a new enough TLS curl https://bootstrap.pypa.io/get-pip.py | sudo $PYTHON_EXE $PYTHON_EXE -m pip install virtualenv + + # Enable TLS 1.3 on macOS + sudo defaults write /Library/Preferences/com.apple.networkd tcp_connect_enable_tls13 1 else pip install virtualenv fi From 694b1645b8c543944ea4923f3a641559b051a9be Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Fri, 7 Dec 2018 16:21:00 -0600 Subject: [PATCH 08/33] Update tornado to 5.1.1 --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index df3ed61097..f9720ebd15 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -3,7 +3,7 @@ coverage==4.5.1 tox==2.9.1 twine==1.11.0 wheel==0.30.0 -tornado==5.0.2 +tornado==5.1.1 PySocks==1.6.8 pkginfo==1.4.2 pytest-timeout==1.3.1 From 9939524126fb2545594c941dcbb71ff6188ddffb Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Mon, 10 Dec 2018 14:32:18 +0000 Subject: [PATCH 09/33] Add documentation updates for TLSv1.3 --- src/urllib3/contrib/securetransport.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 4e698968b4..f14704037a 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -123,7 +123,8 @@ ] # Basically this is simple: for PROTOCOL_SSLv23 we turn it into a low of -# TLSv1 and a high of TLSv1.2. For everything else, we pin to that version. +# TLSv1 and a high of TLSv1.3. For everything else, we pin to that version. +# TLSv1 to 1.2 are supported on macOS 10.8+ and TLSv1.3 is macOS 10.13+ _protocol_to_min_max = { ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocolMaxSupported), } @@ -466,7 +467,8 @@ def handshake(self, # TLS 1.3 isn't necessarily enabled by the OS # so we have to detect when we error out and try - # setting TLS 1.3 if it's allowed. + # setting TLS 1.3 if it's allowed. kTLSProtocolMaxSupported + # was added in macOS 10.13 along with kTLSProtocol13. result = Security.SSLSetProtocolVersionMax(self.context, max_version) if result != 0 and max_version == SecurityConst.kTLSProtocolMaxSupported: result = Security.SSLSetProtocolVersionMax(self.context, SecurityConst.kTLSProtocol12) From 493d78f743199285542db156fd727c02f2de257e Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Mon, 10 Dec 2018 19:07:13 -0600 Subject: [PATCH 10/33] Add wbond/oscrypto license to contrib/securetransport --- src/urllib3/contrib/securetransport.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 4e698968b4..da4271329a 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -23,6 +23,31 @@ urllib3.contrib.securetransport.inject_into_urllib3() Happy TLSing! + +This code is a bastardised version of the code found in Will Bond's oscrypto +library. An enormous debt is owed to him for blazing this trail for us. For +that reason, this code should be considered to be covered both by urllib3's +license and by oscrypto's: + + Copyright (c) 2015-2016 Will Bond + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. """ from __future__ import absolute_import From 9fe526902311c63b5e66c4bd7c55f69d26421df7 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Mon, 10 Dec 2018 19:16:41 -0600 Subject: [PATCH 11/33] Remove all TLS 1.3 ciphersuites from DEFAULT_CIPHERS --- src/urllib3/util/ssl_.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 00a6fdf8b7..62b11c59cc 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -83,17 +83,15 @@ def inet_pton(_, host): # - https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/ # # The general intent is: -# - Prefer TLS 1.3 cipher suites # - prefer cipher suites that offer perfect forward secrecy (DHE/ECDHE), # - prefer ECDHE over DHE for better performance, # - prefer any AES-GCM and ChaCha20 over any AES-CBC for better performance and # security, # - prefer AES-GCM over ChaCha20 because hardware-accelerated AES is common, # - disable NULL authentication, MD5 MACs and DSS for security reasons. +# - NOTE: TLS 1.3 ciphersuites are managed through a different interface +# not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ - 'TLS13-AES-256-GCM-SHA384', - 'TLS13-CHACHA20-POLY1305-SHA256', - 'TLS13-AES-128-GCM-SHA256', 'ECDH+AESGCM', 'ECDH+CHACHA20', 'DH+AESGCM', From b364e7062fae615abac44626612d243afd7a5ed3 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Mon, 17 Dec 2018 13:11:06 -0600 Subject: [PATCH 12/33] Experiment showing cipher list per protocol --- test/with_dummyserver/test_https.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 129c94adbe..87d511f002 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -575,6 +575,11 @@ def test_tls_protocol_name_of_socket(self): self.assertEqual(conn.sock.version(), self.tls_protocol_name) + def test_tls_howsmyssl_ciphers(self): + r = self._pool.request('GET', 'https://howsmyssl.com/a/check') + print(r.data) + assert False + @pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1"), reason="Requires TLSv1 support") class TestHTTPS_TLSv1(TestHTTPS_TLSVersion): From 209873ab0ff066eb90285b209a5d7157b452b80a Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Mon, 17 Dec 2018 13:16:06 -0600 Subject: [PATCH 13/33] Update test_https.py --- test/with_dummyserver/test_https.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 87d511f002..3d8362f1f7 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -27,7 +27,7 @@ fails_on_travis_gce, TARPIT_HOST, ) -from urllib3 import HTTPSConnectionPool +from urllib3 import HTTPSConnectionPool, PoolManager from urllib3.connection import ( VerifiedHTTPSConnection, UnverifiedHTTPSConnection, @@ -576,7 +576,8 @@ def test_tls_protocol_name_of_socket(self): self.assertEqual(conn.sock.version(), self.tls_protocol_name) def test_tls_howsmyssl_ciphers(self): - r = self._pool.request('GET', 'https://howsmyssl.com/a/check') + http = PoolManager(**self.certs()) + r = http.request('GET', 'https://howsmyssl.com/a/check') print(r.data) assert False From 490251b11856d27fc20239dd664f13c6f26348d7 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Mon, 17 Dec 2018 13:22:58 -0600 Subject: [PATCH 14/33] Update test_https.py --- test/with_dummyserver/test_https.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 3d8362f1f7..bfa1a8d974 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -576,7 +576,7 @@ def test_tls_protocol_name_of_socket(self): self.assertEqual(conn.sock.version(), self.tls_protocol_name) def test_tls_howsmyssl_ciphers(self): - http = PoolManager(**self.certs()) + http = PoolManager(ssl_version=self.certs()['ssl_version']) r = http.request('GET', 'https://howsmyssl.com/a/check') print(r.data) assert False From 93f1d3a63b038ae8cf356d2abaa5e13dd6a0f230 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Mon, 17 Dec 2018 21:48:24 +0000 Subject: [PATCH 15/33] Update test_https.py --- test/with_dummyserver/test_https.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index bfa1a8d974..129c94adbe 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -27,7 +27,7 @@ fails_on_travis_gce, TARPIT_HOST, ) -from urllib3 import HTTPSConnectionPool, PoolManager +from urllib3 import HTTPSConnectionPool from urllib3.connection import ( VerifiedHTTPSConnection, UnverifiedHTTPSConnection, @@ -575,12 +575,6 @@ def test_tls_protocol_name_of_socket(self): self.assertEqual(conn.sock.version(), self.tls_protocol_name) - def test_tls_howsmyssl_ciphers(self): - http = PoolManager(ssl_version=self.certs()['ssl_version']) - r = http.request('GET', 'https://howsmyssl.com/a/check') - print(r.data) - assert False - @pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1"), reason="Requires TLSv1 support") class TestHTTPS_TLSv1(TestHTTPS_TLSVersion): From 95e59351118af86c61ee892229ce6b3cf42a0903 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Tue, 18 Dec 2018 19:07:44 +0000 Subject: [PATCH 16/33] Update changelog wording to exclude pyOpenSSL --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 394061aa38..284a567e43 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,7 @@ dev (master) * Implemented a more efficient ``HTTPResponse.__iter__()`` method (Issue #1483) -* Added support for TLSv1.3 support for all ``HTTPSConnection`` implementations. (PR #1496) +* Add support for TLSv1.3 support for CPython and SecureTransport ``SSLContext`` implementations. (PR #1496) * ... [Short description of non-trivial change.] (Issue #) From 1ae867487849ecaa95c6465fa20ed3d0d090192f Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Tue, 18 Dec 2018 19:08:24 +0000 Subject: [PATCH 17/33] minor rewording --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 284a567e43..055ca7177d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,7 @@ dev (master) * Implemented a more efficient ``HTTPResponse.__iter__()`` method (Issue #1483) -* Add support for TLSv1.3 support for CPython and SecureTransport ``SSLContext`` implementations. (PR #1496) +* Add TLSv1.3 support to CPython and SecureTransport ``SSLContext`` implementations. (PR #1496) * ... [Short description of non-trivial change.] (Issue #) From 4f6f74d11ec8583e60fe9834aebb89a92835ae3d Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 18 Dec 2018 23:35:08 -0600 Subject: [PATCH 18/33] Add support for IPv6 in subjectAltName --- dummyserver/certs/server.ipv6_san.crt | 16 +++++++++++++++ dummyserver/server.py | 5 +++++ src/urllib3/contrib/pyopenssl.py | 13 ++++++++----- src/urllib3/contrib/securetransport.py | 2 +- src/urllib3/util/__init__.py | 2 ++ src/urllib3/util/ssl_.py | 27 +++++++++++++++++++------- test/contrib/test_pyopenssl.py | 2 +- test/with_dummyserver/test_https.py | 22 ++++++++++++++++++++- 8 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 dummyserver/certs/server.ipv6_san.crt diff --git a/dummyserver/certs/server.ipv6_san.crt b/dummyserver/certs/server.ipv6_san.crt new file mode 100644 index 0000000000..64acace329 --- /dev/null +++ b/dummyserver/certs/server.ipv6_san.crt @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICfTCCAeagAwIBAgIJAPcpn3/M5+piMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMTgxMjE5MDUyMjUyWhcNNDgxMjE4MDUyMjUyWjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB +gQDXe3FqmCWvP8XPxqtT+0bfL1Tvzvebi46k0WIcUV8bP3vyYiSRXG9ALmyzZH4G +HY9UVs4OEDkCMDOBSezB0y9ai/9doTNcaictdEBu8nfdXKoTtzrn+VX4UPrkH5hm +7NQ1fTQuj1MR7yBCmYqN3Q2Q+Efuujyx0FwBzAuy1aKYuwIDAQABo3UwczAdBgNV +HQ4EFgQUG+dK5Uos08QUwAWofDb3a8YcYlIwHwYDVR0jBBgwFoAUG+dK5Uos08QU +wAWofDb3a8YcYlIwDwYDVR0TAQH/BAUwAwEB/zAgBgNVHREEGTAXggM6OjGHEAAA +AAAAAAAAAAAAAAAAAAEwDQYJKoZIhvcNAQELBQADgYEAjT767TDq6q4lOextf3tZ +BjeuYDUy7bb1fDBAN5rBT1ywr7r0JE6/KOnsZx4jbevx3MllxNpx0gOM2bgwJlnG +8tgwRB6pxDyln01WBj9b5ymK60jdkw7gg3yYpqEs5/VBQidFO3BmDqf5cGO8PU7p +0VWdfJBP2UbwblNXdImI1zk= +-----END CERTIFICATE----- diff --git a/dummyserver/server.py b/dummyserver/server.py index c7da0e9823..b5d21592e0 100755 --- a/dummyserver/server.py +++ b/dummyserver/server.py @@ -58,11 +58,16 @@ 'certfile': os.path.join(CERTS_PATH, 'server.ipv6addr.crt'), 'keyfile': os.path.join(CERTS_PATH, 'server.ipv6addr.key'), } +IPV6_SAN_CERTS = { + 'certfile': os.path.join(CERTS_PATH, 'server.ipv6_san.crt'), + 'keyfile': DEFAULT_CERTS['keyfile'] +} DEFAULT_CA = os.path.join(CERTS_PATH, 'cacert.pem') DEFAULT_CA_BAD = os.path.join(CERTS_PATH, 'client_bad.pem') NO_SAN_CA = os.path.join(CERTS_PATH, 'cacert.no_san.pem') DEFAULT_CA_DIR = os.path.join(CERTS_PATH, 'ca_path_test') IPV6_ADDR_CA = os.path.join(CERTS_PATH, 'server.ipv6addr.crt') +IPV6_SAN_CA = os.path.join(CERTS_PATH, 'server.ipv6_san.crt') COMBINED_CERT_AND_KEY = os.path.join(CERTS_PATH, 'server.combined.pem') diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index be028aaf26..93b6ddba19 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -77,20 +77,19 @@ class UnsupportedExtension(Exception): # Map from urllib3 to PyOpenSSL compatible parameter-values. _openssl_versions = { - ssl.PROTOCOL_SSLv23: OpenSSL.SSL.SSLv23_METHOD, + util.PROTOCOL_TLS: OpenSSL.SSL.SSLv23_METHOD, ssl.PROTOCOL_TLSv1: OpenSSL.SSL.TLSv1_METHOD, } +if hasattr(ssl, 'PROTOCOL_SSLv3') and hasattr(OpenSSL.SSL, 'SSLv3_METHOD'): + _openssl_versions[ssl.PROTOCOL_SSLv3] = OpenSSL.SSL.SSLv3_METHOD + if hasattr(ssl, 'PROTOCOL_TLSv1_1') and hasattr(OpenSSL.SSL, 'TLSv1_1_METHOD'): _openssl_versions[ssl.PROTOCOL_TLSv1_1] = OpenSSL.SSL.TLSv1_1_METHOD if hasattr(ssl, 'PROTOCOL_TLSv1_2') and hasattr(OpenSSL.SSL, 'TLSv1_2_METHOD'): _openssl_versions[ssl.PROTOCOL_TLSv1_2] = OpenSSL.SSL.TLSv1_2_METHOD -try: - _openssl_versions.update({ssl.PROTOCOL_SSLv3: OpenSSL.SSL.SSLv3_METHOD}) -except AttributeError: - pass _stdlib_to_openssl_verify = { ssl.CERT_NONE: OpenSSL.SSL.VERIFY_NONE, @@ -184,6 +183,10 @@ def idna_encode(name): except idna.core.IDNAError: return None + # Don't send IPv6 addresses through the IDNA encoder. + if ':' in name: + return name + name = idna_encode(name) if name is None: return None diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 38d953894f..cb4d9620b7 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -151,7 +151,7 @@ # TLSv1 and a high of TLSv1.3. For everything else, we pin to that version. # TLSv1 to 1.2 are supported on macOS 10.8+ and TLSv1.3 is macOS 10.13+ _protocol_to_min_max = { - ssl.PROTOCOL_SSLv23: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocolMaxSupported), + util.PROTOCOL_TLS: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocolMaxSupported), } if hasattr(ssl, "PROTOCOL_SSLv2"): diff --git a/src/urllib3/util/__init__.py b/src/urllib3/util/__init__.py index 2f2770b622..2914bb468b 100644 --- a/src/urllib3/util/__init__.py +++ b/src/urllib3/util/__init__.py @@ -12,6 +12,7 @@ resolve_cert_reqs, resolve_ssl_version, ssl_wrap_socket, + PROTOCOL_TLS, ) from .timeout import ( current_time, @@ -35,6 +36,7 @@ 'IS_PYOPENSSL', 'IS_SECURETRANSPORT', 'SSLContext', + 'PROTOCOL_TLS', 'Retry', 'Timeout', 'Url', diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 62b11c59cc..5094104c77 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -43,17 +43,28 @@ def _const_compare_digest_backport(a, b): try: # Test for SSL features import ssl - from ssl import wrap_socket, CERT_NONE, PROTOCOL_SSLv23 + from ssl import wrap_socket, CERT_NONE from ssl import HAS_SNI # Has SNI? except ImportError: pass +try: # Platform-specific: Python 3.6 + from ssl import PROTOCOL_TLS + PROTOCOL_SSLv23 = PROTOCOL_TLS +except ImportError: + try: + from ssl import PROTOCOL_SSLv23 as PROTOCOL_TLS + PROTOCOL_SSLv23 = PROTOCOL_TLS + except ImportError: + PROTOCOL_SSLv23 = PROTOCOL_TLS = 2 + try: - from ssl import OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_COMPRESSION + from ssl import OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_COMPRESSION, OP_ALL except ImportError: OP_NO_SSLv2, OP_NO_SSLv3 = 0x1000000, 0x2000000 OP_NO_COMPRESSION = 0x20000 + OP_ALL = 0 # Python 2.7 doesn't have inet_pton on non-Linux so we fallback on inet_aton in @@ -89,7 +100,7 @@ def inet_pton(_, host): # security, # - prefer AES-GCM over ChaCha20 because hardware-accelerated AES is common, # - disable NULL authentication, MD5 MACs and DSS for security reasons. -# - NOTE: TLS 1.3 ciphersuites are managed through a different interface +# - NOTE: TLS 1.3 cipher suites are managed through a different interface # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ 'ECDH+AESGCM', @@ -105,6 +116,7 @@ def inet_pton(_, host): '!aNULL', '!eNULL', '!MD5', + '!DSS', ]) try: @@ -117,7 +129,7 @@ def __init__(self, protocol_version): self.check_hostname = False self.verify_mode = ssl.CERT_NONE self.ca_certs = None - self.options = 0 + self.options = OP_ALL self.certfile = None self.keyfile = None self.ciphers = None @@ -211,7 +223,7 @@ def resolve_ssl_version(candidate): like resolve_cert_reqs """ if candidate is None: - return PROTOCOL_SSLv23 + return PROTOCOL_TLS if isinstance(candidate, str): res = getattr(ssl, candidate, None) @@ -257,7 +269,7 @@ def create_urllib3_context(ssl_version=None, cert_reqs=None, Constructed SSLContext object with specified options :rtype: SSLContext """ - context = SSLContext(ssl_version or ssl.PROTOCOL_SSLv23) + context = SSLContext(ssl_version or PROTOCOL_TLS) context.set_ciphers(ciphers or DEFAULT_CIPHERS) @@ -265,7 +277,8 @@ def create_urllib3_context(ssl_version=None, cert_reqs=None, cert_reqs = ssl.CERT_REQUIRED if cert_reqs is None else cert_reqs if options is None: - options = 0 + # Work-arounds for bugs in other SSL implementations + options = OP_ALL # SSLv2 is easily broken and is considered harmful and dangerous options |= OP_NO_SSLv2 # SSLv3 has several problems and is now dangerous diff --git a/test/contrib/test_pyopenssl.py b/test/contrib/test_pyopenssl.py index 237d229bc2..31fb299a6d 100644 --- a/test/contrib/test_pyopenssl.py +++ b/test/contrib/test_pyopenssl.py @@ -34,7 +34,7 @@ def teardown_module(): from ..with_dummyserver.test_https import ( # noqa: F401 TestHTTPS, TestHTTPS_TLSv1, TestHTTPS_TLSv1_1, TestHTTPS_TLSv1_2, TestHTTPS_TLSv1_3, TestHTTPS_IPSAN, - TestHTTPS_IPv6Addr, TestHTTPS_NoSAN + TestHTTPS_IPv6Addr, TestHTTPS_NoSAN, TestHTTPS_IPV6SAN ) from ..with_dummyserver.test_socketlevel import ( # noqa: F401 TestSNI, TestSocketClosing, TestClientCerts diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 129c94adbe..cbc6cc313e 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -17,7 +17,7 @@ DEFAULT_CLIENT_NO_INTERMEDIATE_CERTS, NO_SAN_CERTS, NO_SAN_CA, DEFAULT_CA_DIR, IPV6_ADDR_CERTS, IPV6_ADDR_CA, HAS_IPV6, - IP_SAN_CERTS) + IP_SAN_CERTS, IPV6_SAN_CERTS, IPV6_SAN_CA) from test import ( onlyPy279OrNewer, @@ -674,5 +674,25 @@ def test_strip_square_brackets_before_validating(self): self.assertEqual(r.status, 200) +class TestHTTPS_IPV6SAN(IPV6HTTPSDummyServerTestCase): + @classmethod + def certs(cls): + return IPV6_SAN_CERTS + + def test_can_validate_ipv6_san(self): + """Ensure that urllib3 can validate SANs with IPv6 addresses in them.""" + try: + import ipaddress # noqa: F401 + except ImportError: + pytest.skip("Only runs on systems with an ipaddress module") + + https_pool = HTTPSConnectionPool('[::1]', self.port, + cert_reqs='CERT_REQUIRED', + ca_certs=IPV6_SAN_CA) + self.addCleanup(https_pool.close) + r = https_pool.request('GET', '/') + self.assertEqual(r.status, 200) + + if __name__ == '__main__': unittest.main() From a18f62394e93617ff928dfe39035bbb35ecb6856 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 18 Dec 2018 23:48:40 -0600 Subject: [PATCH 19/33] Don't use OP_ALL --- src/urllib3/util/ssl_.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 5094104c77..e57de47c8b 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -60,11 +60,10 @@ def _const_compare_digest_backport(a, b): try: - from ssl import OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_COMPRESSION, OP_ALL + from ssl import OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_COMPRESSION except ImportError: OP_NO_SSLv2, OP_NO_SSLv3 = 0x1000000, 0x2000000 OP_NO_COMPRESSION = 0x20000 - OP_ALL = 0 # Python 2.7 doesn't have inet_pton on non-Linux so we fallback on inet_aton in @@ -129,7 +128,7 @@ def __init__(self, protocol_version): self.check_hostname = False self.verify_mode = ssl.CERT_NONE self.ca_certs = None - self.options = OP_ALL + self.options = 0 self.certfile = None self.keyfile = None self.ciphers = None @@ -277,8 +276,7 @@ def create_urllib3_context(ssl_version=None, cert_reqs=None, cert_reqs = ssl.CERT_REQUIRED if cert_reqs is None else cert_reqs if options is None: - # Work-arounds for bugs in other SSL implementations - options = OP_ALL + options = 0 # SSLv2 is easily broken and is considered harmful and dangerous options |= OP_NO_SSLv2 # SSLv3 has several problems and is now dangerous From a97cefe92f0e22180c65b7c03b1202f110953d36 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Wed, 19 Dec 2018 15:05:44 +0000 Subject: [PATCH 20/33] Update CHANGES.rst --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 055ca7177d..39e4548d8b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,6 +8,8 @@ dev (master) * Add TLSv1.3 support to CPython and SecureTransport ``SSLContext`` implementations. (PR #1496) +* Add support for IPv6 addresses in subjectAltName section of certificates. (Issue #1269) + * ... [Short description of non-trivial change.] (Issue #) From ca52ca5b19f50767c67e28ea81a0f0ca8c46c701 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Fri, 21 Dec 2018 16:46:24 +0000 Subject: [PATCH 21/33] No PROTOCOL_TLSv1_3 --- test/with_dummyserver/test_https.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index cbc6cc313e..f1dea7b764 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -609,14 +609,14 @@ def certs(cls): return certs -@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_3"), reason="Requires TLSv1.3 support") +@pytest.mark.skipif(not getattr(ssl, "HAS_TLSv1_3", False), reason="Requires TLSv1.3 support") class TestHTTPS_TLSv1_3(TestHTTPS_TLSVersion): tls_protocol_name = 'TLSv1.3' @classmethod def certs(cls): certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLSv1_3 + certs['ssl_version'] = ssl.PROTOCOL_TLS # No dedicated TLS 1.3 Protocol return certs From 7e4e485090b0f928ee69c0fbe7fcad06e17a2202 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Wed, 26 Dec 2018 13:43:43 -0600 Subject: [PATCH 22/33] Remove DSS, rearrange SecureTransport ciphers --- CHANGES.rst | 5 ++++- .../contrib/_securetransport/bindings.py | 12 +++++----- src/urllib3/contrib/securetransport.py | 22 ++++++------------- src/urllib3/util/ssl_.py | 6 ++++- test/with_dummyserver/test_https.py | 8 +++++++ 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 39e4548d8b..8c13487442 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,10 @@ dev (master) * Implemented a more efficient ``HTTPResponse.__iter__()`` method (Issue #1483) -* Add TLSv1.3 support to CPython and SecureTransport ``SSLContext`` implementations. (PR #1496) +* Add TLSv1.3 support to CPython and SecureTransport ``SSLContext`` implementations. (Pull #1496) + +* Drop ciphers using DSS key exchange from default TLS cipher suites. + Improve default ciphers when using SecureTransport. (Pull #1496) * Add support for IPv6 addresses in subjectAltName section of certificates. (Issue #1269) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index dbb050f44a..5a64a55152 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -564,26 +564,23 @@ class SecurityConst(object): TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 = 0xC030 TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 = 0xC02B TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 = 0xC02F - TLS_DHE_DSS_WITH_AES_256_GCM_SHA384 = 0x00A3 + TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 = 0xCCA9 + TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 = 0xCCA8 + TLS_CHACHA20_POLY1305_SHA256 = 0x1303 TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 = 0x009F - TLS_DHE_DSS_WITH_AES_128_GCM_SHA256 = 0x00A2 TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 = 0x009E TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 = 0xC024 TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 = 0xC028 TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA = 0xC00A TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA = 0xC014 TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 = 0x006B - TLS_DHE_DSS_WITH_AES_256_CBC_SHA256 = 0x006A TLS_DHE_RSA_WITH_AES_256_CBC_SHA = 0x0039 - TLS_DHE_DSS_WITH_AES_256_CBC_SHA = 0x0038 TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 = 0xC023 TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 = 0xC027 TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA = 0xC009 TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA = 0xC013 TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 = 0x0067 - TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 = 0x0040 TLS_DHE_RSA_WITH_AES_128_CBC_SHA = 0x0033 - TLS_DHE_DSS_WITH_AES_128_CBC_SHA = 0x0032 TLS_RSA_WITH_AES_256_GCM_SHA384 = 0x009D TLS_RSA_WITH_AES_128_GCM_SHA256 = 0x009C TLS_RSA_WITH_AES_256_CBC_SHA256 = 0x003D @@ -592,4 +589,5 @@ class SecurityConst(object): TLS_RSA_WITH_AES_128_CBC_SHA = 0x002F TLS_AES_128_GCM_SHA256 = 0x1301 TLS_AES_256_GCM_SHA384 = 0x1302 - TLS_CHACHA20_POLY1305_SHA256 = 0x1303 + TLS_AES_128_CCM_8_SHA256 = 0x1305 + TLS_AES_128_CCM_SHA256 = 0x1304 diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index cb4d9620b7..ea2224c900 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -112,35 +112,33 @@ # individual cipher suites. We need to do this because this is how # SecureTransport wants them. CIPHER_SUITES = [ - SecurityConst.TLS_AES_256_GCM_SHA384, - SecurityConst.TLS_CHACHA20_POLY1305_SHA256, - SecurityConst.TLS_AES_128_GCM_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - SecurityConst.TLS_DHE_DSS_WITH_AES_256_GCM_SHA384, + SecurityConst.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + SecurityConst.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, - SecurityConst.TLS_DHE_DSS_WITH_AES_128_GCM_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_128_GCM_SHA256, + SecurityConst.TLS_CHACHA20_POLY1305_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384, SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, - SecurityConst.TLS_DHE_DSS_WITH_AES_256_CBC_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA, - SecurityConst.TLS_DHE_DSS_WITH_AES_256_CBC_SHA, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, SecurityConst.TLS_DHE_RSA_WITH_AES_128_CBC_SHA256, - SecurityConst.TLS_DHE_DSS_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_128_CBC_SHA, - SecurityConst.TLS_DHE_DSS_WITH_AES_128_CBC_SHA, + SecurityConst.TLS_AES_256_GCM_SHA384, + SecurityConst.TLS_AES_128_GCM_SHA256, SecurityConst.TLS_RSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_RSA_WITH_AES_128_GCM_SHA256, + SecurityConst.TLS_AES_128_CCM_8_SHA256, + SecurityConst.TLS_AES_128_CCM_SHA256, SecurityConst.TLS_RSA_WITH_AES_256_CBC_SHA256, SecurityConst.TLS_RSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_RSA_WITH_AES_256_CBC_SHA, @@ -174,12 +172,6 @@ _protocol_to_min_max[ssl.PROTOCOL_TLSv1_2] = ( SecurityConst.kTLSProtocol12, SecurityConst.kTLSProtocol12 ) -if hasattr(ssl, "PROTOCOL_TLSv1_3") and version_info >= (10, 13): - _protocol_to_min_max[ssl.PROTOCOL_TLSv1_3] = ( - SecurityConst.kTLSProtocol13, SecurityConst.kTLSProtocol13 - ) -if hasattr(ssl, "PROTOCOL_TLS"): - _protocol_to_min_max[ssl.PROTOCOL_TLS] = _protocol_to_min_max[ssl.PROTOCOL_SSLv23] def inject_into_urllib3(): diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index e57de47c8b..28263e8b66 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -98,7 +98,8 @@ def inet_pton(_, host): # - prefer any AES-GCM and ChaCha20 over any AES-CBC for better performance and # security, # - prefer AES-GCM over ChaCha20 because hardware-accelerated AES is common, -# - disable NULL authentication, MD5 MACs and DSS for security reasons. +# - disable NULL authentication, MD5 MACs, DSS, and other +# insecure ciphers for security reasons. # - NOTE: TLS 1.3 cipher suites are managed through a different interface # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ @@ -116,6 +117,9 @@ def inet_pton(_, host): '!eNULL', '!MD5', '!DSS', + '!PSK', + '!IDEA', + '!SEED', ]) try: diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index f1dea7b764..94f41f9c80 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -553,6 +553,14 @@ def _request_without_resource_warnings(self, method, url): return [x for x in w if not isinstance(x.message, ResourceWarning)] + def test_show_ciphers(self): + from urllib3 import PoolManager + import urllib3.util + http = PoolManager() + r = http.request('GET', 'https://howsmyssl.com/a/check') + print((urllib3.util.IS_SECURETRANSPORT, urllib3.util.IS_PYOPENSSL, r.data)) + assert False + class TestHTTPS_TLSVersion(TestHTTPS): tls_protocol_name = None From 5745dfb9fb8147d28586b5a838f9e90afdd80296 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Wed, 26 Dec 2018 13:55:31 -0600 Subject: [PATCH 23/33] Use ECDSA before RSA with ECDHE --- src/urllib3/util/ssl_.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 28263e8b66..2809b379ae 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -103,12 +103,16 @@ def inet_pton(_, host): # - NOTE: TLS 1.3 cipher suites are managed through a different interface # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ + 'ECDH+ECDSA+AESGCM', 'ECDH+AESGCM', + 'ECDH+ECDSA+CHACHA20', 'ECDH+CHACHA20', 'DH+AESGCM', 'DH+CHACHA20', + 'ECDH+ECDSA+AES256', 'ECDH+AES256', 'DH+AES256', + 'ECDH+ECDSA+AES128', 'ECDH+AES128', 'DH+AES', 'RSA+AESGCM', From 2bc274201aae244945ff93089f21e8d992d2a86d Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Wed, 26 Dec 2018 21:34:52 -0600 Subject: [PATCH 24/33] ReviReorder ciphers --- src/urllib3/contrib/_securetransport/bindings.py | 1 - src/urllib3/contrib/securetransport.py | 13 ++++++------- src/urllib3/util/ssl_.py | 15 ++++++--------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index 5a64a55152..7c992697c3 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -566,7 +566,6 @@ class SecurityConst(object): TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 = 0xC02F TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 = 0xCCA9 TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 = 0xCCA8 - TLS_CHACHA20_POLY1305_SHA256 = 0x1303 TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 = 0x009F TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 = 0x009E TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 = 0xC024 diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index ea2224c900..155ded8387 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -113,24 +113,23 @@ # SecureTransport wants them. CIPHER_SUITES = [ SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, SecurityConst.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_DHE_RSA_WITH_AES_128_GCM_SHA256, - SecurityConst.TLS_CHACHA20_POLY1305_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384, - SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, - SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, - SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, - SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, + SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, + SecurityConst.TLS_DHE_RSA_WITH_AES_256_CBC_SHA, SecurityConst.TLS_DHE_RSA_WITH_AES_128_CBC_SHA256, SecurityConst.TLS_DHE_RSA_WITH_AES_128_CBC_SHA, SecurityConst.TLS_AES_256_GCM_SHA384, diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 2809b379ae..ec262d1c0d 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -104,26 +104,23 @@ def inet_pton(_, host): # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ 'ECDH+ECDSA+AESGCM', - 'ECDH+AESGCM', 'ECDH+ECDSA+CHACHA20', + 'ECDH+AESGCM', 'ECDH+CHACHA20', - 'DH+AESGCM', - 'DH+CHACHA20', + 'DHE+AESGCM', + 'DHE+CHACHA20', 'ECDH+ECDSA+AES256', - 'ECDH+AES256', - 'DH+AES256', 'ECDH+ECDSA+AES128', + 'ECDH+AES256', 'ECDH+AES128', - 'DH+AES', + 'DHE+AES256', + 'DHE+AES', 'RSA+AESGCM', 'RSA+AES', '!aNULL', '!eNULL', '!MD5', '!DSS', - '!PSK', - '!IDEA', - '!SEED', ]) try: From 6a4d3dcb663579bfba148e261dacfb377b189da7 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Thu, 27 Dec 2018 10:20:00 -0600 Subject: [PATCH 25/33] ECDHE --- src/urllib3/util/ssl_.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index ec262d1c0d..a247a5b070 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -103,16 +103,16 @@ def inet_pton(_, host): # - NOTE: TLS 1.3 cipher suites are managed through a different interface # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ - 'ECDH+ECDSA+AESGCM', - 'ECDH+ECDSA+CHACHA20', - 'ECDH+AESGCM', - 'ECDH+CHACHA20', + 'ECDHE+ECDSA+AESGCM', + 'ECDHE+ECDSA+CHACHA20', + 'ECDHE+AESGCM', + 'ECDHE+CHACHA20', 'DHE+AESGCM', 'DHE+CHACHA20', - 'ECDH+ECDSA+AES256', - 'ECDH+ECDSA+AES128', - 'ECDH+AES256', - 'ECDH+AES128', + 'ECDHE+ECDSA+AES256', + 'ECDHE+ECDSA+AES128', + 'ECDHE+AES256', + 'ECDHE+AES128', 'DHE+AES256', 'DHE+AES', 'RSA+AESGCM', From 9fc3c5ac739773dd0f4f209ea750c844a1a16de0 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Fri, 28 Dec 2018 10:24:02 -0600 Subject: [PATCH 26/33] Update test_https.py --- test/with_dummyserver/test_https.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 94f41f9c80..f1dea7b764 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -553,14 +553,6 @@ def _request_without_resource_warnings(self, method, url): return [x for x in w if not isinstance(x.message, ResourceWarning)] - def test_show_ciphers(self): - from urllib3 import PoolManager - import urllib3.util - http = PoolManager() - r = http.request('GET', 'https://howsmyssl.com/a/check') - print((urllib3.util.IS_SECURETRANSPORT, urllib3.util.IS_PYOPENSSL, r.data)) - assert False - class TestHTTPS_TLSVersion(TestHTTPS): tls_protocol_name = None From 423df773120c4f5a8eb00ee6f785aff65c941105 Mon Sep 17 00:00:00 2001 From: "Seth M. Larson" Date: Fri, 28 Dec 2018 10:38:43 -0600 Subject: [PATCH 27/33] Turns out we don't need version detection --- src/urllib3/contrib/securetransport.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 155ded8387..309f79ab75 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -69,7 +69,6 @@ _assert_no_error, _cert_array_from_pem, _temporary_keychain, _load_client_cert_chain ) -from ._securetransport.bindings import version_info try: # Platform-specific: Python 2 from socket import _fileobject From 4244f758597a40ff17e9dbaa2d9ae0d037fef4a1 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Wed, 30 Jan 2019 08:14:47 -0600 Subject: [PATCH 28/33] Reorder per Hyneks post and favor ephemeral --- src/urllib3/contrib/_securetransport/bindings.py | 1 + src/urllib3/util/ssl_.py | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index 7c992697c3..be34215359 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -560,6 +560,7 @@ class SecurityConst(object): errSecInvalidTrustSettings = -25262 # Cipher suites. We only pick the ones our default cipher string allows. + # Source: https://developer.apple.com/documentation/security/1550981-ssl_cipher_suite_values TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 = 0xC02C TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 = 0xC030 TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 = 0xC02B diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index a247a5b070..e821a7d7eb 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -103,18 +103,14 @@ def inet_pton(_, host): # - NOTE: TLS 1.3 cipher suites are managed through a different interface # not exposed by CPython (yet!) and are enabled by default if they're available. DEFAULT_CIPHERS = ':'.join([ - 'ECDHE+ECDSA+AESGCM', - 'ECDHE+ECDSA+CHACHA20', 'ECDHE+AESGCM', 'ECDHE+CHACHA20', 'DHE+AESGCM', 'DHE+CHACHA20', - 'ECDHE+ECDSA+AES256', - 'ECDHE+ECDSA+AES128', - 'ECDHE+AES256', - 'ECDHE+AES128', - 'DHE+AES256', - 'DHE+AES', + 'ECDH+AESGCM', + 'DH+AESGCM', + 'ECDH+AES', + 'DH+AES', 'RSA+AESGCM', 'RSA+AES', '!aNULL', From 18001cda8abc4f38499d487528dc53fdc46ef4a5 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 19 Feb 2019 13:21:05 -0600 Subject: [PATCH 29/33] Refactor HTTPS unit tests --- CHANGES.rst | 3 +- dummyserver/testcase.py | 12 +--- src/urllib3/contrib/pyopenssl.py | 10 ++++ test/__init__.py | 20 +++++++ test/with_dummyserver/test_https.py | 91 ++++++++++++----------------- 5 files changed, 73 insertions(+), 63 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 8c13487442..aa6dd56b5b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,8 @@ dev (master) * Implemented a more efficient ``HTTPResponse.__iter__()`` method (Issue #1483) -* Add TLSv1.3 support to CPython and SecureTransport ``SSLContext`` implementations. (Pull #1496) +* Add TLSv1.3 support to CPython, pyOpenSSL, and SecureTransport ``SSLContext`` + implementations. (Pull #1496) * Drop ciphers using DSS key exchange from default TLS cipher suites. Improve default ciphers when using SecureTransport. (Pull #1496) diff --git a/dummyserver/testcase.py b/dummyserver/testcase.py index 14216791be..ebd0bcb841 100644 --- a/dummyserver/testcase.py +++ b/dummyserver/testcase.py @@ -115,16 +115,13 @@ class HTTPDummyServerTestCase(unittest.TestCase): scheme = 'http' host = 'localhost' host_alt = '127.0.0.1' # Some tests need two hosts - - @classmethod - def certs(cls): - return DEFAULT_CERTS + certs = DEFAULT_CERTS @classmethod def _start_server(cls): cls.io_loop = ioloop.IOLoop.current() app = web.Application([(r".*", TestingApp)]) - cls.server, cls.port = run_tornado_app(app, cls.io_loop, cls.certs(), + cls.server, cls.port = run_tornado_app(app, cls.io_loop, cls.certs, cls.scheme, cls.host) cls.server_thread = run_loop_in_thread(cls.io_loop) @@ -146,10 +143,7 @@ def tearDownClass(cls): class HTTPSDummyServerTestCase(HTTPDummyServerTestCase): scheme = 'https' host = 'localhost' - - @classmethod - def certs(cls): - return DEFAULT_CERTS + certs = DEFAULT_CERTS @pytest.mark.skipif(not HAS_IPV6, reason='IPv6 not available') diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index 93b6ddba19..15aa2f0706 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -69,6 +69,8 @@ class UnsupportedExtension(Exception): import sys from .. import util +from ..connection import BaseSSLError + __all__ = ['inject_into_urllib3', 'extract_from_urllib3'] @@ -289,6 +291,10 @@ def recv(self, *args, **kwargs): raise timeout('The read operation timed out') else: return self.recv(*args, **kwargs) + + # TLS 1.3 post-handshake authentication + except OpenSSL.SSL.Error as e: + raise ssl.SSLError("read error: %r" % e) else: return data @@ -311,6 +317,10 @@ def recv_into(self, *args, **kwargs): else: return self.recv_into(*args, **kwargs) + # TLS 1.3 post-handshake authentication + except OpenSSL.SSL.Error as e: + raise ssl.SSLError("read error: %r" % e) + def settimeout(self, timeout): return self.socket.settimeout(timeout) diff --git a/test/__init__.py b/test/__init__.py index 40f00413ba..f4d94fb319 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -152,6 +152,26 @@ def wrapper(*args, **kwargs): return wrapper +def requiresTLSv1(): + """Test requires TLSv1 available""" + return pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1"), reason="Test requires TLSv1") + + +def requiresTLSv1_1(): + """Test requires TLSv1.1 available""" + return pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_1"), reason="Test requires TLSv1.1") + + +def requiresTLSv1_2(): + """Test requires TLSv1.2 available""" + return pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_2"), reason="Test requires TLSv1.2") + + +def requiresTLSv1_3(): + """Test requires TLSv1.3 available""" + return pytest.mark.skipif(not getattr(ssl, "HAS_TLSv1_3", False), reason="Test requires TLSv1.3") + + class _ListHandler(logging.Handler): def __init__(self): super(_ListHandler, self).__init__() diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index f1dea7b764..cd8fed76a4 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -25,6 +25,10 @@ notOpenSSL098, requires_network, fails_on_travis_gce, + requiresTLSv1, + requiresTLSv1_1, + requiresTLSv1_2, + requiresTLSv1_3, TARPIT_HOST, ) from urllib3 import HTTPSConnectionPool @@ -57,7 +61,19 @@ log.addHandler(logging.StreamHandler(sys.stdout)) +TLSv1_CERTS = DEFAULT_CERTS.copy() +TLSv1_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1 + +TLSv1_1_CERTS = DEFAULT_CERTS.copy() +TLSv1_1_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1_1 + +TLSv1_2_CERTS = DEFAULT_CERTS.copy() +TLSv1_2_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1_2 + + class TestHTTPS(HTTPSDummyServerTestCase): + tls_protocol_name = None + def setUp(self): self._pool = HTTPSConnectionPool(self.host, self.port) self.addCleanup(self._pool.close) @@ -553,20 +569,18 @@ def _request_without_resource_warnings(self, method, url): return [x for x in w if not isinstance(x.message, ResourceWarning)] - -class TestHTTPS_TLSVersion(TestHTTPS): - tls_protocol_name = None - - @classmethod - def certs(cls): - return pytest.skip('Don\'t run parent version.') - def test_set_ssl_version_to_tls_version(self): - self._pool.ssl_version = self.certs()['ssl_version'] + if self.tls_protocol_name is None: + pytest.skip("Skipping base test class") + + self._pool.ssl_version = self.certs['ssl_version'] r = self._pool.request('GET', '/') self.assertEqual(r.status, 200, r.data) def test_tls_protocol_name_of_socket(self): + if self.tls_protocol_name is None: + pytest.skip("Skipping base test class") + conn = self._pool._get_conn() conn.connect() @@ -576,54 +590,31 @@ def test_tls_protocol_name_of_socket(self): self.assertEqual(conn.sock.version(), self.tls_protocol_name) -@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1"), reason="Requires TLSv1 support") -class TestHTTPS_TLSv1(TestHTTPS_TLSVersion): +@requiresTLSv1() +class TestHTTPS_TLSv1(TestHTTPS): tls_protocol_name = 'TLSv1' + certs = TLSv1_CERTS - @classmethod - def certs(cls): - certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLSv1 - return certs - -@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_1"), reason="Requires TLSv1.1 support") -class TestHTTPS_TLSv1_1(TestHTTPS_TLSVersion): +@requiresTLSv1_1() +class TestHTTPS_TLSv1_1(TestHTTPS): tls_protocol_name = 'TLSv1.1' - - @classmethod - def certs(cls): - certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLSv1_1 - return certs + certs = TLSv1_1_CERTS -@pytest.mark.skipif(not hasattr(ssl, "PROTOCOL_TLSv1_2"), reason="Requires TLSv1.2 support") -class TestHTTPS_TLSv1_2(TestHTTPS_TLSVersion): +@requiresTLSv1_2() +class TestHTTPS_TLSv1_2(TestHTTPS): tls_protocol_name = 'TLSv1.2' + certs = TLSv1_2_CERTS - @classmethod - def certs(cls): - certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLSv1_2 - return certs - -@pytest.mark.skipif(not getattr(ssl, "HAS_TLSv1_3", False), reason="Requires TLSv1.3 support") -class TestHTTPS_TLSv1_3(TestHTTPS_TLSVersion): +@requiresTLSv1_3() +class TestHTTPS_TLSv1_3(TestHTTPS): tls_protocol_name = 'TLSv1.3' - @classmethod - def certs(cls): - certs = DEFAULT_CERTS.copy() - certs['ssl_version'] = ssl.PROTOCOL_TLS # No dedicated TLS 1.3 Protocol - return certs - class TestHTTPS_NoSAN(HTTPSDummyServerTestCase): - @classmethod - def certs(cls): - return NO_SAN_CERTS + certs = NO_SAN_CERTS def test_warning_for_certs_without_a_san(self): """Ensure that a warning is raised when the cert from the server has @@ -639,9 +630,7 @@ def test_warning_for_certs_without_a_san(self): class TestHTTPS_IPSAN(HTTPSDummyServerTestCase): - @classmethod - def certs(cls): - return IP_SAN_CERTS + certs = IP_SAN_CERTS def test_can_validate_ip_san(self): """Ensure that urllib3 can validate SANs with IP addresses in them.""" @@ -659,9 +648,7 @@ def test_can_validate_ip_san(self): class TestHTTPS_IPv6Addr(IPV6HTTPSDummyServerTestCase): - @classmethod - def certs(cls): - return IPV6_ADDR_CERTS + certs = IPV6_ADDR_CERTS @pytest.mark.skipif(not HAS_IPV6, reason='Only runs on IPv6 systems') def test_strip_square_brackets_before_validating(self): @@ -675,9 +662,7 @@ def test_strip_square_brackets_before_validating(self): class TestHTTPS_IPV6SAN(IPV6HTTPSDummyServerTestCase): - @classmethod - def certs(cls): - return IPV6_SAN_CERTS + certs = IPV6_SAN_CERTS def test_can_validate_ipv6_san(self): """Ensure that urllib3 can validate SANs with IPv6 addresses in them.""" From 3b9c529cb207939d1fa459e80f343f835c7fadb1 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 19 Feb 2019 13:44:25 -0600 Subject: [PATCH 30/33] Fix up tests --- _travis/downstream/requests.sh | 2 +- src/urllib3/contrib/pyopenssl.py | 1 - src/urllib3/util/ssl_.py | 2 +- test/__init__.py | 4 +++- test/with_dummyserver/test_https.py | 6 +++--- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/_travis/downstream/requests.sh b/_travis/downstream/requests.sh index a3a32ac736..f986385749 100755 --- a/_travis/downstream/requests.sh +++ b/_travis/downstream/requests.sh @@ -4,7 +4,7 @@ set -exo pipefail case "${1}" in install) - git clone --depth 1 https://github.com/requests/requests + git clone --depth 1 https://github.com/kennethreitz/requests cd requests git rev-parse HEAD python -m pip install --upgrade pipenv diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index 8e26e93e50..821c174fdc 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -69,7 +69,6 @@ class UnsupportedExtension(Exception): import sys from .. import util -from ..connection import BaseSSLError __all__ = ['inject_into_urllib3', 'extract_from_urllib3'] diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index c017730db3..0327a923ad 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -54,7 +54,7 @@ def _const_compare_digest_backport(a, b): try: # Test for SSL features import ssl - from ssl import wrap_socket, CERT_REQUIRED, PROTOCOL_SSLv23 + from ssl import wrap_socket, CERT_REQUIRED from ssl import HAS_SNI # Has SNI? except ImportError: pass diff --git a/test/__init__.py b/test/__init__.py index d88c4eab0a..ed4e78c77d 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -194,7 +194,9 @@ def requiresTLSv1_2(): def requiresTLSv1_3(): """Test requires TLSv1.3 available""" - return pytest.mark.skipif(not getattr(ssl, "HAS_TLSv1_3", False), reason="Test requires TLSv1.3") + return pytest.mark.skipif( + not getattr(ssl, "HAS_TLSv1_3", False), reason="Test requires TLSv1.3" + ) class _ListHandler(logging.Handler): diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index e15199ded5..2652a90234 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -63,13 +63,13 @@ TLSv1_CERTS = DEFAULT_CERTS.copy() -TLSv1_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1 +TLSv1_CERTS["ssl_version"] = getattr(ssl, "PROTOCOL_TLSv1", None) TLSv1_1_CERTS = DEFAULT_CERTS.copy() -TLSv1_1_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1_1 +TLSv1_1_CERTS["ssl_version"] = getattr(ssl, "PROTOCOL_TLSv1_1", None) TLSv1_2_CERTS = DEFAULT_CERTS.copy() -TLSv1_2_CERTS["ssl_version"] = ssl.PROTOCOL_TLSv1_2 +TLSv1_2_CERTS["ssl_version"] = getattr(ssl, "PROTOCOL_TLSv1_2", None) class TestHTTPS(HTTPSDummyServerTestCase): From ccb3737ccb3f6e5ec9d58c97a1c56ed0b8c19678 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 26 Feb 2019 20:44:41 -0600 Subject: [PATCH 31/33] Test locking pytest-httpbin --- _travis/downstream/requests.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/_travis/downstream/requests.sh b/_travis/downstream/requests.sh index f986385749..df0ce89bec 100755 --- a/_travis/downstream/requests.sh +++ b/_travis/downstream/requests.sh @@ -9,6 +9,9 @@ case "${1}" in git rev-parse HEAD python -m pip install --upgrade pipenv pipenv install --dev --skip-lock + + # See: kennethreitz/requests/5004 + python -m pip install pytest-httpbin<1.0.0 ;; run) cd requests From 9e7823146b1e05902c289010a32978c75316ee8a Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 26 Feb 2019 20:55:24 -0600 Subject: [PATCH 32/33] Update requests.sh --- _travis/downstream/requests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_travis/downstream/requests.sh b/_travis/downstream/requests.sh index df0ce89bec..3c5f4551db 100755 --- a/_travis/downstream/requests.sh +++ b/_travis/downstream/requests.sh @@ -11,7 +11,7 @@ case "${1}" in pipenv install --dev --skip-lock # See: kennethreitz/requests/5004 - python -m pip install pytest-httpbin<1.0.0 + python -m pip install pytest-httpbin==0.3.0 ;; run) cd requests From 15c3af787d621033fcda9c3ae4b409ffe63d49d8 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Wed, 27 Feb 2019 09:48:53 -0600 Subject: [PATCH 33/33] remove whitespace --- test/with_dummyserver/test_https.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py index 2652a90234..eafd40b0b6 100644 --- a/test/with_dummyserver/test_https.py +++ b/test/with_dummyserver/test_https.py @@ -610,7 +610,7 @@ def test_set_ssl_version_to_tls_version(self): self._pool.ssl_version = self.certs['ssl_version'] r = self._pool.request('GET', '/') self.assertEqual(r.status, 200, r.data) - + def test_set_cert_default_cert_required(self): conn = VerifiedHTTPSConnection(self.host, self.port) conn.set_cert()