From af9c34f6ef8dceb21871206eb3e4d350f6e3d3dc Mon Sep 17 00:00:00 2001 From: Benjamin Fogle Date: Wed, 22 Mar 2023 10:08:41 -0400 Subject: gh-96931: Fix incorrect results in ssl.SSLSocket.shared_ciphers (#96932) --- Doc/library/ssl.rst | 2 +- Lib/test/test_ssl.py | 6 ++-- .../2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst | 1 + Modules/_ssl.c | 36 +++++++++++++++++----- 4 files changed, 33 insertions(+), 12 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-09-19-08-12-58.gh-issue-96931.x0WQhh.rst diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst index 30f2a07..4b60b7c 100644 --- a/Doc/library/ssl.rst +++ b/Doc/library/ssl.rst @@ -1218,7 +1218,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 76669bd..1317efb 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2082,13 +2082,13 @@ class SimpleBackgroundTests(unittest.TestCase): 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: @@ -4051,7 +4051,7 @@ class ThreadedTests(unittest.TestCase): 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 0000000..766b1d4 --- /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 321b6ec..36b66cd 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -1998,24 +1998,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; } -- cgit v0.12