Skip to content

Commit 1229664

Browse files
miss-islingtonzooba
miss-islington
and
zooba
authored
bpo-32533: Fixed thread-safety of error handling in _ssl. (GH-7158)
(cherry picked from commit c6fd1c1) Co-authored-by: Steve Dower <steve.dower@microsoft.com>
1 parent f8e34ee commit 1229664

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
(void) ERR_get_state();
@@ -977,7 +978,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
977978
/*[clinic end generated code: output=6c0898a8936548f6 input=d2d737de3df018c8]*/
978979
{
979980
int ret;
980-
int err;
981+
_PySSLError err;
981982
int sockstate, nonblocking;
982983
PySocketSockObject *sock = GET_SOCKET(self);
983984
_PyTime_t timeout, deadline = 0;
@@ -1007,19 +1008,19 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10071008
do {
10081009
PySSL_BEGIN_ALLOW_THREADS
10091010
ret = SSL_do_handshake(self->ssl);
1010-
_PySSL_UPDATE_ERRNO_IF(ret < 1, self, ret);
1011+
err = _PySSL_errno(ret < 1, self->ssl, ret);
10111012
PySSL_END_ALLOW_THREADS
1012-
err = self->ssl_errno;
1013+
self->err = err;
10131014

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

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

1020-
if (err == SSL_ERROR_WANT_READ) {
1021+
if (err.ssl == SSL_ERROR_WANT_READ) {
10211022
sockstate = PySSL_select(sock, 0, timeout);
1022-
} else if (err == SSL_ERROR_WANT_WRITE) {
1023+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
10231024
sockstate = PySSL_select(sock, 1, timeout);
10241025
} else {
10251026
sockstate = SOCKET_OPERATION_OK;
@@ -1040,7 +1041,8 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10401041
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
10411042
break;
10421043
}
1043-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
1044+
} while (err.ssl == SSL_ERROR_WANT_READ ||
1045+
err.ssl == SSL_ERROR_WANT_WRITE);
10441046
Py_XDECREF(sock);
10451047
if (ret < 1)
10461048
return PySSL_SetError(self, ret, __FILE__, __LINE__);
@@ -2229,7 +2231,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
22292231
{
22302232
int len;
22312233
int sockstate;
2232-
int err;
2234+
_PySSLError err;
22332235
int nonblocking;
22342236
PySocketSockObject *sock = GET_SOCKET(self);
22352237
_PyTime_t timeout, deadline = 0;
@@ -2280,19 +2282,19 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
22802282
do {
22812283
PySSL_BEGIN_ALLOW_THREADS
22822284
len = SSL_write(self->ssl, b->buf, (int)b->len);
2283-
_PySSL_UPDATE_ERRNO_IF(len <= 0, self, len);
2285+
err = _PySSL_errno(len <= 0, self->ssl, len);
22842286
PySSL_END_ALLOW_THREADS
2285-
err = self->ssl_errno;
2287+
self->err = err;
22862288

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

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

2293-
if (err == SSL_ERROR_WANT_READ) {
2295+
if (err.ssl == SSL_ERROR_WANT_READ) {
22942296
sockstate = PySSL_select(sock, 0, timeout);
2295-
} else if (err == SSL_ERROR_WANT_WRITE) {
2297+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
22962298
sockstate = PySSL_select(sock, 1, timeout);
22972299
} else {
22982300
sockstate = SOCKET_OPERATION_OK;
@@ -2309,7 +2311,8 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
23092311
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
23102312
break;
23112313
}
2312-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2314+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2315+
err.ssl == SSL_ERROR_WANT_WRITE);
23132316

23142317
Py_XDECREF(sock);
23152318
if (len > 0)
@@ -2333,11 +2336,14 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
23332336
/*[clinic end generated code: output=983d9fecdc308a83 input=2b77487d6dfd597f]*/
23342337
{
23352338
int count = 0;
2339+
_PySSLError err;
23362340

23372341
PySSL_BEGIN_ALLOW_THREADS
23382342
count = SSL_pending(self->ssl);
2339-
_PySSL_UPDATE_ERRNO_IF(count < 0, self, count);
2343+
err = _PySSL_errno(count < 0, self->ssl, count);
23402344
PySSL_END_ALLOW_THREADS
2345+
self->err = err;
2346+
23412347
if (count < 0)
23422348
return PySSL_SetError(self, count, __FILE__, __LINE__);
23432349
else
@@ -2364,7 +2370,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
23642370
char *mem;
23652371
int count;
23662372
int sockstate;
2367-
int err;
2373+
_PySSLError err;
23682374
int nonblocking;
23692375
PySocketSockObject *sock = GET_SOCKET(self);
23702376
_PyTime_t timeout, deadline = 0;
@@ -2425,21 +2431,21 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
24252431
do {
24262432
PySSL_BEGIN_ALLOW_THREADS
24272433
count = SSL_read(self->ssl, mem, len);
2428-
_PySSL_UPDATE_ERRNO_IF(count <= 0, self, count);
2434+
err = _PySSL_errno(count <= 0, self->ssl, count);
24292435
PySSL_END_ALLOW_THREADS
2436+
self->err = err;
24302437

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

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

2437-
err = self->ssl_errno;
2438-
if (err == SSL_ERROR_WANT_READ) {
2444+
if (err.ssl == SSL_ERROR_WANT_READ) {
24392445
sockstate = PySSL_select(sock, 0, timeout);
2440-
} else if (err == SSL_ERROR_WANT_WRITE) {
2446+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
24412447
sockstate = PySSL_select(sock, 1, timeout);
2442-
} else if (err == SSL_ERROR_ZERO_RETURN &&
2448+
} else if (err.ssl == SSL_ERROR_ZERO_RETURN &&
24432449
SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN)
24442450
{
24452451
count = 0;
@@ -2455,7 +2461,8 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
24552461
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
24562462
break;
24572463
}
2458-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2464+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2465+
err.ssl == SSL_ERROR_WANT_WRITE);
24592466

24602467
if (count <= 0) {
24612468
PySSL_SetError(self, count, __FILE__, __LINE__);
@@ -2489,7 +2496,8 @@ static PyObject *
24892496
_ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
24902497
/*[clinic end generated code: output=ca1aa7ed9d25ca42 input=11d39e69b0a2bf4a]*/
24912498
{
2492-
int err, sockstate, nonblocking;
2499+
_PySSLError err;
2500+
int sockstate, nonblocking, ret;
24932501
int zeros = 0;
24942502
PySocketSockObject *sock = GET_SOCKET(self);
24952503
_PyTime_t timeout, deadline = 0;
@@ -2527,14 +2535,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25272535
*/
25282536
if (self->shutdown_seen_zero)
25292537
SSL_set_read_ahead(self->ssl, 0);
2530-
err = SSL_shutdown(self->ssl);
2531-
_PySSL_UPDATE_ERRNO_IF(err < 0, self, err);
2538+
ret = SSL_shutdown(self->ssl);
2539+
err = _PySSL_errno(ret < 0, self->ssl, ret);
25322540
PySSL_END_ALLOW_THREADS
2541+
self->err = err;
25332542

25342543
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
2535-
if (err > 0)
2544+
if (ret > 0)
25362545
break;
2537-
if (err == 0) {
2546+
if (ret == 0) {
25382547
/* Don't loop endlessly; instead preserve legacy
25392548
behaviour of trying SSL_shutdown() only twice.
25402549
This looks necessary for OpenSSL < 0.9.8m */
@@ -2549,16 +2558,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25492558
timeout = deadline - _PyTime_GetMonotonicClock();
25502559

25512560
/* Possibly retry shutdown until timeout or failure */
2552-
_PySSL_UPDATE_ERRNO(self, err);
2553-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2561+
if (err.ssl == SSL_ERROR_WANT_READ)
25542562
sockstate = PySSL_select(sock, 0, timeout);
2555-
else if (self->ssl_errno == SSL_ERROR_WANT_WRITE)
2563+
else if (err.ssl == SSL_ERROR_WANT_WRITE)
25562564
sockstate = PySSL_select(sock, 1, timeout);
25572565
else
25582566
break;
25592567

25602568
if (sockstate == SOCKET_HAS_TIMED_OUT) {
2561-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2569+
if (err.ssl == SSL_ERROR_WANT_READ)
25622570
PyErr_SetString(PySocketModule.timeout_error,
25632571
"The read operation timed out");
25642572
else
@@ -2576,9 +2584,9 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
25762584
break;
25772585
}
25782586

2579-
if (err < 0) {
2587+
if (err.ssl < 0) {
25802588
Py_XDECREF(sock);
2581-
return PySSL_SetError(self, err, __FILE__, __LINE__);
2589+
return PySSL_SetError(self, err.ssl, __FILE__, __LINE__);
25822590
}
25832591
if (sock)
25842592
/* It's already INCREF'ed */

0 commit comments

Comments
 (0)