Skip to content

Commit

Permalink
Only buffer up to 512 KiB of pending CRYPTO frames [#501]. (#505)
Browse files Browse the repository at this point in the history
  • Loading branch information
rthalley authored Jun 18, 2024
1 parent b507364 commit 174a2eb
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 9 deletions.
14 changes: 9 additions & 5 deletions src/aioquic/quic/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@
push_ack_frame,
push_quic_transport_parameters,
)
from .packet_builder import (
QuicDeliveryState,
QuicPacketBuilder,
QuicPacketBuilderStop,
)
from .packet_builder import QuicDeliveryState, QuicPacketBuilder, QuicPacketBuilderStop
from .recovery import QuicPacketRecovery, QuicPacketSpace
from .stream import FinalSizeError, QuicStream, StreamFinishedError

Expand Down Expand Up @@ -95,6 +91,7 @@
STREAM_COUNT_MAX = 0x1000000000000000
UDP_HEADER_SIZE = 8
MAX_PENDING_RETIRES = 100
MAX_PENDING_CRYPTO = 524288 # in bytes

NetworkAddress = Any

Expand Down Expand Up @@ -1628,6 +1625,13 @@ def _handle_crypto_frame(
)

stream = self._crypto_streams[context.epoch]
pending = offset + length - stream.receiver.starting_offset()
if pending > MAX_PENDING_CRYPTO:
raise QuicConnectionError(
error_code=QuicErrorCode.CRYPTO_BUFFER_EXCEEDED,
frame_type=frame_type,
reason_phrase="too much crypto buffering",
)
event = stream.receiver.handle_frame(frame)
if event is not None:
# pass data to TLS layer
Expand Down
3 changes: 3 additions & 0 deletions src/aioquic/quic/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ def get_stop_frame(self) -> QuicStopSendingFrame:
stream_id=self._stream_id,
)

def starting_offset(self) -> int:
return self._buffer_start

def handle_frame(
self, frame: QuicStreamFrame
) -> Optional[events.StreamDataReceived]:
Expand Down
34 changes: 30 additions & 4 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from aioquic.quic.configuration import SMALLEST_MAX_DATAGRAM_SIZE, QuicConfiguration
from aioquic.quic.connection import (
MAX_LOCAL_CHALLENGES,
MAX_PENDING_CRYPTO,
STREAM_COUNT_MAX,
NetworkAddress,
QuicConnection,
Expand All @@ -30,10 +31,7 @@
encode_quic_version_negotiation,
push_quic_transport_parameters,
)
from aioquic.quic.packet_builder import (
QuicDeliveryState,
QuicPacketBuilder,
)
from aioquic.quic.packet_builder import QuicDeliveryState, QuicPacketBuilder
from aioquic.quic.recovery import QuicPacketPacer

from .utils import (
Expand Down Expand Up @@ -1365,6 +1363,34 @@ def test_handle_crypto_frame_over_largest_offset(self):
cm.exception.reason_phrase, "offset + length cannot exceed 2^62 - 1"
)

def test_excessive_crypto_buffering(self):
with client_and_server() as (client, server):
# Client receives data that causes more than 512K of buffering; note that
# because the stream buffer is a single buffer and not a set of fragments,
# the total buffering size depends not on how much data is received, but
# how much buffering is needed. We send fragments of only 100 bytes
# at offsets 10000, 20000, 30000 etc.
highest_good_offset = 0
with self.assertRaises(QuicConnectionError) as cm:
# We don't start at zero as we want to force buffering, not cause
# a TLS error.
for offset in range(10000, 1000000, 10000):
client._handle_crypto_frame(
client_receive_context(client),
QuicFrameType.CRYPTO,
Buffer(
data=encode_uint_var(offset)
+ encode_uint_var(100)
+ b"\x00" * 100
),
)
highest_good_offset = offset
self.assertEqual(
cm.exception.error_code, QuicErrorCode.CRYPTO_BUFFER_EXCEEDED
)
self.assertEqual(cm.exception.frame_type, QuicFrameType.CRYPTO)
self.assertEqual(highest_good_offset, (MAX_PENDING_CRYPTO // 10000) * 10000)

def test_handle_data_blocked_frame(self):
with client_and_server() as (client, server):
# client receives DATA_BLOCKED: 12345
Expand Down

0 comments on commit 174a2eb

Please sign in to comment.