Skip to content

Commit f3434d8

Browse files
committed
gh-160513: Call connection_lost() when start_tls() handshake is lost
SSLProtocol.connection_lost() did not notify the application protocol while a handshake was in progress, even when start_tls() had handed over a protocol that had already seen connection_made(). With streams this left StreamWriter.wait_closed() waiting forever.
1 parent 3b3c692 commit f3434d8

4 files changed

Lines changed: 140 additions & 9 deletions

File tree

‎Lib/asyncio/sslproto.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -406,13 +406,15 @@ def connection_lost(self, exc):
406406
if self._app_transport is not None:
407407
self._app_transport._closed = True
408408

409-
if self._state != SSLProtocolState.DO_HANDSHAKE:
410-
if (
411-
self._app_state == AppProtocolState.STATE_CON_MADE or
412-
self._app_state == AppProtocolState.STATE_EOF
413-
):
414-
self._app_state = AppProtocolState.STATE_CON_LOST
415-
self._loop.call_soon(self._app_protocol.connection_lost, exc)
409+
# The app protocol gets connection_lost() only if it got
410+
# connection_made(). After start_tls() that is already the case
411+
# while the handshake is in progress.
412+
if (
413+
self._app_state == AppProtocolState.STATE_CON_MADE or
414+
self._app_state == AppProtocolState.STATE_EOF
415+
):
416+
self._app_state = AppProtocolState.STATE_CON_LOST
417+
self._loop.call_soon(self._app_protocol.connection_lost, exc)
416418
self._set_state(SSLProtocolState.UNWRAPPED)
417419
self._transport = None
418420
self._app_transport = None

‎Lib/test/test_asyncio/test_sslproto.py‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import logging
44
import socket
5+
import struct
56
import unittest
67
import weakref
78
from test import support
@@ -153,6 +154,29 @@ def test_connection_lost(self):
153154
test_utils.run_briefly(self.loop)
154155
self.assertIsInstance(waiter.exception(), ConnectionAbortedError)
155156

157+
def test_connection_lost_during_handshake(self):
158+
# The app protocol gets connection_lost() if and only if it got
159+
# connection_made(). With start_tls() (call_connection_made=False)
160+
# it is already connected when the handshake starts.
161+
for call_connection_made in (True, False):
162+
with self.subTest(call_connection_made=call_connection_made):
163+
app_proto = mock.Mock(spec=asyncio.Protocol)
164+
ssl_proto = sslproto.SSLProtocol(
165+
self.loop, app_proto, test_utils.dummy_ssl_context(),
166+
waiter=None, call_connection_made=call_connection_made)
167+
self.connection_made(
168+
ssl_proto,
169+
do_handshake=mock.Mock(side_effect=ssl.SSLWantReadError)
170+
)
171+
exc = ConnectionResetError()
172+
ssl_proto.connection_lost(exc)
173+
test_utils.run_briefly(self.loop)
174+
app_proto.connection_made.assert_not_called()
175+
if call_connection_made:
176+
app_proto.connection_lost.assert_not_called()
177+
else:
178+
app_proto.connection_lost.assert_called_once_with(exc)
179+
156180
def test_connection_lost_when_busy(self):
157181
# gh-118950: SSLProtocol.connection_lost not being called when OSError
158182
# is thrown on asyncio.write.
@@ -570,9 +594,10 @@ def serve(sock):
570594
sock.close()
571595

572596
class ClientProto(asyncio.Protocol):
573-
def __init__(self, on_data, on_eof):
597+
def __init__(self, on_data, on_eof, on_con_lost):
574598
self.on_data = on_data
575599
self.on_eof = on_eof
600+
self.on_con_lost = on_con_lost
576601
self.con_made_cnt = 0
577602

578603
def connection_made(proto, tr):
@@ -586,14 +611,18 @@ def data_received(self, data):
586611
def eof_received(self):
587612
self.on_eof.set_result(True)
588613

614+
def connection_lost(self, exc):
615+
self.on_con_lost.set_result(exc)
616+
589617
async def client(addr):
590618
await asyncio.sleep(0.5)
591619

592620
on_data = self.loop.create_future()
593621
on_eof = self.loop.create_future()
622+
on_con_lost = self.loop.create_future()
594623

595624
tr, proto = await self.loop.create_connection(
596-
lambda: ClientProto(on_data, on_eof), *addr)
625+
lambda: ClientProto(on_data, on_eof, on_con_lost), *addr)
597626

598627
tr.write(HELLO_MSG)
599628

@@ -604,11 +633,58 @@ async def client(addr):
604633
self.loop.start_tls(tr, proto, client_context),
605634
0.5)
606635

636+
self.assertIsNone(await on_con_lost)
637+
607638
with self.tcp_server(serve, timeout=self.TIMEOUT) as srv:
608639
self.loop.run_until_complete(
609640
asyncio.wait_for(client(srv.addr),
610641
timeout=support.SHORT_TIMEOUT))
611642

643+
def _test_start_tls_streams_failed_handshake(self, serve, expected_exc,
644+
**kwargs):
645+
# wait_closed() must not hang if the connection is lost while
646+
# the handshake started by start_tls() is still in progress.
647+
client_context = test_utils.simple_client_sslcontext()
648+
649+
async def client(addr):
650+
reader, writer = await asyncio.open_connection(*addr)
651+
with self.assertRaises(expected_exc):
652+
await writer.start_tls(client_context, **kwargs)
653+
writer.close()
654+
with self.assertRaises(expected_exc):
655+
await writer.wait_closed()
656+
657+
with self.tcp_server(serve, timeout=self.TIMEOUT) as srv:
658+
self.loop.run_until_complete(
659+
asyncio.wait_for(client(srv.addr),
660+
timeout=support.SHORT_TIMEOUT))
661+
662+
def test_start_tls_streams_handshake_reset(self):
663+
def serve(sock):
664+
sock.settimeout(self.TIMEOUT)
665+
sock.recv(1024 * 1024) # ClientHello
666+
# Send RST instead of FIN.
667+
sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER,
668+
struct.pack('ii', 1, 0))
669+
sock.close()
670+
671+
self._test_start_tls_streams_failed_handshake(
672+
serve, ConnectionError)
673+
674+
def test_start_tls_streams_handshake_timeout(self):
675+
def serve(sock):
676+
sock.settimeout(self.TIMEOUT)
677+
try:
678+
while sock.recv(1024 * 1024):
679+
pass
680+
except ConnectionError:
681+
pass
682+
finally:
683+
sock.close()
684+
685+
self._test_start_tls_streams_failed_handshake(
686+
serve, ConnectionAbortedError, ssl_handshake_timeout=0.1)
687+
612688
@socket_helper.skip_if_tcp_blackhole
613689
def test_start_tls_server_1(self):
614690
HELLO_MSG = b'1' * self.PAYLOAD_SIZE

‎Lib/test/test_asyncio/test_streams.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,54 @@ async def run_test():
861861
self.loop.run_until_complete(run_test())
862862
self.assertEqual(messages, [])
863863

864+
@unittest.skipIf(ssl is None, 'No ssl module')
865+
def test_start_tls_connection_lost_during_handshake(self):
866+
# The protocol must get connection_lost() when the connection is
867+
# lost while the start_tls() handshake is in progress, so that
868+
# wait_closed() completes instead of hanging forever.
869+
870+
async def server_handler(client_reader, client_writer):
871+
line = await client_reader.readline()
872+
self.assertEqual(line, b"STARTTLS\n")
873+
client_writer.write(b"go ahead\n")
874+
await client_writer.drain()
875+
with self.assertRaises(ConnectionAbortedError):
876+
await client_writer.start_tls(
877+
test_utils.simple_server_sslcontext(),
878+
ssl_handshake_timeout=0.1)
879+
client_writer.close()
880+
with self.assertRaises(ConnectionAbortedError):
881+
await asyncio.wait_for(client_writer.wait_closed(), 10)
882+
handler_done.set()
883+
884+
async def client(addr):
885+
reader, writer = await asyncio.open_connection(*addr)
886+
writer.write(b"STARTTLS\n")
887+
await writer.drain()
888+
line = await reader.readline()
889+
self.assertEqual(line, b"go ahead\n")
890+
# Never send a ClientHello: the server's handshake times out.
891+
data = await reader.read()
892+
self.assertEqual(data, b"")
893+
writer.close()
894+
await writer.wait_closed()
895+
896+
async def run_test():
897+
server = await asyncio.start_server(
898+
server_handler, socket_helper.HOSTv4, 0)
899+
server_addr = server.sockets[0].getsockname()
900+
901+
await client(server_addr)
902+
await asyncio.wait_for(handler_done.wait(), 10)
903+
server.close()
904+
await server.wait_closed()
905+
906+
handler_done = asyncio.Event()
907+
messages = []
908+
self.loop.set_exception_handler(lambda loop, ctx: messages.append(ctx))
909+
self.loop.run_until_complete(run_test())
910+
self.assertEqual(messages, [])
911+
864912
def test_streamwriter_start_tls_updates_reader_transport(self):
865913
reader = asyncio.StreamReader(loop=self.loop)
866914
protocol = asyncio.StreamReaderProtocol(reader, loop=self.loop)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix :meth:`asyncio.loop.start_tls` and :meth:`asyncio.StreamWriter.start_tls`
2+
not calling the protocol's :meth:`~asyncio.BaseProtocol.connection_lost`
3+
when the connection is lost, the handshake times out or the call is
4+
cancelled during the TLS handshake. :meth:`asyncio.StreamWriter.wait_closed`
5+
no longer waits forever after such a failure.

0 commit comments

Comments
 (0)