]> granicus.if.org Git - python/commitdiff
Merged revisions 80451-80452 via svnmerge from
authorAntoine Pitrou <solipsis@pitrou.net>
Sat, 24 Apr 2010 21:26:44 +0000 (21:26 +0000)
committerAntoine Pitrou <solipsis@pitrou.net>
Sat, 24 Apr 2010 21:26:44 +0000 (21:26 +0000)
svn+ssh://pythondev@svn.python.org/python/trunk

........
  r80451 | antoine.pitrou | 2010-04-24 21:57:01 +0200 (sam., 24 avril 2010) | 4 lines

  The do_handshake() method of SSL objects now adjusts the blocking mode of
  the SSL structure if necessary (as other methods already do).
........
  r80452 | antoine.pitrou | 2010-04-24 22:04:58 +0200 (sam., 24 avril 2010) | 4 lines

  Issue #5103: SSL handshake would ignore the socket timeout and block
  indefinitely if the other end didn't respond.
........

Lib/test/test_poplib.py
Lib/test/test_ssl.py
Misc/NEWS
Modules/_ssl.c

index 27e3a9fb567ada623c65b066da611af5cd4591e3..aa703c50bd63edaab6057393e762f1774f591a2e 100644 (file)
@@ -10,6 +10,7 @@ import asynchat
 import socket
 import os
 import time
+import errno
 
 from unittest import TestCase
 from test import support as test_support
@@ -241,13 +242,39 @@ if hasattr(poplib, 'POP3_SSL'):
         def __init__(self, conn):
             asynchat.async_chat.__init__(self, conn)
             ssl_socket = ssl.wrap_socket(self.socket, certfile=CERTFILE,
-                                          server_side=True)
+                                          server_side=True,
+                                          do_handshake_on_connect=False)
             self.del_channel()
             self.set_socket(ssl_socket)
+            # Must try handshake before calling push()
+            self._ssl_accepting = True
+            self._do_ssl_handshake()
             self.set_terminator(b"\r\n")
             self.in_buffer = []
             self.push('+OK dummy pop3 server ready. <timestamp>')
 
+        def _do_ssl_handshake(self):
+            try:
+                self.socket.do_handshake()
+            except ssl.SSLError as err:
+                if err.args[0] in (ssl.SSL_ERROR_WANT_READ,
+                                   ssl.SSL_ERROR_WANT_WRITE):
+                    return
+                elif err.args[0] == ssl.SSL_ERROR_EOF:
+                    return self.handle_close()
+                raise
+            except socket.error as err:
+                if err.args[0] == errno.ECONNABORTED:
+                    return self.handle_close()
+            else:
+                self._ssl_accepting = False
+
+        def handle_read(self):
+            if self._ssl_accepting:
+                self._do_ssl_handshake()
+            else:
+                DummyPOP3Handler.handle_read(self)
+
     class TestPOP3_SSLClass(TestPOP3Class):
         # repeat previous tests by using poplib.POP3_SSL
 
index 2b1ba8a0ce22daa85e75cb46cf59059cef9319a7..bd2aed578b5cbbf8b54ccf6bda07791159bdad61 100644 (file)
@@ -616,11 +616,8 @@ else:
                                                   certfile=certfile,
                                                   do_handshake_on_connect=False)
                     asyncore.dispatcher_with_send.__init__(self, self.socket)
-                    # now we have to do the handshake
-                    # we'll just do it the easy way, and block the connection
-                    # till it's finished.  If we were doing it right, we'd
-                    # do this in multiple calls to handle_read...
-                    self.do_handshake(block=True)
+                    self._ssl_accepting = True
+                    self._do_ssl_handshake()
 
                 def readable(self):
                     if isinstance(self.socket, ssl.SSLSocket):
@@ -628,18 +625,37 @@ else:
                             self.handle_read_event()
                     return True
 
+                def _do_ssl_handshake(self):
+                    try:
+                        self.socket.do_handshake()
+                    except ssl.SSLError as err:
+                        if err.args[0] in (ssl.SSL_ERROR_WANT_READ,
+                                           ssl.SSL_ERROR_WANT_WRITE):
+                            return
+                        elif err.args[0] == ssl.SSL_ERROR_EOF:
+                            return self.handle_close()
+                        raise
+                    except socket.error as err:
+                        if err.args[0] == errno.ECONNABORTED:
+                            return self.handle_close()
+                    else:
+                        self._ssl_accepting = False
+
                 def handle_read(self):
-                    data = self.recv(1024)
-                    if support.verbose:
-                        sys.stdout.write(" server:  read %s from client\n" % repr(data))
-                    if not data:
-                        self.close()
+                    if self._ssl_accepting:
+                        self._do_ssl_handshake()
                     else:
-                        self.send(str(data, 'ASCII', 'strict').lower().encode('ASCII', 'strict'))
+                        data = self.recv(1024)
+                        if support.verbose:
+                            sys.stdout.write(" server:  read %s from client\n" % repr(data))
+                        if not data:
+                            self.close()
+                        else:
+                            self.send(str(data, 'ASCII', 'strict').lower().encode('ASCII', 'strict'))
 
                 def handle_close(self):
                     self.close()
-                    if support.verbose:
+                    if test_support.verbose:
                         sys.stdout.write(" server:  closed connection %s\n" % self.socket)
 
                 def handle_error(self):
@@ -1266,6 +1282,55 @@ else:
                 server.stop()
                 server.join()
 
+        def test_handshake_timeout(self):
+            # Issue #5103: SSL handshake must respect the socket timeout
+            server = socket.socket(socket.AF_INET)
+            host = "127.0.0.1"
+            port = support.bind_port(server)
+            started = threading.Event()
+            finish = False
+
+            def serve():
+                server.listen(5)
+                started.set()
+                conns = []
+                while not finish:
+                    r, w, e = select.select([server], [], [], 0.1)
+                    if server in r:
+                        # Let the socket hang around rather than having
+                        # it closed by garbage collection.
+                        conns.append(server.accept()[0])
+
+            t = threading.Thread(target=serve)
+            t.start()
+            started.wait()
+
+            try:
+                if 0:
+                    # Disabled until #8524 finds a solution
+                    try:
+                        c = socket.socket(socket.AF_INET)
+                        c.settimeout(1.0)
+                        c.connect((host, port))
+                        # Will attempt handshake and time out
+                        self.assertRaisesRegexp(ssl.SSLError, "timed out",
+                                                ssl.wrap_socket, c)
+                    finally:
+                        c.close()
+                try:
+                    c = socket.socket(socket.AF_INET)
+                    c = ssl.wrap_socket(c)
+                    c.settimeout(0.2)
+                    # Will attempt handshake and time out
+                    self.assertRaisesRegexp(ssl.SSLError, "timed out",
+                                            c.connect, (host, port))
+                finally:
+                    c.close()
+            finally:
+                finish = True
+                t.join()
+                server.close()
+
 
 def test_main(verbose=False):
     if skip_expected:
index 0d72c83815cc935f52012ab60b2bb8019eb38a58..e98e95d5ffd3efc2ba36394b80919ac3e7a8e960 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -339,6 +339,12 @@ C-API
 Library
 -------
 
+- Issue #5103: SSL handshake would ignore the socket timeout and block
+  indefinitely if the other end didn't respond.
+
+- The do_handshake() method of SSL objects now adjusts the blocking mode of
+  the SSL structure if necessary (as other methods already do).
+
 - Issue #8391: os.execvpe() and os.getenv() supports unicode with surrogates
   and bytes strings for environment keys and values
 
index e06b7e041277159194baa3dabc4ee8179296089b..e89e5b776a9d5716c76eee1ce7652a9e7b102ff5 100644 (file)
@@ -455,20 +455,25 @@ static PyObject *PySSL_SSLdo_handshake(PySSLObject *self)
 {
        int ret;
        int err;
-       int sockstate;
+       int sockstate, nonblocking;
+        PySocketSockObject *sock
+          = (PySocketSockObject *) PyWeakref_GetObject(self->Socket);
+
+       if (((PyObject*)sock) == Py_None) {
+                _setSSLError("Underlying socket connection gone",
+                             PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
+                return NULL;
+        }
+
+       /* just in case the blocking state of the socket has been changed */
+       nonblocking = (sock->sock_timeout >= 0.0);
+       BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking);
+       BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking);
 
        /* Actually negotiate SSL connection */
        /* XXX If SSL_do_handshake() returns 0, it's also a failure. */
        sockstate = 0;
        do {
-                PySocketSockObject *sock
-                  = (PySocketSockObject *) PyWeakref_GetObject(self->Socket);
-                if (((PyObject*)sock) == Py_None) {
-                        _setSSLError("Underlying socket connection gone",
-                                     PY_SSL_ERROR_NO_SOCKET, __FILE__, __LINE__);
-                        return NULL;
-                }
-
                PySSL_BEGIN_ALLOW_THREADS
                ret = SSL_do_handshake(self->ssl);
                err = SSL_get_error(self->ssl, ret);