From cd666b308dec810a84ef2f39b1b69346ade86702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jeremy=20Lain=C3=A9?= Date: Tue, 12 Dec 2023 08:26:37 +0100 Subject: [PATCH] Close connection if client's first INITIAL contains no CRYPTO 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. --- src/aioquic/quic/connection.py | 22 ++++++++++++++++++++-- tests/test_connection.py | 13 +++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/aioquic/quic/connection.py b/src/aioquic/quic/connection.py index 8dfcca8c6..a879fcd1d 100644 --- a/src/aioquic/quic/connection.py +++ b/src/aioquic/quic/connection.py @@ -908,6 +908,7 @@ 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 @@ -915,6 +916,7 @@ def receive_datagram(self, data: bytes, addr: NetworkAddress, now: float) -> Non 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) @@ -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) @@ -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 @@ -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 @@ -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: diff --git a/tests/test_connection.py b/tests/test_connection.py index 7ec9b6645..ead835d2a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -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): """