bpo-31432: Clarify ssl CERT_NONE/OPTIONAL/REQUIRED docs. (GH-3530)

The documentation for CERT_NONE, CERT_OPTIONAL, and CERT_REQUIRED were
misleading and partly wrong. It fails to explain that OpenSSL behaves
differently in client and server mode. Also OpenSSL does validate the
cert chain everytime. With SSL_VERIFY_NONE a validation error is not
fatal in client mode and does not request a client cert in server mode.
Also discourage people from using CERT_OPTIONAL in client mode.

4 files changed, 32 insertions, 16 deletions

diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
index 625c692..9b16a8b 100644
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -541,20 +541,28 @@ Constants
 .. data:: CERT_NONE
 
    Possible value for :attr:`SSLContext.verify_mode`, or the ``cert_reqs``
-   parameter to :func:`wrap_socket`.  In this mode (the default), no
-   certificates will be required from the other side of the socket connection.
-   If a certificate is received from the other end, no attempt to validate it
-   is made.
+   parameter to :func:`wrap_socket`.  Except for :const:`PROTOCOL_TLS_CLIENT`,
+   it is the default mode.  With client-side sockets, just about any
+   cert is accepted.  Validation errors, such as untrusted or expired cert,
+   are ignored and do not abort the TLS/SSL handshake.
+
+   In server mode, no certificate is requested from the client, so the client
+   does not send any for client cert authentication.
 
    See the discussion of :ref:`ssl-security` below.
 
 .. data:: CERT_OPTIONAL
 
    Possible value for :attr:`SSLContext.verify_mode`, or the ``cert_reqs``
-   parameter to :func:`wrap_socket`.  In this mode no certificates will be
-   required from the other side of the socket connection; but if they
-   are provided, validation will be attempted and an :class:`SSLError`
-   will be raised on failure.
+   parameter to :func:`wrap_socket`.  In client mode, :const:`CERT_OPTIONAL`
+   has the same meaning as :const:`CERT_REQUIRED`. It is recommended to
+   use :const:`CERT_REQUIRED` for client-side sockets instead.
+
+   In server mode, a client certificate request is sent to the client.  The
+   client may either ignore the request or send a certificate in order
+   perform TLS client cert authentication.  If the client chooses to send
+   a certificate, it is verified.  Any verification error immediately aborts
+   the TLS handshake.
 
    Use of this setting requires a valid set of CA certificates to
    be passed, either to :meth:`SSLContext.load_verify_locations` or as a
@@ -566,6 +574,15 @@ Constants
    parameter to :func:`wrap_socket`.  In this mode, certificates are
    required from the other side of the socket connection; an :class:`SSLError`
    will be raised if no certificate is provided, or if its validation fails.
+   This mode is **not** sufficient to verify a certificate in client mode as
+   it does not match hostnames.  :attr:`~SSLContext.check_hostname` must be
+   enabled as well to verify the authenticity of a cert.
+   :const:`PROTOCOL_TLS_CLIENT` uses :const:`CERT_REQUIRED` and
+   enables :attr:`~SSLContext.check_hostname` by default.
+
+   With server socket, this mode provides mandatory TLS client cert
+   authentication.  A client certificate request is sent to the client and
+   the client must provide a valid and trusted certificate.
 
    Use of this setting requires a valid set of CA certificates to
    be passed, either to :meth:`SSLContext.load_verify_locations` or as a
@@ -2537,11 +2554,6 @@ In server mode, if you want to authenticate your clients using the SSL layer
 (rather than using a higher-level authentication mechanism), you'll also have
 to specify :const:`CERT_REQUIRED` and similarly check the client certificate.
 
-   .. note::
-
-      In client mode, :const:`CERT_OPTIONAL` and :const:`CERT_REQUIRED` are
-      equivalent unless anonymous ciphers are enabled (they are disabled
-      by default).
 
 Protocol versions
 '''''''''''''''''
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 73d3e3b..3d6ae02 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -4074,7 +4074,9 @@ class ThreadedTests(unittest.TestCase):
                 self.assertTrue(session)
                 with self.assertRaises(TypeError) as e:
                     s.session = object
-                self.assertEqual(str(e.exception), 'Value is not a SSLSession.')
+                self.assertEqual(
+                    str(e.exception), 'Value is not an SSLSession.'
+                )
 
             with client_context.wrap_socket(socket.socket(),
                                             server_hostname=hostname) as s:
diff --git a/Misc/NEWS.d/next/Documentation/2017-09-13-07-14-59.bpo-31432.yAY4Z3.rst b/Misc/NEWS.d/next/Documentation/2017-09-13-07-14-59.bpo-31432.yAY4Z3.rst
new file mode 100644
index 0000000..18e5353
--- /dev/null
+++ b/Misc/NEWS.d/next/Documentation/2017-09-13-07-14-59.bpo-31432.yAY4Z3.rst
@@ -0,0 +1,2 @@
+Clarify meaning of CERT_NONE, CERT_OPTIONAL, and CERT_REQUIRED flags for
+ssl.SSLContext.verify_mode.
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index 2bce481..00d648d 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -2066,7 +2066,7 @@ static int PySSL_set_context(PySSLSocket *self, PyObject *value,
         SSL_set_SSL_CTX(self->ssl, self->ctx->ctx);
 #endif
     } else {
-        PyErr_SetString(PyExc_TypeError, "The value must be a SSLContext");
+        PyErr_SetString(PyExc_TypeError, "The value must be an SSLContext.");
         return -1;
     }
 
@@ -2725,7 +2725,7 @@ static int PySSL_set_session(PySSLSocket *self, PyObject *value,
     int result;
 
     if (!PySSLSession_Check(value)) {
-        PyErr_SetString(PyExc_TypeError, "Value is not a SSLSession.");
+        PyErr_SetString(PyExc_TypeError, "Value is not an SSLSession.");
         return -1;
     }
     pysess = (PySSLSession *)value;