Skip to content

Commit 1a107ee

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

File tree

2 files changed

+68
-59
lines changed

2 files changed

+68
-59
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: 67 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,14 @@ typedef struct {
325325
int check_hostname;
326326
} PySSLContext;
327327

328+
typedef struct {
329+
int ssl; /* last seen error from SSL */
330+
int c; /* last seen error from libc */
331+
#ifdef MS_WINDOWS
332+
int ws; /* last seen error from winsock */
333+
#endif
334+
} _PySSLError;
335+
328336
typedef struct {
329337
PyObject_HEAD
330338
PyObject *Socket; /* weakref to socket on which we're layered */
@@ -334,11 +342,7 @@ typedef struct {
334342
enum py_ssl_server_or_client socket_type;
335343
PyObject *owner; /* Python level "owner" passed to servername callback */
336344
PyObject *server_hostname;
337-
int ssl_errno; /* last seen error from SSL */
338-
int c_errno; /* last seen error from libc */
339-
#ifdef MS_WINDOWS
340-
int ws_errno; /* last seen error from winsock */
341-
#endif
345+
_PySSLError err; /* last seen error from various sources */
342346
} PySSLSocket;
343347

344348
typedef struct {
@@ -358,19 +362,18 @@ static PyTypeObject PySSLSocket_Type;
358362
static PyTypeObject PySSLMemoryBIO_Type;
359363
static PyTypeObject PySSLSession_Type;
360364

365+
static inline _PySSLError _PySSL_errno(int failed, const SSL *ssl, int retcode)
366+
{
367+
_PySSLError err = { 0 };
368+
if (failed) {
361369
#ifdef MS_WINDOWS
362-
#define _PySSL_UPDATE_ERRNO_IF(cond, sock, retcode) if (cond) { \
363-
(sock)->ws_errno = WSAGetLastError(); \
364-
(sock)->c_errno = errno; \
365-
(sock)->ssl_errno = SSL_get_error((sock->ssl), (retcode)); \
366-
} else { sock->ws_errno = 0; sock->c_errno = 0; sock->ssl_errno = 0; }
367-
#else
368-
#define _PySSL_UPDATE_ERRNO_IF(cond, sock, retcode) if (cond) { \
369-
(sock)->c_errno = errno; \
370-
(sock)->ssl_errno = SSL_get_error((sock->ssl), (retcode)); \
371-
} else { (sock)->c_errno = 0; (sock)->ssl_errno = 0; }
370+
err.ws = WSAGetLastError();
372371
#endif
373-
#define _PySSL_UPDATE_ERRNO(sock, retcode) _PySSL_UPDATE_ERRNO_IF(1, (sock), (retcode))
372+
err.c = errno;
373+
err.ssl = SSL_get_error(ssl, retcode);
374+
}
375+
return err;
376+
}
374377

375378
/*[clinic input]
376379
module _ssl
@@ -537,17 +540,17 @@ PySSL_SetError(PySSLSocket *obj, int ret, const char *filename, int lineno)
537540
{
538541
PyObject *type = PySSLErrorObject;
539542
char *errstr = NULL;
540-
int err;
543+
_PySSLError err;
541544
enum py_ssl_error p = PY_SSL_ERROR_NONE;
542545
unsigned long e = 0;
543546

544547
assert(ret <= 0);
545548
e = ERR_peek_last_error();
546549

547550
if (obj->ssl != NULL) {
548-
err = obj->ssl_errno;
551+
err = obj->err;
549552

550-
switch (err) {
553+
switch (err.ssl) {
551554
case SSL_ERROR_ZERO_RETURN:
552555
errstr = "TLS/SSL connection has been closed (EOF)";
553556
type = PySSLZeroReturnErrorObject;
@@ -583,11 +586,12 @@ PySSL_SetError(PySSLSocket *obj, int ret, const char *filename, int lineno)
583586
/* underlying BIO reported an I/O error */
584587
ERR_clear_error();
585588
#ifdef MS_WINDOWS
586-
if (obj->ws_errno)
587-
return PyErr_SetFromWindowsErr(obj->ws_errno);
589+
if (err.ws) {
590+
return PyErr_SetFromWindowsErr(err.ws);
591+
}
588592
#endif
589-
if (obj->c_errno) {
590-
errno = obj->c_errno;
593+
if (err.c) {
594+
errno = err.c;
591595
return PyErr_SetFromErrno(PyExc_OSError);
592596
}
593597
Py_INCREF(s);
@@ -647,6 +651,7 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
647651
PySSLSocket *self;
648652
SSL_CTX *ctx = sslctx->ctx;
649653
long mode;
654+
_PySSLError err = { 0 };
650655

651656
self = PyObject_New(PySSLSocket, &PySSLSocket_Type);
652657
if (self == NULL)
@@ -668,11 +673,7 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
668673
}
669674
self->server_hostname = hostname;
670675
}
671-
self->ssl_errno = 0;
672-
self->c_errno = 0;
673-
#ifdef MS_WINDOWS
674-
self->ws_errno = 0;
675-
#endif
676+
self->err = err;
676677

677678
/* Make sure the SSL error state is initialized */
678679
(void) ERR_get_state();
@@ -773,7 +774,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
773774
/*[clinic end generated code: output=6c0898a8936548f6 input=d2d737de3df018c8]*/
774775
{
775776
int ret;
776-
int err;
777+
_PySSLError err;
777778
int sockstate, nonblocking;
778779
PySocketSockObject *sock = GET_SOCKET(self);
779780
_PyTime_t timeout, deadline = 0;
@@ -803,19 +804,19 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
803804
do {
804805
PySSL_BEGIN_ALLOW_THREADS
805806
ret = SSL_do_handshake(self->ssl);
806-
_PySSL_UPDATE_ERRNO_IF(ret < 1, self, ret);
807+
err = _PySSL_errno(ret < 1, self->ssl, ret);
807808
PySSL_END_ALLOW_THREADS
808-
err = self->ssl_errno;
809+
self->err = err;
809810

810811
if (PyErr_CheckSignals())
811812
goto error;
812813

813814
if (has_timeout)
814815
timeout = deadline - _PyTime_GetMonotonicClock();
815816

816-
if (err == SSL_ERROR_WANT_READ) {
817+
if (err.ssl == SSL_ERROR_WANT_READ) {
817818
sockstate = PySSL_select(sock, 0, timeout);
818-
} else if (err == SSL_ERROR_WANT_WRITE) {
819+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
819820
sockstate = PySSL_select(sock, 1, timeout);
820821
} else {
821822
sockstate = SOCKET_OPERATION_OK;
@@ -836,7 +837,8 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
836837
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
837838
break;
838839
}
839-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
840+
} while (err.ssl == SSL_ERROR_WANT_READ ||
841+
err.ssl == SSL_ERROR_WANT_WRITE);
840842
Py_XDECREF(sock);
841843
if (ret < 1)
842844
return PySSL_SetError(self, ret, __FILE__, __LINE__);
@@ -2050,7 +2052,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
20502052
{
20512053
int len;
20522054
int sockstate;
2053-
int err;
2055+
_PySSLError err;
20542056
int nonblocking;
20552057
PySocketSockObject *sock = GET_SOCKET(self);
20562058
_PyTime_t timeout, deadline = 0;
@@ -2101,19 +2103,19 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
21012103
do {
21022104
PySSL_BEGIN_ALLOW_THREADS
21032105
len = SSL_write(self->ssl, b->buf, (int)b->len);
2104-
_PySSL_UPDATE_ERRNO_IF(len <= 0, self, len);
2106+
err = _PySSL_errno(len <= 0, self->ssl, len);
21052107
PySSL_END_ALLOW_THREADS
2106-
err = self->ssl_errno;
2108+
self->err = err;
21072109

21082110
if (PyErr_CheckSignals())
21092111
goto error;
21102112

21112113
if (has_timeout)
21122114
timeout = deadline - _PyTime_GetMonotonicClock();
21132115

2114-
if (err == SSL_ERROR_WANT_READ) {
2116+
if (err.ssl == SSL_ERROR_WANT_READ) {
21152117
sockstate = PySSL_select(sock, 0, timeout);
2116-
} else if (err == SSL_ERROR_WANT_WRITE) {
2118+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
21172119
sockstate = PySSL_select(sock, 1, timeout);
21182120
} else {
21192121
sockstate = SOCKET_OPERATION_OK;
@@ -2130,7 +2132,8 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
21302132
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
21312133
break;
21322134
}
2133-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2135+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2136+
err.ssl == SSL_ERROR_WANT_WRITE);
21342137

21352138
Py_XDECREF(sock);
21362139
if (len > 0)
@@ -2154,11 +2157,14 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
21542157
/*[clinic end generated code: output=983d9fecdc308a83 input=2b77487d6dfd597f]*/
21552158
{
21562159
int count = 0;
2160+
_PySSLError err;
21572161

21582162
PySSL_BEGIN_ALLOW_THREADS
21592163
count = SSL_pending(self->ssl);
2160-
_PySSL_UPDATE_ERRNO_IF(count < 0, self, count);
2164+
err = _PySSL_errno(count < 0, self->ssl, count);
21612165
PySSL_END_ALLOW_THREADS
2166+
self->err = err;
2167+
21622168
if (count < 0)
21632169
return PySSL_SetError(self, count, __FILE__, __LINE__);
21642170
else
@@ -2185,7 +2191,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
21852191
char *mem;
21862192
int count;
21872193
int sockstate;
2188-
int err;
2194+
_PySSLError err;
21892195
int nonblocking;
21902196
PySocketSockObject *sock = GET_SOCKET(self);
21912197
_PyTime_t timeout, deadline = 0;
@@ -2246,21 +2252,21 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
22462252
do {
22472253
PySSL_BEGIN_ALLOW_THREADS
22482254
count = SSL_read(self->ssl, mem, len);
2249-
_PySSL_UPDATE_ERRNO_IF(count <= 0, self, count);
2255+
err = _PySSL_errno(count <= 0, self->ssl, count);
22502256
PySSL_END_ALLOW_THREADS
2257+
self->err = err;
22512258

22522259
if (PyErr_CheckSignals())
22532260
goto error;
22542261

22552262
if (has_timeout)
22562263
timeout = deadline - _PyTime_GetMonotonicClock();
22572264

2258-
err = self->ssl_errno;
2259-
if (err == SSL_ERROR_WANT_READ) {
2265+
if (err.ssl == SSL_ERROR_WANT_READ) {
22602266
sockstate = PySSL_select(sock, 0, timeout);
2261-
} else if (err == SSL_ERROR_WANT_WRITE) {
2267+
} else if (err.ssl == SSL_ERROR_WANT_WRITE) {
22622268
sockstate = PySSL_select(sock, 1, timeout);
2263-
} else if (err == SSL_ERROR_ZERO_RETURN &&
2269+
} else if (err.ssl == SSL_ERROR_ZERO_RETURN &&
22642270
SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN)
22652271
{
22662272
count = 0;
@@ -2276,7 +2282,8 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, int len, int group_right_1,
22762282
} else if (sockstate == SOCKET_IS_NONBLOCKING) {
22772283
break;
22782284
}
2279-
} while (err == SSL_ERROR_WANT_READ || err == SSL_ERROR_WANT_WRITE);
2285+
} while (err.ssl == SSL_ERROR_WANT_READ ||
2286+
err.ssl == SSL_ERROR_WANT_WRITE);
22802287

22812288
if (count <= 0) {
22822289
PySSL_SetError(self, count, __FILE__, __LINE__);
@@ -2312,7 +2319,8 @@ static PyObject *
23122319
_ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
23132320
/*[clinic end generated code: output=ca1aa7ed9d25ca42 input=ede2cc1a2ddf0ee4]*/
23142321
{
2315-
int err, sockstate, nonblocking;
2322+
_PySSLError err;
2323+
int sockstate, nonblocking, ret;
23162324
int zeros = 0;
23172325
PySocketSockObject *sock = GET_SOCKET(self);
23182326
_PyTime_t timeout, deadline = 0;
@@ -2350,14 +2358,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
23502358
*/
23512359
if (self->shutdown_seen_zero)
23522360
SSL_set_read_ahead(self->ssl, 0);
2353-
err = SSL_shutdown(self->ssl);
2354-
_PySSL_UPDATE_ERRNO_IF(err < 0, self, err);
2361+
ret = SSL_shutdown(self->ssl);
2362+
err = _PySSL_errno(ret < 0, self->ssl, ret);
23552363
PySSL_END_ALLOW_THREADS
2364+
self->err = err;
23562365

23572366
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
2358-
if (err > 0)
2367+
if (ret > 0)
23592368
break;
2360-
if (err == 0) {
2369+
if (ret == 0) {
23612370
/* Don't loop endlessly; instead preserve legacy
23622371
behaviour of trying SSL_shutdown() only twice.
23632372
This looks necessary for OpenSSL < 0.9.8m */
@@ -2372,16 +2381,15 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
23722381
timeout = deadline - _PyTime_GetMonotonicClock();
23732382

23742383
/* Possibly retry shutdown until timeout or failure */
2375-
_PySSL_UPDATE_ERRNO(self, err);
2376-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2384+
if (err.ssl == SSL_ERROR_WANT_READ)
23772385
sockstate = PySSL_select(sock, 0, timeout);
2378-
else if (self->ssl_errno == SSL_ERROR_WANT_WRITE)
2386+
else if (err.ssl == SSL_ERROR_WANT_WRITE)
23792387
sockstate = PySSL_select(sock, 1, timeout);
23802388
else
23812389
break;
23822390

23832391
if (sockstate == SOCKET_HAS_TIMED_OUT) {
2384-
if (self->ssl_errno == SSL_ERROR_WANT_READ)
2392+
if (err.ssl == SSL_ERROR_WANT_READ)
23852393
PyErr_SetString(PySocketModule.timeout_error,
23862394
"The read operation timed out");
23872395
else
@@ -2399,9 +2407,9 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
23992407
break;
24002408
}
24012409

2402-
if (err < 0) {
2410+
if (err.ssl < 0) {
24032411
Py_XDECREF(sock);
2404-
return PySSL_SetError(self, err, __FILE__, __LINE__);
2412+
return PySSL_SetError(self, err.ssl, __FILE__, __LINE__);
24052413
}
24062414
if (sock)
24072415
/* It's already INCREF'ed */

0 commit comments

Comments
 (0)