From ec146185c4382206ecbbcaa505c81b8c04992f3a Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 24 Apr 2010 21:30:20 +0000 Subject: [PATCH] Merged revisions 80454 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k ................ r80454 | antoine.pitrou | 2010-04-24 23:26:44 +0200 (sam., 24 avril 2010) | 15 lines Merged revisions 80451-80452 via svnmerge from 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 | 29 ++++++++++++- Lib/test/test_ssl.py | 91 +++++++++++++++++++++++++++++++++++------ Misc/NEWS | 6 +++ Modules/_ssl.c | 23 +++++++---- 4 files changed, 126 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py index 520c20f49a0..5659b78628f 100644 --- 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 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. ') + 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 diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 30923e7a8c6..403f6bf46c5 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -580,11 +580,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): @@ -592,18 +589,37 @@ else: self.handle_read_event() return True - 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() + 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.send(str(data, 'ASCII', 'strict').lower().encode('ASCII', 'strict')) + self._ssl_accepting = False + + def handle_read(self): + if self._ssl_accepting: + self._do_ssl_handshake() + else: + 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): @@ -1222,6 +1238,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: diff --git a/Misc/NEWS b/Misc/NEWS index a619192ff0d..df89d37dff8 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -40,6 +40,12 @@ 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). + - Issue #8108: Fix the unwrap() method of SSL objects when the socket has a non-infinite timeout. Also make that method friendlier with applications wanting to continue using the socket in clear-text mode, by disabling diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 5e0f473d405..c21ac5d380a 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -445,20 +445,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);