Commit 14690708 authored by Victor Stinner's avatar Victor Stinner

Issue #23853: Methods of SSL socket don't reset the socket timeout anymore each

time bytes are received or sent. The socket timeout is now the maximum total
duration of the method.

This change fixes a denial of service if the application is regulary
interrupted by a signal and the signal handler does not raise an exception.
parent 222dfc7d
...@@ -830,6 +830,11 @@ SSL Sockets ...@@ -830,6 +830,11 @@ SSL Sockets
.. versionchanged:: 3.5 .. versionchanged:: 3.5
The :meth:`sendfile` method was added. The :meth:`sendfile` method was added.
.. versionchanged:: 3.5
The :meth:`shutdown` does not reset the socket timeout each time bytes
are received or sent. The socket timeout is now to maximum total duration
of the shutdown.
SSL sockets also have the following additional methods and attributes: SSL sockets also have the following additional methods and attributes:
...@@ -845,6 +850,11 @@ SSL sockets also have the following additional methods and attributes: ...@@ -845,6 +850,11 @@ SSL sockets also have the following additional methods and attributes:
As at any time a re-negotiation is possible, a call to :meth:`read` can also As at any time a re-negotiation is possible, a call to :meth:`read` can also
cause write operations. cause write operations.
.. versionchanged:: 3.5
The socket timeout is no more reset each time bytes are received or sent.
The socket timeout is now to maximum total duration to read up to *len*
bytes.
.. method:: SSLSocket.write(buf) .. method:: SSLSocket.write(buf)
Write *buf* to the SSL socket and return the number of bytes written. The Write *buf* to the SSL socket and return the number of bytes written. The
...@@ -856,6 +866,10 @@ SSL sockets also have the following additional methods and attributes: ...@@ -856,6 +866,10 @@ SSL sockets also have the following additional methods and attributes:
As at any time a re-negotiation is possible, a call to :meth:`write` can As at any time a re-negotiation is possible, a call to :meth:`write` can
also cause read operations. also cause read operations.
.. versionchanged:: 3.5
The socket timeout is no more reset each time bytes are received or sent.
The socket timeout is now to maximum total duration to write *buf*.
.. note:: .. note::
The :meth:`~SSLSocket.read` and :meth:`~SSLSocket.write` methods are the The :meth:`~SSLSocket.read` and :meth:`~SSLSocket.write` methods are the
...@@ -877,6 +891,10 @@ SSL sockets also have the following additional methods and attributes: ...@@ -877,6 +891,10 @@ SSL sockets also have the following additional methods and attributes:
:attr:`~SSLContext.check_hostname` attribute of the socket's :attr:`~SSLContext.check_hostname` attribute of the socket's
:attr:`~SSLSocket.context` is true. :attr:`~SSLSocket.context` is true.
.. versionchanged:: 3.5
The socket timeout is no more reset each time bytes are received or sent.
The socket timeout is now to maximum total duration of the handshake.
.. method:: SSLSocket.getpeercert(binary_form=False) .. method:: SSLSocket.getpeercert(binary_form=False)
If there is no certificate for the peer on the other end of the connection, If there is no certificate for the peer on the other end of the connection,
......
...@@ -223,7 +223,7 @@ static PyTypeObject PySSLMemoryBIO_Type; ...@@ -223,7 +223,7 @@ static PyTypeObject PySSLMemoryBIO_Type;
static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args); static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args);
static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args); static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args);
static int PySSL_select(PySocketSockObject *s, int writing); static int PySSL_select(PySocketSockObject *s, int writing, _PyTime_t timeout);
static PyObject *PySSL_peercert(PySSLSocket *self, PyObject *args); static PyObject *PySSL_peercert(PySSLSocket *self, PyObject *args);
static PyObject *PySSL_cipher(PySSLSocket *self); static PyObject *PySSL_cipher(PySSLSocket *self);
...@@ -248,6 +248,10 @@ typedef enum { ...@@ -248,6 +248,10 @@ typedef enum {
#define GET_SOCKET(obj) ((obj)->Socket ? \ #define GET_SOCKET(obj) ((obj)->Socket ? \
(PySocketSockObject *) PyWeakref_GetObject((obj)->Socket) : NULL) (PySocketSockObject *) PyWeakref_GetObject((obj)->Socket) : NULL)
/* If sock is NULL, use a timeout of 0 second */
#define GET_SOCKET_TIMEOUT(sock) \
((sock != NULL) ? (sock)->sock_timeout : 0)
/* /*
* SSL errors. * SSL errors.
*/ */
...@@ -565,6 +569,8 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self) ...@@ -565,6 +569,8 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self)
int err; int err;
int sockstate, nonblocking; int sockstate, nonblocking;
PySocketSockObject *sock = GET_SOCKET(self); PySocketSockObject *sock = GET_SOCKET(self);
_PyTime_t timeout, deadline = 0;
int has_timeout;
if (sock) { if (sock) {
if (((PyObject*)sock) == Py_None) { if (((PyObject*)sock) == Py_None) {
...@@ -580,6 +586,11 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self) ...@@ -580,6 +586,11 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self)
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking); BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
} }
timeout = GET_SOCKET_TIMEOUT(sock);
has_timeout = (timeout > 0);
if (has_timeout)
deadline = _PyTime_GetMonotonicClock() + timeout;
/* Actually negotiate SSL connection */ /* Actually negotiate SSL connection */
/* XXX If SSL_do_handshake() returns 0, it's also a failure. */ /* XXX If SSL_do_handshake() returns 0, it's also a failure. */
do { do {
...@@ -591,10 +602,13 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self) ...@@ -591,10 +602,13 @@ static PyObject *PySSL_SSLdo_handshake(PySSLSocket *self)
if (PyErr_CheckSignals()) if (PyErr_CheckSignals())
goto error; goto error;
if (has_timeout)
timeout = deadline - _PyTime_GetMonotonicClock();
if (err == SSL_ERROR_WANT_READ) { if (err == SSL_ERROR_WANT_READ) {
sockstate = PySSL_select(sock, 0); sockstate = PySSL_select(sock, 0, timeout);
} else if (err == SSL_ERROR_WANT_WRITE) { } else if (err == SSL_ERROR_WANT_WRITE) {
sockstate = PySSL_select(sock, 1); sockstate = PySSL_select(sock, 1, timeout);
} else { } else {
sockstate = SOCKET_OPERATION_OK; sockstate = SOCKET_OPERATION_OK;
} }
...@@ -1611,7 +1625,7 @@ static void PySSL_dealloc(PySSLSocket *self) ...@@ -1611,7 +1625,7 @@ static void PySSL_dealloc(PySSLSocket *self)
*/ */
static int static int
PySSL_select(PySocketSockObject *s, int writing) PySSL_select(PySocketSockObject *s, int writing, _PyTime_t timeout)
{ {
int rc; int rc;
#ifdef HAVE_POLL #ifdef HAVE_POLL
...@@ -1624,10 +1638,14 @@ PySSL_select(PySocketSockObject *s, int writing) ...@@ -1624,10 +1638,14 @@ PySSL_select(PySocketSockObject *s, int writing)
#endif #endif
/* Nothing to do unless we're in timeout mode (not non-blocking) */ /* Nothing to do unless we're in timeout mode (not non-blocking) */
if ((s == NULL) || (s->sock_timeout == 0)) if ((s == NULL) || (timeout == 0))
return SOCKET_IS_NONBLOCKING; return SOCKET_IS_NONBLOCKING;
else if (s->sock_timeout < 0) else if (timeout < 0) {
if (s->sock_timeout > 0)
return SOCKET_HAS_TIMED_OUT;
else
return SOCKET_IS_BLOCKING; return SOCKET_IS_BLOCKING;
}
/* Guard against closed socket */ /* Guard against closed socket */
if (s->sock_fd < 0) if (s->sock_fd < 0)
...@@ -1639,8 +1657,8 @@ PySSL_select(PySocketSockObject *s, int writing) ...@@ -1639,8 +1657,8 @@ PySSL_select(PySocketSockObject *s, int writing)
pollfd.fd = s->sock_fd; pollfd.fd = s->sock_fd;
pollfd.events = writing ? POLLOUT : POLLIN; pollfd.events = writing ? POLLOUT : POLLIN;
/* s->sock_timeout is in seconds, timeout in ms */ /* timeout is in seconds, poll() uses milliseconds */
ms = (int)_PyTime_AsMilliseconds(s->sock_timeout, _PyTime_ROUND_CEILING); ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
assert(ms <= INT_MAX); assert(ms <= INT_MAX);
PySSL_BEGIN_ALLOW_THREADS PySSL_BEGIN_ALLOW_THREADS
...@@ -1651,7 +1669,7 @@ PySSL_select(PySocketSockObject *s, int writing) ...@@ -1651,7 +1669,7 @@ PySSL_select(PySocketSockObject *s, int writing)
if (!_PyIsSelectable_fd(s->sock_fd)) if (!_PyIsSelectable_fd(s->sock_fd))
return SOCKET_TOO_LARGE_FOR_SELECT; return SOCKET_TOO_LARGE_FOR_SELECT;
_PyTime_AsTimeval_noraise(s->sock_timeout, &tv, _PyTime_ROUND_CEILING); _PyTime_AsTimeval_noraise(timeout, &tv, _PyTime_ROUND_CEILING);
FD_ZERO(&fds); FD_ZERO(&fds);
FD_SET(s->sock_fd, &fds); FD_SET(s->sock_fd, &fds);
...@@ -1679,6 +1697,8 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args) ...@@ -1679,6 +1697,8 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args)
int err; int err;
int nonblocking; int nonblocking;
PySocketSockObject *sock = GET_SOCKET(self); PySocketSockObject *sock = GET_SOCKET(self);
_PyTime_t timeout, deadline = 0;
int has_timeout;
if (sock != NULL) { if (sock != NULL) {
if (((PyObject*)sock) == Py_None) { if (((PyObject*)sock) == Py_None) {
...@@ -1707,7 +1727,12 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args) ...@@ -1707,7 +1727,12 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args)
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking); BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
} }
sockstate = PySSL_select(sock, 1); timeout = GET_SOCKET_TIMEOUT(sock);
has_timeout = (timeout > 0);
if (has_timeout)
deadline = _PyTime_GetMonotonicClock() + timeout;
sockstate = PySSL_select(sock, 1, timeout);
if (sockstate == SOCKET_HAS_TIMED_OUT) { if (sockstate == SOCKET_HAS_TIMED_OUT) {
PyErr_SetString(PySocketModule.timeout_error, PyErr_SetString(PySocketModule.timeout_error,
"The write operation timed out"); "The write operation timed out");
...@@ -1731,10 +1756,13 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args) ...@@ -1731,10 +1756,13 @@ static PyObject *PySSL_SSLwrite(PySSLSocket *self, PyObject *args)
if (PyErr_CheckSignals()) if (PyErr_CheckSignals())
goto error; goto error;
if (has_timeout)
timeout = deadline - _PyTime_GetMonotonicClock();
if (err == SSL_ERROR_WANT_READ) { if (err == SSL_ERROR_WANT_READ) {
sockstate = PySSL_select(sock, 0); sockstate = PySSL_select(sock, 0, timeout);
} else if (err == SSL_ERROR_WANT_WRITE) { } else if (err == SSL_ERROR_WANT_WRITE) {
sockstate = PySSL_select(sock, 1); sockstate = PySSL_select(sock, 1, timeout);
} else { } else {
sockstate = SOCKET_OPERATION_OK; sockstate = SOCKET_OPERATION_OK;
} }
...@@ -1801,6 +1829,8 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args) ...@@ -1801,6 +1829,8 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args)
int err; int err;
int nonblocking; int nonblocking;
PySocketSockObject *sock = GET_SOCKET(self); PySocketSockObject *sock = GET_SOCKET(self);
_PyTime_t timeout, deadline = 0;
int has_timeout;
if (sock != NULL) { if (sock != NULL) {
if (((PyObject*)sock) == Py_None) { if (((PyObject*)sock) == Py_None) {
...@@ -1842,6 +1872,11 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args) ...@@ -1842,6 +1872,11 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args)
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking); BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
} }
timeout = GET_SOCKET_TIMEOUT(sock);
has_timeout = (timeout > 0);
if (has_timeout)
deadline = _PyTime_GetMonotonicClock() + timeout;
do { do {
PySSL_BEGIN_ALLOW_THREADS PySSL_BEGIN_ALLOW_THREADS
count = SSL_read(self->ssl, mem, len); count = SSL_read(self->ssl, mem, len);
...@@ -1851,10 +1886,13 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args) ...@@ -1851,10 +1886,13 @@ static PyObject *PySSL_SSLread(PySSLSocket *self, PyObject *args)
if (PyErr_CheckSignals()) if (PyErr_CheckSignals())
goto error; goto error;
if (has_timeout)
timeout = deadline - _PyTime_GetMonotonicClock();
if (err == SSL_ERROR_WANT_READ) { if (err == SSL_ERROR_WANT_READ) {
sockstate = PySSL_select(sock, 0); sockstate = PySSL_select(sock, 0, timeout);
} else if (err == SSL_ERROR_WANT_WRITE) { } else if (err == SSL_ERROR_WANT_WRITE) {
sockstate = PySSL_select(sock, 1); sockstate = PySSL_select(sock, 1, timeout);
} else if (err == SSL_ERROR_ZERO_RETURN && } else if (err == SSL_ERROR_ZERO_RETURN &&
SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN) SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN)
{ {
...@@ -1908,6 +1946,8 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self) ...@@ -1908,6 +1946,8 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self)
int err, ssl_err, sockstate, nonblocking; int err, ssl_err, sockstate, nonblocking;
int zeros = 0; int zeros = 0;
PySocketSockObject *sock = GET_SOCKET(self); PySocketSockObject *sock = GET_SOCKET(self);
_PyTime_t timeout, deadline = 0;
int has_timeout;
if (sock != NULL) { if (sock != NULL) {
/* Guard against closed socket */ /* Guard against closed socket */
...@@ -1924,6 +1964,11 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self) ...@@ -1924,6 +1964,11 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self)
BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking); BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
} }
timeout = GET_SOCKET_TIMEOUT(sock);
has_timeout = (timeout > 0);
if (has_timeout)
deadline = _PyTime_GetMonotonicClock() + timeout;
while (1) { while (1) {
PySSL_BEGIN_ALLOW_THREADS PySSL_BEGIN_ALLOW_THREADS
/* Disable read-ahead so that unwrap can work correctly. /* Disable read-ahead so that unwrap can work correctly.
...@@ -1953,12 +1998,15 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self) ...@@ -1953,12 +1998,15 @@ static PyObject *PySSL_SSLshutdown(PySSLSocket *self)
continue; continue;
} }
if (has_timeout)
timeout = deadline - _PyTime_GetMonotonicClock();
/* Possibly retry shutdown until timeout or failure */ /* Possibly retry shutdown until timeout or failure */
ssl_err = SSL_get_error(self->ssl, err); ssl_err = SSL_get_error(self->ssl, err);
if (ssl_err == SSL_ERROR_WANT_READ) if (ssl_err == SSL_ERROR_WANT_READ)
sockstate = PySSL_select(sock, 0); sockstate = PySSL_select(sock, 0, timeout);
else if (ssl_err == SSL_ERROR_WANT_WRITE) else if (ssl_err == SSL_ERROR_WANT_WRITE)
sockstate = PySSL_select(sock, 1); sockstate = PySSL_select(sock, 1, timeout);
else else
break; break;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment