bpo-33734: asyncio/ssl: a bunch of bugfixes (#7321)

* Fix AttributeError (not all SSL exceptions have 'errno' attribute)

* Increase default handshake timeout from 10 to 60 seconds
* Make sure start_tls can be cancelled correctly
* Make sure any error in SSLProtocol gets propagated (instead of just being logged)

2 files changed, 214 insertions, 31 deletions

diff --git a/Lib/test/test_asyncio/test_sslproto.py b/Lib/test/test_asyncio/test_sslproto.py
index fb823f8..d02d441 100644
--- a/Lib/test/test_asyncio/test_sslproto.py
+++ b/Lib/test/test_asyncio/test_sslproto.py
@@ -53,35 +53,6 @@ class SslProtoHandshakeTests(test_utils.TestCase):
             ssl_proto.connection_made(transport)
         return transport
 
-    def test_cancel_handshake(self):
-        # Python issue #23197: cancelling a handshake must not raise an
-        # exception or log an error, even if the handshake failed
-        waiter = asyncio.Future(loop=self.loop)
-        ssl_proto = self.ssl_protocol(waiter=waiter)
-        handshake_fut = asyncio.Future(loop=self.loop)
-
-        def do_handshake(callback):
-            exc = Exception()
-            callback(exc)
-            handshake_fut.set_result(None)
-            return []
-
-        waiter.cancel()
-        self.connection_made(ssl_proto, do_handshake=do_handshake)
-
-        with test_utils.disable_logger():
-            self.loop.run_until_complete(handshake_fut)
-
-    def test_handshake_timeout(self):
-        # bpo-29970: Check that a connection is aborted if handshake is not
-        # completed in timeout period, instead of remaining open indefinitely
-        ssl_proto = self.ssl_protocol()
-        transport = self.connection_made(ssl_proto)
-
-        with test_utils.disable_logger():
-            self.loop.run_until_complete(tasks.sleep(0.2, loop=self.loop))
-        self.assertTrue(transport.abort.called)
-
     def test_handshake_timeout_zero(self):
         sslcontext = test_utils.dummy_ssl_context()
         app_proto = mock.Mock()
@@ -392,6 +363,67 @@ class BaseStartTLS(func_tests.FunctionalTestCaseMixin):
                 asyncio.wait_for(client(srv.addr),
                                  loop=self.loop, timeout=self.TIMEOUT))
 
+    def test_start_tls_slow_client_cancel(self):
+        HELLO_MSG = b'1' * self.PAYLOAD_SIZE
+
+        client_context = test_utils.simple_client_sslcontext()
+        server_waits_on_handshake = self.loop.create_future()
+
+        def serve(sock):
+            sock.settimeout(self.TIMEOUT)
+
+            data = sock.recv_all(len(HELLO_MSG))
+            self.assertEqual(len(data), len(HELLO_MSG))
+
+            try:
+                self.loop.call_soon_threadsafe(
+                    server_waits_on_handshake.set_result, None)
+                data = sock.recv_all(1024 * 1024)
+            except ConnectionAbortedError:
+                pass
+            finally:
+                sock.close()
+
+        class ClientProto(asyncio.Protocol):
+            def __init__(self, on_data, on_eof):
+                self.on_data = on_data
+                self.on_eof = on_eof
+                self.con_made_cnt = 0
+
+            def connection_made(proto, tr):
+                proto.con_made_cnt += 1
+                # Ensure connection_made gets called only once.
+                self.assertEqual(proto.con_made_cnt, 1)
+
+            def data_received(self, data):
+                self.on_data.set_result(data)
+
+            def eof_received(self):
+                self.on_eof.set_result(True)
+
+        async def client(addr):
+            await asyncio.sleep(0.5, loop=self.loop)
+
+            on_data = self.loop.create_future()
+            on_eof = self.loop.create_future()
+
+            tr, proto = await self.loop.create_connection(
+                lambda: ClientProto(on_data, on_eof), *addr)
+
+            tr.write(HELLO_MSG)
+
+            await server_waits_on_handshake
+
+            with self.assertRaises(asyncio.TimeoutError):
+                await asyncio.wait_for(
+                    self.loop.start_tls(tr, proto, client_context),
+                    0.5,
+                    loop=self.loop)
+
+        with self.tcp_server(serve, timeout=self.TIMEOUT) as srv:
+            self.loop.run_until_complete(
+                asyncio.wait_for(client(srv.addr), loop=self.loop, timeout=10))
+
     def test_start_tls_server_1(self):
         HELLO_MSG = b'1' * self.PAYLOAD_SIZE
 
@@ -481,6 +513,156 @@ class BaseStartTLS(func_tests.FunctionalTestCaseMixin):
 
         self.loop.run_until_complete(main())
 
+    def test_handshake_timeout(self):
+        # bpo-29970: Check that a connection is aborted if handshake is not
+        # completed in timeout period, instead of remaining open indefinitely
+        client_sslctx = test_utils.simple_client_sslcontext()
+
+        messages = []
+        self.loop.set_exception_handler(lambda loop, ctx: messages.append(ctx))
+
+        server_side_aborted = False
+
+        def server(sock):
+            nonlocal server_side_aborted
+            try:
+                sock.recv_all(1024 * 1024)
+            except ConnectionAbortedError:
+                server_side_aborted = True
+            finally:
+                sock.close()
+
+        async def client(addr):
+            await asyncio.wait_for(
+                self.loop.create_connection(
+                    asyncio.Protocol,
+                    *addr,
+                    ssl=client_sslctx,
+                    server_hostname='',
+                    ssl_handshake_timeout=10.0),
+                0.5,
+                loop=self.loop)
+
+        with self.tcp_server(server,
+                             max_clients=1,
+                             backlog=1) as srv:
+
+            with self.assertRaises(asyncio.TimeoutError):
+                self.loop.run_until_complete(client(srv.addr))
+
+        self.assertTrue(server_side_aborted)
+
+        # Python issue #23197: cancelling a handshake must not raise an
+        # exception or log an error, even if the handshake failed
+        self.assertEqual(messages, [])
+
+    def test_create_connection_ssl_slow_handshake(self):
+        client_sslctx = test_utils.simple_client_sslcontext()
+
+        messages = []
+        self.loop.set_exception_handler(lambda loop, ctx: messages.append(ctx))
+
+        def server(sock):
+            try:
+                sock.recv_all(1024 * 1024)
+            except ConnectionAbortedError:
+                pass
+            finally:
+                sock.close()
+
+        async def client(addr):
+            reader, writer = await asyncio.open_connection(
+                *addr,
+                ssl=client_sslctx,
+                server_hostname='',
+                loop=self.loop,
+                ssl_handshake_timeout=1.0)
+
+        with self.tcp_server(server,
+                             max_clients=1,
+                             backlog=1) as srv:
+
+            with self.assertRaisesRegex(
+                    ConnectionAbortedError,
+                    r'SSL handshake.*is taking longer'):
+
+                self.loop.run_until_complete(client(srv.addr))
+
+        self.assertEqual(messages, [])
+
+    def test_create_connection_ssl_failed_certificate(self):
+        self.loop.set_exception_handler(lambda loop, ctx: None)
+
+        sslctx = test_utils.simple_server_sslcontext()
+        client_sslctx = test_utils.simple_client_sslcontext(
+            disable_verify=False)
+
+        def server(sock):
+            try:
+                sock.start_tls(
+                    sslctx,
+                    server_side=True)
+            except ssl.SSLError:
+                pass
+            finally:
+                sock.close()
+
+        async def client(addr):
+            reader, writer = await asyncio.open_connection(
+                *addr,
+                ssl=client_sslctx,
+                server_hostname='',
+                loop=self.loop,
+                ssl_handshake_timeout=1.0)
+
+        with self.tcp_server(server,
+                             max_clients=1,
+                             backlog=1) as srv:
+
+            with self.assertRaises(ssl.SSLCertVerificationError):
+                self.loop.run_until_complete(client(srv.addr))
+
+    def test_start_tls_client_corrupted_ssl(self):
+        self.loop.set_exception_handler(lambda loop, ctx: None)
+
+        sslctx = test_utils.simple_server_sslcontext()
+        client_sslctx = test_utils.simple_client_sslcontext()
+
+        def server(sock):
+            orig_sock = sock.dup()
+            try:
+                sock.start_tls(
+                    sslctx,
+                    server_side=True)
+                sock.sendall(b'A\n')
+                sock.recv_all(1)
+                orig_sock.send(b'please corrupt the SSL connection')
+            except ssl.SSLError:
+                pass
+            finally:
+                sock.close()
+
+        async def client(addr):
+            reader, writer = await asyncio.open_connection(
+                *addr,
+                ssl=client_sslctx,
+                server_hostname='',
+                loop=self.loop)
+
+            self.assertEqual(await reader.readline(), b'A\n')
+            writer.write(b'B')
+            with self.assertRaises(ssl.SSLError):
+                await reader.readline()
+            return 'OK'
+
+        with self.tcp_server(server,
+                             max_clients=1,
+                             backlog=1) as srv:
+
+            res = self.loop.run_until_complete(client(srv.addr))
+
+        self.assertEqual(res, 'OK')
+
 
 @unittest.skipIf(ssl is None, 'No ssl module')
 class SelectorStartTLSTests(BaseStartTLS, unittest.TestCase):
diff --git a/Lib/test/test_asyncio/utils.py b/Lib/test/test_asyncio/utils.py
index 96dfe2f..5362591 100644
--- a/Lib/test/test_asyncio/utils.py
+++ b/Lib/test/test_asyncio/utils.py
@@ -77,10 +77,11 @@ def simple_server_sslcontext():
     return server_context
 
 
-def simple_client_sslcontext():
+def simple_client_sslcontext(*, disable_verify=True):
     client_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
     client_context.check_hostname = False
-    client_context.verify_mode = ssl.CERT_NONE
+    if disable_verify:
+        client_context.verify_mode = ssl.CERT_NONE
     return client_context