Skip to content

Commit fd565fd

Browse files
ZeroIntensitygpshead
ZeroIntensity
and
gpshead
committed
[3.14] gh-134698: Hold a lock when the thread state is detached in ssl (GH-134724)
Lock when the thread state is detached. (cherry picked from commit e047a35) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 8742298 commit fd565fd

File tree

4 files changed

+87
-48
lines changed

4 files changed

+87
-48
lines changed

‎Lib/test/test_ssl.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,25 @@ def getpass(self):
12391239
# Make sure the password function isn't called if it isn't needed
12401240
ctx.load_cert_chain(CERTFILE, password=getpass_exception)
12411241

1242+
@threading_helper.requires_working_threading()
1243+
def test_load_cert_chain_thread_safety(self):
1244+
# gh-134698: _ssl detaches the thread state (and as such,
1245+
# releases the GIL and critical sections) around expensive
1246+
# OpenSSL calls. Unfortunately, OpenSSL structures aren't
1247+
# thread-safe, so executing these calls concurrently led
1248+
# to crashes.
1249+
ctx = ssl.create_default_context()
1250+
1251+
def race():
1252+
ctx.load_cert_chain(CERTFILE)
1253+
1254+
threads = [threading.Thread(target=race) for _ in range(8)]
1255+
with threading_helper.catch_threading_exception() as cm:
1256+
with threading_helper.start_threads(threads):
1257+
pass
1258+
1259+
self.assertIsNone(cm.exc_value)
1260+
12421261
def test_load_verify_locations(self):
12431262
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
12441263
ctx.load_verify_locations(CERTFILE)

‎Misc/NEWS.d/next/Library/2025-05-26-10-52-27.gh-issue-134698.aJ1mZ1.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a crash when calling methods of :class:`ssl.SSLContext` or
2+
:class:`ssl.SSLSocket` across multiple threads.

‎Modules/_ssl.c

Lines changed: 57 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@
4343
/* Redefined below for Windows debug builds after important #includes */
4444
#define _PySSL_FIX_ERRNO
4545

46-
#define PySSL_BEGIN_ALLOW_THREADS_S(save) \
47-
do { (save) = PyEval_SaveThread(); } while(0)
48-
#define PySSL_END_ALLOW_THREADS_S(save) \
49-
do { PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
50-
#define PySSL_BEGIN_ALLOW_THREADS { \
46+
#define PySSL_BEGIN_ALLOW_THREADS_S(save, mutex) \
47+
do { (save) = PyEval_SaveThread(); PyMutex_Lock(mutex); } while(0)
48+
#define PySSL_END_ALLOW_THREADS_S(save, mutex) \
49+
do { PyMutex_Unlock(mutex); PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
50+
#define PySSL_BEGIN_ALLOW_THREADS(self) { \
5151
PyThreadState *_save = NULL; \
52-
PySSL_BEGIN_ALLOW_THREADS_S(_save);
53-
#define PySSL_END_ALLOW_THREADS PySSL_END_ALLOW_THREADS_S(_save); }
52+
PySSL_BEGIN_ALLOW_THREADS_S(_save, &self->tstate_mutex);
53+
#define PySSL_END_ALLOW_THREADS(self) PySSL_END_ALLOW_THREADS_S(_save, &self->tstate_mutex); }
5454

5555
#if defined(HAVE_POLL_H)
5656
#include <poll.h>
@@ -309,6 +309,9 @@ typedef struct {
309309
PyObject *psk_client_callback;
310310
PyObject *psk_server_callback;
311311
#endif
312+
/* Lock to synchronize calls when the thread state is detached.
313+
See also gh-134698. */
314+
PyMutex tstate_mutex;
312315
} PySSLContext;
313316

314317
#define PySSLContext_CAST(op) ((PySSLContext *)(op))
@@ -336,6 +339,9 @@ typedef struct {
336339
* and shutdown methods check for chained exceptions.
337340
*/
338341
PyObject *exc;
342+
/* Lock to synchronize calls when the thread state is detached.
343+
See also gh-134698. */
344+
PyMutex tstate_mutex;
339345
} PySSLSocket;
340346

341347
#define PySSLSocket_CAST(op) ((PySSLSocket *)(op))
@@ -885,13 +891,14 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
885891
self->server_hostname = NULL;
886892
self->err = err;
887893
self->exc = NULL;
894+
self->tstate_mutex = (PyMutex){0};
888895

889896
/* Make sure the SSL error state is initialized */
890897
ERR_clear_error();
891898

892-
PySSL_BEGIN_ALLOW_THREADS
899+
PySSL_BEGIN_ALLOW_THREADS(sslctx)
893900
self->ssl = SSL_new(ctx);
894-
PySSL_END_ALLOW_THREADS
901+
PySSL_END_ALLOW_THREADS(sslctx)
895902
if (self->ssl == NULL) {
896903
Py_DECREF(self);
897904
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -960,12 +967,12 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
960967
BIO_set_nbio(SSL_get_wbio(self->ssl), 1);
961968
}
962969

963-
PySSL_BEGIN_ALLOW_THREADS
970+
PySSL_BEGIN_ALLOW_THREADS(self)
964971
if (socket_type == PY_SSL_CLIENT)
965972
SSL_set_connect_state(self->ssl);
966973
else
967974
SSL_set_accept_state(self->ssl);
968-
PySSL_END_ALLOW_THREADS
975+
PySSL_END_ALLOW_THREADS(self)
969976

970977
self->socket_type = socket_type;
971978
if (sock != NULL) {
@@ -1034,10 +1041,10 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10341041
/* Actually negotiate SSL connection */
10351042
/* XXX If SSL_do_handshake() returns 0, it's also a failure. */
10361043
do {
1037-
PySSL_BEGIN_ALLOW_THREADS
1044+
PySSL_BEGIN_ALLOW_THREADS(self)
10381045
ret = SSL_do_handshake(self->ssl);
10391046
err = _PySSL_errno(ret < 1, self->ssl, ret);
1040-
PySSL_END_ALLOW_THREADS
1047+
PySSL_END_ALLOW_THREADS(self)
10411048
self->err = err;
10421049

10431050
if (PyErr_CheckSignals())
@@ -2414,9 +2421,10 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24142421
ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
24152422
assert(ms <= INT_MAX);
24162423

2417-
PySSL_BEGIN_ALLOW_THREADS
2424+
Py_BEGIN_ALLOW_THREADS
24182425
rc = poll(&pollfd, 1, (int)ms);
2419-
PySSL_END_ALLOW_THREADS
2426+
Py_END_ALLOW_THREADS
2427+
_PySSL_FIX_ERRNO;
24202428
#else
24212429
/* Guard against socket too large for select*/
24222430
if (!_PyIsSelectable_fd(s->sock_fd))
@@ -2428,13 +2436,14 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24282436
FD_SET(s->sock_fd, &fds);
24292437

24302438
/* Wait until the socket becomes ready */
2431-
PySSL_BEGIN_ALLOW_THREADS
2439+
Py_BEGIN_ALLOW_THREADS
24322440
nfds = Py_SAFE_DOWNCAST(s->sock_fd+1, SOCKET_T, int);
24332441
if (writing)
24342442
rc = select(nfds, NULL, &fds, NULL, &tv);
24352443
else
24362444
rc = select(nfds, &fds, NULL, NULL, &tv);
2437-
PySSL_END_ALLOW_THREADS
2445+
Py_END_ALLOW_THREADS
2446+
_PySSL_FIX_ERRNO;
24382447
#endif
24392448

24402449
/* Return SOCKET_TIMED_OUT on timeout, SOCKET_OPERATION_OK otherwise
@@ -2505,10 +2514,10 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
25052514
}
25062515

25072516
do {
2508-
PySSL_BEGIN_ALLOW_THREADS
2517+
PySSL_BEGIN_ALLOW_THREADS(self)
25092518
retval = SSL_write_ex(self->ssl, b->buf, (size_t)b->len, &count);
25102519
err = _PySSL_errno(retval == 0, self->ssl, retval);
2511-
PySSL_END_ALLOW_THREADS
2520+
PySSL_END_ALLOW_THREADS(self)
25122521
self->err = err;
25132522

25142523
if (PyErr_CheckSignals())
@@ -2566,10 +2575,10 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
25662575
int count = 0;
25672576
_PySSLError err;
25682577

2569-
PySSL_BEGIN_ALLOW_THREADS
2578+
PySSL_BEGIN_ALLOW_THREADS(self)
25702579
count = SSL_pending(self->ssl);
25712580
err = _PySSL_errno(count < 0, self->ssl, count);
2572-
PySSL_END_ALLOW_THREADS
2581+
PySSL_END_ALLOW_THREADS(self)
25732582
self->err = err;
25742583

25752584
if (count < 0)
@@ -2660,10 +2669,10 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
26602669
deadline = _PyDeadline_Init(timeout);
26612670

26622671
do {
2663-
PySSL_BEGIN_ALLOW_THREADS
2672+
PySSL_BEGIN_ALLOW_THREADS(self)
26642673
retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count);
26652674
err = _PySSL_errno(retval == 0, self->ssl, retval);
2666-
PySSL_END_ALLOW_THREADS
2675+
PySSL_END_ALLOW_THREADS(self)
26672676
self->err = err;
26682677

26692678
if (PyErr_CheckSignals())
@@ -2762,7 +2771,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
27622771
}
27632772

27642773
while (1) {
2765-
PySSL_BEGIN_ALLOW_THREADS
2774+
PySSL_BEGIN_ALLOW_THREADS(self)
27662775
/* Disable read-ahead so that unwrap can work correctly.
27672776
* Otherwise OpenSSL might read in too much data,
27682777
* eating clear text data that happens to be
@@ -2775,7 +2784,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
27752784
SSL_set_read_ahead(self->ssl, 0);
27762785
ret = SSL_shutdown(self->ssl);
27772786
err = _PySSL_errno(ret < 0, self->ssl, ret);
2778-
PySSL_END_ALLOW_THREADS
2787+
PySSL_END_ALLOW_THREADS(self)
27792788
self->err = err;
27802789

27812790
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
@@ -3167,9 +3176,10 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
31673176
// no other thread can be touching this object yet.
31683177
// (Technically, we can't even lock if we wanted to, as the
31693178
// lock hasn't been initialized yet.)
3170-
PySSL_BEGIN_ALLOW_THREADS
3179+
Py_BEGIN_ALLOW_THREADS
31713180
ctx = SSL_CTX_new(method);
3172-
PySSL_END_ALLOW_THREADS
3181+
Py_END_ALLOW_THREADS
3182+
_PySSL_FIX_ERRNO;
31733183

31743184
if (ctx == NULL) {
31753185
_setSSLError(get_ssl_state(module), NULL, 0, __FILE__, __LINE__);
@@ -3194,6 +3204,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
31943204
self->psk_client_callback = NULL;
31953205
self->psk_server_callback = NULL;
31963206
#endif
3207+
self->tstate_mutex = (PyMutex){0};
31973208

31983209
/* Don't check host name by default */
31993210
if (proto_version == PY_SSL_VERSION_TLS_CLIENT) {
@@ -3312,9 +3323,10 @@ context_clear(PyObject *op)
33123323
Py_CLEAR(self->psk_server_callback);
33133324
#endif
33143325
if (self->keylog_bio != NULL) {
3315-
PySSL_BEGIN_ALLOW_THREADS
3326+
Py_BEGIN_ALLOW_THREADS
33163327
BIO_free_all(self->keylog_bio);
3317-
PySSL_END_ALLOW_THREADS
3328+
Py_END_ALLOW_THREADS
3329+
_PySSL_FIX_ERRNO;
33183330
self->keylog_bio = NULL;
33193331
}
33203332
return 0;
@@ -4037,7 +4049,8 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
40374049
_PySSLPasswordInfo *pw_info = (_PySSLPasswordInfo*) userdata;
40384050
PyObject *fn_ret = NULL;
40394051

4040-
PySSL_END_ALLOW_THREADS_S(pw_info->thread_state);
4052+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
4053+
_PySSL_FIX_ERRNO;
40414054

40424055
if (pw_info->error) {
40434056
/* already failed previously. OpenSSL 3.0.0-alpha14 invokes the
@@ -4067,13 +4080,13 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
40674080
goto error;
40684081
}
40694082

4070-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4083+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
40714084
memcpy(buf, pw_info->password, pw_info->size);
40724085
return pw_info->size;
40734086

40744087
error:
40754088
Py_XDECREF(fn_ret);
4076-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4089+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
40774090
pw_info->error = 1;
40784091
return -1;
40794092
}
@@ -4126,10 +4139,10 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41264139
SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback);
41274140
SSL_CTX_set_default_passwd_cb_userdata(self->ctx, &pw_info);
41284141
}
4129-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4142+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41304143
r = SSL_CTX_use_certificate_chain_file(self->ctx,
41314144
PyBytes_AS_STRING(certfile_bytes));
4132-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4145+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41334146
if (r != 1) {
41344147
if (pw_info.error) {
41354148
ERR_clear_error();
@@ -4144,11 +4157,11 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41444157
}
41454158
goto error;
41464159
}
4147-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4160+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41484161
r = SSL_CTX_use_PrivateKey_file(self->ctx,
41494162
PyBytes_AS_STRING(keyfile ? keyfile_bytes : certfile_bytes),
41504163
SSL_FILETYPE_PEM);
4151-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4164+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41524165
Py_CLEAR(keyfile_bytes);
41534166
Py_CLEAR(certfile_bytes);
41544167
if (r != 1) {
@@ -4165,9 +4178,9 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41654178
}
41664179
goto error;
41674180
}
4168-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4181+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41694182
r = SSL_CTX_check_private_key(self->ctx);
4170-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4183+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41714184
if (r != 1) {
41724185
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
41734186
goto error;
@@ -4384,9 +4397,9 @@ _ssl__SSLContext_load_verify_locations_impl(PySSLContext *self,
43844397
cafile_buf = PyBytes_AS_STRING(cafile_bytes);
43854398
if (capath)
43864399
capath_buf = PyBytes_AS_STRING(capath_bytes);
4387-
PySSL_BEGIN_ALLOW_THREADS
4400+
PySSL_BEGIN_ALLOW_THREADS(self)
43884401
r = SSL_CTX_load_verify_locations(self->ctx, cafile_buf, capath_buf);
4389-
PySSL_END_ALLOW_THREADS
4402+
PySSL_END_ALLOW_THREADS(self)
43904403
if (r != 1) {
43914404
if (errno != 0) {
43924405
PyErr_SetFromErrno(PyExc_OSError);
@@ -4438,10 +4451,11 @@ _ssl__SSLContext_load_dh_params_impl(PySSLContext *self, PyObject *filepath)
44384451
return NULL;
44394452

44404453
errno = 0;
4441-
PySSL_BEGIN_ALLOW_THREADS
4454+
Py_BEGIN_ALLOW_THREADS
44424455
dh = PEM_read_DHparams(f, NULL, NULL, NULL);
44434456
fclose(f);
4444-
PySSL_END_ALLOW_THREADS
4457+
Py_END_ALLOW_THREADS
4458+
_PySSL_FIX_ERRNO;
44454459
if (dh == NULL) {
44464460
if (errno != 0) {
44474461
PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
@@ -4593,6 +4607,7 @@ _ssl__SSLContext_set_default_verify_paths_impl(PySSLContext *self)
45934607
Py_BEGIN_ALLOW_THREADS
45944608
rc = SSL_CTX_set_default_verify_paths(self->ctx);
45954609
Py_END_ALLOW_THREADS
4610+
_PySSL_FIX_ERRNO;
45964611
if (!rc) {
45974612
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
45984613
return NULL;

‎Modules/_ssl/debughelpers.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,15 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line)
140140
* critical debug helper.
141141
*/
142142

143-
PySSL_BEGIN_ALLOW_THREADS
143+
assert(PyMutex_IsLocked(&ssl_obj->tstate_mutex));
144+
Py_BEGIN_ALLOW_THREADS
144145
PyThread_acquire_lock(lock, 1);
145146
res = BIO_printf(ssl_obj->ctx->keylog_bio, "%s\n", line);
146147
e = errno;
147148
(void)BIO_flush(ssl_obj->ctx->keylog_bio);
148149
PyThread_release_lock(lock);
149-
PySSL_END_ALLOW_THREADS
150+
Py_END_ALLOW_THREADS
151+
_PySSL_FIX_ERRNO;
150152

151153
if (res == -1) {
152154
errno = e;
@@ -187,9 +189,10 @@ _PySSLContext_set_keylog_filename(PyObject *op, PyObject *arg,
187189
if (self->keylog_bio != NULL) {
188190
BIO *bio = self->keylog_bio;
189191
self->keylog_bio = NULL;
190-
PySSL_BEGIN_ALLOW_THREADS
192+
Py_BEGIN_ALLOW_THREADS
191193
BIO_free_all(bio);
192-
PySSL_END_ALLOW_THREADS
194+
Py_END_ALLOW_THREADS
195+
_PySSL_FIX_ERRNO;
193196
}
194197

195198
if (arg == Py_None) {
@@ -211,13 +214,13 @@ _PySSLContext_set_keylog_filename(PyObject *op, PyObject *arg,
211214
self->keylog_filename = Py_NewRef(arg);
212215

213216
/* Write a header for seekable, empty files (this excludes pipes). */
214-
PySSL_BEGIN_ALLOW_THREADS
217+
PySSL_BEGIN_ALLOW_THREADS(self)
215218
if (BIO_tell(self->keylog_bio) == 0) {
216219
BIO_puts(self->keylog_bio,
217220
"# TLS secrets log file, generated by OpenSSL / Python\n");
218221
(void)BIO_flush(self->keylog_bio);
219222
}
220-
PySSL_END_ALLOW_THREADS
223+
PySSL_END_ALLOW_THREADS(self)
221224
SSL_CTX_set_keylog_callback(self->ctx, _PySSL_keylog_callback);
222225
return 0;
223226
}

0 commit comments

Comments
 (0)