From: Christian Heimes <christian@python.org>
Date: Fri, 15 Sep 2017 18:29:57 +0000 (+0200)
Subject: bpo-31431: SSLContext.check_hostname auto-sets CERT_REQUIRED (#3531)
X-Git-Tag: v3.7.0a1~19
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e82c034496512139e9ea3f68ceda86c04bc7baab;p=python

bpo-31431: SSLContext.check_hostname auto-sets CERT_REQUIRED (#3531)

Signed-off-by: Christian Heimes <christian@python.org>
---

diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
index eb4d8ace3d..1f3e8d5f78 100644
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -1674,7 +1674,10 @@ to speed up repeated connections from the same clients.
    :meth:`SSLSocket.do_handshake`. The context's
    :attr:`~SSLContext.verify_mode` must be set to :data:`CERT_OPTIONAL` or
    :data:`CERT_REQUIRED`, and you must pass *server_hostname* to
-   :meth:`~SSLContext.wrap_socket` in order to match the hostname.
+   :meth:`~SSLContext.wrap_socket` in order to match the hostname.  Enabling
+   hostname checking automatically sets :attr:`~SSLContext.verify_mode` from
+   :data:`CERT_NONE` to :data:`CERT_REQUIRED`.  It cannot be set back to
+   :data:`CERT_NONE` as long as hostname checking is enabled.
 
    Example::
 
@@ -1691,6 +1694,13 @@ to speed up repeated connections from the same clients.
 
    .. versionadded:: 3.4
 
+   .. versionchanged:: 3.7
+
+      :attr:`~SSLContext.verify_mode` is now automatically changed
+      to :data:`CERT_REQUIRED`  when hostname checking is enabled and
+      :attr:`~SSLContext.verify_mode` is :data:`CERT_NONE`. Previously
+      the same operation would have failed with a :exc:`ValueError`.
+
    .. note::
 
      This features requires OpenSSL 0.9.8f or newer.
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 2978b8b3eb..aa2429ac98 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -1363,24 +1363,45 @@ class ContextTests(unittest.TestCase):
     def test_check_hostname(self):
         ctx = ssl.SSLContext(ssl.PROTOCOL_TLS)
         self.assertFalse(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_NONE)
 
-        # Requires CERT_REQUIRED or CERT_OPTIONAL
-        with self.assertRaises(ValueError):
-            ctx.check_hostname = True
+        # Auto set CERT_REQUIRED
+        ctx.check_hostname = True
+        self.assertTrue(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_REQUIRED)
+        ctx.check_hostname = False
         ctx.verify_mode = ssl.CERT_REQUIRED
         self.assertFalse(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_REQUIRED)
+
+        # Changing verify_mode does not affect check_hostname
+        ctx.check_hostname = False
+        ctx.verify_mode = ssl.CERT_NONE
+        ctx.check_hostname = False
+        self.assertFalse(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_NONE)
+        # Auto set
         ctx.check_hostname = True
         self.assertTrue(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_REQUIRED)
 
+        ctx.check_hostname = False
         ctx.verify_mode = ssl.CERT_OPTIONAL
+        ctx.check_hostname = False
+        self.assertFalse(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_OPTIONAL)
+        # keep CERT_OPTIONAL
         ctx.check_hostname = True
         self.assertTrue(ctx.check_hostname)
+        self.assertEqual(ctx.verify_mode, ssl.CERT_OPTIONAL)
 
         # Cannot set CERT_NONE with check_hostname enabled
         with self.assertRaises(ValueError):
             ctx.verify_mode = ssl.CERT_NONE
         ctx.check_hostname = False
         self.assertFalse(ctx.check_hostname)
+        ctx.verify_mode = ssl.CERT_NONE
+        self.assertEqual(ctx.verify_mode, ssl.CERT_NONE)
 
     def test_context_client_server(self):
         # PROTOCOL_TLS_CLIENT has sane defaults
diff --git a/Misc/NEWS.d/next/Library/2017-09-13-07-37-20.bpo-31431.dj994R.rst b/Misc/NEWS.d/next/Library/2017-09-13-07-37-20.bpo-31431.dj994R.rst
new file mode 100644
index 0000000000..6083fd65bd
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-09-13-07-37-20.bpo-31431.dj994R.rst
@@ -0,0 +1,2 @@
+SSLContext.check_hostname now automatically sets SSLContext.verify_mode to
+ssl.CERT_REQUIRED instead of failing with a ValueError.
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index 8aaaa3212c..73abad3dcf 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -3227,10 +3227,10 @@ set_check_hostname(PySSLContext *self, PyObject *arg, void *c)
         return -1;
     if (check_hostname &&
             SSL_CTX_get_verify_mode(self->ctx) == SSL_VERIFY_NONE) {
-        PyErr_SetString(PyExc_ValueError,
-                        "check_hostname needs a SSL context with either "
-                        "CERT_OPTIONAL or CERT_REQUIRED");
-        return -1;
+        /* check_hostname = True sets verify_mode = CERT_REQUIRED */
+        if (_set_verify_mode(self->ctx, PY_SSL_CERT_REQUIRED) == -1) {
+            return -1;
+        }
     }
     self->check_hostname = check_hostname;
     return 0;