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.

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 (file)
index 0000000..fc7eeff
--- /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 2bcf864e759b912a18b226bc3d52db097b8c0d72..e240b889d86a2d5203c7bd9c7140ac47e0bbc910 100644 (file)
--- 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 <poll.h>
@@ -4543,60 +4546,25 @@ error:
     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=30bc7e967ea01a58]*/
+{
+    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 d1fb024903e1572525e887cf28d6e3dc52203032..8c35c8443b775ae1a52c6f344be306fa1cd7d92d 100644 (file)
--- 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]*/