FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-134698: Hold a lock when the thread state is detached in `ssl` (GH… · python/cpython@e047a35 · GitHub

  1. python
  2. cpython

Repository navigation

Commit e047a35

Browse files
ZeroIntensity
and
gpshead
authored
gh-134698: Hold a lock when the thread state is detached in ssl (GH-134724)
Lock when the thread state is detached. Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent cb93b6f commit e047a35

4 files changed

Lines changed: 89 additions & 50 deletions

File tree

‎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: 59 additions & 44 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>
@@ -336,6 +336,9 @@ typedef struct {
336336
PyObject *psk_client_callback;
337337
PyObject *psk_server_callback;
338338
#endif
339+
/* Lock to synchronize calls when the thread state is detached.
340+
See also gh-134698. */
341+
PyMutex tstate_mutex;
339342
} PySSLContext;
340343

341344
#define PySSLContext_CAST(op) ((PySSLContext *)(op))
@@ -363,6 +366,9 @@ typedef struct {
363366
* and shutdown methods check for chained exceptions.
364367
*/
365368
PyObject *exc;
369+
/* Lock to synchronize calls when the thread state is detached.
370+
See also gh-134698. */
371+
PyMutex tstate_mutex;
366372
} PySSLSocket;
367373

368374
#define PySSLSocket_CAST(op) ((PySSLSocket *)(op))
@@ -912,13 +918,14 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
912918
self->server_hostname = NULL;
913919
self->err = err;
914920
self->exc = NULL;
921+
self->tstate_mutex = (PyMutex){0};
915922

916923
/* Make sure the SSL error state is initialized */
917924
ERR_clear_error();
918925

919-
PySSL_BEGIN_ALLOW_THREADS
926+
PySSL_BEGIN_ALLOW_THREADS(sslctx)
920927
self->ssl = SSL_new(ctx);
921-
PySSL_END_ALLOW_THREADS
928+
PySSL_END_ALLOW_THREADS(sslctx)
922929
if (self->ssl == NULL) {
923930
Py_DECREF(self);
924931
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -987,12 +994,12 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
987994
BIO_set_nbio(SSL_get_wbio(self->ssl), 1);
988995
}
989996

990-
PySSL_BEGIN_ALLOW_THREADS
997+
PySSL_BEGIN_ALLOW_THREADS(self)
991998
if (socket_type == PY_SSL_CLIENT)
992999
SSL_set_connect_state(self->ssl);
9931000
else
9941001
SSL_set_accept_state(self->ssl);
995-
PySSL_END_ALLOW_THREADS
1002+
PySSL_END_ALLOW_THREADS(self)
9961003

9971004
self->socket_type = socket_type;
9981005
if (sock != NULL) {
@@ -1061,10 +1068,10 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10611068
/* Actually negotiate SSL connection */
10621069
/* XXX If SSL_do_handshake() returns 0, it's also a failure. */
10631070
do {
1064-
PySSL_BEGIN_ALLOW_THREADS
1071+
PySSL_BEGIN_ALLOW_THREADS(self)
10651072
ret = SSL_do_handshake(self->ssl);
10661073
err = _PySSL_errno(ret < 1, self->ssl, ret);
1067-
PySSL_END_ALLOW_THREADS
1074+
PySSL_END_ALLOW_THREADS(self)
10681075
self->err = err;
10691076

10701077
if (PyErr_CheckSignals())
@@ -2441,9 +2448,10 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24412448
ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
24422449
assert(ms <= INT_MAX);
24432450

2444-
PySSL_BEGIN_ALLOW_THREADS
2451+
Py_BEGIN_ALLOW_THREADS
24452452
rc = poll(&pollfd, 1, (int)ms);
2446-
PySSL_END_ALLOW_THREADS
2453+
Py_END_ALLOW_THREADS
2454+
_PySSL_FIX_ERRNO;
24472455
#else
24482456
/* Guard against socket too large for select*/
24492457
if (!_PyIsSelectable_fd(s->sock_fd))
@@ -2455,13 +2463,14 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24552463
FD_SET(s->sock_fd, &fds);
24562464

24572465
/* Wait until the socket becomes ready */
2458-
PySSL_BEGIN_ALLOW_THREADS
2466+
Py_BEGIN_ALLOW_THREADS
24592467
nfds = Py_SAFE_DOWNCAST(s->sock_fd+1, SOCKET_T, int);
24602468
if (writing)
24612469
rc = select(nfds, NULL, &fds, NULL, &tv);
24622470
else
24632471
rc = select(nfds, &fds, NULL, NULL, &tv);
2464-
PySSL_END_ALLOW_THREADS
2472+
Py_END_ALLOW_THREADS
2473+
_PySSL_FIX_ERRNO;
24652474
#endif
24662475

24672476
/* Return SOCKET_TIMED_OUT on timeout, SOCKET_OPERATION_OK otherwise
@@ -2579,10 +2588,10 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset,
25792588
}
25802589

25812590
do {
2582-
PySSL_BEGIN_ALLOW_THREADS
2591+
PySSL_BEGIN_ALLOW_THREADS(self)
25832592
retval = SSL_sendfile(self->ssl, fd, (off_t)offset, size, flags);
25842593
err = _PySSL_errno(retval < 0, self->ssl, (int)retval);
2585-
PySSL_END_ALLOW_THREADS
2594+
PySSL_END_ALLOW_THREADS(self)
25862595
self->err = err;
25872596

25882597
if (PyErr_CheckSignals()) {
@@ -2710,10 +2719,10 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
27102719
}
27112720

27122721
do {
2713-
PySSL_BEGIN_ALLOW_THREADS
2722+
PySSL_BEGIN_ALLOW_THREADS(self)
27142723
retval = SSL_write_ex(self->ssl, b->buf, (size_t)b->len, &count);
27152724
err = _PySSL_errno(retval == 0, self->ssl, retval);
2716-
PySSL_END_ALLOW_THREADS
2725+
PySSL_END_ALLOW_THREADS(self)
27172726
self->err = err;
27182727

27192728
if (PyErr_CheckSignals())
@@ -2771,10 +2780,10 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
27712780
int count = 0;
27722781
_PySSLError err;
27732782

2774-
PySSL_BEGIN_ALLOW_THREADS
2783+
PySSL_BEGIN_ALLOW_THREADS(self)
27752784
count = SSL_pending(self->ssl);
27762785
err = _PySSL_errno(count < 0, self->ssl, count);
2777-
PySSL_END_ALLOW_THREADS
2786+
PySSL_END_ALLOW_THREADS(self)
27782787
self->err = err;
27792788

27802789
if (count < 0)
@@ -2865,10 +2874,10 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
28652874
deadline = _PyDeadline_Init(timeout);
28662875

28672876
do {
2868-
PySSL_BEGIN_ALLOW_THREADS
2877+
PySSL_BEGIN_ALLOW_THREADS(self)
28692878
retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count);
28702879
err = _PySSL_errno(retval == 0, self->ssl, retval);
2871-
PySSL_END_ALLOW_THREADS
2880+
PySSL_END_ALLOW_THREADS(self)
28722881
self->err = err;
28732882

28742883
if (PyErr_CheckSignals())
@@ -2967,7 +2976,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
29672976
}
29682977

29692978
while (1) {
2970-
PySSL_BEGIN_ALLOW_THREADS
2979+
PySSL_BEGIN_ALLOW_THREADS(self)
29712980
/* Disable read-ahead so that unwrap can work correctly.
29722981
* Otherwise OpenSSL might read in too much data,
29732982
* eating clear text data that happens to be
@@ -2980,7 +2989,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
29802989
SSL_set_read_ahead(self->ssl, 0);
29812990
ret = SSL_shutdown(self->ssl);
29822991
err = _PySSL_errno(ret < 0, self->ssl, ret);
2983-
PySSL_END_ALLOW_THREADS
2992+
PySSL_END_ALLOW_THREADS(self)
29842993
self->err = err;
29852994

29862995
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
@@ -3375,9 +3384,10 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
33753384
// no other thread can be touching this object yet.
33763385
// (Technically, we can't even lock if we wanted to, as the
33773386
// lock hasn't been initialized yet.)
3378-
PySSL_BEGIN_ALLOW_THREADS
3387+
Py_BEGIN_ALLOW_THREADS
33793388
ctx = SSL_CTX_new(method);
3380-
PySSL_END_ALLOW_THREADS
3389+
Py_END_ALLOW_THREADS
3390+
_PySSL_FIX_ERRNO;
33813391

33823392
if (ctx == NULL) {
33833393
_setSSLError(get_ssl_state(module), NULL, 0, __FILE__, __LINE__);
@@ -3402,6 +3412,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
34023412
self->psk_client_callback = NULL;
34033413
self->psk_server_callback = NULL;
34043414
#endif
3415+
self->tstate_mutex = (PyMutex){0};
34053416

34063417
/* Don't check host name by default */
34073418
if (proto_version == PY_SSL_VERSION_TLS_CLIENT) {
@@ -3520,9 +3531,10 @@ context_clear(PyObject *op)
35203531
Py_CLEAR(self->psk_server_callback);
35213532
#endif
35223533
if (self->keylog_bio != NULL) {
3523-
PySSL_BEGIN_ALLOW_THREADS
3534+
Py_BEGIN_ALLOW_THREADS
35243535
BIO_free_all(self->keylog_bio);
3525-
PySSL_END_ALLOW_THREADS
3536+
Py_END_ALLOW_THREADS
3537+
_PySSL_FIX_ERRNO;
35263538
self->keylog_bio = NULL;
35273539
}
35283540
return 0;
@@ -4245,7 +4257,8 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
42454257
_PySSLPasswordInfo *pw_info = (_PySSLPasswordInfo*) userdata;
42464258
PyObject *fn_ret = NULL;
42474259

4248-
PySSL_END_ALLOW_THREADS_S(pw_info->thread_state);
4260+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
4261+
_PySSL_FIX_ERRNO;
42494262

42504263
if (pw_info->error) {
42514264
/* already failed previously. OpenSSL 3.0.0-alpha14 invokes the
@@ -4275,13 +4288,13 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
42754288
goto error;
42764289
}
42774290

4278-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4291+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
42794292
memcpy(buf, pw_info->password, pw_info->size);
42804293
return pw_info->size;
42814294

42824295
error:
42834296
Py_XDECREF(fn_ret);
4284-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4297+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
42854298
pw_info->error = 1;
42864299
return -1;
42874300
}
@@ -4334,10 +4347,10 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
43344347
SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback);
43354348
SSL_CTX_set_default_passwd_cb_userdata(self->ctx, &pw_info);
43364349
}
4337-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4350+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43384351
r = SSL_CTX_use_certificate_chain_file(self->ctx,
43394352
PyBytes_AS_STRING(certfile_bytes));
4340-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4353+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43414354
if (r != 1) {
43424355
if (pw_info.error) {
43434356
ERR_clear_error();
@@ -4352,11 +4365,11 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
43524365
}
43534366
goto error;
43544367
}
4355-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4368+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43564369
r = SSL_CTX_use_PrivateKey_file(self->ctx,
43574370
PyBytes_AS_STRING(keyfile ? keyfile_bytes : certfile_bytes),
43584371
SSL_FILETYPE_PEM);
4359-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4372+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43604373
Py_CLEAR(keyfile_bytes);
43614374
Py_CLEAR(certfile_bytes);
43624375
if (r != 1) {
@@ -4373,9 +4386,9 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
43734386
}
43744387
goto error;
43754388
}
4376-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4389+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43774390
r = SSL_CTX_check_private_key(self->ctx);
4378-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4391+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
43794392
if (r != 1) {
43804393
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
43814394
goto error;
@@ -4592,9 +4605,9 @@ _ssl__SSLContext_load_verify_locations_impl(PySSLContext *self,
45924605
cafile_buf = PyBytes_AS_STRING(cafile_bytes);
45934606
if (capath)
45944607
capath_buf = PyBytes_AS_STRING(capath_bytes);
4595-
PySSL_BEGIN_ALLOW_THREADS
4608+
PySSL_BEGIN_ALLOW_THREADS(self)
45964609
r = SSL_CTX_load_verify_locations(self->ctx, cafile_buf, capath_buf);
4597-
PySSL_END_ALLOW_THREADS
4610+
PySSL_END_ALLOW_THREADS(self)
45984611
if (r != 1) {
45994612
if (errno != 0) {
46004613
PyErr_SetFromErrno(PyExc_OSError);
@@ -4646,10 +4659,11 @@ _ssl__SSLContext_load_dh_params_impl(PySSLContext *self, PyObject *filepath)
46464659
return NULL;
46474660

46484661
errno = 0;
4649-
PySSL_BEGIN_ALLOW_THREADS
4662+
Py_BEGIN_ALLOW_THREADS
46504663
dh = PEM_read_DHparams(f, NULL, NULL, NULL);
46514664
fclose(f);
4652-
PySSL_END_ALLOW_THREADS
4665+
Py_END_ALLOW_THREADS
4666+
_PySSL_FIX_ERRNO;
46534667
if (dh == NULL) {
46544668
if (errno != 0) {
46554669
PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
@@ -4801,6 +4815,7 @@ _ssl__SSLContext_set_default_verify_paths_impl(PySSLContext *self)
48014815
Py_BEGIN_ALLOW_THREADS
48024816
rc = SSL_CTX_set_default_verify_paths(self->ctx);
48034817
Py_END_ALLOW_THREADS
4818+
_PySSL_FIX_ERRNO;
48044819
if (!rc) {
48054820
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
48064821
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)
Back | FazBrowse Home | New Git URL