Skip to content

Commit 357c650

Browse files
gh-98078: fix asyncio SSL transports not sending the fatal TLS alerts (#153620)
1 parent 7fb315b commit 357c650

3 files changed

Lines changed: 153 additions & 4 deletions

File tree

Lib/asyncio/sslproto.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,12 @@ def _on_handshake_complete(self, handshake_exc):
588588
msg = 'SSL handshake failed on verifying the certificate'
589589
else:
590590
msg = 'SSL handshake failed'
591+
# gh-98078: When the handshake fails, OpenSSL leaves the fatal
592+
# TLS alert (for example "bad certificate" or "protocol
593+
# version") in the outgoing BIO. Send it to the peer before
594+
# closing the transport so that it knows why the handshake
595+
# failed.
596+
self._process_outgoing()
591597
self._fatal_error(exc, msg)
592598
self._wakeup_waiter(exc)
593599
return
@@ -652,6 +658,10 @@ def _do_shutdown(self):
652658
except SSLAgainErrors:
653659
self._process_outgoing()
654660
except ssl.SSLError as exc:
661+
# gh-98078: send what OpenSSL left in the outgoing BIO, e.g.
662+
# the close_notify alert, to the peer before closing (see
663+
# _on_handshake_complete()).
664+
self._process_outgoing()
655665
self._on_shutdown_complete(exc)
656666
else:
657667
self._process_outgoing()
@@ -743,6 +753,11 @@ def _do_read(self):
743753
else:
744754
self._process_outgoing()
745755
self._control_ssl_reading()
756+
except ssl.SSLError as ex:
757+
# gh-98078: send the fatal TLS alert left in the outgoing
758+
# BIO to the peer (see _on_handshake_complete()).
759+
self._process_outgoing()
760+
self._fatal_error(ex, 'Fatal error on SSL protocol')
746761
except Exception as ex:
747762
self._fatal_error(ex, 'Fatal error on SSL protocol')
748763

Lib/test/test_asyncio/test_sslproto.py

Lines changed: 132 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -753,14 +753,16 @@ def test_create_connection_ssl_failed_certificate(self):
753753
sslctx = test_utils.simple_server_sslcontext()
754754
client_sslctx = test_utils.simple_client_sslcontext(
755755
disable_verify=False)
756+
server_err = None
756757

757758
def server(sock):
759+
nonlocal server_err
758760
try:
759761
sock.start_tls(
760762
sslctx,
761763
server_side=True)
762-
except ssl.SSLError:
763-
pass
764+
except ssl.SSLError as exc:
765+
server_err = exc
764766
except OSError:
765767
pass
766768
finally:
@@ -780,13 +782,71 @@ async def client(addr):
780782
with self.assertRaises(ssl.SSLCertVerificationError):
781783
self.loop.run_until_complete(client(srv.addr))
782784

785+
# gh-98078: the client must send a fatal TLS alert to the
786+
# server instead of just closing the connection, so that the
787+
# server knows why the handshake failed.
788+
self.assertIsInstance(server_err, ssl.SSLError)
789+
self.assertIn('ALERT_UNKNOWN_CA', server_err.reason or '')
790+
791+
def test_create_server_ssl_failed_handshake_sends_alert(self):
792+
# gh-98078: when the handshake fails, the server must send the
793+
# fatal TLS alert generated by OpenSSL to the client before
794+
# closing the connection, so that the client knows why the
795+
# handshake failed (here: no TLS version in common).
796+
if not ssl.HAS_TLSv1_3 or not ssl.HAS_TLSv1_2:
797+
self.skipTest('needs TLSv1.2 and TLSv1.3 support')
798+
799+
self.loop.set_exception_handler(lambda loop, ctx: None)
800+
801+
server_context = test_utils.simple_server_sslcontext()
802+
server_context.minimum_version = ssl.TLSVersion.TLSv1_3
803+
client_context = test_utils.simple_client_sslcontext()
804+
client_context.maximum_version = ssl.TLSVersion.TLSv1_2
805+
806+
client_done = self.loop.create_future()
807+
client_err = None
808+
809+
def client(sock, addr):
810+
nonlocal client_err
811+
try:
812+
sock.settimeout(self.TIMEOUT)
813+
sock.connect(addr)
814+
try:
815+
sock.start_tls(client_context)
816+
except OSError as exc:
817+
client_err = exc
818+
finally:
819+
sock.close()
820+
finally:
821+
self.loop.call_soon_threadsafe(
822+
client_done.set_result, None)
823+
824+
async def run_main():
825+
server = await self.loop.create_server(
826+
asyncio.Protocol, '127.0.0.1', 0, ssl=server_context)
827+
addr = server.sockets[0].getsockname()
828+
try:
829+
with self.tcp_client(lambda sock: client(sock, addr),
830+
timeout=self.TIMEOUT):
831+
await asyncio.wait_for(client_done, self.TIMEOUT)
832+
finally:
833+
server.close()
834+
await server.wait_closed()
835+
836+
self.loop.run_until_complete(run_main())
837+
838+
self.assertIsInstance(client_err, ssl.SSLError)
839+
self.assertIn('ALERT_PROTOCOL_VERSION', client_err.reason or '')
840+
783841
def test_start_tls_client_corrupted_ssl(self):
784842
self.loop.set_exception_handler(lambda loop, ctx: None)
785843

786844
sslctx = test_utils.simple_server_sslcontext()
787845
client_sslctx = test_utils.simple_client_sslcontext()
846+
server_err = None
788847

789848
def server(sock):
849+
nonlocal server_err
790850
orig_sock = sock.dup()
791851
try:
792852
sock.start_tls(
@@ -795,8 +855,11 @@ def server(sock):
795855
sock.sendall(b'A\n')
796856
sock.recv_all(1)
797857
orig_sock.send(b'please corrupt the SSL connection')
798-
except ssl.SSLError:
799-
pass
858+
# gh-98078: receive the fatal TLS alert sent by the
859+
# client before it closed the connection
860+
sock.recv(16)
861+
except ssl.SSLError as exc:
862+
server_err = exc
800863
finally:
801864
orig_sock.close()
802865
sock.close()
@@ -822,6 +885,71 @@ async def client(addr):
822885
res = self.loop.run_until_complete(client(srv.addr))
823886

824887
self.assertEqual(res, 'OK')
888+
# gh-98078: the client must send a fatal TLS alert to the
889+
# server instead of just closing the connection, so that the
890+
# server knows why the connection was dropped.
891+
self.assertIsInstance(server_err, ssl.SSLError)
892+
self.assertIn('ALERT_', server_err.reason or '')
893+
894+
def test_shutdown_corrupted_ssl_sends_close_notify(self):
895+
# gh-98078: when the TLS shutdown fails (here: on a corrupted
896+
# record that was buffered while the application had reading
897+
# paused), the close_notify alert that OpenSSL already
898+
# generated must be sent to the peer before the transport is
899+
# closed, so that the peer sees a clean TLS EOF instead of a
900+
# connection reset.
901+
self.loop.set_exception_handler(lambda loop, ctx: None)
902+
903+
sslctx = test_utils.simple_server_sslcontext()
904+
client_sslctx = test_utils.simple_client_sslcontext()
905+
server_err = None
906+
907+
def server(sock):
908+
nonlocal server_err
909+
orig_sock = sock.dup()
910+
try:
911+
sock.start_tls(
912+
sslctx,
913+
server_side=True)
914+
sock.sendall(b'A\n')
915+
sock.recv_all(1)
916+
orig_sock.send(b'please corrupt the SSL connection')
917+
# the client now closes the connection; although its
918+
# TLS shutdown fails on the corrupted record, it must
919+
# still send close_notify, completing our unwrap()
920+
sock.unwrap()
921+
except ssl.SSLError as exc:
922+
server_err = exc
923+
finally:
924+
orig_sock.close()
925+
sock.close()
926+
927+
async def client(addr):
928+
reader, writer = await asyncio.open_connection(
929+
*addr,
930+
ssl=client_sslctx,
931+
server_hostname='')
932+
# drain the post-handshake data (e.g. TLS session tickets)
933+
# so that only the corrupted record can be buffered next
934+
self.assertEqual(await reader.readline(), b'A\n')
935+
# keep the corrupted record buffered in the incoming BIO
936+
writer.transport.pause_reading()
937+
writer.write(b'B')
938+
await writer.drain()
939+
# wait for the corrupted record to arrive in the read buffer
940+
async with asyncio.timeout(support.SHORT_TIMEOUT):
941+
while not writer.transport.get_read_buffer_size():
942+
await asyncio.sleep(0)
943+
writer.close()
944+
with self.assertRaises(ssl.SSLError):
945+
await writer.wait_closed()
946+
947+
with self.tcp_server(server,
948+
max_clients=1,
949+
backlog=1) as srv:
950+
self.loop.run_until_complete(client(srv.addr))
951+
952+
self.assertIsNone(server_err)
825953

826954

827955
@unittest.skipIf(ssl is None, 'No ssl module')
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fix :mod:`asyncio` SSL transports not sending the fatal TLS alert to the
2+
peer when the TLS handshake fails or when receiving corrupted data, and
3+
not sending the ``close_notify`` alert to the peer when the TLS shutdown
4+
fails. The peer can now tell why the connection was dropped (for example,
5+
certificate verification failure or no TLS version in common) instead of
6+
seeing the connection abruptly closed.

0 commit comments

Comments
 (0)