[Python-checkins] [3.11] gh-96931: Fix incorrect results in ssl.SSLSocket.shared_ciphers (GH-96932) (#102918)

[Yhg1s]

https://github.com/python/cpython/commit/e075631f672730067fe467fd6d058e6bfec9cbec
commit: e075631f672730067fe467fd6d058e6bfec9cbec
branch: 3.11
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Yhg1s <thomas at python.org>
date: 2023-03-24T13:33:24+01:00
summary:

[3.11] gh-96931: Fix incorrect results in ssl.SSLSocket.shared_ciphers (GH-96932) (#102918)

gh-96931: Fix incorrect results in ssl.SSLSocket.shared_ciphers (GH-96932)
(cherry picked from commit af9c34f6ef8dceb21871206eb3e4d350f6e3d3dc)

Co-authored-by: Benjamin Fogle <benfogle at gmail.com>

files:
A Misc/NEWS.d/next/Library/2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst
M Doc/library/ssl.rst
M Lib/test/test_ssl.py
M Modules/_ssl.c

diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
index 372b1243dd75..6ac08fd6548e 100644
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -1309,7 +1309,7 @@ SSL sockets also have the following additional methods and attributes:
 
 .. method:: SSLSocket.shared_ciphers()
 
-   Return the list of ciphers shared by the client during the handshake.  Each
+   Return the list of ciphers available in both the client and server.  Each
    entry of the returned list is a three-value tuple containing the name of the
    cipher, the version of the SSL protocol that defines its use, and the number
    of secret bits the cipher uses.  :meth:`~SSLSocket.shared_ciphers` returns
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 3b3b869bb53a..5c6ddf12ec35 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -2326,13 +2326,13 @@ def test_bio_handshake(self):
         self.assertIs(sslobj._sslobj.owner, sslobj)
         self.assertIsNone(sslobj.cipher())
         self.assertIsNone(sslobj.version())
-        self.assertIsNotNone(sslobj.shared_ciphers())
+        self.assertIsNone(sslobj.shared_ciphers())
         self.assertRaises(ValueError, sslobj.getpeercert)
         if 'tls-unique' in ssl.CHANNEL_BINDING_TYPES:
             self.assertIsNone(sslobj.get_channel_binding('tls-unique'))
         self.ssl_io_loop(sock, incoming, outgoing, sslobj.do_handshake)
         self.assertTrue(sslobj.cipher())
-        self.assertIsNotNone(sslobj.shared_ciphers())
+        self.assertIsNone(sslobj.shared_ciphers())
         self.assertIsNotNone(sslobj.version())
         self.assertTrue(sslobj.getpeercert())
         if 'tls-unique' in ssl.CHANNEL_BINDING_TYPES:
@@ -4302,7 +4302,7 @@ def cb_wrong_return_type(ssl_sock, server_name, initial_context):
     def test_shared_ciphers(self):
         client_context, server_context, hostname = testing_context()
         client_context.set_ciphers("AES128:AES256")
-        server_context.set_ciphers("AES256")
+        server_context.set_ciphers("AES256:eNULL")
         expected_algs = [
             "AES256", "AES-256",
             # TLS 1.3 ciphers are always enabled
diff --git a/Misc/NEWS.d/next/Library/2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst b/Misc/NEWS.d/next/Library/2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst
new file mode 100644
index 000000000000..766b1d4d477b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst
@@ -0,0 +1 @@
+Fix incorrect results from :meth:`ssl.SSLSocket.shared_ciphers`
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index 6071202104fa..79f78007b68d 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -2000,24 +2000,44 @@ static PyObject *
 _ssl__SSLSocket_shared_ciphers_impl(PySSLSocket *self)
 /*[clinic end generated code: output=3d174ead2e42c4fd input=0bfe149da8fe6306]*/
 {
-    STACK_OF(SSL_CIPHER) *ciphers;
-    int i;
+    STACK_OF(SSL_CIPHER) *server_ciphers;
+    STACK_OF(SSL_CIPHER) *client_ciphers;
+    int i, len;
     PyObject *res;
+    const SSL_CIPHER* cipher;
+
+    /* Rather than use SSL_get_shared_ciphers, we use an equivalent algorithm because:
+
+       1) It returns a colon seperated list of strings, in an undefined
+          order, that we would have to post process back into tuples.
+       2) It will return a truncated string with no indication that it has
+          done so, if the buffer is too small.
+     */
 
-    ciphers = SSL_get_ciphers(self->ssl);
-    if (!ciphers)
+    server_ciphers = SSL_get_ciphers(self->ssl);
+    if (!server_ciphers)
         Py_RETURN_NONE;
-    res = PyList_New(sk_SSL_CIPHER_num(ciphers));
+    client_ciphers = SSL_get_client_ciphers(self->ssl);
+    if (!client_ciphers)
+        Py_RETURN_NONE;
+
+    res = PyList_New(sk_SSL_CIPHER_num(server_ciphers));
     if (!res)
         return NULL;
-    for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
-        PyObject *tup = cipher_to_tuple(sk_SSL_CIPHER_value(ciphers, i));
+    len = 0;
+    for (i = 0; i < sk_SSL_CIPHER_num(server_ciphers); i++) {
+        cipher = sk_SSL_CIPHER_value(server_ciphers, i);
+        if (sk_SSL_CIPHER_find(client_ciphers, cipher) < 0)
+            continue;
+
+        PyObject *tup = cipher_to_tuple(cipher);
         if (!tup) {
             Py_DECREF(res);
             return NULL;
         }
-        PyList_SET_ITEM(res, i, tup);
+        PyList_SET_ITEM(res, len++, tup);
     }
+    Py_SET_SIZE(res, len);
     return res;
 }