[Python-checkins] cpython: Issue #23853: Methods of SSL socket don't reset the socket timeout anymore each

victor.stinner python-checkins at python.org
Mon Apr 6 23:25:15 CEST 2015


https://hg.python.org/cpython/rev/5983f3fdacdb
changeset:   95461:5983f3fdacdb
user:        Victor Stinner <victor.stinner at gmail.com>
date:        Mon Apr 06 22:46:13 2015 +0200
summary:
  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.

files:
  Doc/library/ssl.rst |  18 +++++++
  Modules/_ssl.c      |  82 ++++++++++++++++++++++++++------
  2 files changed, 83 insertions(+), 17 deletions(-)


diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -830,6 +830,11 @@
    .. versionchanged:: 3.5
       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:
 
@@ -845,6 +850,11 @@
    As at any time a re-negotiation is possible, a call to :meth:`read` can also
    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)
 
    Write *buf* to the SSL socket and return the number of bytes written. The
@@ -856,6 +866,10 @@
    As at any time a re-negotiation is possible, a call to :meth:`write` can
    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::
 
    The :meth:`~SSLSocket.read` and :meth:`~SSLSocket.write` methods are the
@@ -877,6 +891,10 @@
       :attr:`~SSLContext.check_hostname` attribute of the socket's
       :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)
 
    If there is no certificate for the peer on the other end of the connection,
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -223,7 +223,7 @@
 
 static PyObject *PySSL_SSLwrite(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_cipher(PySSLSocket *self);
 
@@ -248,6 +248,10 @@
 #define GET_SOCKET(obj) ((obj)->Socket ? \
     (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.
  */
@@ -565,6 +569,8 @@
     int err;
     int sockstate, nonblocking;
     PySocketSockObject *sock = GET_SOCKET(self);
+    _PyTime_t timeout, deadline = 0;
+    int has_timeout;
 
     if (sock) {
         if (((PyObject*)sock) == Py_None) {
@@ -580,6 +586,11 @@
         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 */
     /* XXX If SSL_do_handshake() returns 0, it's also a failure. */
     do {
@@ -591,10 +602,13 @@
         if (PyErr_CheckSignals())
             goto error;
 
+        if (has_timeout)
+            timeout = deadline - _PyTime_GetMonotonicClock();
+
         if (err == SSL_ERROR_WANT_READ) {
-            sockstate = PySSL_select(sock, 0);
+            sockstate = PySSL_select(sock, 0, timeout);
         } else if (err == SSL_ERROR_WANT_WRITE) {
-            sockstate = PySSL_select(sock, 1);
+            sockstate = PySSL_select(sock, 1, timeout);
         } else {
             sockstate = SOCKET_OPERATION_OK;
         }
@@ -1611,7 +1625,7 @@
  */
 
 static int
-PySSL_select(PySocketSockObject *s, int writing)
+PySSL_select(PySocketSockObject *s, int writing, _PyTime_t timeout)
 {
     int rc;
 #ifdef HAVE_POLL
@@ -1624,10 +1638,14 @@
 #endif
 
     /* 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;
-    else if (s->sock_timeout < 0)
-        return SOCKET_IS_BLOCKING;
+    else if (timeout < 0) {
+        if (s->sock_timeout > 0)
+            return SOCKET_HAS_TIMED_OUT;
+        else
+            return SOCKET_IS_BLOCKING;
+    }
 
     /* Guard against closed socket */
     if (s->sock_fd < 0)
@@ -1639,8 +1657,8 @@
     pollfd.fd = s->sock_fd;
     pollfd.events = writing ? POLLOUT : POLLIN;
 
-    /* s->sock_timeout is in seconds, timeout in ms */
-    ms = (int)_PyTime_AsMilliseconds(s->sock_timeout, _PyTime_ROUND_CEILING);
+    /* timeout is in seconds, poll() uses milliseconds */
+    ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
     assert(ms <= INT_MAX);
 
     PySSL_BEGIN_ALLOW_THREADS
@@ -1651,7 +1669,7 @@
     if (!_PyIsSelectable_fd(s->sock_fd))
         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_SET(s->sock_fd, &fds);
@@ -1679,6 +1697,8 @@
     int err;
     int nonblocking;
     PySocketSockObject *sock = GET_SOCKET(self);
+    _PyTime_t timeout, deadline = 0;
+    int has_timeout;
 
     if (sock != NULL) {
         if (((PyObject*)sock) == Py_None) {
@@ -1707,7 +1727,12 @@
         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) {
         PyErr_SetString(PySocketModule.timeout_error,
                         "The write operation timed out");
@@ -1731,10 +1756,13 @@
         if (PyErr_CheckSignals())
             goto error;
 
+        if (has_timeout)
+            timeout = deadline - _PyTime_GetMonotonicClock();
+
         if (err == SSL_ERROR_WANT_READ) {
-            sockstate = PySSL_select(sock, 0);
+            sockstate = PySSL_select(sock, 0, timeout);
         } else if (err == SSL_ERROR_WANT_WRITE) {
-            sockstate = PySSL_select(sock, 1);
+            sockstate = PySSL_select(sock, 1, timeout);
         } else {
             sockstate = SOCKET_OPERATION_OK;
         }
@@ -1801,6 +1829,8 @@
     int err;
     int nonblocking;
     PySocketSockObject *sock = GET_SOCKET(self);
+    _PyTime_t timeout, deadline = 0;
+    int has_timeout;
 
     if (sock != NULL) {
         if (((PyObject*)sock) == Py_None) {
@@ -1842,6 +1872,11 @@
         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 {
         PySSL_BEGIN_ALLOW_THREADS
         count = SSL_read(self->ssl, mem, len);
@@ -1851,10 +1886,13 @@
         if (PyErr_CheckSignals())
             goto error;
 
+        if (has_timeout)
+            timeout = deadline - _PyTime_GetMonotonicClock();
+
         if (err == SSL_ERROR_WANT_READ) {
-            sockstate = PySSL_select(sock, 0);
+            sockstate = PySSL_select(sock, 0, timeout);
         } 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 &&
                    SSL_get_shutdown(self->ssl) == SSL_RECEIVED_SHUTDOWN)
         {
@@ -1908,6 +1946,8 @@
     int err, ssl_err, sockstate, nonblocking;
     int zeros = 0;
     PySocketSockObject *sock = GET_SOCKET(self);
+    _PyTime_t timeout, deadline = 0;
+    int has_timeout;
 
     if (sock != NULL) {
         /* Guard against closed socket */
@@ -1924,6 +1964,11 @@
         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) {
         PySSL_BEGIN_ALLOW_THREADS
         /* Disable read-ahead so that unwrap can work correctly.
@@ -1953,12 +1998,15 @@
             continue;
         }
 
+        if (has_timeout)
+            timeout = deadline - _PyTime_GetMonotonicClock();
+
         /* Possibly retry shutdown until timeout or failure */
         ssl_err = SSL_get_error(self->ssl, err);
         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)
-            sockstate = PySSL_select(sock, 1);
+            sockstate = PySSL_select(sock, 1, timeout);
         else
             break;
 

-- 
Repository URL: https://hg.python.org/cpython


More information about the Python-checkins mailing list