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.
This commit is contained in:
Sam Gross 2026-01-22 14:02:30 -05:00 committed by GitHub
parent a966d94e76
commit bcf9cb0217
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 87 additions and 72 deletions

View file

@ -0,0 +1 @@
Fix potential thread safety issues in :mod:`ssl` module.

View file

@ -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 <poll.h>
@ -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

View file

@ -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]*/