From a090f01bb63d73382e6e65b0364047c50afae5c2 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Sun, 7 Dec 2014 13:18:25 -0500 Subject: HTTPSConnection: prefer the context's check_hostname attribute over the constructor parameter (#22959) --- Doc/library/http.client.rst | 11 +++-------- Lib/http/client.py | 4 ++-- Lib/test/test_httplib.py | 14 ++++++++++++++ Misc/NEWS | 3 +++ 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index 35b9355..94f7b13 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -69,17 +69,12 @@ The module provides the following classes: *key_file* and *cert_file* are deprecated, please use :meth:`ssl.SSLContext.load_cert_chain` instead, or let :func:`ssl.create_default_context` select the system's trusted CA - certificates for you. + certificates for you. The *check_hostname* parameter is also deprecated; the + :attr:`SSLContext.check_hostname` attribute of *context* should be used + instead. Please read :ref:`ssl-security` for more information on best practices. - .. note:: - If *context* is specified and has a :attr:`~ssl.SSLContext.verify_mode` - of either :data:`~ssl.CERT_OPTIONAL` or :data:`~ssl.CERT_REQUIRED`, then - by default *host* is matched against the host name(s) allowed by the - server's certificate. If you want to change that behaviour, you can - explicitly set *check_hostname* to False. - .. versionchanged:: 3.2 *source_address*, *context* and *check_hostname* were added. diff --git a/Lib/http/client.py b/Lib/http/client.py index 281e7f2..c0760dd 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -1210,8 +1210,8 @@ else: context = ssl._create_default_https_context() will_verify = context.verify_mode != ssl.CERT_NONE if check_hostname is None: - check_hostname = will_verify - elif check_hostname and not will_verify: + check_hostname = context.check_hostname + if check_hostname and not will_verify: raise ValueError("check_hostname needs a SSL context with " "either CERT_OPTIONAL or CERT_REQUIRED") if key_file or cert_file: diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 933e5c4..49d767d 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -882,6 +882,7 @@ class HTTPSTest(TestCase): server = self.make_server(CERT_fakehostname) context = ssl.SSLContext(ssl.PROTOCOL_TLSv1) context.verify_mode = ssl.CERT_REQUIRED + context.check_hostname = True context.load_verify_locations(CERT_fakehostname) h = client.HTTPSConnection('localhost', server.port, context=context) with self.assertRaises(ssl.CertificateError): @@ -892,11 +893,24 @@ class HTTPSTest(TestCase): with self.assertRaises(ssl.CertificateError): h.request('GET', '/') # With check_hostname=False, the mismatching is ignored + context.check_hostname = False h = client.HTTPSConnection('localhost', server.port, context=context, check_hostname=False) h.request('GET', '/nonexistent') resp = h.getresponse() self.assertEqual(resp.status, 404) + # The context's check_hostname setting is used if one isn't passed to + # HTTPSConnection. + context.check_hostname = False + h = client.HTTPSConnection('localhost', server.port, context=context) + h.request('GET', '/nonexistent') + self.assertEqual(h.getresponse().status, 404) + # Passing check_hostname to HTTPSConnection should override the + # context's setting. + h = client.HTTPSConnection('localhost', server.port, context=context, + check_hostname=True) + with self.assertRaises(ssl.CertificateError): + h.request('GET', '/') @unittest.skipIf(not hasattr(client, 'HTTPSConnection'), 'http.client.HTTPSConnection not available') diff --git a/Misc/NEWS b/Misc/NEWS index d5bb074..732236d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -39,6 +39,9 @@ Core and Builtins Library ------- +- Issue #22959: In the constructor of http.client.HTTPSConnection, prefer the + context's check_hostname attribute over the *check_hostname* parameter. + - Issue #16043: Add a default limit for the amount of data xmlrpclib.gzip_decode will return. This resolves CVE-2013-1753. -- cgit v0.12