From e3e7d40514e5dd0c3847682a719577efcfae1d8f Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Sun, 23 Nov 2014 21:02:02 -0600 Subject: pep 476: verify certificates by default (#22417) --- Doc/library/httplib.rst | 8 +++++--- Doc/library/xmlrpclib.rst | 7 +++---- Doc/whatsnew/2.7.rst | 23 +++++++++++++++++++++++ Lib/httplib.py | 2 +- Lib/ssl.py | 11 +++++++++-- Lib/test/test_httplib.py | 3 +-- Lib/test/test_urllib2_localnet.py | 27 +++++++++++++++++++++++++-- Misc/NEWS | 2 ++ 8 files changed, 69 insertions(+), 14 deletions(-) diff --git a/Doc/library/httplib.rst b/Doc/library/httplib.rst index 23b0e64..b659fd0 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 720da39..3aa8be0 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 9b02687..65eaf17 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 48fbcb6..7c27906 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 c9f25c0..0f82227 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 60165c1..e68555a 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 8fc90af..a24a077 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 3ad4862..923bfb7 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. -- cgit v0.12