Improved path challenge handling. (#483)

1) We are willing to reply to up to 5 outstanding challenges
on a connection, as it is legal for a client to send more than
one.  We limit to 5 to prevent excessive challenging.

2) We now accept path challege responses on any network path,
in accordance with RFC 9000 section 8.2.3.

src/aioquic/quic/connection.py

@@ -75,6 +75,8 @@
     "1": tls.Epoch.ONE_RTT,
 }
 MAX_EARLY_DATA = 0xFFFFFFFF
+MAX_REMOTE_CHALLENGES = 5
+MAX_LOCAL_CHALLENGES = 5
 SECRETS_LABELS = [
     [
         None,
@@ -198,14 +200,14 @@ class QuicConnectionState(Enum):
     TERMINATED = 4
 
 
-@dataclass
 class QuicNetworkPath:
-    addr: NetworkAddress
-    bytes_received: int = 0
-    bytes_sent: int = 0
-    is_validated: bool = False
-    local_challenge: Optional[bytes] = None
-    remote_challenge: Optional[bytes] = None
+    def __init__(self, addr: NetworkAddress, is_validated: bool = False):
+        self.addr: NetworkAddress = addr
+        self.bytes_received: int = 0
+        self.bytes_sent: int = 0
+        self.is_validated: bool = is_validated
+        self.local_challenge_sent: bool = False
+        self.remote_challenges: Deque[bytes] = deque()
 
     def can_send(self, size: int) -> bool:
         return self.is_validated or (self.bytes_sent + size) <= 3 * self.bytes_received
@@ -309,6 +311,7 @@ def __init__(
         self._host_cid_seq = 1
         self._local_ack_delay_exponent = 3
         self._local_active_connection_id_limit = 8
+        self._local_challenges: Dict[bytes, QuicNetworkPath] = {}
         self._local_initial_source_connection_id = self._host_cids[0].cid
         self._local_max_data = Limit(
             frame_type=QuicFrameType.MAX_DATA,
@@ -2037,7 +2040,7 @@ def _handle_path_challenge_frame(
                 self._quic_logger.encode_path_challenge_frame(data=data)
             )
 
-        context.network_path.remote_challenge = data
+        context.network_path.remote_challenges.append(data)
 
     def _handle_path_response_frame(
         self, context: QuicReceiveContext, frame_type: int, buf: Buffer
@@ -2053,16 +2056,16 @@ def _handle_path_response_frame(
                 self._quic_logger.encode_path_response_frame(data=data)
             )
 
-        if data != context.network_path.local_challenge:
+        try:
+            network_path = self._local_challenges.pop(data)
+        except KeyError:
             raise QuicConnectionError(
                 error_code=QuicErrorCode.PROTOCOL_VIOLATION,
                 frame_type=frame_type,
                 reason_phrase="Response does not match challenge",
             )
-        self._logger.debug(
-            "Network path %s validated by challenge", context.network_path.addr
-        )
-        context.network_path.is_validated = True
+        self._logger.debug("Network path %s validated by challenge", network_path.addr)
+        network_path.is_validated = True
 
     def _handle_ping_frame(
         self, context: QuicReceiveContext, frame_type: int, buf: Buffer
@@ -2780,6 +2783,14 @@ def _update_traffic_key(
                 cipher_suite=cipher_suite, secret=secret, version=self._version
             )
 
+    def _add_local_challenge(self, challenge: bytes, network_path: QuicNetworkPath):
+        self._local_challenges[challenge] = network_path
+        while len(self._local_challenges) > MAX_LOCAL_CHALLENGES:
+            # Dictionaries are ordered, so pop the first key until we are below the
+            # limit.
+            key = next(iter(self._local_challenges.keys()))
+            del self._local_challenges[key]
+
     def _write_application(
         self, builder: QuicPacketBuilder, network_path: QuicNetworkPath, now: float
     ) -> None:
@@ -2814,22 +2825,22 @@ def _write_application(
                     self._handshake_done_pending = False
 
                 # PATH CHALLENGE
-                if (
-                    not network_path.is_validated
-                    and network_path.local_challenge is None
-                ):
+                if not (network_path.is_validated or network_path.local_challenge_sent):
                     challenge = os.urandom(8)
+                    self._add_local_challenge(
+                        challenge=challenge, network_path=network_path
+                    )
                     self._write_path_challenge_frame(
                         builder=builder, challenge=challenge
                     )
-                    network_path.local_challenge = challenge
+                    network_path.local_challenge_sent = True
 
                 # PATH RESPONSE
-                if network_path.remote_challenge is not None:
+                while len(network_path.remote_challenges) > 0:
+                    challenge = network_path.remote_challenges.popleft()
                     self._write_path_response_frame(
-                        builder=builder, challenge=network_path.remote_challenge
+                        builder=builder, challenge=challenge
                     )
-                    network_path.remote_challenge = None
 
                 # NEW_CONNECTION_ID
                 for connection_id in self._host_cids:

tests/test_connection.py

@@ -10,6 +10,7 @@
 from aioquic.quic import events
 from aioquic.quic.configuration import SMALLEST_MAX_DATAGRAM_SIZE, QuicConfiguration
 from aioquic.quic.connection import (
+    MAX_LOCAL_CHALLENGES,
     STREAM_COUNT_MAX,
     NetworkAddress,
     QuicConnection,
@@ -1809,6 +1810,47 @@ def test_handle_path_challenge_frame(self):
             self.assertEqual(server._network_paths[1].addr, ("1.2.3.4", 1234))
             self.assertTrue(server._network_paths[1].is_validated)
 
+    def test_handle_path_challenge_response_on_different_path(self):
+        with client_and_server() as (client, server):
+            # client changes address and sends some data
+            client.send_stream_data(0, b"01234567")
+            for data, addr in client.datagrams_to_send(now=time.time()):
+                server.receive_datagram(data, ("1.2.3.4", 2345), now=time.time())
+
+            # check paths
+            self.assertEqual(len(server._network_paths), 2)
+            self.assertEqual(server._network_paths[0].addr, ("1.2.3.4", 2345))
+            self.assertFalse(server._network_paths[0].is_validated)
+            self.assertEqual(server._network_paths[1].addr, ("1.2.3.4", 1234))
+            self.assertTrue(server._network_paths[1].is_validated)
+
+            # server sends PATH_CHALLENGE and receives PATH_RESPONSE on the 1234
+            # path instead of the expected 2345 path.
+            for data, addr in server.datagrams_to_send(now=time.time()):
+                client.receive_datagram(data, SERVER_ADDR, now=time.time())
+            for data, addr in client.datagrams_to_send(now=time.time()):
+                server.receive_datagram(data, ("1.2.3.4", 1234), now=time.time())
+
+            # check paths; note that the order is backwards from the prior test
+            # as receiving on 1234 promotes it to first in the list
+            self.assertEqual(server._network_paths[0].addr, ("1.2.3.4", 1234))
+            self.assertTrue(server._network_paths[0].is_validated)
+            self.assertEqual(server._network_paths[1].addr, ("1.2.3.4", 2345))
+            self.assertTrue(server._network_paths[1].is_validated)
+
+    def test_local_path_challenges_are_bounded(self):
+        with client_and_server() as (client, server):
+            for i in range(MAX_LOCAL_CHALLENGES + 2):
+                server._add_local_challenge(
+                    int.to_bytes(i, 8, "big"), QuicNetworkPath(f"1.2.3.{i}")
+                )
+            self.assertEqual(len(server._local_challenges), MAX_LOCAL_CHALLENGES)
+            for i in range(2, MAX_LOCAL_CHALLENGES + 2):
+                self.assertEqual(
+                    server._local_challenges[int.to_bytes(i, 8, "big")].addr,
+                    f"1.2.3.{i}",
+                )
+
     def test_handle_path_response_frame_bad(self):
         with client_and_server() as (client, server):
             # server receives unsolicited PATH_RESPONSE