From 4109a334f93926838d4eb0daba3194913e0db52d Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 13 Jan 2026 16:20:21 -0500 Subject: [PATCH 1/2] gh-143756: Fix potential data race in SSLContext.load_cert_chain Concurrent calls to `load_cert_chain` caused data races in OpenSSL code. --- ...-01-13-16-19-50.gh-issue-143756.LQOra1.rst | 1 + Modules/_ssl.c | 148 ++++++++++-------- Modules/clinic/_ssl.c.h | 4 +- 3 files changed, 84 insertions(+), 69 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-01-13-16-19-50.gh-issue-143756.LQOra1.rst diff --git a/Misc/NEWS.d/next/Library/2026-01-13-16-19-50.gh-issue-143756.LQOra1.rst b/Misc/NEWS.d/next/Library/2026-01-13-16-19-50.gh-issue-143756.LQOra1.rst new file mode 100644 index 00000000000000..fc7eefff8619ae --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-01-13-16-19-50.gh-issue-143756.LQOra1.rst @@ -0,0 +1 @@ +Fix potential thread safety issues in :mod:`ssl` module. diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 7dd57e7892af41..a25ff42d7ab697 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -43,14 +43,17 @@ /* Redefined below for Windows debug builds after important #includes */ #define _PySSL_FIX_ERRNO -#define PySSL_BEGIN_ALLOW_THREADS_S(save, mutex) \ - do { (save) = PyEval_SaveThread(); PyMutex_Lock(mutex); } while(0) -#define PySSL_END_ALLOW_THREADS_S(save, mutex) \ - do { PyMutex_Unlock(mutex); PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0) +#define PySSL_BEGIN_ALLOW_THREADS_S(save) \ + do { (save) = PyEval_SaveThread(); } while(0) +#define PySSL_END_ALLOW_THREADS_S(save) \ + do { PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0) #define PySSL_BEGIN_ALLOW_THREADS(self) { \ PyThreadState *_save = NULL; \ - PySSL_BEGIN_ALLOW_THREADS_S(_save, &self->tstate_mutex); -#define PySSL_END_ALLOW_THREADS(self) PySSL_END_ALLOW_THREADS_S(_save, &self->tstate_mutex); } + PySSL_BEGIN_ALLOW_THREADS_S(_save); \ + PyMutex_Lock(&(self)->tstate_mutex); +#define PySSL_END_ALLOW_THREADS(self) \ + PyMutex_Unlock(&(self)->tstate_mutex); \ + PySSL_END_ALLOW_THREADS_S(_save); } #if defined(HAVE_POLL_H) #include @@ -4543,60 +4546,25 @@ _password_callback(char *buf, int size, int rwflag, void *userdata) return -1; } -/*[clinic input] -@critical_section -_ssl._SSLContext.load_cert_chain - certfile: object - keyfile: object = None - password: object = None - -[clinic start generated code]*/ - static PyObject * -_ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, - PyObject *keyfile, PyObject *password) -/*[clinic end generated code: output=9480bc1c380e2095 input=6c7c5e8b73e4264b]*/ +load_cert_chain_lock_held(PySSLContext *self, _PySSLPasswordInfo *pw_info, + PyObject *certfile_bytes, PyObject *keyfile_bytes) { - PyObject *certfile_bytes = NULL, *keyfile_bytes = NULL; + int r; + PyObject *ret = NULL; + pem_password_cb *orig_passwd_cb = SSL_CTX_get_default_passwd_cb(self->ctx); void *orig_passwd_userdata = SSL_CTX_get_default_passwd_cb_userdata(self->ctx); - _PySSLPasswordInfo pw_info = { NULL, NULL, NULL, 0, 0 }; - int r; - errno = 0; - ERR_clear_error(); - if (keyfile == Py_None) - keyfile = NULL; - if (!PyUnicode_FSConverter(certfile, &certfile_bytes)) { - if (PyErr_ExceptionMatches(PyExc_TypeError)) { - PyErr_SetString(PyExc_TypeError, - "certfile should be a valid filesystem path"); - } - return NULL; - } - if (keyfile && !PyUnicode_FSConverter(keyfile, &keyfile_bytes)) { - if (PyErr_ExceptionMatches(PyExc_TypeError)) { - PyErr_SetString(PyExc_TypeError, - "keyfile should be a valid filesystem path"); - } - goto error; - } - if (password != Py_None) { - if (PyCallable_Check(password)) { - pw_info.callable = password; - } else if (!_pwinfo_set(&pw_info, password, - "password should be a string or callable")) { - goto error; - } - SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback); - SSL_CTX_set_default_passwd_cb_userdata(self->ctx, &pw_info); - } - PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); + SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback); + SSL_CTX_set_default_passwd_cb_userdata(self->ctx, pw_info); + + PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state); r = SSL_CTX_use_certificate_chain_file(self->ctx, PyBytes_AS_STRING(certfile_bytes)); - PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); + PySSL_END_ALLOW_THREADS_S(pw_info->thread_state); if (r != 1) { - if (pw_info.error) { + if (pw_info->error) { ERR_clear_error(); /* the password callback has already set the error information */ } @@ -4609,15 +4577,14 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, } goto error; } - PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); + + PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state); r = SSL_CTX_use_PrivateKey_file(self->ctx, - PyBytes_AS_STRING(keyfile ? keyfile_bytes : certfile_bytes), + PyBytes_AS_STRING(keyfile_bytes ? keyfile_bytes : certfile_bytes), SSL_FILETYPE_PEM); - PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); - Py_CLEAR(keyfile_bytes); - Py_CLEAR(certfile_bytes); + PySSL_END_ALLOW_THREADS_S(pw_info->thread_state); if (r != 1) { - if (pw_info.error) { + if (pw_info->error) { ERR_clear_error(); /* the password callback has already set the error information */ } @@ -4630,25 +4597,74 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, } goto error; } - PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); + + PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state); r = SSL_CTX_check_private_key(self->ctx); - PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); + PySSL_END_ALLOW_THREADS_S(pw_info->thread_state); if (r != 1) { _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); goto error; } - SSL_CTX_set_default_passwd_cb(self->ctx, orig_passwd_cb); - SSL_CTX_set_default_passwd_cb_userdata(self->ctx, orig_passwd_userdata); - PyMem_Free(pw_info.password); - Py_RETURN_NONE; - + ret = Py_None; error: SSL_CTX_set_default_passwd_cb(self->ctx, orig_passwd_cb); SSL_CTX_set_default_passwd_cb_userdata(self->ctx, orig_passwd_userdata); + return ret; +} + +/*[clinic input] +_ssl._SSLContext.load_cert_chain + certfile: object + keyfile: object = None + password: object = None + +[clinic start generated code]*/ + +static PyObject * +_ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, + PyObject *keyfile, PyObject *password) +/*[clinic end generated code: output=9480bc1c380e2095 input=6c7c5e8b73e4264b]*/ +{ + PyObject *certfile_bytes = NULL, *keyfile_bytes = NULL; + _PySSLPasswordInfo pw_info = { NULL, NULL, NULL, 0, 0 }; + PyObject *ret = NULL; + + errno = 0; + ERR_clear_error(); + if (keyfile == Py_None) + keyfile = NULL; + if (!PyUnicode_FSConverter(certfile, &certfile_bytes)) { + if (PyErr_ExceptionMatches(PyExc_TypeError)) { + PyErr_SetString(PyExc_TypeError, + "certfile should be a valid filesystem path"); + } + return NULL; + } + if (keyfile && !PyUnicode_FSConverter(keyfile, &keyfile_bytes)) { + if (PyErr_ExceptionMatches(PyExc_TypeError)) { + PyErr_SetString(PyExc_TypeError, + "keyfile should be a valid filesystem path"); + } + goto done; + } + if (password != Py_None) { + if (PyCallable_Check(password)) { + pw_info.callable = password; + } else if (!_pwinfo_set(&pw_info, password, + "password should be a string or callable")) { + goto done; + } + } + + PyMutex_Lock(&self->tstate_mutex); + ret = load_cert_chain_lock_held(self, &pw_info, certfile_bytes, keyfile_bytes); + PyMutex_Unlock(&self->tstate_mutex); + +done: PyMem_Free(pw_info.password); Py_XDECREF(keyfile_bytes); Py_XDECREF(certfile_bytes); - return NULL; + return ret; } /* internal helper function, returns -1 on error diff --git a/Modules/clinic/_ssl.c.h b/Modules/clinic/_ssl.c.h index d1fb024903e157..8c35c8443b775a 100644 --- a/Modules/clinic/_ssl.c.h +++ b/Modules/clinic/_ssl.c.h @@ -1829,9 +1829,7 @@ _ssl__SSLContext_load_cert_chain(PyObject *self, PyObject *const *args, Py_ssize } password = args[2]; skip_optional_pos: - Py_BEGIN_CRITICAL_SECTION(self); return_value = _ssl__SSLContext_load_cert_chain_impl((PySSLContext *)self, certfile, keyfile, password); - Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -3325,4 +3323,4 @@ _ssl_enum_crls(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObje #ifndef _SSL_ENUM_CRLS_METHODDEF #define _SSL_ENUM_CRLS_METHODDEF #endif /* !defined(_SSL_ENUM_CRLS_METHODDEF) */ -/*[clinic end generated code: output=3b6c9cbfc4660ecb input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e29d5ada294f97bb input=a9049054013a1b77]*/ From 46d033a177ae8cd7ab8edc99e60e4835bc07b615 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 13 Jan 2026 16:31:01 -0500 Subject: [PATCH 2/2] Update clinic file --- Modules/_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index a25ff42d7ab697..c9bbf57be59721 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -4623,7 +4623,7 @@ _ssl._SSLContext.load_cert_chain static PyObject * _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, PyObject *keyfile, PyObject *password) -/*[clinic end generated code: output=9480bc1c380e2095 input=6c7c5e8b73e4264b]*/ +/*[clinic end generated code: output=9480bc1c380e2095 input=30bc7e967ea01a58]*/ { PyObject *certfile_bytes = NULL, *keyfile_bytes = NULL; _PySSLPasswordInfo pw_info = { NULL, NULL, NULL, 0, 0 };