Skip to content

Commit c6fd1c1

Browse files
zoobazooba
authored
bpo-32533: Fixed thread-safety of error handling in _ssl. (GH-7158)
1 parent 12a69db commit c6fd1c1

File tree

2 files changed

+69
-60
lines changed

2 files changed

+69
-60
lines changed

‎Misc/NEWS.d/next/Security/2018-05-28-08-55-30.bpo-32533.IzwkBI.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed thread-safety of error handling in _ssl.

‎Modules/_ssl.c

Lines changed: 68 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,14 @@ typedef struct {
423423
int protocol;
424424
} PySSLContext;
425425

426+
typedef struct {
427+
int ssl; /* last seen error from SSL */
428+
int c; /* last seen error from libc */
429+
#ifdef MS_WINDOWS
430+
int ws; /* last seen error from winsock */
431+
#endif
432+
} _PySSLError;
433+
426434
typedef struct {
427435
PyObject_HEAD
428436
PyObject *Socket; /* weakref to socket on which we're layered */
@@ -432,11 +440,7 @@ typedef struct {
432440
enum py_ssl_server_or_client socket_type;
433441
PyObject *owner; /* Python level "owner" passed to servername callback */
434442
PyObject *server_hostname;
435-
int ssl_errno; /* last seen error from SSL */
436-
int c_errno; /* last seen error from libc */
437-
#ifdef MS_WINDOWS
438-
int ws_errno; /* last seen error from winsock */
439-
#endif
443+
_PySSLError err; /* last seen error from various sources */
440444
} PySSLSocket;
441445

442446
typedef struct {
@@ -456,20 +460,19 @@ static PyTypeObject PySSLSocket_Type;
456460
static PyTypeObject PySSLMemoryBIO_Type;
457461
static PyTypeObject PySSLSession_Type;
458462

463+
static inline _PySSLError _PySSL_errno(int failed, const SSL *ssl, int retcode)
464+
{
465+
_PySSLError err = { 0 };
466+
if (failed) {
459467
#ifdef MS_WINDOWS
460-
#define _PySSL_UPDATE_ERRNO_IF(cond, sock, retcode) if (cond) { \
461-
(sock)->ws_errno = WSAGetLastError(); \
462-
_PySSL_FIX_ERRNO; \
463-
(sock)->c_errno = errno; \
464-
(sock)->ssl_errno = SSL_get_error((sock->ssl), (retcode)); \
465-
} else { sock->ws_errno = 0; sock->c_errno = 0; sock->ssl_errno = 0; }
466-
#else
467-
#define _PySSL_UPDATE_ERRNO_IF(cond, sock, retcode) if (cond) { \
468-
(sock)->c_errno = errno; \
469-
(sock)->ssl_errno = SSL_get_error((sock->ssl), (retcode)); \
470-
} else { (sock)->c_errno = 0; (sock)->ssl_errno = 0; }
468+
err.ws = WSAGetLastError();
469+
_PySSL_FIX_ERRNO;
471470
#endif
472-
#define _PySSL_UPDATE_ERRNO(sock, retcode) _PySSL_UPDATE_ERRNO_IF(1, (sock), (retcode))
471+
err.c = errno;
472+
err.ssl = SSL_get_error(ssl, retcode);
473+
}
474+
return err;
475+
}
473476

474477
/*[clinic input]
475478
module _ssl
@@ -703,17 +706,17 @@ PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno)
703706
{
704707
PyObject *type = PySSLErrorObject;
705708
char *errstr = NULL;
706-
int err;
709+
_PySSLError err;
707710
enum py_ssl_error p = PY_SSL_ERROR_NONE;
708711
unsigned long e = 0;
709712

710713
assert(ret <= 0);
711714
e = ERR_peek_last_error();
712715

713716
if (sslsock->ssl != NULL) {
714-
err = sslsock->ssl_errno;
717+
err = sslsock->err;
715718

716-
switch (err) {
719+
switch (err.ssl) {
717720
case SSL_ERROR_ZERO_RETURN:
718721
errstr = "TLS/SSL connection has been closed (EOF)";
719722
type = PySSLZeroReturnErrorObject;
@@ -749,11 +752,12 @@ PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno)
749752
/* underlying BIO reported an I/O error */
750753
ERR_clear_error();
751754
#ifdef MS_WINDOWS
752-
if (sslsock->ws_errno)
753-
return PyErr_SetFromWindowsErr(sslsock->ws_errno);
755+
if (err.ws) {
756+
return PyErr_SetFromWindowsErr(err.ws);
757+
}
754758
#endif
755-
if (sslsock->c_errno) {
756-
errno = sslsock->c_errno;
759+
if (err.c) {
760+
errno = err.c;
757761
return PyErr_SetFromErrno(PyExc_OSError);
758762
}
759763
Py_INCREF(s);
@@ -883,6 +887,7 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
883887
{
884888
PySSLSocket *self;
885889
SSL_CTX *ctx = sslctx->ctx;
890+
_PySSLError err = { 0 };
886891

887892
self = PyObject_New(PySSLSocket, &PySSLSocket_Type);
888893
if (self == NULL)
@@ -895,11 +900,7 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
895900
self->shutdown_seen_zero = 0;
896901
self->owner = NULL;
897902
self->server_hostname = NULL;
898-
self->ssl_errno = 0;
899-
self->c_errno = 0;
900-
#ifdef MS_WINDOWS
901-
self->ws_errno = 0;
902-
#endif
903+
self->err = err;
903904

904905
/* Make sure the SSL error state is initialized */
905906
ERR_clear_error();
@@ -976,7 +977,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
976977
/*[clinic end generated code: output=6c0898a8936548f6 input=d2d737de3df018c8]*/
977978
{
978979
int ret;
979-
int err;
980+
_PySSLError err;
980981
int sockstate, nonblocking;
981982
PySocketSockObject *sock = GET_SOCKET(self);
982983
_PyTime_t timeout, deadline = 0;
@@ -1006,19 +1007,19 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10061007
do {
10071008
PySSL_BEGIN_ALLOW_THREADS
10081009
ret = SSL_do_handshake(self->ssl);
1009-
_PySSL_UPDATE_ERRNO_IF(ret < 1, self, ret);
1010+
err = _PySSL_errno(ret < 1, self->ssl, ret);
10101011
PySSL_END_ALLOW_THREADS
1011-
err = self->ssl_errno;
1012+
self->err = err;
10121013

10131014
if (PyErr_CheckSignals())
10141015
goto error;
10151016

10161017
if (has_timeout)
10171018
timeout = deadline - _PyTime_GetMonotonicClock();
10181019

1019-
if (err == SSL_ERROR_WANT_READ) {
1020+
if (err.ssl == SSL_ERROR_WANT_READ) {
10201021
sockstate = PySSL_select(sock, 0, timeout);
1021-
} else if (err == SSL_ERROR_WANT_WRITE) {
1022+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
10221023
sockstate = PySSL_select(sock, 1, timeout);
10231024
} else {
10241025
sockstate = SOCKET_OPERATION_OK;
@@ -1039,7 +1040,8 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10391040
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
10401041
break;
10411042
}
1042-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
1043+
} while (err.ssl == SSL_ERROR_WANT_READ ||
1044+
err.ssl == SSL_ERROR_WANT_WRITE);
10431045
Py_XDECREF(sock);
10441046
if (ret < 1)
10451047
return PySSL_SetError(self, ret, __FILE__, __LINE__);
@@ -2228,7 +2230,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
22282230
{
22292231
int len;
22302232
int sockstate;
2231-
int err;
2233+
_PySSLError err;
22322234
int nonblocking;
22332235
PySocketSockObject *sock = GET_SOCKET(self);
22342236
_PyTime_t timeout, deadline = 0;
@@ -2279,19 +2281,19 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
22792281
do {
22802282
PySSL_BEGIN_ALLOW_THREADS
22812283
len = SSL_write(self->ssl, b->buf, (int)b->len);
2282-
_PySSL_UPDATE_ERRNO_IF(len <= 0, self, len);
2284+
err = _PySSL_errno(len <= 0, self->ssl, len);
22832285
PySSL_END_ALLOW_THREADS
2284-
err = self->ssl_errno;
2286+
self->err = err;
22852287

22862288
if (PyErr_CheckSignals())
22872289
goto error;
22882290

22892291
if (has_timeout)
22902292
timeout = deadline - _PyTime_GetMonotonicClock();
22912293

2292-
if (err == SSL_ERROR_WANT_READ) {
2294+
if (err.ssl == SSL_ERROR_WANT_READ) {
22932295
sockstate = PySSL_select(sock, 0, timeout);
2294-
} else if (err == SSL_ERROR_WANT_WRITE) {
2296+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
22952297
sockstate = PySSL_select(sock, 1, timeout);
22962298
} else {
22972299
sockstate = SOCKET_OPERATION_OK;
@@ -2308,7 +2310,8 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
23082310
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
23092311
break;
23102312
}
2311-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2313+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2314+
err.ssl == SSL_ERROR_WANT_WRITE);
23122315

23132316
Py_XDECREF(sock);
23142317
if (len > 0)
@@ -2332,11 +2335,14 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
23322335
/*[clinic end generated code: output=983d9fecdc308a83 input=2b77487d6dfd597f]*/
23332336
{
23342337
int count = 0;
2338+
_PySSLError err;
23352339

23362340
PySSL_BEGIN_ALLOW_THREADS
23372341
count = SSL_pending(self->ssl);
2338-
_PySSL_UPDATE_ERRNO_IF(count < 0, self, count);
2342+
err = _PySSL_errno(count < 0, self->ssl, count);
23392343
PySSL_END_ALLOW_THREADS
2344+
self->err = err;
2345+
23402346
if (count < 0)
23412347
return PySSL_SetError(self, count, __FILE__, __LINE__);
23422348
else
@@ -2363,7 +2369,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
23632369
char *mem;
23642370
int count;
23652371
int sockstate;
2366-
int err;
2372+
_PySSLError err;
23672373
int nonblocking;
23682374
PySocketSockObject *sock = GET_SOCKET(self);
23692375
_PyTime_t timeout, deadline = 0;
@@ -2424,21 +2430,21 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
24242430
do {
24252431
PySSL_BEGIN_ALLOW_THREADS
24262432
count = SSL_read(self->ssl, mem, len);
2427-
_PySSL_UPDATE_ERRNO_IF(count <= 0, self, count);
2433+
err = _PySSL_errno(count <= 0, self->ssl, count);
24282434
PySSL_END_ALLOW_THREADS
2435+
self->err = err;
24292436

24302437
if (PyErr_CheckSignals())
24312438
goto error;
24322439

24332440
if (has_timeout)
24342441
timeout = deadline - _PyTime_GetMonotonicClock();
24352442

2436-
err = self->ssl_errno;
2437-
if (err == SSL_ERROR_WANT_READ) {
2443+
if (err.ssl == SSL_ERROR_WANT_READ) {
24382444
sockstate = PySSL_select(sock, 0, timeout);
2439-
} else if (err == SSL_ERROR_WANT_WRITE) {
2445+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
24402446
sockstate = PySSL_select(sock, 1, timeout);
2441-
} else if (err == SSL_ERROR_ZERO_RETURN &&
2447+
} else if (err.ssl == SSL_ERROR_ZERO_RETURN &&
24422448
SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN)
24432449
{
24442450
count = 0;
@@ -2454,7 +2460,8 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
24542460
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
24552461
break;
24562462
}
2457-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2463+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2464+
err.ssl == SSL_ERROR_WANT_WRITE);
24582465

24592466
if (count <= 0) {
24602467
PySSL_SetError(self, count, __FILE__, __LINE__);
@@ -2488,7 +2495,8 @@ static PyObject *
24882495
_ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
24892496
/*[clinic end generated code: output=ca1aa7ed9d25ca42 input=11d39e69b0a2bf4a]*/
24902497
{
2491-
int err, sockstate, nonblocking;
2498+
_PySSLError err;
2499+
int sockstate, nonblocking, ret;
24922500
int zeros = 0;
24932501
PySocketSockObject *sock = GET_SOCKET(self);
24942502
_PyTime_t timeout, deadline = 0;
@@ -2526,14 +2534,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25262534
*/
25272535
if (self->shutdown_seen_zero)
25282536
SSL_set_read_ahead(self->ssl, 0);
2529-
err = SSL_shutdown(self->ssl);
2530-
_PySSL_UPDATE_ERRNO_IF(err < 0, self, err);
2537+
ret = SSL_shutdown(self->ssl);
2538+
err = _PySSL_errno(ret < 0, self->ssl, ret);
25312539
PySSL_END_ALLOW_THREADS
2540+
self->err = err;
25322541

25332542
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
2534-
if (err > 0)
2543+
if (ret > 0)
25352544
break;
2536-
if (err == 0) {
2545+
if (ret == 0) {
25372546
/* Don't loop endlessly; instead preserve legacy
25382547
behaviour of trying SSL_shutdown() only twice.
25392548
This looks necessary for OpenSSL < 0.9.8m */
@@ -2548,16 +2557,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25482557
timeout = deadline - _PyTime_GetMonotonicClock();
25492558

25502559
/* Possibly retry shutdown until timeout or failure */
2551-
_PySSL_UPDATE_ERRNO(self, err);
2552-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2560+
if (err.ssl == SSL_ERROR_WANT_READ)
25532561
sockstate = PySSL_select(sock, 0, timeout);
2554-
else if (self->ssl_errno == SSL_ERROR_WANT_WRITE)
2562+
else if (err.ssl == SSL_ERROR_WANT_WRITE)
25552563
sockstate = PySSL_select(sock, 1, timeout);
25562564
else
25572565
break;
25582566

25592567
if (sockstate == SOCKET_HAS_TIMED_OUT) {
2560-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2568+
if (err.ssl == SSL_ERROR_WANT_READ)
25612569
PyErr_SetString(PySocketModule.timeout_error,
25622570
"The read operation timed out");
25632571
else
@@ -2575,9 +2583,9 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25752583
break;
25762584
}
25772585

2578-
if (err < 0) {
2586+
if (err.ssl < 0) {
25792587
Py_XDECREF(sock);
2580-
return PySSL_SetError(self, err, __FILE__, __LINE__);
2588+
return PySSL_SetError(self, err.ssl, __FILE__, __LINE__);
25812589
}
25822590
if (sock)
25832591
/* It's already INCREF'ed */

0 commit comments

Comments
 (0)