From edbdc3444eaa8ce02984e6543b36e03c3a1adc9f Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Sun, 22 Nov 2020 16:23:05 -0600 Subject: [PATCH 1/6] Drop RSA from default TLS ciphers --- .../contrib/_securetransport/bindings.py | 1 + src/urllib3/contrib/securetransport.py | 17 +++++++---------- src/urllib3/util/ssl_.py | 2 -- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index 7f6e461e24..6b07e8fc56 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -513,5 +513,6 @@ class SecurityConst: 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 1855bd5c3b..34c60ebf19 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -112,6 +112,13 @@ # individual cipher suites. We need to do this because this is how # SecureTransport wants them. CIPHER_SUITES = [ + # TLS 1.3 + SecurityConst.TLS_AES_256_GCM_SHA384, + SecurityConst.TLS_AES_128_GCM_SHA256, + SecurityConst.TLS_CHACHA20_POLY1305_SHA256, + SecurityConst.TLS_AES_128_CCM_8_SHA256, + SecurityConst.TLS_AES_128_CCM_SHA256, + # TLS 1.2 SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, SecurityConst.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, @@ -132,16 +139,6 @@ 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, - 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, - SecurityConst.TLS_RSA_WITH_AES_128_CBC_SHA, ] # Basically this is simple: for PROTOCOL_SSLv23 we turn it into a low of diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index cf54d41d2e..a7cbf47fca 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -84,8 +84,6 @@ def _const_compare_digest_backport(a, b): "DH+AESGCM", "ECDH+AES", "DH+AES", - "RSA+AESGCM", - "RSA+AES", "!aNULL", "!eNULL", "!MD5", From 4323bd3d949eb85f5963354c16f4a534a58c0677 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Mon, 23 Nov 2020 22:06:23 -0600 Subject: [PATCH 2/6] Use system TLS ciphers in OpenSSL 1.1.1+ --- src/urllib3/contrib/pyopenssl.py | 8 + src/urllib3/contrib/securetransport.py | 3 + src/urllib3/util/ssl_.py | 29 ++- test/contrib/test_pyopenssl.py | 5 +- test/test_ssl.py | 337 +++++++++++++------------ 5 files changed, 221 insertions(+), 161 deletions(-) diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index b19655664f..8564dc3c2f 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -80,6 +80,11 @@ class UnsupportedExtension(Exception): # SNI always works. HAS_SNI = True +# Use system TLS ciphers on OpenSSL 1.1.1+ +USE_SYSTEM_SSL_CIPHERS = util.ssl_._is_ge_openssl_v1_1_1( + openssl_backend.openssl_version_text(), openssl_backend.openssl_version_number() +) + # Map from urllib3 to PyOpenSSL compatible parameter-values. _openssl_versions = { util.PROTOCOL_TLS: OpenSSL.SSL.SSLv23_METHOD, @@ -109,6 +114,7 @@ class UnsupportedExtension(Exception): orig_util_HAS_SNI = util.HAS_SNI orig_util_SSLContext = util.ssl_.SSLContext +orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_SYSTEM_SSL_CIPHERS log = logging.getLogger(__name__) @@ -125,6 +131,7 @@ def inject_into_urllib3(): util.ssl_.HAS_SNI = HAS_SNI util.IS_PYOPENSSL = True util.ssl_.IS_PYOPENSSL = True + util.ssl_.USE_SYSTEM_SSL_CIPHERS = USE_SYSTEM_SSL_CIPHERS def extract_from_urllib3(): @@ -136,6 +143,7 @@ def extract_from_urllib3(): util.ssl_.HAS_SNI = orig_util_HAS_SNI util.IS_PYOPENSSL = False util.ssl_.IS_PYOPENSSL = False + util.ssl_.USE_SYSTEM_SSL_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS def _validate_dependencies_met(): diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 34c60ebf19..9171419c53 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -84,6 +84,7 @@ orig_util_HAS_SNI = util.HAS_SNI orig_util_SSLContext = util.ssl_.SSLContext +orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_SYSTEM_SSL_CIPHERS # This dictionary is used by the read callback to obtain a handle to the # calling wrapped socket. This is a pretty silly approach, but for now it'll @@ -185,6 +186,7 @@ def inject_into_urllib3(): util.ssl_.HAS_SNI = HAS_SNI util.IS_SECURETRANSPORT = True util.ssl_.IS_SECURETRANSPORT = True + util.ssl_.USE_SYSTEM_SSL_CIPHERS = False def extract_from_urllib3(): @@ -197,6 +199,7 @@ def extract_from_urllib3(): util.ssl_.HAS_SNI = orig_util_HAS_SNI util.IS_SECURETRANSPORT = False util.ssl_.IS_SECURETRANSPORT = False + util.ssl_.USE_SYSTEM_SSL_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS def _read_callback(connection_id, data_buffer, data_length_pointer): diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index a7cbf47fca..9639f19d50 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -14,6 +14,7 @@ IS_PYOPENSSL = False IS_SECURETRANSPORT = False ALPN_PROTOCOLS = ["http/1.1"] +USE_SYSTEM_SSL_CIPHERS = False # Maps the length of a digest to a possible hash function producing this digest HASHFUNC_MAP = {32: md5, 40: sha1, 64: sha256} @@ -34,6 +35,21 @@ def _const_compare_digest_backport(a, b): _const_compare_digest = getattr(hmac, "compare_digest", _const_compare_digest_backport) + +def _is_ge_openssl_v1_1_1( + openssl_version_text: str, openssl_version_number: int +) -> bool: + """Returns True for OpenSSL 1.1.1+ (>=0x10101000) + + LibreSSL reports a version number of 0x20000000 for + OpenSSL version number so we need to filter out LibreSSL. + """ + return ( + not openssl_version_text.startswith("LibreSSL") + and openssl_version_number >= 0x10101000 + ) + + try: # Do we have ssl at all? import ssl from ssl import ( @@ -41,12 +57,17 @@ def _const_compare_digest_backport(a, b): HAS_SNI, OP_NO_COMPRESSION, OP_NO_TICKET, + OPENSSL_VERSION, + OPENSSL_VERSION_NUMBER, PROTOCOL_TLS, OP_NO_SSLv2, OP_NO_SSLv3, SSLContext, ) + USE_SYSTEM_SSL_CIPHERS = _is_ge_openssl_v1_1_1( + OPENSSL_VERSION, OPENSSL_VERSION_NUMBER + ) PROTOCOL_SSLv23 = PROTOCOL_TLS from .ssltransport import SSLTransport except ImportError: @@ -189,14 +210,18 @@ def create_urllib3_context( Specific OpenSSL options. These default to ``ssl.OP_NO_SSLv2``, ``ssl.OP_NO_SSLv3``, ``ssl.OP_NO_COMPRESSION``, and ``ssl.OP_NO_TICKET``. :param ciphers: - Which cipher suites to allow the server to select. + Which cipher suites to allow the server to select. Defaults to either system configured + ciphers if OpenSSL 1.1.1+, otherwise uses a secure default set of ciphers. :returns: Constructed SSLContext object with specified options :rtype: SSLContext """ context = SSLContext(ssl_version or PROTOCOL_TLS) - context.set_ciphers(ciphers or DEFAULT_CIPHERS) + # Unless we're given ciphers defer to either system ciphers in + # the case of OpenSSL 1.1.1+ or use our own secure default ciphers. + if ciphers is not None or not USE_SYSTEM_SSL_CIPHERS: + context.set_ciphers(ciphers or DEFAULT_CIPHERS) # Setting the default here, as we may have no ssl module on import cert_reqs = ssl.CERT_REQUIRED if cert_reqs is None else cert_reqs diff --git a/test/contrib/test_pyopenssl.py b/test/contrib/test_pyopenssl.py index 05d2c7e221..3d1bfbf308 100644 --- a/test/contrib/test_pyopenssl.py +++ b/test/contrib/test_pyopenssl.py @@ -30,6 +30,7 @@ def teardown_module(): pass +from ..test_ssl import TestSSL # noqa: E402, F401 from ..test_util import TestUtilSSL # noqa: E402, F401 from ..with_dummyserver.test_https import ( # noqa: E402, F401 TestHTTPS, @@ -46,7 +47,9 @@ def teardown_module(): TestClientCerts, TestSNI, TestSocketClosing, - TestSSL, +) +from ..with_dummyserver.test_socketlevel import ( # noqa: E402, F401 + TestSSL as TestSocketSSL, ) diff --git a/test/test_ssl.py b/test/test_ssl.py index a8b592544b..7cd9aae370 100644 --- a/test/test_ssl.py +++ b/test/test_ssl.py @@ -6,163 +6,184 @@ from urllib3.util import ssl_ -@pytest.mark.parametrize( - "addr", - [ - # IPv6 - "::1", - "::", - "FE80::8939:7684:D84b:a5A4%251", - # IPv4 - "127.0.0.1", - "8.8.8.8", - b"127.0.0.1", - # IPv6 w/ Zone IDs - "FE80::8939:7684:D84b:a5A4%251", - b"FE80::8939:7684:D84b:a5A4%251", - "FE80::8939:7684:D84b:a5A4%19", - b"FE80::8939:7684:D84b:a5A4%19", - ], -) -def test_is_ipaddress_true(addr): - assert ssl_.is_ipaddress(addr) - - -@pytest.mark.parametrize( - "addr", - [ - "www.python.org", - b"www.python.org", - "v2.sg.media-imdb.com", - b"v2.sg.media-imdb.com", - ], -) -def test_is_ipaddress_false(addr): - assert not ssl_.is_ipaddress(addr) - - -@pytest.mark.parametrize( - ["has_sni", "server_hostname", "uses_sni"], - [ - (True, "127.0.0.1", False), - (False, "www.python.org", False), - (False, "0.0.0.0", False), - (True, "www.google.com", True), - (True, None, False), - (False, None, False), - ], -) -def test_context_sni_with_ip_address(monkeypatch, has_sni, server_hostname, uses_sni): - monkeypatch.setattr(ssl_, "HAS_SNI", has_sni) - - sock = mock.Mock() - context = mock.create_autospec(ssl_.SSLContext) - - ssl_.ssl_wrap_socket(sock, server_hostname=server_hostname, ssl_context=context) - - if uses_sni: - context.wrap_socket.assert_called_with(sock, server_hostname=server_hostname) - else: - context.wrap_socket.assert_called_with(sock) - - -@pytest.mark.parametrize( - ["has_sni", "server_hostname", "should_warn"], - [ - (True, "www.google.com", False), - (True, "127.0.0.1", False), - (False, "127.0.0.1", False), - (False, "www.google.com", True), - (True, None, False), - (False, None, False), - ], -) -def test_sni_missing_warning_with_ip_addresses( - monkeypatch, has_sni, server_hostname, should_warn -): - monkeypatch.setattr(ssl_, "HAS_SNI", has_sni) - - sock = mock.Mock() - context = mock.create_autospec(ssl_.SSLContext) - - with mock.patch("warnings.warn") as warn: - ssl_.ssl_wrap_socket(sock, server_hostname=server_hostname, ssl_context=context) - - if should_warn: - assert warn.call_count >= 1 - warnings = [call[0][1] for call in warn.call_args_list] - assert SNIMissingWarning in warnings - else: - assert warn.call_count == 0 - - -@pytest.mark.parametrize( - ["ciphers", "expected_ciphers"], - [ - (None, ssl_.DEFAULT_CIPHERS), - ("ECDH+AESGCM:ECDH+CHACHA20", "ECDH+AESGCM:ECDH+CHACHA20"), - ], -) -def test_create_urllib3_context_set_ciphers(monkeypatch, ciphers, expected_ciphers): - - context = mock.create_autospec(ssl_.SSLContext) - context.set_ciphers = mock.Mock() - context.options = 0 - monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) - - assert ssl_.create_urllib3_context(ciphers=ciphers) is context - - assert context.set_ciphers.call_count == 1 - assert context.set_ciphers.call_args == mock.call(expected_ciphers) - - -def test_wrap_socket_given_context_no_load_default_certs(): - context = mock.create_autospec(ssl_.SSLContext) - context.load_default_certs = mock.Mock() - - sock = mock.Mock() - ssl_.ssl_wrap_socket(sock, ssl_context=context) - - context.load_default_certs.assert_not_called() - +class TestSSL: + @pytest.mark.parametrize( + "addr", + [ + # IPv6 + "::1", + "::", + "FE80::8939:7684:D84b:a5A4%251", + # IPv4 + "127.0.0.1", + "8.8.8.8", + b"127.0.0.1", + # IPv6 w/ Zone IDs + "FE80::8939:7684:D84b:a5A4%251", + b"FE80::8939:7684:D84b:a5A4%251", + "FE80::8939:7684:D84b:a5A4%19", + b"FE80::8939:7684:D84b:a5A4%19", + ], + ) + def test_is_ipaddress_true(self, addr): + assert ssl_.is_ipaddress(addr) + + @pytest.mark.parametrize( + "addr", + [ + "www.python.org", + b"www.python.org", + "v2.sg.media-imdb.com", + b"v2.sg.media-imdb.com", + ], + ) + def test_is_ipaddress_false(self, addr): + assert not ssl_.is_ipaddress(addr) + + @pytest.mark.parametrize( + ["has_sni", "server_hostname", "uses_sni"], + [ + (True, "127.0.0.1", False), + (False, "www.python.org", False), + (False, "0.0.0.0", False), + (True, "www.google.com", True), + (True, None, False), + (False, None, False), + ], + ) + def test_context_sni_with_ip_address( + self, monkeypatch, has_sni, server_hostname, uses_sni + ): + monkeypatch.setattr(ssl_, "HAS_SNI", has_sni) + + sock = mock.Mock() + context = mock.create_autospec(ssl_.SSLContext) -def test_wrap_socket_given_ca_certs_no_load_default_certs(monkeypatch): - context = mock.create_autospec(ssl_.SSLContext) - context.load_default_certs = mock.Mock() - context.options = 0 - - monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) - - sock = mock.Mock() - ssl_.ssl_wrap_socket(sock, ca_certs="/tmp/fake-file") - - context.load_default_certs.assert_not_called() - context.load_verify_locations.assert_called_with("/tmp/fake-file", None, None) - - -def test_wrap_socket_default_loads_default_certs(monkeypatch): - context = mock.create_autospec(ssl_.SSLContext) - context.load_default_certs = mock.Mock() - context.options = 0 - - monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) - - sock = mock.Mock() - ssl_.ssl_wrap_socket(sock) - - context.load_default_certs.assert_called_with() - - -@pytest.mark.parametrize( - ["pha", "expected_pha"], [(None, None), (False, True), (True, True)] -) -def test_create_urllib3_context_pha(monkeypatch, pha, expected_pha): - context = mock.create_autospec(ssl_.SSLContext) - context.set_ciphers = mock.Mock() - context.options = 0 - context.post_handshake_auth = pha - monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) - - assert ssl_.create_urllib3_context() is context + ssl_.ssl_wrap_socket(sock, server_hostname=server_hostname, ssl_context=context) - assert context.post_handshake_auth == expected_pha + if uses_sni: + context.wrap_socket.assert_called_with( + sock, server_hostname=server_hostname + ) + else: + context.wrap_socket.assert_called_with(sock) + + @pytest.mark.parametrize( + ["has_sni", "server_hostname", "should_warn"], + [ + (True, "www.google.com", False), + (True, "127.0.0.1", False), + (False, "127.0.0.1", False), + (False, "www.google.com", True), + (True, None, False), + (False, None, False), + ], + ) + def test_sni_missing_warning_with_ip_addresses( + self, monkeypatch, has_sni, server_hostname, should_warn + ): + monkeypatch.setattr(ssl_, "HAS_SNI", has_sni) + + sock = mock.Mock() + context = mock.create_autospec(ssl_.SSLContext) + + with mock.patch("warnings.warn") as warn: + ssl_.ssl_wrap_socket( + sock, server_hostname=server_hostname, ssl_context=context + ) + + if should_warn: + assert warn.call_count >= 1 + warnings = [call[0][1] for call in warn.call_args_list] + assert SNIMissingWarning in warnings + else: + assert warn.call_count == 0 + + @pytest.mark.parametrize( + ["ciphers", "expected_ciphers"], + [ + (None, ssl_.DEFAULT_CIPHERS), + ("ECDH+AESGCM:ECDH+CHACHA20", "ECDH+AESGCM:ECDH+CHACHA20"), + ], + ) + def test_create_urllib3_context_set_ciphers( + self, monkeypatch, ciphers, expected_ciphers + ): + + context = mock.create_autospec(ssl_.SSLContext) + context.set_ciphers = mock.Mock() + context.options = 0 + monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) + + assert ssl_.create_urllib3_context(ciphers=ciphers) is context + + if ciphers is None and ssl_.USE_SYSTEM_SSL_CIPHERS: + assert context.set_ciphers.call_count == 0 + else: + assert context.set_ciphers.call_count == 1 + assert context.set_ciphers.call_args == mock.call(expected_ciphers) + + def test_wrap_socket_given_context_no_load_default_certs(self): + context = mock.create_autospec(ssl_.SSLContext) + context.load_default_certs = mock.Mock() + + sock = mock.Mock() + ssl_.ssl_wrap_socket(sock, ssl_context=context) + + context.load_default_certs.assert_not_called() + + def test_wrap_socket_given_ca_certs_no_load_default_certs(self, monkeypatch): + context = mock.create_autospec(ssl_.SSLContext) + context.load_default_certs = mock.Mock() + context.options = 0 + + monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) + + sock = mock.Mock() + ssl_.ssl_wrap_socket(sock, ca_certs="/tmp/fake-file") + + context.load_default_certs.assert_not_called() + context.load_verify_locations.assert_called_with("/tmp/fake-file", None, None) + + def test_wrap_socket_default_loads_default_certs(self, monkeypatch): + context = mock.create_autospec(ssl_.SSLContext) + context.load_default_certs = mock.Mock() + context.options = 0 + + monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) + + sock = mock.Mock() + ssl_.ssl_wrap_socket(sock) + + context.load_default_certs.assert_called_with() + + @pytest.mark.parametrize( + ["pha", "expected_pha"], [(None, None), (False, True), (True, True)] + ) + def test_create_urllib3_context_pha(self, monkeypatch, pha, expected_pha): + context = mock.create_autospec(ssl_.SSLContext) + context.set_ciphers = mock.Mock() + context.options = 0 + context.post_handshake_auth = pha + monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) + + assert ssl_.create_urllib3_context() is context + + assert context.post_handshake_auth == expected_pha + + @pytest.mark.parametrize("use_system_ssl_ciphers", [True, False]) + def test_create_urllib3_context_default_ciphers( + self, monkeypatch, use_system_ssl_ciphers + ): + context = mock.create_autospec(ssl_.SSLContext) + context.set_ciphers = mock.Mock() + context.options = 0 + monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) + monkeypatch.setattr(ssl_, "USE_SYSTEM_SSL_CIPHERS", use_system_ssl_ciphers) + + ssl_.create_urllib3_context() + + if use_system_ssl_ciphers: + context.set_ciphers.assert_not_called() + else: + context.set_ciphers.assert_called_with(ssl_.DEFAULT_CIPHERS) From 59d2343e9f4b6c70fbcfda08a7dc4774f44e5182 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Fri, 11 Dec 2020 17:52:10 -0600 Subject: [PATCH 3/6] Don't set ciphers on SecureTransport --- .../contrib/_securetransport/bindings.py | 34 ------------------- src/urllib3/contrib/securetransport.py | 22 ++---------- 2 files changed, 2 insertions(+), 54 deletions(-) diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py index 6b07e8fc56..f8fe10eedd 100644 --- a/src/urllib3/contrib/_securetransport/bindings.py +++ b/src/urllib3/contrib/_securetransport/bindings.py @@ -482,37 +482,3 @@ class SecurityConst: errSecNoTrustSettings = -25263 errSecItemNotFound = -25300 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 - 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_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 - 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_RSA_WITH_AES_256_CBC_SHA = 0x0039 - 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_RSA_WITH_AES_128_CBC_SHA = 0x0033 - TLS_RSA_WITH_AES_256_GCM_SHA384 = 0x009D - TLS_RSA_WITH_AES_128_GCM_SHA256 = 0x009C - TLS_RSA_WITH_AES_256_CBC_SHA256 = 0x003D - TLS_RSA_WITH_AES_128_CBC_SHA256 = 0x003C - TLS_RSA_WITH_AES_256_CBC_SHA = 0x0035 - 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 9171419c53..bca625e8bb 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -186,7 +186,7 @@ def inject_into_urllib3(): util.ssl_.HAS_SNI = HAS_SNI util.IS_SECURETRANSPORT = True util.ssl_.IS_SECURETRANSPORT = True - util.ssl_.USE_SYSTEM_SSL_CIPHERS = False + util.ssl_.USE_SYSTEM_SSL_CIPHERS = True def extract_from_urllib3(): @@ -362,19 +362,6 @@ def _raise_on_error(self): self.close() raise exception - def _set_ciphers(self): - """ - Sets up the allowed ciphers. By default this matches the set in - util.ssl_.DEFAULT_CIPHERS, at least as supported by macOS. This is done - custom and doesn't allow changing at this time, mostly because parsing - OpenSSL cipher strings is going to be a freaking nightmare. - """ - ciphers = (Security.SSLCipherSuite * len(CIPHER_SUITES))(*CIPHER_SUITES) - result = Security.SSLSetEnabledCiphers( - self.context, ciphers, len(CIPHER_SUITES) - ) - _assert_no_error(result) - def _set_alpn_protocols(self, protocols): """ Sets up the ALPN protocols on the context. @@ -511,9 +498,6 @@ def handshake( ) _assert_no_error(result) - # Setup the ciphers. - self._set_ciphers() - # Setup the ALPN protocols. self._set_alpn_protocols(alpn_protocols) @@ -838,9 +822,7 @@ def load_default_certs(self): return self.set_default_verify_paths() def set_ciphers(self, ciphers): - # For now, we just require the default cipher string. - if ciphers != util.ssl_.DEFAULT_CIPHERS: - raise ValueError("SecureTransport doesn't support custom cipher strings") + raise ValueError("SecureTransport doesn't support custom cipher strings") def load_verify_locations(self, cafile=None, capath=None, cadata=None): # OK, we only really support cadata and cafile. From d5220b2028220fc09167f9c8b3d32722e4c3e64a Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Sat, 12 Dec 2020 15:48:03 -0600 Subject: [PATCH 4/6] Restore the gt OpenSSL 1.1.1 function --- src/urllib3/util/ssl_.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 851ef120e1..1150cb7a24 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -20,6 +20,19 @@ HASHFUNC_MAP = {32: md5, 40: sha1, 64: sha256} +def _is_ge_openssl_v1_1_1( + openssl_version_text: str, openssl_version_number: int +) -> bool: + """Returns True for OpenSSL 1.1.1+ (>=0x10101000) + LibreSSL reports a version number of 0x20000000 for + OpenSSL version number so we need to filter out LibreSSL. + """ + return ( + not openssl_version_text.startswith("LibreSSL") + and openssl_version_number >= 0x10101000 + ) + + try: # Do we have ssl at all? import ssl from ssl import ( From f5fcded67255834ea60122370aa05e08c56e1ae2 Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Sat, 12 Dec 2020 15:54:19 -0600 Subject: [PATCH 5/6] Remove cipher list from ST --- src/urllib3/contrib/securetransport.py | 33 -------------------------- 1 file changed, 33 deletions(-) diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index a0eb050cca..fe738178e1 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -107,39 +107,6 @@ # for no better reason than we need *a* limit, and this one is right there. SSL_WRITE_BLOCKSIZE = 16384 -# This is our equivalent of util.ssl_.DEFAULT_CIPHERS, but expanded out to -# individual cipher suites. We need to do this because this is how -# SecureTransport wants them. -CIPHER_SUITES = [ - # TLS 1.3 - SecurityConst.TLS_AES_256_GCM_SHA384, - SecurityConst.TLS_AES_128_GCM_SHA256, - SecurityConst.TLS_CHACHA20_POLY1305_SHA256, - SecurityConst.TLS_AES_128_CCM_8_SHA256, - SecurityConst.TLS_AES_128_CCM_SHA256, - # TLS 1.2 - SecurityConst.TLS_ECDHE_ECDSA_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_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384, - SecurityConst.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - SecurityConst.TLS_ECDHE_ECDSA_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, -] - # 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 to 1.2 are supported on macOS 10.8+ From 0d5030c906c136b2faca48816139d6f98e6a675a Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Thu, 31 Dec 2020 16:49:03 -0600 Subject: [PATCH 6/6] Rename parameter to be easier to understand --- src/urllib3/contrib/pyopenssl.py | 8 ++++---- src/urllib3/contrib/securetransport.py | 6 +++--- src/urllib3/util/ssl_.py | 9 ++++++--- test/test_ssl.py | 12 +++++++----- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index 3e3058339a..93611ef626 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -74,7 +74,7 @@ class UnsupportedExtension(Exception): HAS_SNI = True # Use system TLS ciphers on OpenSSL 1.1.1+ -USE_SYSTEM_SSL_CIPHERS = util.ssl_._is_ge_openssl_v1_1_1( +USE_DEFAULT_SSLCONTEXT_CIPHERS = util.ssl_._is_ge_openssl_v1_1_1( openssl_backend.openssl_version_text(), openssl_backend.openssl_version_number() ) @@ -107,7 +107,7 @@ class UnsupportedExtension(Exception): orig_util_HAS_SNI = util.HAS_SNI orig_util_SSLContext = util.ssl_.SSLContext -orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_SYSTEM_SSL_CIPHERS +orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS log = logging.getLogger(__name__) @@ -124,7 +124,7 @@ def inject_into_urllib3(): util.ssl_.HAS_SNI = HAS_SNI util.IS_PYOPENSSL = True util.ssl_.IS_PYOPENSSL = True - util.ssl_.USE_SYSTEM_SSL_CIPHERS = USE_SYSTEM_SSL_CIPHERS + util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS = USE_DEFAULT_SSLCONTEXT_CIPHERS def extract_from_urllib3(): @@ -136,7 +136,7 @@ def extract_from_urllib3(): util.ssl_.HAS_SNI = orig_util_HAS_SNI util.IS_PYOPENSSL = False util.ssl_.IS_PYOPENSSL = False - util.ssl_.USE_SYSTEM_SSL_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS + util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS def _validate_dependencies_met(): diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index fe738178e1..b7f8653637 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -82,7 +82,7 @@ orig_util_HAS_SNI = util.HAS_SNI orig_util_SSLContext = util.ssl_.SSLContext -orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_SYSTEM_SSL_CIPHERS +orig_util_USE_SYSTEM_SSL_CIPHERS = util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS # This dictionary is used by the read callback to obtain a handle to the # calling wrapped socket. This is a pretty silly approach, but for now it'll @@ -151,7 +151,7 @@ def inject_into_urllib3(): util.ssl_.HAS_SNI = HAS_SNI util.IS_SECURETRANSPORT = True util.ssl_.IS_SECURETRANSPORT = True - util.ssl_.USE_SYSTEM_SSL_CIPHERS = True + util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS = True def extract_from_urllib3(): @@ -164,7 +164,7 @@ def extract_from_urllib3(): util.ssl_.HAS_SNI = orig_util_HAS_SNI util.IS_SECURETRANSPORT = False util.ssl_.IS_SECURETRANSPORT = False - util.ssl_.USE_SYSTEM_SSL_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS + util.ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS = orig_util_USE_SYSTEM_SSL_CIPHERS def _read_callback(connection_id, data_buffer, data_length_pointer): diff --git a/src/urllib3/util/ssl_.py b/src/urllib3/util/ssl_.py index 1150cb7a24..c325c2c855 100644 --- a/src/urllib3/util/ssl_.py +++ b/src/urllib3/util/ssl_.py @@ -14,7 +14,7 @@ IS_PYOPENSSL = False IS_SECURETRANSPORT = False ALPN_PROTOCOLS = ["http/1.1"] -USE_SYSTEM_SSL_CIPHERS = False +USE_DEFAULT_SSLCONTEXT_CIPHERS = False # Maps the length of a digest to a possible hash function producing this digest HASHFUNC_MAP = {32: md5, 40: sha1, 64: sha256} @@ -48,7 +48,7 @@ def _is_ge_openssl_v1_1_1( SSLContext, ) - USE_SYSTEM_SSL_CIPHERS = _is_ge_openssl_v1_1_1( + USE_DEFAULT_SSLCONTEXT_CIPHERS = _is_ge_openssl_v1_1_1( OPENSSL_VERSION, OPENSSL_VERSION_NUMBER ) PROTOCOL_SSLv23 = PROTOCOL_TLS @@ -88,10 +88,13 @@ def _is_ge_openssl_v1_1_1( "DH+AESGCM", "ECDH+AES", "DH+AES", + "RSA+AESGCM", + "RSA+AES", "!aNULL", "!eNULL", "!MD5", "!DSS", + "!AESCCM", ] ) @@ -203,7 +206,7 @@ def create_urllib3_context( # Unless we're given ciphers defer to either system ciphers in # the case of OpenSSL 1.1.1+ or use our own secure default ciphers. - if ciphers is not None or not USE_SYSTEM_SSL_CIPHERS: + if ciphers is not None or not USE_DEFAULT_SSLCONTEXT_CIPHERS: context.set_ciphers(ciphers or DEFAULT_CIPHERS) # Setting the default here, as we may have no ssl module on import diff --git a/test/test_ssl.py b/test/test_ssl.py index 7cd9aae370..7a45abe943 100644 --- a/test/test_ssl.py +++ b/test/test_ssl.py @@ -117,7 +117,7 @@ def test_create_urllib3_context_set_ciphers( assert ssl_.create_urllib3_context(ciphers=ciphers) is context - if ciphers is None and ssl_.USE_SYSTEM_SSL_CIPHERS: + if ciphers is None and ssl_.USE_DEFAULT_SSLCONTEXT_CIPHERS: assert context.set_ciphers.call_count == 0 else: assert context.set_ciphers.call_count == 1 @@ -171,19 +171,21 @@ def test_create_urllib3_context_pha(self, monkeypatch, pha, expected_pha): assert context.post_handshake_auth == expected_pha - @pytest.mark.parametrize("use_system_ssl_ciphers", [True, False]) + @pytest.mark.parametrize("use_default_sslcontext_ciphers", [True, False]) def test_create_urllib3_context_default_ciphers( - self, monkeypatch, use_system_ssl_ciphers + self, monkeypatch, use_default_sslcontext_ciphers ): context = mock.create_autospec(ssl_.SSLContext) context.set_ciphers = mock.Mock() context.options = 0 monkeypatch.setattr(ssl_, "SSLContext", lambda *_, **__: context) - monkeypatch.setattr(ssl_, "USE_SYSTEM_SSL_CIPHERS", use_system_ssl_ciphers) + monkeypatch.setattr( + ssl_, "USE_DEFAULT_SSLCONTEXT_CIPHERS", use_default_sslcontext_ciphers + ) ssl_.create_urllib3_context() - if use_system_ssl_ciphers: + if use_default_sslcontext_ciphers: context.set_ciphers.assert_not_called() else: context.set_ciphers.assert_called_with(ssl_.DEFAULT_CIPHERS)