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
OpenSSL's internal readahead.  Thanks to Darryl Miles for guidance.

Issue #8108: test_ftplib's non-blocking SSL server now has proper handling
of SSL shutdowns.

Lib/test/test_ftplib.py

@@ -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_chat):
         self.baseclass.last_received_data += self.recv(1024)
 
     def handle_close(self):
+        # 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

Misc/NEWS

@@ -25,6 +25,11 @@ Core and Builtins
 Library
 -------
 
+- 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
+  OpenSSL's internal readahead.  Thanks to Darryl Miles for guidance.
+
 - Issue #8484: Load all ciphers and digest algorithms when initializing
   the _ssl extension, such that verification of some SSL certificates
   doesn't fail because of an "unknown algorithm".
@@ -68,6 +73,12 @@ Extension Modules
 
 - Build the ossaudio extension on GNU/kFreeBSD.
 
+Tests
+-----
+
+- Issue #8108: test_ftplib's non-blocking SSL server now has proper handling
+  of SSL shutdowns.
+
 
 What's New in Python 2.7 beta 1?
 ================================

Modules/_ssl.c

@@ -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;
 
@@ -1357,7 +1361,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) {
@@ -1366,13 +1371,65 @@ static PyObject *PySSL_SSLshutdown(PySSLObject *self)
 		return NULL;
 	}
 
+        /* 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);
-	if (err == 0) {
-		/* we need to call it again to finish the shutdown */
-		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;
+	}
 
 	if (err < 0)
 		return PySSL_SetError(self, err, __FILE__, __LINE__);