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

Author: mithrandi Reviewer: glyph Fixes: twisted#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. git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/trunk@46833 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
TaurusTiger · Feb 19, 2016 · a6c08ae · a6c08ae
1 parent ba899ea
commit a6c08ae
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 22 deletions.
diff --git a/twisted/protocols/loopback.py b/twisted/protocols/loopback.py
@@ -79,6 +79,14 @@ def loseConnection(self):
         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()
 

diff --git a/twisted/protocols/test/test_tls.py b/twisted/protocols/test/test_tls.py
@@ -708,6 +708,7 @@ def cbHandshake(ignored):
             # 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 @@ def test_loseConnectionTwice(self):
         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):
         """

diff --git a/twisted/protocols/tls.py b/twisted/protocols/tls.py
@@ -275,6 +275,7 @@ class TLSMemoryBIOProtocol(ProtocolWrapper):
     _writeBlockedOnRead = False
     _producer = None
     _aborted = False
+    _shuttingDown = False
 
     def __init__(self, factory, wrappedProtocol, _connectWrapped=True):
         ProtocolWrapper.__init__(self, factory, wrappedProtocol)
@@ -318,15 +319,20 @@ def makeConnection(self, transport):
         # 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 @@ def _shutdownTLS(self):
         """
         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 @@ def loseConnection(self):
         """
         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()

diff --git a/twisted/test/proto_helpers.py b/twisted/test/proto_helpers.py
@@ -204,6 +204,13 @@ def loseConnection(self):
         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)

diff --git a/twisted/test/test_sslverify.py b/twisted/test/test_sslverify.py
@@ -1826,7 +1826,7 @@ def test_surpriseFromInfoCallback(self):
         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)
 

diff --git a/twisted/topfiles/8189.bugfix b/twisted/topfiles/8189.bugfix
@@ -0,0 +1 @@
+Twisted is now compatible with OpenSSL 1.0.2f.
diff --git a/twisted/web/test/test_agent.py b/twisted/web/test/test_agent.py
@@ -2888,6 +2888,7 @@ def test_deprecatedTransport(self):
         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 '