From bcf9cb0217fdbab5dc6b812648e61bfa196e7110 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 22 Jan 2026 14:02:30 -0500 Subject: [PATCH] gh-143756: Fix potential data race in SSLContext.load_cert_chain (gh-143818) 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 | 154 ++++++++++-------- Modules/clinic/_ssl.c.h | 4 +- 3 files changed, 87 insertions(+), 72 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 00000000000..fc7eefff861 --- /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 2bcf864e759..e240b889d86 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,8 +4546,73 @@ _password_callback(char *buf, int size, int rwflag, void *userdata) return -1; } +static PyObject * +load_cert_chain_lock_held(PySSLContext *self, _PySSLPasswordInfo *pw_info, + PyObject *certfile_bytes, PyObject *keyfile_bytes) +{ + 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); + + 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); + if (r != 1) { + if (pw_info->error) { + ERR_clear_error(); + /* the password callback has already set the error information */ + } + else if (errno != 0) { + PyErr_SetFromErrno(PyExc_OSError); + ERR_clear_error(); + } + else { + _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); + } + goto error; + } + + PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state); + r = SSL_CTX_use_PrivateKey_file(self->ctx, + PyBytes_AS_STRING(keyfile_bytes ? keyfile_bytes : certfile_bytes), + SSL_FILETYPE_PEM); + PySSL_END_ALLOW_THREADS_S(pw_info->thread_state); + if (r != 1) { + if (pw_info->error) { + ERR_clear_error(); + /* the password callback has already set the error information */ + } + else if (errno != 0) { + PyErr_SetFromErrno(PyExc_OSError); + ERR_clear_error(); + } + else { + _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); + } + goto error; + } + + 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); + if (r != 1) { + _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); + goto error; + } + 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] -@critical_section _ssl._SSLContext.load_cert_chain certfile: object keyfile: object = None @@ -4555,13 +4623,11 @@ _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; - 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; + PyObject *ret = NULL; errno = 0; ERR_clear_error(); @@ -4579,76 +4645,26 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile, PyErr_SetString(PyExc_TypeError, "keyfile should be a valid filesystem path"); } - goto error; + 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 error; + goto done; } - 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); - 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); - if (r != 1) { - if (pw_info.error) { - ERR_clear_error(); - /* the password callback has already set the error information */ - } - else if (errno != 0) { - PyErr_SetFromErrno(PyExc_OSError); - ERR_clear_error(); - } - else { - _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); - } - goto error; - } - PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); - r = SSL_CTX_use_PrivateKey_file(self->ctx, - PyBytes_AS_STRING(keyfile ? 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); - if (r != 1) { - if (pw_info.error) { - ERR_clear_error(); - /* the password callback has already set the error information */ - } - else if (errno != 0) { - PyErr_SetFromErrno(PyExc_OSError); - ERR_clear_error(); - } - else { - _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); - } - goto error; - } - PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); - r = SSL_CTX_check_private_key(self->ctx); - PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex); - 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; -error: - SSL_CTX_set_default_passwd_cb(self->ctx, orig_passwd_cb); - SSL_CTX_set_default_passwd_cb_userdata(self->ctx, orig_passwd_userdata); + 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 d1fb024903e..8c35c8443b7 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 @@ exit: #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]*/