Skip to content

Commit 8f1b557

Browse files
committed
gh-160499: Pause asyncio TLS reads when SSL replies pile up
SSLProtocol read flow control only looked at the incoming BIO, so records OpenSSL writes on its own while reading a non-reading peer (renegotiation alerts, KeyUpdate replies) grew the outgoing BIO without bound. Count that non-application output toward the read high-water mark and re-check it when writing resumes. ANT-2026-RNWRQ22W
1 parent 3b3c692 commit 8f1b557

3 files changed

Lines changed: 59 additions & 1 deletion

File tree

‎Lib/asyncio/sslproto.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,8 @@ def __init__(self, loop, app_protocol, sslcontext, waiter,
308308
# App data write buffering
309309
self._write_backlog = collections.deque()
310310
self._write_buffer_size = 0
311+
# How much of self._outgoing is encrypted app data
312+
self._outgoing_app_size = 0
311313

312314
self._waiter = waiter
313315
self._loop = loop
@@ -710,6 +712,7 @@ def _write_appdata(self, list_of_data):
710712
self._fatal_error(ex, 'Fatal error on SSL protocol')
711713

712714
def _do_write(self):
715+
pending = self._outgoing.pending
713716
try:
714717
while self._write_backlog:
715718
data = self._write_backlog[0]
@@ -723,11 +726,13 @@ def _do_write(self):
723726
self._write_buffer_size -= data_len
724727
except SSLAgainErrors:
725728
pass
729+
self._outgoing_app_size += self._outgoing.pending - pending
726730
self._process_outgoing()
727731

728732
def _process_outgoing(self):
729733
if not self._ssl_writing_paused:
730734
data = self._outgoing.read()
735+
self._outgoing_app_size = 0
731736
if len(data):
732737
self._transport.write(data)
733738
self._control_app_writing()
@@ -894,7 +899,14 @@ def resume():
894899
# Flow control for reads from SSL socket
895900

896901
def _control_ssl_reading(self):
897-
size = self._get_read_buffer_size()
902+
# Also count the records OpenSSL writes on its own while reading
903+
# (alerts, KeyUpdate replies, ...) which the peer does not read:
904+
# the app is not involved in producing them, so pausing it does
905+
# not bound them; only pausing reads from the peer does. App data
906+
# is left out so that two peers which both write before they read
907+
# cannot deadlock.
908+
size = (self._get_read_buffer_size() +
909+
self._outgoing.pending - self._outgoing_app_size)
898910
if size >= self._incoming_high_water and not self._ssl_reading_paused:
899911
self._ssl_reading_paused = True
900912
self._transport.pause_reading()
@@ -927,6 +939,7 @@ def resume_writing(self):
927939
assert self._ssl_writing_paused
928940
self._ssl_writing_paused = False
929941
self._process_outgoing()
942+
self._control_ssl_reading()
930943

931944
def _fatal_error(self, exc, message='Fatal error on transport'):
932945
if self._transport:

‎Lib/test/test_asyncio/test_sslproto.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,46 @@ def test_write_after_closing(self):
265265
# should not raise
266266
self.assertIsNone(transp.write(b'data'))
267267

268+
def test_pause_reading_if_ssl_replies_are_not_read(self):
269+
# Output which OpenSSL produces on its own while reading (alerts,
270+
# KeyUpdate, ...) must not pile up if the peer does not read it.
271+
ssl_proto = self.ssl_protocol()
272+
transport = self.connection_made(ssl_proto, do_handshake=mock.Mock())
273+
reply = b'x' * (ssl_proto._incoming_high_water // 2)
274+
275+
def read(*args):
276+
ssl_proto._outgoing.write(reply)
277+
raise ssl.SSLWantReadError
278+
ssl_proto._sslobj.read.side_effect = read
279+
280+
ssl_proto.pause_writing()
281+
ssl_proto.buffer_updated(0)
282+
transport.pause_reading.assert_not_called()
283+
ssl_proto.buffer_updated(0)
284+
transport.pause_reading.assert_called_once_with()
285+
transport.resume_reading.assert_not_called()
286+
transport.write.assert_not_called()
287+
288+
ssl_proto.resume_writing()
289+
transport.write.assert_called_once_with(reply * 2)
290+
transport.resume_reading.assert_called_once_with()
291+
292+
def test_no_pause_reading_if_app_data_is_not_read(self):
293+
# Peers which both write a lot before they read must not deadlock.
294+
ssl_proto = self.ssl_protocol()
295+
transport = self.connection_made(ssl_proto, do_handshake=mock.Mock())
296+
ssl_proto._sslobj.write.side_effect = ssl_proto._outgoing.write
297+
data = b'x' * ssl_proto._incoming_high_water
298+
299+
ssl_proto.pause_writing()
300+
ssl_proto._app_transport.write(data)
301+
ssl_proto.buffer_updated(0)
302+
transport.pause_reading.assert_not_called()
303+
304+
ssl_proto.resume_writing()
305+
transport.write.assert_called_once_with(data)
306+
transport.resume_reading.assert_not_called()
307+
268308

269309
##############################################################################
270310
# Start TLS Tests
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix unbounded memory growth in :mod:`asyncio` TLS connections when a peer
2+
stops reading but keeps sending records that make OpenSSL generate its own
3+
replies (such as TLS 1.2 renegotiation alerts). Reading from the peer is now
4+
paused when this output accumulates, instead of only when application data
5+
does.

0 commit comments

Comments
 (0)