gh-137197: Address review comments

ronf · ronf · commit 48e5164e8487 · 2025-08-09T09:16:49.000-07:00
diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
@@ -1685,28 +1685,29 @@ to speed up repeated connections from the same clients.
 .. method:: SSLContext.set_ciphers(ciphers)
 
    Set the allowed ciphers for sockets created with this context when
-   connecting using TLS 1.2 and earlier.  It should be a string in the `OpenSSL
-   cipher list format <https://docs.openssl.org/master/man1/ciphers/>`_.
+   connecting using TLS 1.2 and earlier.  The *ciphers* argument should
+   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`.
-   below.  If no cipher can be selected (because compile-time options or other
+   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.
 
    .. note::
       when connected, the :meth:`SSLSocket.cipher` method of SSL sockets will
-      return the negotiated cipher and associated TLS version.
+      return details about the negotiated cipher.
 
 .. method:: SSLContext.set_ciphersuites(ciphersuites)
 
    Set the allowed ciphers for sockets created with this context when
-   connecting using TLS 1.3.  It should be a colon-separate string of TLS 1.3
-   cipher names.  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.
+   connecting using TLS 1.3.  The *ciphersuites* argument should be a
+   colon-separate string of TLS 1.3 cipher names.  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.
 
    .. note::
       when connected, the :meth:`SSLSocket.cipher` method of SSL sockets will
-      return the negotiated cipher and associated TLS version.
+      return details about the negotiated cipher.
 
 .. method:: SSLContext.set_groups(groups)
 
@@ -2860,8 +2861,9 @@ of TLS/SSL. Some new TLS 1.3 features are not yet available.
   called instead of :meth:`SSLContext.set_ciphers`, which only affects
   ciphers in older TLS versions.  The method :meth:`SSLContext.get_ciphers`
   returns information about ciphers for both TLS 1.3 and earlier versions
-  and the method :meth:`SSLSocket.cipher` returns the negotiated cipher and
-  the associated TLS version once a connection is established.
+  and the method :meth:`SSLSocket.cipher` returns information about the
+  negotiated cipher for both TLS 1.3 and earlier versions once a connection
+  is established.
 - Session tickets are no longer sent as part of the initial handshake and
   are handled differently.  :attr:`SSLSocket.session` and :class:`SSLSession`
   are not compatible with TLS 1.3.
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
@@ -261,9 +261,11 @@ def utc_offset(): #NOTE: ignore issues like #1647654
 )
 
 
-def test_wrap_socket(sock, *, cert_reqs=ssl.CERT_NONE, ca_certs=None,
+def test_wrap_socket(sock, *,
+                     cert_reqs=ssl.CERT_NONE, ca_certs=None,
                      ciphers=None, ciphersuites=None, min_version=None,
-                     certfile=None, keyfile=None, **kwargs):
+                     certfile=None, keyfile=None,
+                     **kwargs):
     if not kwargs.get("server_side"):
         kwargs["server_hostname"] = SIGNED_CERTFILE_HOSTNAME
         context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
@@ -1866,6 +1868,10 @@ class SimpleBackgroundTests(unittest.TestCase):
 
     def setUp(self):
         self.server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
+
+        if has_tls_version('TLSv1_3'):
+            self.server_context.set_ciphersuites('TLS_AES_256_GCM_SHA384')
+
         self.server_context.load_cert_chain(SIGNED_CERTFILE)
         server = ThreadedEchoServer(context=self.server_context)
         self.enterContext(server)
@@ -2112,27 +2118,32 @@ def test_ciphers(self):
                                     cert_reqs=ssl.CERT_NONE, ciphers="^$:,;?*'dorothyx")
                 s.connect(self.server_addr)
 
+    @requires_tls_version('TLSv1_3')
     def test_ciphersuites(self):
-        with test_wrap_socket(socket.socket(socket.AF_INET),
-                              cert_reqs=ssl.CERT_NONE,
-                              min_version=ssl.TLSVersion.TLSv1_3) as s:
-            s.connect(self.server_addr)
-            self.assertEqual(s.cipher()[1], "TLSv1.3")
+        # Test successful TLS 1.3 handshake
         with test_wrap_socket(socket.socket(socket.AF_INET),
                               cert_reqs=ssl.CERT_NONE,
                               ciphersuites="TLS_AES_256_GCM_SHA384",
                               min_version=ssl.TLSVersion.TLSv1_3) as s:
             s.connect(self.server_addr)
             self.assertEqual(s.cipher(),
                              ("TLS_AES_256_GCM_SHA384", "TLSv1.3", 256))
-        # Error checking can happen at instantiation or when connecting
+
+        # Test mismatched TLS 1.3 cipher suites
+        with test_wrap_socket(socket.socket(socket.AF_INET),
+                              cert_reqs=ssl.CERT_NONE,
+                              ciphersuites="TLS_AES_128_GCM_SHA256",
+                              min_version=ssl.TLSVersion.TLSv1_3) as s:
+            with self.assertRaises(ssl.SSLError):
+                s.connect(self.server_addr)
+
+        # Test unrecognized TLS 1.3 cipher suite name
         with self.assertRaisesRegex(ssl.SSLError,
                                     "No cipher suite can be selected"):
             with socket.socket(socket.AF_INET) as sock:
                 s = test_wrap_socket(sock, cert_reqs=ssl.CERT_NONE,
                                      ciphersuites="XXX",
                                      min_version=ssl.TLSVersion.TLSv1_3)
-                s.connect(self.server_addr)
 
     def test_get_ca_certs_capath(self):
         # capath certs are loaded on request
