r46833 - Merge openssl-f-8189-2: Fix compatibility with OpenSSL 1.0.2f

mithrandi-TA+aISz0psMTMxyoc4vAAJOcrHinNvQL0E9HWUfgJXw@public.gmane.org
Newsgroups gmane.comp.python.twisted.commits
Message-ID <[email protected]>
Author: mithrandi
Date: Thu Feb 18 21:49:00 2016
New Revision: 46833

Added:
   trunk/twisted/topfiles/8189.bugfix
Modified:
   trunk/twisted/protocols/loopback.py
   trunk/twisted/protocols/test/test_tls.py
   trunk/twisted/protocols/tls.py
   trunk/twisted/test/proto_helpers.py
   trunk/twisted/test/test_sslverify.py
   trunk/twisted/web/test/test_agent.py

Log:
Merge openssl-f-8189-2: Fix compatibility with OpenSSL 1.0.2f

Author: mithrandi
Reviewer: glyph
Fixes: #8189

OpenSSL 1.0.2f handles shutdown slightly differently to earlier
versions; loseConnection() now calls abortConnection() in cases where
the connection cannot be cleanly shut down.

Modified: trunk/twisted/protocols/loopback.py
==============================================================================
--- trunk/twisted/protocols/loopback.py	(original)
+++ trunk/twisted/protocols/loopback.py	Thu Feb 18 21:49:00 2016
@@ -79,6 +79,14 @@
         self.q.disconnect = True
         self.q.put(None)
 
+
+    def abortConnection(self):
+        """
+        Abort the connection. Same as L{loseConnection}.
+        """
+        self.loseConnection()
+
+
     def getPeer(self):
         return _LoopbackAddress()
 

Modified: trunk/twisted/protocols/test/test_tls.py
==============================================================================
--- trunk/twisted/protocols/test/test_tls.py	(original)
+++ trunk/twisted/protocols/test/test_tls.py	Thu Feb 18 21:49:00 2016
@@ -708,6 +708,7 @@
             # will be written out before the connection is closed, rather than
             # just small amounts that can be returned in a single bio_read:
             clientProtocol.transport.write(chunkOfBytes)
+            serverProtocol.transport.write(b'x')
             serverProtocol.transport.loseConnection()
 
             # Now wait for the client and server to notice.
@@ -772,28 +773,28 @@
         If TLSMemoryBIOProtocol.loseConnection is called multiple times, all
         but the first call have no effect.
         """
-        wrapperFactory = TLSMemoryBIOFactory(ClientTLSContext(),
-                                             True, ClientFactory())
-        tlsProtocol = TLSMemoryBIOProtocol(wrapperFactory, Protocol())
-        transport = StringTransport()
-        tlsProtocol.makeConnection(transport)
-        self.assertEqual(tlsProtocol.disconnecting, False)
-
+        tlsClient, tlsServer, handshakeDeferred, disconnectDeferred = (
+            self.handshakeProtocols())
+        self.successResultOf(handshakeDeferred)
         # Make sure loseConnection calls _shutdownTLS the first time (mostly
         # to make sure we've overriding it correctly):
         calls = []
-        def _shutdownTLS(shutdown=tlsProtocol._shutdownTLS):
+        def _shutdownTLS(shutdown=tlsClient._shutdownTLS):
             calls.append(1)
             return shutdown()
-        tlsProtocol._shutdownTLS = _shutdownTLS
-        tlsProtocol.loseConnection()
-        self.assertEqual(tlsProtocol.disconnecting, True)
+        tlsClient._shutdownTLS = _shutdownTLS
+        tlsClient.write(b'x')
+        tlsClient.loseConnection()
+        self.assertEqual(tlsClient.disconnecting, True)
         self.assertEqual(calls, [1])
 
         # Make sure _shutdownTLS isn't called a second time:
-        tlsProtocol.loseConnection()
+        tlsClient.loseConnection()
         self.assertEqual(calls, [1])
 
+        # We do successfully disconnect at some point:
+        return disconnectDeferred
+
 
     def test_unexpectedEOF(self):
         """

Modified: trunk/twisted/protocols/tls.py
==============================================================================
--- trunk/twisted/protocols/tls.py	(original)
+++ trunk/twisted/protocols/tls.py	Thu Feb 18 21:49:00 2016
@@ -275,6 +275,7 @@
     _writeBlockedOnRead = False
     _producer = None
     _aborted = False
+    _shuttingDown = False
 
     def __init__(self, factory, wrappedProtocol, _connectWrapped=True):
         ProtocolWrapper.__init__(self, factory, wrappedProtocol)
@@ -318,15 +319,20 @@
         # Now that we ourselves have a transport (initialized by the
         # ProtocolWrapper.makeConnection call above), kick off the TLS
         # handshake.
-        try:
-            self._tlsConnection.do_handshake()
-        except WantReadError:
-            # This is the expected case - there's no data in the connection's
-            # input buffer yet, so it won't be able to complete the whole
-            # handshake now.  If this is the speak-first side of the
-            # connection, then some bytes will be in the send buffer now; flush
-            # them.
-            self._flushSendBIO()
+
+        # The connection might already be aborted (eg. by a callback during
+        # connection setup), so don't even bother trying to handshake in that
+        # case.
+        if not self._aborted:
+            try:
+                self._tlsConnection.do_handshake()
+            except WantReadError:
+                # This is the expected case - there's no data in the
+                # connection's input buffer yet, so it won't be able to
+                # complete the whole handshake now. If this is the speak-first
+                # side of the connection, then some bytes will be in the send
+                # buffer now; flush them.
+                self._flushSendBIO()
 
 
     def _flushSendBIO(self):
@@ -426,6 +432,7 @@
         """
         Initiate, or reply to, the shutdown handshake of the TLS layer.
         """
+        self._shuttingDown = True
         try:
             shutdownSuccess = self._tlsConnection.shutdown()
         except Error:
@@ -483,6 +490,14 @@
         """
         if self.disconnecting:
             return
+        # If connection setup has not finished, OpenSSL 1.0.2f+ will not shut
+        # down the connection until we write some data to the connection which
+        # allows the handshake to complete. However, since no data should be
+        # written after loseConnection, this means we'll be stuck forever
+        # waiting for shutdown to complete. Instead, we simply abort the
+        # connection without trying to shut down cleanly:
+        if not self._handshakeDone and not self._writeBlockedOnRead:
+            self.abortConnection()
         self.disconnecting = True
         if not self._writeBlockedOnRead and self._producer is None:
             self._shutdownTLS()

Modified: trunk/twisted/test/proto_helpers.py
==============================================================================
--- trunk/twisted/test/proto_helpers.py	(original)
+++ trunk/twisted/test/proto_helpers.py	Thu Feb 18 21:49:00 2016
@@ -204,6 +204,13 @@
         self.disconnecting = True
 
 
+    def abortConnection(self):
+        """
+        Abort the connection. Same as C{loseConnection}.
+        """
+        self.loseConnection()
+
+
     def getPeer(self):
         if self.peerAddr is None:
             return address.IPv4Address('TCP', '192.168.1.1', 54321)

Modified: trunk/twisted/test/test_sslverify.py
==============================================================================
--- trunk/twisted/test/test_sslverify.py	(original)
+++ trunk/twisted/test/test_sslverify.py	Thu Feb 18 21:49:00 2016
@@ -1826,7 +1826,7 @@
         sErr = sProto.wrappedProtocol.lostReason.value
 
         self.assertIsInstance(cErr, ZeroDivisionError)
-        self.assertIsInstance(sErr, ConnectionClosed)
+        self.assertIsInstance(sErr, (ConnectionClosed, SSL.Error))
         errors = self.flushLoggedErrors(ZeroDivisionError)
         self.assertTrue(errors)
 

Modified: trunk/twisted/web/test/test_agent.py
==============================================================================
--- trunk/twisted/web/test/test_agent.py	(original)
+++ trunk/twisted/web/test/test_agent.py	Thu Feb 18 21:49:00 2016
@@ -2888,6 +2888,7 @@
         warning, but no exception when cancelling.
         """
         response = DummyResponse(transportFactory=StringTransport)
+        response.transport.abortConnection = None
         d = self.assertWarns(
             DeprecationWarning,
             'Using readBody with a transport that does not have an '
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.