Address review comments

ronf · ronf · commit bc84599dd55a · 2025-09-02T20:36:13.000-07:00
diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
@@ -1300,16 +1300,16 @@ SSL sockets also have the following additional methods and attributes:
 .. method:: SSLSocket.client_sigalg()
 
    Return the signature algorithm used for performing certificate-based client
-   authentication on this connection. If no connection has been established
-   or client authentication didn't occur, this method returns ``None``.
+   authentication on this connection, or ``None`` if no connection has been
+   established or client authentication didn't occur.
 
    .. versionadded:: next
 
 .. method:: SSLSocket.server_sigalg()
 
    Return the signature algorithm used by the server to complete the TLS
-   handshake on this connection. If no connection has been established
-   or the cipher suite has no signature, this method returns ``None``.
+   handshake on this connection, or ``None`` if no connection has been
+   established or the cipher suite has no signature.
 
    .. versionadded:: next
 
diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst
@@ -433,7 +433,7 @@ ssl
   connection is made.
   (Contributed by Ron Frederick in :gh:`137197`.)
 
-* Added new methods for managing signature algorithms
+* Added new methods for managing signature algorithms:
 
   * :meth:`ssl.SSLContext.set_client_sigalgs` sets the signature algorithms
     allowed for certificate-based client authentication.
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
@@ -999,7 +999,7 @@ def test_get_groups(self):
         self.assertIn('P-256', ctx.get_groups(include_aliases=True))
 
     @unittest.skipUnless(CAN_SET_CLIENT_SIGALGS,
-                         "AWS-LC doesn't support setting client sigalgs")
+                         "SSL library doesn't support setting client sigalgs")
     def test_set_client_sigalgs(self):
         ctx = ssl.create_default_context()
 
@@ -4311,7 +4311,7 @@ def test_groups(self):
                                sni_name=hostname)
 
     @unittest.skipUnless(CAN_SET_CLIENT_SIGALGS,
-                         "AWS-LC doesn't support setting client sigalgs")
+                         "SSL library doesn't support setting client sigalgs")
     def test_client_sigalgs(self):
         # no mutual auth, so cient_sigalg should be None
         client_context, server_context, hostname = testing_context()
@@ -4322,16 +4322,19 @@ def test_client_sigalgs(self):
             self.assertIsNone(stats['client_sigalg'])
 
         # server auto, client rsa_pss_rsae_sha384
+        sigalg = "rsa_pss_rsae_sha384"
         client_context, server_context, hostname = \
             testing_context(client_cert=SIGNED_CERTFILE)
-        client_context.set_client_sigalgs("rsa_pss_rsae_sha384")
+        client_context.set_client_sigalgs(sigalg)
         stats = server_params_test(client_context, server_context,
                                    chatty=True, connectionchatty=True,
                                    sni_name=hostname)
         if CAN_GET_SELECTED_OPENSSL_SIGALG:
-            self.assertEqual(stats['client_sigalg'], "rsa_pss_rsae_sha384")
+            self.assertEqual(stats['client_sigalg'], sigalg)
 
-        # server / client sigalg mismatch
+    @unittest.skipUnless(CAN_SET_CLIENT_SIGALGS,
+                         "SSL library doesn't support setting client sigalgs")
+    def test_client_sigalgs_mismatch(self):
         client_context, server_context, hostname = \
             testing_context(client_cert=SIGNED_CERTFILE)
         client_context.set_client_sigalgs("rsa_pss_rsae_sha256")
@@ -4345,24 +4348,25 @@ def test_client_sigalgs(self):
 
     def test_server_sigalgs(self):
         # server rsa_pss_rsae_sha384, client auto
+        sigalg = "rsa_pss_rsae_sha384"
         client_context, server_context, hostname = testing_context()
-        server_context.set_server_sigalgs("rsa_pss_rsae_sha384")
+        server_context.set_server_sigalgs(sigalg)
         stats = server_params_test(client_context, server_context,
                                    chatty=True, connectionchatty=True,
                                    sni_name=hostname)
         if CAN_GET_SELECTED_OPENSSL_SIGALG:
-            self.assertEqual(stats['server_sigalg'], "rsa_pss_rsae_sha384")
+            self.assertEqual(stats['server_sigalg'], sigalg)
 
         # server auto, client rsa_pss_rsae_sha384
         client_context, server_context, hostname = testing_context()
-        client_context.set_server_sigalgs("rsa_pss_rsae_sha384")
+        client_context.set_server_sigalgs(sigalg)
         stats = server_params_test(client_context, server_context,
                                    chatty=True, connectionchatty=True,
                                    sni_name=hostname)
         if CAN_GET_SELECTED_OPENSSL_SIGALG:
-            self.assertEqual(stats['server_sigalg'], "rsa_pss_rsae_sha384")
+            self.assertEqual(stats['server_sigalg'], sigalg)
 
-        # server / client sigalg mismatch
+    def test_server_sigalgs_mismatch(self):
         client_context, server_context, hostname = testing_context()
         client_context.set_server_sigalgs("rsa_pss_rsae_sha256")
         server_context.set_server_sigalgs("rsa_pss_rsae_sha384")
diff --git a/Misc/NEWS.d/next/Library/2025-08-30-17-58-04.gh-issue-138252.CDiEby.rst b/Misc/NEWS.d/next/Library/2025-08-30-17-58-04.gh-issue-138252.CDiEby.rst
@@ -1,2 +1,4 @@
-:class:`~ssl.SSLContext` objects can now set client and server TLS signature algorithms and
-:class:`~ssl.SSLSocket` objects can return the signature algorithms selected on a connection
+:mod:`ssl`: :class:`~ssl.SSLContext` objects can now set client and server
+TLS signature algorithms. If Python has been built with OpenSSL 3.5 or later,
+:class:`~ssl.SSLSocket` objects can return the signature algorithms selected
+on a connection.
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
@@ -2200,30 +2200,24 @@ _ssl__SSLSocket_group_impl(PySSLSocket *self)
 #endif
 }
 
-/*[clinic input]
-@critical_section
-_ssl._SSLSocket.client_sigalg
-[clinic start generated code]*/
-
 static PyObject *
-_ssl__SSLSocket_client_sigalg_impl(PySSLSocket *self)
-/*[clinic end generated code: output=499dd7fbf021a47b input=a0d9696b5414c627]*/
+ssl_socket_signame_impl(PySSLSocket *socket,
+                        enum py_ssl_server_or_client self_socket_type)
 {
 #if OPENSSL_VERSION_NUMBER >= 0x30500000L
     int ret;
     const char *sigalg;
 
-    if (self->ssl == NULL) {
+    if (socket->ssl == NULL) {
         Py_RETURN_NONE;
     }
-    if (self->socket_type == PY_SSL_CLIENT) {
-        ret = SSL_get0_signature_name(self->ssl, &sigalg);
-    } else {
-        ret = SSL_get0_peer_signature_name(self->ssl, &sigalg);
-    }
+    ret = (socket->socket_type == self_socket_type)
+        ? SSL_get0_signature_name(socket->ssl, &sigalg)
+        : SSL_get0_peer_signature_name(socket->ssl, &sigalg);
     if (ret == 0) {
         Py_RETURN_NONE;
     }
+    assert(sigalg != NULL);
     return PyUnicode_DecodeFSDefault(sigalg);
 #else
     PyErr_SetString(PyExc_NotImplementedError,
@@ -2232,6 +2226,18 @@ _ssl__SSLSocket_client_sigalg_impl(PySSLSocket *self)
 #endif
 }
 
+/*[clinic input]
+@critical_section
+_ssl._SSLSocket.client_sigalg
+[clinic start generated code]*/
+
+static PyObject *
+_ssl__SSLSocket_client_sigalg_impl(PySSLSocket *self)
+/*[clinic end generated code: output=499dd7fbf021a47b input=a0d9696b5414c627]*/
+{
+    return ssl_socket_signame_impl(self, PY_SSL_CLIENT);
+}
+
 /*[clinic input]
 @critical_section
 _ssl._SSLSocket.server_sigalg
@@ -2241,27 +2247,7 @@ static PyObject *
 _ssl__SSLSocket_server_sigalg_impl(PySSLSocket *self)
 /*[clinic end generated code: output=c508a766a8e275dc input=9063e562a1e6b946]*/
 {
-#if OPENSSL_VERSION_NUMBER >= 0x30500000L
-    int ret;
-    const char *sigalg;
-
-    if (self->ssl == NULL) {
-        Py_RETURN_NONE;
-    }
-    if (self->socket_type == PY_SSL_CLIENT) {
-        ret = SSL_get0_peer_signature_name(self->ssl, &sigalg);
-    } else {
-        ret = SSL_get0_signature_name(self->ssl, &sigalg);
-    }
-    if (ret == 0) {
-        Py_RETURN_NONE;
-    }
-    return PyUnicode_DecodeFSDefault(sigalg);
-#else
-    PyErr_SetString(PyExc_NotImplementedError,
-                    "Getting sig algorithms requires OpenSSL 3.5 or later.");
-    return NULL;
-#endif
+    return ssl_socket_signame_impl(self, PY_SSL_SERVER);
 }
 
 /*[clinic input]
