diff -r fc724972f325 Lib/test/test_ftplib.py --- a/Lib/test/test_ftplib.py Fri Apr 16 04:46:46 2010 +0200 +++ b/Lib/test/test_ftplib.py Fri Apr 16 21:20:31 2010 +0200 @@ -29,6 +29,7 @@ NLST_DATA = 'foo\r\nbar\r\n' class DummyDTPHandler(asynchat.async_chat): + dtp_conn_closed = False def __init__(self, conn, baseclass): asynchat.async_chat.__init__(self, conn) @@ -39,8 +40,13 @@ class DummyDTPHandler(asynchat.async_cha self.baseclass.last_received_data += self.recv(1024) def handle_close(self): - self.baseclass.push('226 transfer complete') - self.close() + # XXX: this method can be called many times in a row for a single + # connection, including in clear-text (non-TLS) mode. + # (behaviour witnessed with test_data_connection) + if not self.dtp_conn_closed: + self.baseclass.push('226 transfer complete') + self.close() + self.dtp_conn_closed = True class DummyFTPHandler(asynchat.async_chat): @@ -253,6 +259,7 @@ if ssl is not None: """An asyncore.dispatcher subclass supporting TLS/SSL.""" _ssl_accepting = False + _ssl_closing = False def secure_connection(self): self.socket = ssl.wrap_socket(self.socket, suppress_ragged_eofs=False, @@ -277,15 +284,36 @@ if ssl is not None: else: self._ssl_accepting = False + def _do_ssl_shutdown(self): + self._ssl_closing = True + try: + self.socket = self.socket.unwrap() + except ssl.SSLError, err: + if err.args[0] in (ssl.SSL_ERROR_WANT_READ, + ssl.SSL_ERROR_WANT_WRITE): + return + except socket.error, err: + # Any "socket error" corresponds to a SSL_ERROR_SYSCALL return + # from OpenSSL's SSL_shutdown(), corresponding to a + # closed socket condition. See also: + # http://www.mail-archive.com/openssl-users@openssl.org/msg60710.html + pass + self._ssl_closing = False + super(SSLConnection, self).close() + def handle_read_event(self): if self._ssl_accepting: self._do_ssl_handshake() + elif self._ssl_closing: + self._do_ssl_shutdown() else: super(SSLConnection, self).handle_read_event() def handle_write_event(self): if self._ssl_accepting: self._do_ssl_handshake() + elif self._ssl_closing: + self._do_ssl_shutdown() else: super(SSLConnection, self).handle_write_event() @@ -315,12 +343,9 @@ if ssl is not None: raise def close(self): - try: - if isinstance(self.socket, ssl.SSLSocket): - if self.socket._sslobj is not None: - self.socket.unwrap() - finally: - super(SSLConnection, self).close() + if (isinstance(self.socket, ssl.SSLSocket) and + self.socket._sslobj is not None): + self._do_ssl_shutdown() class DummyTLS_DTPHandler(SSLConnection, DummyDTPHandler): @@ -597,21 +622,21 @@ class TestTLS_FTPClass(TestCase): sock = self.client.transfercmd('list') self.assertNotIsInstance(sock, ssl.SSLSocket) sock.close() - self.client.voidresp() + self.assertEqual(self.client.voidresp(), "226 transfer complete") # secured, after PROT P self.client.prot_p() sock = self.client.transfercmd('list') self.assertIsInstance(sock, ssl.SSLSocket) sock.close() - self.client.voidresp() + self.assertEqual(self.client.voidresp(), "226 transfer complete") # PROT C is issued, the connection must be in cleartext again self.client.prot_c() sock = self.client.transfercmd('list') self.assertNotIsInstance(sock, ssl.SSLSocket) sock.close() - self.client.voidresp() + self.assertEqual(self.client.voidresp(), "226 transfer complete") def test_login(self): # login() is supposed to implicitly secure the control connection diff -r fc724972f325 Modules/_ssl.c --- a/Modules/_ssl.c Fri Apr 16 04:46:46 2010 +0200 +++ b/Modules/_ssl.c Fri Apr 16 21:20:31 2010 +0200 @@ -9,6 +9,9 @@ directly. XXX should partial writes be enabled, SSL_MODE_ENABLE_PARTIAL_WRITE? + + XXX integrate several "shutdown modes" as suggested in + http://bugs.python.org/issue8108#msg102867 ? */ #include "Python.h" @@ -115,6 +118,7 @@ typedef struct { X509* peer_cert; char server[X509_NAME_MAXLEN]; char issuer[X509_NAME_MAXLEN]; + int shutdown_seen_zero; } PySSLObject; @@ -1347,7 +1351,8 @@ Read up to len bytes from the SSL socket static PyObject *PySSL_SSLshutdown(PySSLObject *self) { - int err; + int err, ssl_err, sockstate, nonblocking; + int zeros = 0; /* Guard against closed socket */ if (self->Socket->sock_fd < 0) { @@ -1356,13 +1361,65 @@ static PyObject *PySSL_SSLshutdown(PySSL return NULL; } - PySSL_BEGIN_ALLOW_THREADS - err = SSL_shutdown(self->ssl); - if (err == 0) { - /* we need to call it again to finish the shutdown */ + /* Just in case the blocking state of the socket has been changed */ + nonblocking = (self->Socket->sock_timeout >= 0.0); + BIO_set_nbio(SSL_get_rbio(self->ssl), nonblocking); + BIO_set_nbio(SSL_get_wbio(self->ssl), nonblocking); + + while (1) { + PySSL_BEGIN_ALLOW_THREADS + /* Disable read-ahead so that unwrap can work correctly. + * Otherwise OpenSSL might read in too much data, + * eating clear text data that happens to be + * transmitted after the SSL shutdown. + * Should be safe to call repeatedly everytime this + * function is used and the shutdown_seen_zero != 0 + * condition is met. + */ + if (self->shutdown_seen_zero) + SSL_set_read_ahead(self->ssl, 0); err = SSL_shutdown(self->ssl); + PySSL_END_ALLOW_THREADS + /* If err == 1, a secure shutdown with SSL_shutdown() is complete */ + if (err > 0) + break; + if (err == 0) { + /* Don't loop endlessly; instead preserve legacy + behaviour of trying SSL_shutdown() only twice. + This looks necessary for OpenSSL < 0.9.8m */ + if (++zeros > 1) + break; + /* Shutdown was sent, now try receiving */ + self->shutdown_seen_zero = 1; + continue; + } + + /* Possibly retry shutdown until timeout or failure */ + ssl_err = SSL_get_error(self->ssl, err); + if (ssl_err == SSL_ERROR_WANT_READ) + sockstate = check_socket_and_wait_for_timeout(self->Socket, 0); + else if (ssl_err == SSL_ERROR_WANT_WRITE) + sockstate = check_socket_and_wait_for_timeout(self->Socket, 1); + else + break; + if (sockstate == SOCKET_HAS_TIMED_OUT) { + if (ssl_err == SSL_ERROR_WANT_READ) + PyErr_SetString(PySSLErrorObject, + "The read operation timed out"); + else + PyErr_SetString(PySSLErrorObject, + "The write operation timed out"); + return NULL; + } + else if (sockstate == SOCKET_TOO_LARGE_FOR_SELECT) { + PyErr_SetString(PySSLErrorObject, + "Underlying socket too large for select()."); + return NULL; + } + else if (sockstate != SOCKET_OPERATION_OK) + /* Retain the SSL error code */ + break; } - PySSL_END_ALLOW_THREADS if (err < 0) return PySSL_SetError(self, err, __FILE__, __LINE__);