Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Doc/library/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,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
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -4310,7 +4310,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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix incorrect results from :meth:`ssl.SSLSocket.shared_ciphers`
36 changes: 28 additions & 8 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2005,24 +2005,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;
}

Expand Down