Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate negotiating TLSv1 and TLSv1.1 by default #2002

Merged
merged 3 commits into from Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/urllib3/connection.py
Expand Up @@ -398,6 +398,23 @@ def connect(self):
ssl_context=context,
)

# If we're using all defaults and the connection
# is TLSv1 or TLSv1.1 we throw a DeprecationWarning
# for the host.
if (
default_ssl_context
and self.ssl_version is None
and hasattr(self.sock, "version")
and self.sock.version() in {"TLSv1", "TLSv1.1"}
):
warnings.warn(
"Negotiating TLSv1/TLSv1.1 by default is deprecated "
"and will be disabled in urllib3 v2.0.0. Connecting to"
"'%s' with '%s' can be enabled by explicitly opting-in "
"with 'ssl_version'" % (self.host, self.sock.version()),
DeprecationWarning,
)

if self.assert_fingerprint:
assert_fingerprint(
self.sock.getpeercert(binary_form=True), self.assert_fingerprint
Expand Down
86 changes: 66 additions & 20 deletions test/with_dummyserver/test_https.py
Expand Up @@ -87,6 +87,9 @@
class TestHTTPS(HTTPSDummyServerTestCase):
tls_protocol_name = None

def tls_protocol_deprecated(self):
return self.tls_protocol_name in {"TLSv1", "TLSv1.1"}

@classmethod
def setup_class(cls):
super(TestHTTPS, cls).setup_class()
Expand Down Expand Up @@ -213,26 +216,25 @@ def test_verified(self):
conn = https_pool._new_conn()
assert conn.__class__ == VerifiedHTTPSConnection

with mock.patch("warnings.warn") as warn:
with warnings.catch_warnings(record=True) as w:
r = https_pool.request("GET", "/")
assert r.status == 200

# Modern versions of Python, or systems using PyOpenSSL, don't
# emit warnings.
if (
sys.version_info >= (2, 7, 9)
or util.IS_PYOPENSSL
or util.IS_SECURETRANSPORT
):
assert not warn.called, warn.call_args_list
else:
assert warn.called
if util.HAS_SNI:
call = warn.call_args_list[0]
else:
call = warn.call_args_list[1]
error = call[0][1]
assert error == InsecurePlatformWarning
# If we're using a deprecated TLS version we can remove 'DeprecationWarning'
if self.tls_protocol_deprecated():
w = [x for x in w if x.category != DeprecationWarning]

# Modern versions of Python, or systems using PyOpenSSL, don't
# emit warnings.
if (
sys.version_info >= (2, 7, 9)
or util.IS_PYOPENSSL
or util.IS_SECURETRANSPORT
):
assert w == []
else:
assert len(w) > 1
assert any(x.category == InsecureRequestWarning for x in w)

def test_verified_with_context(self):
ctx = util.ssl_.create_urllib3_context(cert_reqs=ssl.CERT_REQUIRED)
Expand Down Expand Up @@ -306,10 +308,15 @@ def test_ca_dir_verified(self, tmpdir):
conn = https_pool._new_conn()
assert conn.__class__ == VerifiedHTTPSConnection

with mock.patch("warnings.warn") as warn:
with warnings.catch_warnings(record=True) as w:
r = https_pool.request("GET", "/")
assert r.status == 200
assert not warn.called, warn.call_args_list

# If we're using a deprecated TLS version we can remove 'DeprecationWarning'
if self.tls_protocol_deprecated():
w = [x for x in w if x.category != DeprecationWarning]

assert w == []

def test_invalid_common_name(self):
with HTTPSConnectionPool(
Expand Down Expand Up @@ -391,6 +398,11 @@ def test_ssl_unverified_with_ca_certs(self):
# the unverified warning. Older systems may also emit other
# warnings, which we want to ignore here.
calls = warn.call_args_list

# If we're using a deprecated TLS version we can remove 'DeprecationWarning'
if self.tls_protocol_deprecated():
calls = [call for call in calls if call[0][1] != DeprecationWarning]

if (
sys.version_info >= (2, 7, 9)
or util.IS_PYOPENSSL
Expand Down Expand Up @@ -665,7 +677,13 @@ def _request_without_resource_warnings(self, method, url):
) as https_pool:
https_pool.request(method, url)

return [x for x in w if not isinstance(x.message, ResourceWarning)]
w = [x for x in w if not isinstance(x.message, ResourceWarning)]

# If we're using a deprecated TLS version we can remove 'DeprecationWarning'
if self.tls_protocol_deprecated():
w = [x for x in w if x.category != DeprecationWarning]

return w

def test_set_ssl_version_to_tls_version(self):
if self.tls_protocol_name is None:
Expand Down Expand Up @@ -699,6 +717,34 @@ def test_tls_protocol_name_of_socket(self):
finally:
conn.close()

def test_default_tls_version_deprecations(self):
if self.tls_protocol_name is None:
pytest.skip("Skipping base test class")

with HTTPSConnectionPool(
self.host, self.port, ca_certs=DEFAULT_CA
) as https_pool:
conn = https_pool._get_conn()
try:
with warnings.catch_warnings(record=True) as w:
conn.connect()
if not hasattr(conn.sock, "version"):
pytest.skip("SSLSocket.version() not available")
finally:
conn.close()

assert w is not None
if self.tls_protocol_deprecated():
assert len(w) == 1
assert str(w[0].message) == (
"Negotiating TLSv1/TLSv1.1 by default is deprecated "
"and will be disabled in urllib3 v2.0.0. Connecting to"
"'%s' with '%s' can be enabled by explicitly opting-in "
"with 'ssl_version'" % (self.host, self.tls_protocol_name)
)
else:
assert w == []

@pytest.mark.skipif(sys.version_info < (3, 8), reason="requires python 3.8+")
def test_sslkeylogfile(self, tmpdir, monkeypatch):
if not hasattr(util.SSLContext, "keylog_filename"):
Expand Down