Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Lib/asyncio/sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,12 @@ def _on_handshake_complete(self, handshake_exc):
msg = 'SSL handshake failed on verifying the certificate'
else:
msg = 'SSL handshake failed'
# gh-98078: When the handshake fails, OpenSSL leaves the fatal
# TLS alert (for example "bad certificate" or "protocol
# version") in the outgoing BIO. Send it to the peer before
# closing the transport so that it knows why the handshake
# failed.
self._process_outgoing()
self._fatal_error(exc, msg)
self._wakeup_waiter(exc)
return
Expand Down Expand Up @@ -652,6 +658,10 @@ def _do_shutdown(self):
except SSLAgainErrors:
self._process_outgoing()
except ssl.SSLError as exc:
# gh-98078: send what OpenSSL left in the outgoing BIO, e.g.
# the close_notify alert, to the peer before closing (see
# _on_handshake_complete()).
self._process_outgoing()
self._on_shutdown_complete(exc)
else:
self._process_outgoing()
Expand Down Expand Up @@ -743,6 +753,11 @@ def _do_read(self):
else:
self._process_outgoing()
self._control_ssl_reading()
except ssl.SSLError as ex:
# gh-98078: send the fatal TLS alert left in the outgoing
# BIO to the peer (see _on_handshake_complete()).
self._process_outgoing()
self._fatal_error(ex, 'Fatal error on SSL protocol')
except Exception as ex:
self._fatal_error(ex, 'Fatal error on SSL protocol')

Expand Down
136 changes: 132 additions & 4 deletions Lib/test/test_asyncio/test_sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,14 +753,16 @@ def test_create_connection_ssl_failed_certificate(self):
sslctx = test_utils.simple_server_sslcontext()
client_sslctx = test_utils.simple_client_sslcontext(
disable_verify=False)
server_err = None

def server(sock):
nonlocal server_err
try:
sock.start_tls(
sslctx,
server_side=True)
except ssl.SSLError:
pass
except ssl.SSLError as exc:
server_err = exc
except OSError:
pass
finally:
Expand All @@ -780,13 +782,71 @@ async def client(addr):
with self.assertRaises(ssl.SSLCertVerificationError):
self.loop.run_until_complete(client(srv.addr))

# gh-98078: the client must send a fatal TLS alert to the
# server instead of just closing the connection, so that the
# server knows why the handshake failed.
self.assertIsInstance(server_err, ssl.SSLError)
self.assertIn('ALERT_UNKNOWN_CA', server_err.reason or '')

def test_create_server_ssl_failed_handshake_sends_alert(self):
# gh-98078: when the handshake fails, the server must send the
# fatal TLS alert generated by OpenSSL to the client before
# closing the connection, so that the client knows why the
# handshake failed (here: no TLS version in common).
if not ssl.HAS_TLSv1_3 or not ssl.HAS_TLSv1_2:
self.skipTest('needs TLSv1.2 and TLSv1.3 support')

self.loop.set_exception_handler(lambda loop, ctx: None)

server_context = test_utils.simple_server_sslcontext()
server_context.minimum_version = ssl.TLSVersion.TLSv1_3
client_context = test_utils.simple_client_sslcontext()
client_context.maximum_version = ssl.TLSVersion.TLSv1_2

client_done = self.loop.create_future()
client_err = None

def client(sock, addr):
nonlocal client_err
try:
sock.settimeout(self.TIMEOUT)
sock.connect(addr)
try:
sock.start_tls(client_context)
except OSError as exc:
client_err = exc
finally:
sock.close()
finally:
self.loop.call_soon_threadsafe(
client_done.set_result, None)

async def run_main():
server = await self.loop.create_server(
asyncio.Protocol, '127.0.0.1', 0, ssl=server_context)
addr = server.sockets[0].getsockname()
try:
with self.tcp_client(lambda sock: client(sock, addr),
timeout=self.TIMEOUT):
await asyncio.wait_for(client_done, self.TIMEOUT)
finally:
server.close()
await server.wait_closed()

self.loop.run_until_complete(run_main())

self.assertIsInstance(client_err, ssl.SSLError)
self.assertIn('ALERT_PROTOCOL_VERSION', client_err.reason or '')

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()
server_err = None

def server(sock):
nonlocal server_err
orig_sock = sock.dup()
try:
sock.start_tls(
Expand All @@ -795,8 +855,11 @@ def server(sock):
sock.sendall(b'A\n')
sock.recv_all(1)
orig_sock.send(b'please corrupt the SSL connection')
except ssl.SSLError:
pass
# gh-98078: receive the fatal TLS alert sent by the
# client before it closed the connection
sock.recv(16)
except ssl.SSLError as exc:
server_err = exc
finally:
orig_sock.close()
sock.close()
Expand All @@ -822,6 +885,71 @@ async def client(addr):
res = self.loop.run_until_complete(client(srv.addr))

self.assertEqual(res, 'OK')
# gh-98078: the client must send a fatal TLS alert to the
# server instead of just closing the connection, so that the
# server knows why the connection was dropped.
self.assertIsInstance(server_err, ssl.SSLError)
self.assertIn('ALERT_', server_err.reason or '')

def test_shutdown_corrupted_ssl_sends_close_notify(self):
# gh-98078: when the TLS shutdown fails (here: on a corrupted
# record that was buffered while the application had reading
# paused), the close_notify alert that OpenSSL already
# generated must be sent to the peer before the transport is
# closed, so that the peer sees a clean TLS EOF instead of a
# connection reset.
self.loop.set_exception_handler(lambda loop, ctx: None)

sslctx = test_utils.simple_server_sslcontext()
client_sslctx = test_utils.simple_client_sslcontext()
server_err = None

def server(sock):
nonlocal server_err
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')
# the client now closes the connection; although its
# TLS shutdown fails on the corrupted record, it must
# still send close_notify, completing our unwrap()
sock.unwrap()
except ssl.SSLError as exc:
server_err = exc
finally:
orig_sock.close()
sock.close()

async def client(addr):
reader, writer = await asyncio.open_connection(
*addr,
ssl=client_sslctx,
server_hostname='')
# drain the post-handshake data (e.g. TLS session tickets)
# so that only the corrupted record can be buffered next
self.assertEqual(await reader.readline(), b'A\n')
# keep the corrupted record buffered in the incoming BIO
writer.transport.pause_reading()
writer.write(b'B')
await writer.drain()
# wait for the corrupted record to arrive in the read buffer
async with asyncio.timeout(support.SHORT_TIMEOUT):
while not writer.transport.get_read_buffer_size():
await asyncio.sleep(0)
writer.close()
with self.assertRaises(ssl.SSLError):
await writer.wait_closed()

with self.tcp_server(server,
max_clients=1,
backlog=1) as srv:
self.loop.run_until_complete(client(srv.addr))

self.assertIsNone(server_err)


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