Skip to content

Commit

Permalink
Close connection if client's first INITIAL contains no CRYPTO
Browse files Browse the repository at this point in the history
According to section 17.2.2 of RFC 9000, the client's first INITIAL
packet must contain a CRYPTO frame. Ensure this is indeed to case to
avoid connections in a half-connected state.
  • Loading branch information
jlaine committed Dec 12, 2023
1 parent 18214db commit cd666b3
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/aioquic/quic/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -908,13 +908,15 @@ def receive_datagram(self, data: bytes, addr: NetworkAddress, now: float) -> Non
)
return

crypto_frame_required = False
network_path = self._find_network_path(addr)

# server initialization
if not self._is_client and self._state == QuicConnectionState.FIRSTFLIGHT:
assert (
header.packet_type == PACKET_TYPE_INITIAL
), "first packet must be INITIAL"
crypto_frame_required = True
self._network_paths = [network_path]
self._version = QuicProtocolVersion(header.version)
self._initialize(header.destination_cid)
Expand Down Expand Up @@ -1042,7 +1044,7 @@ def receive_datagram(self, data: bytes, addr: NetworkAddress, now: float) -> Non
)
try:
is_ack_eliciting, is_probing = self._payload_received(
context, plain_payload
context, plain_payload, crypto_frame_required=crypto_frame_required
)
except QuicConnectionError as exc:
self._logger.warning(exc)
Expand Down Expand Up @@ -2329,13 +2331,17 @@ def _on_retire_connection_id_delivery(
self._retire_connection_ids.append(sequence_number)

def _payload_received(
self, context: QuicReceiveContext, plain: bytes
self,
context: QuicReceiveContext,
plain: bytes,
crypto_frame_required: bool = False,
) -> Tuple[bool, bool]:
"""
Handle a QUIC packet payload.
"""
buf = Buffer(data=plain)

crypto_frame_found = False
frame_found = False
is_ack_eliciting = False
is_probing = None
Expand Down Expand Up @@ -2384,6 +2390,9 @@ def _payload_received(
# update ACK only / probing flags
frame_found = True

if frame_type == QuicFrameType.CRYPTO:
crypto_frame_found = True

if frame_type not in NON_ACK_ELICITING_FRAME_TYPES:
is_ack_eliciting = True

Expand All @@ -2399,6 +2408,15 @@ def _payload_received(
reason_phrase="Packet contains no frames",
)

# RFC 9000 - 17.2.2. Initial Packet
# The first packet sent by a client always includes a CRYPTO frame.
if crypto_frame_required and not crypto_frame_found:
raise QuicConnectionError(
error_code=QuicErrorCode.PROTOCOL_VIOLATION,
frame_type=QuicFrameType.PADDING,
reason_phrase="Packet contains no CRYPTO frame",
)

return is_ack_eliciting, bool(is_probing)

def _replenish_connection_ids(self) -> None:
Expand Down
13 changes: 13 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,19 @@ def test_connect_with_loss_5(self):
self.assertFalse(server._handshake_done_pending)
self.assertEqual(datagram_sizes(items), [224])

def test_connect_with_no_crypto_frame(self):
def patch(client):
"""
Patch client to send PING instead of CRYPTO.
"""
client._push_crypto_data = client._send_probe

with client_and_server(client_patch=patch) as (client, server):
self.assertEqual(
server._close_event.reason_phrase,
"Packet contains no CRYPTO frame",
)

def test_connect_with_no_transport_parameters(self):
def patch(client):
"""
Expand Down

0 comments on commit cd666b3

Please sign in to comment.