pep 476: verify certificates by default (#22417)

8 files changed, 69 insertions, 14 deletions

diff --git a/Doc/library/httplib.rst b/Doc/library/httplib.rst
index 23b0e64290..b659fd0970 100644
--- a/Doc/library/httplib.rst
+++ b/Doc/library/httplib.rst
@@ -90,9 +90,6 @@ The module provides the following classes:
       server's certificate.  If you want to change that behaviour, you can
       explicitly set *check_hostname* to False.
 
-   .. warning::
-      This does not do any verification of the server's certificate.
-
    .. versionadded:: 2.0
 
    .. versionchanged:: 2.6
@@ -104,6 +101,11 @@ The module provides the following classes:
    .. versionchanged:: 2.7.9
       *context* and *check_hostname* was added.
 
+      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, strict=0)
 
diff --git a/Doc/library/xmlrpclib.rst b/Doc/library/xmlrpclib.rst
index 720da39d55..3aa8be0708 100644
--- a/Doc/library/xmlrpclib.rst
+++ b/Doc/library/xmlrpclib.rst
@@ -34,11 +34,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:`xmlrpclib` does not do any verification of
-   the server's certificate.
+.. versionchanged:: 2.7.9
 
+   For https URIs, :mod:`xmlrpclib` now performs all the necessary certificate
+   and hostname checks by default
 
 .. class:: ServerProxy(uri[, transport[, encoding[, verbose[,  allow_none[, use_datetime]]]]])
 
diff --git a/Doc/whatsnew/2.7.rst b/Doc/whatsnew/2.7.rst
index 9b02687378..65eaf17bcc 100644
--- a/Doc/whatsnew/2.7.rst
+++ b/Doc/whatsnew/2.7.rst
@@ -2646,6 +2646,29 @@ and :ref:`distutils-index`.
       PEP written by Donald Stufft and Nick Coghlan, implemented by
       Donald Stufft, Nick Coghlan, Martin von Löwis and Ned Deily.
 
+PEP 476: Enabling certificate verification by default for stdlib http clients
+-----------------------------------------------------------------------------
+
+:mod:`httplib` and modules which use it, such as :mod:`urllib2` and
+:mod:`xmlrpclib`, 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 urllib2
+    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")
+
+    urllib2.urlopen("https://invalid-cert", context=context)
 
 .. ======================================================================
 
diff --git a/Lib/httplib.py b/Lib/httplib.py
index 48fbcb61b2..7c27906858 100644
--- a/Lib/httplib.py
+++ b/Lib/httplib.py
@@ -1193,7 +1193,7 @@ else:
             self.key_file = key_file
             self.cert_file = cert_file
             if context is None:
-                context = ssl.create_default_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 c9f25c0bf0..0f822277e9 100644
--- a/Lib/ssl.py
+++ b/Lib/ssl.py
@@ -427,8 +427,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):
@@ -469,6 +468,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 60165c1029..e68555a169 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -1,10 +1,9 @@
 import httplib
 import array
-import httplib
-import os
 import StringIO
 import socket
 import errno
+import os
 
 import unittest
 TestCase = unittest.TestCase
diff --git a/Lib/test/test_urllib2_localnet.py b/Lib/test/test_urllib2_localnet.py
index 8fc90af18d..a24a077b2c 100644
--- a/Lib/test/test_urllib2_localnet.py
+++ b/Lib/test/test_urllib2_localnet.py
@@ -5,6 +5,7 @@ import urllib2
 import BaseHTTPServer
 import unittest
 import hashlib
+import ssl
 
 from test import test_support
 
@@ -562,15 +563,37 @@ class TestUrlopen(BaseTestCase):
                             cafile=CERT_localhost)
         self.assertEqual(data, b"we care a bit")
         # Bad cert
-        with self.assertRaises(urllib2.URLError) as cm:
+        with self.assertRaises(urllib2.URLError):
             self.urlopen("https://localhost:%s/bizarre" % handler.port,
                          cafile=CERT_fakehostname)
         # Good cert, but mismatching hostname
         handler = self.start_https_server(certfile=CERT_fakehostname)
-        with self.assertRaises(ssl.CertificateError) as cm:
+        with self.assertRaises(ssl.CertificateError):
             self.urlopen("https://localhost:%s/bizarre" % handler.port,
                          cafile=CERT_fakehostname)
 
+    def test_https_with_cadefault(self):
+        handler = self.start_https_server(certfile=CERT_localhost)
+        # Self-signed cert should fail verification with system certificate store
+        with self.assertRaises(urllib2.URLError):
+            self.urlopen("https://localhost:%s/bizarre" % handler.port,
+                         cadefault=True)
+
+    def test_https_sni(self):
+        if ssl is None:
+            self.skipTest("ssl module required")
+        if not ssl.HAS_SNI:
+            self.skipTest("SNI support required in OpenSSL")
+        sni_name = [None]
+        def cb_sni(ssl_sock, server_name, initial_context):
+            sni_name[0] = server_name
+        context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
+        context.set_servername_callback(cb_sni)
+        handler = self.start_https_server(context=context, certfile=CERT_localhost)
+        context = ssl.create_default_context(cafile=CERT_localhost)
+        self.urlopen("https://localhost:%s" % handler.port, context=context)
+        self.assertEqual(sni_name[0], "localhost")
+
     def test_sending_headers(self):
         handler = self.start_server([(200, [], "we don't care")])
 
diff --git a/Misc/NEWS b/Misc/NEWS
index 3ad48623f9..923bfb7861 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -42,6 +42,8 @@ Core and Builtins
 Library
 -------
 
+- Issue #22417: Verify certificates by default in httplib (PEP 476).
+
 - Issue #22927: Allow urllib.urlopen to take a *context* parameter to control
   SSL settings for HTTPS connections.