gh-137197: Address review comments

Author: ronf

Doc/library/ssl.rst

@@ -1689,6 +1689,7 @@ to speed up repeated connections from the same clients.
    be a string in the `OpenSSL cipher list format
    <https://docs.openssl.org/master/man1/ciphers/>`_.
    To set allowed TLS 1.3 ciphers, use :meth:`SSLContext.set_ciphersuites`.
+
    If no cipher can be selected (because compile-time options or other
    configuration forbids use of all the specified ciphers), an
    :class:`SSLError` will be raised.
@@ -1709,6 +1710,8 @@ to speed up repeated connections from the same clients.
       When connected, the :meth:`SSLSocket.cipher` method of SSL sockets will
       return details about the negotiated cipher.
 
+   .. versionadded:: next
+
 .. method:: SSLContext.set_groups(groups)
 
    Set the groups allowed for key agreement for sockets created with this
@@ -2856,10 +2859,10 @@ The TLS 1.3 protocol behaves slightly differently than previous version
 of TLS/SSL. Some new TLS 1.3 features are not yet available.
 
 - TLS 1.3 uses a disjunct set of cipher suites.  All AES-GCM and ChaCha20
-  cipher suites are enabled by default.  To restrict which TLS1.3 ciphers
+  cipher suites are enabled by default.  To restrict which TLS 1.3 ciphers
   are allowed, the method :meth:`SSLContext.set_ciphersuites` should be
   called instead of :meth:`SSLContext.set_ciphers`, which only affects
-  ciphers in older TLS versions.  The method :meth:`SSLContext.get_ciphers`
+  ciphers in older TLS versions.  The :meth:`SSLContext.get_ciphers` method
   returns information about ciphers for both TLS 1.3 and earlier versions
   and the method :meth:`SSLSocket.cipher` returns information about the
   negotiated cipher for both TLS 1.3 and earlier versions once a connection

Lib/test/test_ssl.py

@@ -263,7 +263,8 @@ def utc_offset(): #NOTE: ignore issues like #1647654
 
 def test_wrap_socket(sock, *,
                      cert_reqs=ssl.CERT_NONE, ca_certs=None,
-                     ciphers=None, ciphersuites=None, min_version=None,
+                     ciphers=None, ciphersuites=None,
+                     min_version=None, max_version=None,
                      certfile=None, keyfile=None,
                      **kwargs):
     if not kwargs.get("server_side"):
@@ -285,6 +286,8 @@ def test_wrap_socket(sock, *,
         context.set_ciphersuites(ciphersuites)
     if min_version is not None:
         context.minimum_version = min_version
+    if max_version is not None:
+        context.maximum_version = max_version
     return context.wrap_socket(sock, **kwargs)
 
 
@@ -2245,7 +2248,7 @@ def test_transport_eof(self):
 
 @requires_tls_version('TLSv1_3')
 class SimpleBackgroundTestsTLS_1_3(unittest.TestCase):
-    """Tests that connect to a simple server running in the background"""
+    """Tests that connect to a simple server running in the background."""
 
     def setUp(self):
         ciphers = [cipher['name'] for cipher in ctx.get_ciphers()
@@ -2278,17 +2281,17 @@ def test_ciphersuites(self):
             s.connect(self.server_addr)
             self.assertEqual(s.cipher()[0], self.matching_cipher)
 
-        # Test mismatched TLS 1.3 cipher suites
-        if self.matching_client != self.mismatched_cipher:
-            with test_wrap_socket(socket.socket(socket.AF_INET),
-                                  cert_reqs=ssl.CERT_NONE,
-                                  ciphersuites=self.mismatched_cipher,
-                                  min_version=ssl.TLSVersion.TLSv1_3) as s:
-                with self.assertRaises(ssl.SSLError):
-                    s.connect(self.server_addr)
-        else:
+    def test_ciphersuite_mismatch(self):
+        if self.matching_cipher == self.mismatched_cipher:
             self.skipTest("Multiple TLS 1.3 ciphers are not available")
 
+        with test_wrap_socket(socket.socket(socket.AF_INET),
+                              cert_reqs=ssl.CERT_NONE,
+                              ciphersuites=self.mismatched_cipher,
+                              min_version=ssl.TLSVersion.TLSv1_3) as s:
+            with self.assertRaises(ssl.SSLError):
+                s.connect(self.server_addr)
+
 
 @support.requires_resource('network')
 class NetworkedTests(unittest.TestCase):

Modules/_ssl.c

@@ -3595,7 +3595,12 @@ _ssl__SSLContext_set_ciphers_impl(PySSLContext *self, const char *cipherlist)
 {
     int ret = SSL_CTX_set_cipher_list(self->ctx, cipherlist);
     if (ret == 0) {
-        _setSSLError(get_state_ctx(self), "No cipher can be selected.", 0, __FILE__, __LINE__);
+        /* Clearing the error queue is necessary on some OpenSSL versions,
+           otherwise the error will be reported again when another SSL call
+           is done. */
+        ERR_clear_error();
+        PyErr_SetString(get_state_ctx(self)->PySSLErrorObject,
+                        "No cipher can be selected.");
         return NULL;
     }
     Py_RETURN_NONE;