From c145c03281af850130dd7f75abc19c6ea49626e6 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Sun, 21 Mar 2021 08:39:15 -0700 Subject: [PATCH] bpo-43577: Fix deadlock with SSLContext._msg_callback and sni_callback (GH-24957) OpenSSL copies the internal message callback from SSL_CTX->msg_callback to SSL->msg_callback. SSL_set_SSL_CTX() does not update SSL->msg_callback to use the callback value of the new context. PySSL_set_context() now resets the callback and _PySSL_msg_callback() resets thread state in error path. Signed-off-by: Christian Heimes (cherry picked from commit 77cde5042a2f1eae489c11a67540afaf43cd5cdf) Co-authored-by: Christian Heimes --- Lib/test/test_ssl.py | 22 +++++++++++++++++++ .../2021-03-21-10-13-17.bpo-43577.m7JnAV.rst | 1 + Modules/_ssl.c | 5 +++++ Modules/_ssl/debughelpers.c | 1 + 4 files changed, 29 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-03-21-10-13-17.bpo-43577.m7JnAV.rst diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index e5b4b11d37f..738929e3195 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4739,6 +4739,28 @@ class TestSSLDebug(unittest.TestCase): msg ) + def test_msg_callback_deadlock_bpo43577(self): + client_context, server_context, hostname = testing_context() + server_context2 = testing_context()[1] + + def msg_cb(conn, direction, version, content_type, msg_type, data): + pass + + def sni_cb(sock, servername, ctx): + sock.context = server_context2 + + server_context._msg_callback = msg_cb + server_context.sni_callback = sni_cb + + server = ThreadedEchoServer(context=server_context, chatty=False) + with server: + with client_context.wrap_socket(socket.socket(), + server_hostname=hostname) as s: + s.connect((HOST, server.port)) + with client_context.wrap_socket(socket.socket(), + server_hostname=hostname) as s: + s.connect((HOST, server.port)) + def test_main(verbose=False): if support.verbose: diff --git a/Misc/NEWS.d/next/Library/2021-03-21-10-13-17.bpo-43577.m7JnAV.rst b/Misc/NEWS.d/next/Library/2021-03-21-10-13-17.bpo-43577.m7JnAV.rst new file mode 100644 index 00000000000..a7db48bc626 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-03-21-10-13-17.bpo-43577.m7JnAV.rst @@ -0,0 +1 @@ +Fix deadlock when using :class:`ssl.SSLContext` debug callback with :meth:`ssl.SSLContext.sni_callback`. diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 7bdde452513..aab57139975 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -2203,6 +2203,11 @@ static int PySSL_set_context(PySSLSocket *self, PyObject *value, Py_INCREF(value); Py_SETREF(self->ctx, (PySSLContext *)value); SSL_set_SSL_CTX(self->ssl, self->ctx->ctx); + /* Set SSL* internal msg_callback to state of new context's state */ + SSL_set_msg_callback( + self->ssl, + self->ctx->msg_cb ? _PySSL_msg_callback : NULL + ); #endif } else { PyErr_SetString(PyExc_TypeError, "The value must be a SSLContext"); diff --git a/Modules/_ssl/debughelpers.c b/Modules/_ssl/debughelpers.c index b840da2f663..af56f9d28d1 100644 --- a/Modules/_ssl/debughelpers.c +++ b/Modules/_ssl/debughelpers.c @@ -23,6 +23,7 @@ _PySSL_msg_callback(int write_p, int version, int content_type, ssl_obj = (PySSLSocket *)SSL_get_app_data(ssl); assert(PySSLSocket_Check(ssl_obj)); if (ssl_obj->ctx->msg_cb == NULL) { + PyGILState_Release(threadstate); return; }