PEP 476: enable HTTPS certificate verification by default (#22417)

Patch by Alex Gaynor with some modifications by me.

11 files changed, 116 insertions, 69 deletions

diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst
index 9f6bcd1..35b9355 100644
--- a/Doc/library/http.client.rst
+++ b/Doc/library/http.client.rst
@@ -71,12 +71,6 @@ The module provides the following classes:
    :func:`ssl.create_default_context` select the system's trusted CA
    certificates for you.
 
-   The recommended way to connect to HTTPS hosts on the Internet is as
-   follows::
-
-      context = ssl.create_default_context()
-      h = client.HTTPSConnection('www.python.org', 443, context=context)
-
    Please read :ref:`ssl-security` for more information on best practices.
 
    .. note::
@@ -97,6 +91,12 @@ The module provides the following classes:
       The *strict* parameter was removed. HTTP 0.9-style "Simple Responses" are
       no longer supported.
 
+   .. versionchanged:: 3.4.3
+      This class now performs all the necessary certificate and hostname checks
+      by default. To revert to the previous, unverified, behavior
+      :func:`ssl._create_unverified_context` can be passed to the *context*
+      parameter.
+
 
 .. class:: HTTPResponse(sock, debuglevel=0, method=None, url=None)
 
diff --git a/Doc/library/urllib.request.rst b/Doc/library/urllib.request.rst
index bd7340b..ea9050f 100644
--- a/Doc/library/urllib.request.rst
+++ b/Doc/library/urllib.request.rst
@@ -62,11 +62,6 @@ The :mod:`urllib.request` module defines the following functions:
    *cafile* and *capath* parameters are omitted.  This will only work on
    some non-Windows platforms.
 
-   .. warning::
-      If neither *cafile* nor *capath* is specified, and *cadefault* is ``False``,
-      an HTTPS request will not do any verification of the server's
-      certificate.
-
    For http and https urls, this function returns a
    :class:`http.client.HTTPResponse` object which has the following
    :ref:`httpresponse-objects` methods.
diff --git a/Doc/library/xmlrpc.client.rst b/Doc/library/xmlrpc.client.rst
index d6360df..c06345e 100644
--- a/Doc/library/xmlrpc.client.rst
+++ b/Doc/library/xmlrpc.client.rst
@@ -27,11 +27,10 @@ between conformable Python objects and XML on the wire.
    constructed data.  If you need to parse untrusted or unauthenticated data see
    :ref:`xml-vulnerabilities`.
 
-.. warning::
-
-   In the case of https URIs, :mod:`xmlrpc.client` does not do any verification
-   of the server's certificate.
+.. versionchanged:: 3.4.3
 
+   For https URIs, :mod:`xmlrpc.client` now performs all the necessary
+   certificate and hostname checks by default
 
 .. class:: ServerProxy(uri, transport=None, encoding=None, verbose=False, \
                        allow_none=False, use_datetime=False, \
diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst
index 7129f54..bc3a6cc 100644
--- a/Doc/whatsnew/3.4.rst
+++ b/Doc/whatsnew/3.4.rst
@@ -2504,3 +2504,32 @@ Changes in the C API
 * The ``f_tstate`` (thread state) field of the :c:type:`PyFrameObject`
   structure has been removed to fix a bug: see :issue:`14432` for the
   rationale.
+
+Changed in 3.4.3
+================
+
+.. _pep-476:
+
+PEP 476: Enabling certificate verification by default for stdlib http clients
+-----------------------------------------------------------------------------
+
+:mod:`http.client` and modules which use it, such as :mod:`urllib.request` and
+:mod:`xmlrpc.client`, will now verify that the server presents a certificate
+which is signed by a CA in the platform trust store and whose hostname matches
+the hostname being requested by default, significantly improving security for
+many applications.
+
+For applications which require the old previous behavior, they can pass an
+alternate context::
+
+    import urllib.request
+    import ssl
+
+    # This disables all verification
+    context = ssl._create_unverified_context()
+
+    # This allows using a specific certificate for the host, which doesn't need
+    # to be in the trust store
+    context = ssl.create_default_context(cafile="/path/to/file.crt")
+
+    urllib.request.urlopen("https://invalid-cert", context=context)
diff --git a/Lib/http/client.py b/Lib/http/client.py
index d2013f2..b4cdec4 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -1203,7 +1203,7 @@ else:
             self.key_file = key_file
             self.cert_file = cert_file
             if context is None:
-                context = ssl._create_stdlib_context()
+                context = ssl._create_default_https_context()
             will_verify = context.verify_mode != ssl.CERT_NONE
             if check_hostname is None:
                 check_hostname = will_verify
diff --git a/Lib/ssl.py b/Lib/ssl.py
index e2636ef..b6e6f16 100644
--- a/Lib/ssl.py
+++ b/Lib/ssl.py
@@ -441,8 +441,7 @@ def create_default_context(purpose=Purpose.SERVER_AUTH, *, cafile=None,
         context.load_default_certs(purpose)
     return context
 
-
-def _create_stdlib_context(protocol=PROTOCOL_SSLv23, *, cert_reqs=None,
+def _create_unverified_context(protocol=PROTOCOL_SSLv23, *, cert_reqs=None,
                            check_hostname=False, purpose=Purpose.SERVER_AUTH,
                            certfile=None, keyfile=None,
                            cafile=None, capath=None, cadata=None):
@@ -480,6 +479,14 @@ def _create_stdlib_context(protocol=PROTOCOL_SSLv23, *, cert_reqs=None,
 
     return context
 
+# Used by http.client if no context is explicitly passed.
+_create_default_https_context = create_default_context
+
+
+# Backwards compatibility alias, even though it's not a public name.
+_create_stdlib_context = _create_unverified_context
+
+
 class SSLSocket(socket):
     """This class implements a subtype of socket.socket that wraps
     the underlying OS socket in an SSL context when necessary, and
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index 22f7329..2ded60e 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -778,13 +778,36 @@ class HTTPSTest(TestCase):
         self.assertIn('Apache', server_string)
 
     def test_networked(self):
-        # Default settings: no cert verification is done
+        # Default settings: requires a valid cert from a trusted CA
+        import ssl
         support.requires('network')
-        with support.transient_internet('svn.python.org'):
-            h = client.HTTPSConnection('svn.python.org', 443)
+        with support.transient_internet('self-signed.pythontest.net'):
+            h = client.HTTPSConnection('self-signed.pythontest.net', 443)
+            with self.assertRaises(ssl.SSLError) as exc_info:
+                h.request('GET', '/')
+            self.assertEqual(exc_info.exception.reason, 'CERTIFICATE_VERIFY_FAILED')
+
+    def test_networked_noverification(self):
+        # Switch off cert verification
+        import ssl
+        support.requires('network')
+        with support.transient_internet('self-signed.pythontest.net'):
+            context = ssl._create_unverified_context()
+            h = client.HTTPSConnection('self-signed.pythontest.net', 443,
+                                       context=context)
             h.request('GET', '/')
             resp = h.getresponse()
-            self._check_svn_python_org(resp)
+            self.assertIn('nginx', resp.getheader('server'))
+
+    def test_networked_trusted_by_default_cert(self):
+        # Default settings: requires a valid cert from a trusted CA
+        support.requires('network')
+        with support.transient_internet('www.python.org'):
+            h = client.HTTPSConnection('www.python.org', 443)
+            h.request('GET', '/')
+            resp = h.getresponse()
+            content_type = resp.getheader('content-type')
+            self.assertIn('text/html', content_type)
 
     def test_networked_good_cert(self):
         # We feed a CA cert that validates the server's cert
@@ -803,13 +826,23 @@ class HTTPSTest(TestCase):
         # We feed a "CA" cert that is unrelated to the server's cert
         import ssl
         support.requires('network')
-        with support.transient_internet('svn.python.org'):
+        with support.transient_internet('self-signed.pythontest.net'):
             context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
             context.verify_mode = ssl.CERT_REQUIRED
             context.load_verify_locations(CERT_localhost)
-            h = client.HTTPSConnection('svn.python.org', 443, context=context)
-            with self.assertRaises(ssl.SSLError):
+            h = client.HTTPSConnection('self-signed.pythontest.net', 443, context=context)
+            with self.assertRaises(ssl.SSLError) as exc_info:
                 h.request('GET', '/')
+            self.assertEqual(exc_info.exception.reason, 'CERTIFICATE_VERIFY_FAILED')
+
+    def test_local_unknown_cert(self):
+        # The custom cert isn't known to the default trust bundle
+        import ssl
+        server = self.make_server(CERT_localhost)
+        h = client.HTTPSConnection('localhost', server.port)
+        with self.assertRaises(ssl.SSLError) as exc_info:
+            h.request('GET', '/')
+        self.assertEqual(exc_info.exception.reason, 'CERTIFICATE_VERIFY_FAILED')
 
     def test_local_good_hostname(self):
         # The (valid) cert validates the HTTP hostname
@@ -822,7 +855,6 @@ class HTTPSTest(TestCase):
         h.request('GET', '/nonexistent')
         resp = h.getresponse()
         self.assertEqual(resp.status, 404)
-        del server
 
     def test_local_bad_hostname(self):
         # The (valid) cert doesn't validate the HTTP hostname
@@ -845,7 +877,6 @@ class HTTPSTest(TestCase):
         h.request('GET', '/nonexistent')
         resp = h.getresponse()
         self.assertEqual(resp.status, 404)
-        del server
 
     @unittest.skipIf(not hasattr(client, 'HTTPSConnection'),
                      'http.client.HTTPSConnection not available')
diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py
index d5aec9a..ba790d1 100644
--- a/Lib/test/test_logging.py
+++ b/Lib/test/test_logging.py
@@ -1631,36 +1631,6 @@ class UnixSysLogHandlerTest(SysLogHandlerTest):
 class HTTPHandlerTest(BaseTest):
     """Test for HTTPHandler."""
 
-    PEMFILE = """-----BEGIN RSA PRIVATE KEY-----
-MIICXQIBAAKBgQDGT4xS5r91rbLJQK2nUDenBhBG6qFk+bVOjuAGC/LSHlAoBnvG
-zQG3agOG+e7c5z2XT8m2ktORLqG3E4mYmbxgyhDrzP6ei2Anc+pszmnxPoK3Puh5
-aXV+XKt0bU0C1m2+ACmGGJ0t3P408art82nOxBw8ZHgIg9Dtp6xIUCyOqwIDAQAB
-AoGBAJFTnFboaKh5eUrIzjmNrKsG44jEyy+vWvHN/FgSC4l103HxhmWiuL5Lv3f7
-0tMp1tX7D6xvHwIG9VWvyKb/Cq9rJsDibmDVIOslnOWeQhG+XwJyitR0pq/KlJIB
-5LjORcBw795oKWOAi6RcOb1ON59tysEFYhAGQO9k6VL621gRAkEA/Gb+YXULLpbs
-piXN3q4zcHzeaVANo69tUZ6TjaQqMeTxE4tOYM0G0ZoSeHEdaP59AOZGKXXNGSQy
-2z/MddcYGQJBAMkjLSYIpOLJY11ja8OwwswFG2hEzHe0cS9bzo++R/jc1bHA5R0Y
-i6vA5iPi+wopPFvpytdBol7UuEBe5xZrxWMCQQCWxELRHiP2yWpEeLJ3gGDzoXMN
-PydWjhRju7Bx3AzkTtf+D6lawz1+eGTuEss5i0JKBkMEwvwnN2s1ce+EuF4JAkBb
-E96h1lAzkVW5OAfYOPY8RCPA90ZO/hoyg7PpSxR0ECuDrgERR8gXIeYUYfejBkEa
-rab4CfRoVJKKM28Yq/xZAkBvuq670JRCwOgfUTdww7WpdOQBYPkzQccsKNCslQW8
-/DyW6y06oQusSENUvynT6dr3LJxt/NgZPhZX2+k1eYDV
------END RSA PRIVATE KEY-----
------BEGIN CERTIFICATE-----
-MIICGzCCAYSgAwIBAgIJAIq84a2Q/OvlMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNV
-BAMTCWxvY2FsaG9zdDAeFw0xMTA1MjExMDIzMzNaFw03NTAzMjEwMzU1MTdaMBQx
-EjAQBgNVBAMTCWxvY2FsaG9zdDCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA
-xk+MUua/da2yyUCtp1A3pwYQRuqhZPm1To7gBgvy0h5QKAZ7xs0Bt2oDhvnu3Oc9
-l0/JtpLTkS6htxOJmJm8YMoQ68z+notgJ3PqbM5p8T6Ctz7oeWl1flyrdG1NAtZt
-vgAphhidLdz+NPGq7fNpzsQcPGR4CIPQ7aesSFAsjqsCAwEAAaN1MHMwHQYDVR0O
-BBYEFLWaUPO6N7efGiuoS9i3DVYcUwn0MEQGA1UdIwQ9MDuAFLWaUPO6N7efGiuo
-S9i3DVYcUwn0oRikFjAUMRIwEAYDVQQDEwlsb2NhbGhvc3SCCQCKvOGtkPzr5TAM
-BgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4GBAMK5whPjLNQK1Ivvk88oqJqq
-4f889OwikGP0eUhOBhbFlsZs+jq5YZC2UzHz+evzKBlgAP1u4lP/cB85CnjvWqM+
-1c/lywFHQ6HOdDeQ1L72tSYMrNOG4XNmLn0h7rx6GoTU7dcFRfseahBCq8mv0IDt
-IRbTpvlHWPjsSvHz0ZOH
------END CERTIFICATE-----"""
-
     def setUp(self):
         """Set up an HTTP server to receive log messages, and a HTTPHandler
         pointing to that server's address and port."""
@@ -1690,15 +1660,26 @@ IRbTpvlHWPjsSvHz0ZOH
             if secure:
                 try:
                     import ssl
-                    fd, fn = tempfile.mkstemp()
-                    os.close(fd)
-                    with open(fn, 'w') as f:
-                        f.write(self.PEMFILE)
-                    sslctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
-                    sslctx.load_cert_chain(fn)
-                    os.unlink(fn)
                 except ImportError:
                     sslctx = None
+                else:
+                    here = os.path.dirname(__file__)
+                    localhost_cert = os.path.join(here, "keycert.pem")
+                    sslctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
+                    sslctx.load_cert_chain(localhost_cert)
+                    # Unfortunately, HTTPHandler doesn't allow us to change the
+                    # SSLContext used by HTTPSConnection, so we have to
+                    # monkeypatch. This can be cleaned up if issue 22788 is
+                    # fixed.
+                    old = ssl._create_default_https_context
+                    def restore_handler():
+                        ssl._create_default_https_context = old
+                    self.addCleanup(restore_handler)
+                    def hack_create_ctx():
+                        ctx = old()
+                        ctx.load_verify_locations(localhost_cert)
+                        return ctx
+                    ssl._create_default_https_context = hack_create_ctx
             else:
                 sslctx = None
             self.server = server = TestHTTPServer(addr, self.handle_request,
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index e71a400..f95d3e5 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -2337,9 +2337,10 @@ else:
                 d1 = f.read()
             d2 = ''
             # now fetch the same data from the HTTPS server
-            url = 'https://%s:%d/%s' % (
-                HOST, server.port, os.path.split(CERTFILE)[1])
-            f = urllib.request.urlopen(url)
+            url = 'https://localhost:%d/%s' % (
+                server.port, os.path.split(CERTFILE)[1])
+            context = ssl.create_default_context(cafile=CERTFILE)
+            f = urllib.request.urlopen(url, context=context)
             try:
                 dlen = f.info().get("content-length")
                 if dlen and (int(dlen) > 0):
diff --git a/Lib/test/test_urllib2_localnet.py b/Lib/test/test_urllib2_localnet.py
index d0a8a0f..e17b225 100644
--- a/Lib/test/test_urllib2_localnet.py
+++ b/Lib/test/test_urllib2_localnet.py
@@ -545,7 +545,8 @@ class TestUrlopen(unittest.TestCase):
 
     def test_https(self):
         handler = self.start_https_server()
-        data = self.urlopen("https://localhost:%s/bizarre" % handler.port)
+        context = ssl.create_default_context(cafile=CERT_localhost)
+        data = self.urlopen("https://localhost:%s/bizarre" % handler.port, context=context)
         self.assertEqual(data, b"we care a bit")
 
     def test_https_with_cafile(self):
@@ -584,7 +585,8 @@ class TestUrlopen(unittest.TestCase):
         context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
         context.set_servername_callback(cb_sni)
         handler = self.start_https_server(context=context, certfile=CERT_localhost)
-        self.urlopen("https://localhost:%s" % handler.port)
+        context = ssl.create_default_context(cafile=CERT_localhost)
+        self.urlopen("https://localhost:%s" % handler.port, context=context)
         self.assertEqual(sni_name, "localhost")
 
     def test_sending_headers(self):
diff --git a/Misc/NEWS b/Misc/NEWS
index ee1c53f..d31dbe9 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -36,6 +36,8 @@ Core and Builtins
 Library
 -------
 
+- Issue #22417: Verify certificates by default in httplib (PEP 476).
+
 - Issue #22775: Fixed unpickling of http.cookies.SimpleCookie with protocol 2
   and above.  Patch by Tim Graham.