Issue #5103: SSL handshake would ignore the socket timeout and block

indefinitely if the other end didn't respond.

diff --git a/Lib/ssl.py b/Lib/ssl.py
index 9e88073916bd55d2e30e02e0e47167ae61707aed..1d29bef72d62062d7ab14397eab1ed9c23bfdcf5 100644 (file)
--- a/Lib/ssl.py
+++ b/Lib/ssl.py
@@ -115,12 +115,7 @@ class SSLSocket(socket):
                                         cert_reqs, ssl_version, ca_certs,
                                         ciphers)
             if do_handshake_on_connect:
-                timeout = self.gettimeout()
-                try:
-                    self.settimeout(None)
-                    self.do_handshake()
-                finally:
-                    self.settimeout(timeout)
+                self.do_handshake()
         self.keyfile = keyfile
         self.certfile = certfile
         self.cert_reqs = cert_reqs

diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py
index 345a1c75d22d7bad3d2ad020546431e2354a734a..f75f0b1a519475c8ab4bec1faea2fecaa707bc36 100644 (file)
--- a/Lib/test/test_poplib.py
+++ b/Lib/test/test_poplib.py
@@ -10,6 +10,7 @@ import asynchat
 import socket
 import os
 import time
+import errno
 
 from unittest import TestCase
 from test import test_support
@@ -231,11 +232,37 @@ if hasattr(poplib, 'POP3_SSL'):
         def __init__(self, conn):
             asynchat.async_chat.__init__(self, conn)
             self.socket = ssl.wrap_socket(self.socket, certfile=CERTFILE,
-                                          server_side=True)
+                                          server_side=True,
+                                          do_handshake_on_connect=False)
+            # Must try handshake before calling push()
+            self._ssl_accepting = True
+            self._do_ssl_handshake()
             self.set_terminator("\r\n")
             self.in_buffer = []
             self.push('+OK dummy pop3 server ready.')
 
+        def _do_ssl_handshake(self):
+            try:
+                self.socket.do_handshake()
+            except ssl.SSLError, 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, 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
 

diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 2c401dc69060a2a6840521ecb0c6ac567a2876e9..bbccaeb4ddf53c320b0ce4d66ef5e7930f17aec2 100644 (file)
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -494,7 +494,8 @@ else:
                     asyncore.dispatcher_with_send.__init__(self, conn)
                     self.socket = ssl.wrap_socket(conn, server_side=True,
                                                   certfile=certfile,
-                                                  do_handshake_on_connect=True)
+                                                  do_handshake_on_connect=False)
+                    self._ssl_accepting = True
 
                 def readable(self):
                     if isinstance(self.socket, ssl.SSLSocket):
@@ -502,9 +503,28 @@ else:
                             self.handle_read_event()
                     return True
 
+                def _do_ssl_handshake(self):
+                    try:
+                        self.socket.do_handshake()
+                    except ssl.SSLError, 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, err:
+                        if err.args[0] == errno.ECONNABORTED:
+                            return self.handle_close()
+                    else:
+                        self._ssl_accepting = False
+
                 def handle_read(self):
-                    data = self.recv(1024)
-                    self.send(data.lower())
+                    if self._ssl_accepting:
+                        self._do_ssl_handshake()
+                    else:
+                        data = self.recv(1024)
+                        self.send(data.lower())
 
                 def handle_close(self):
                     self.close()
@@ -1271,6 +1291,53 @@ 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 = test_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:
+                try:
+                    c = socket.socket(socket.AF_INET)
+                    c.settimeout(0.2)
+                    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.settimeout(0.2)
+                    c = ssl.wrap_socket(c)
+                    # 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:

diff --git a/Misc/NEWS b/Misc/NEWS
index b9bdc9716a9ba1d329279cd4e3baf0389e11aedd..39341d479f0bb71733f7509237439d0b1279e0a0 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -25,6 +25,9 @@ Core and Builtins
 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).