From 4a80c3f7c1c63b00ed64ea833e1b3fb3d088a6b3 Mon Sep 17 00:00:00 2001
From: Phoenix Kahlo <kahlo.phoenix@gmail.com>
Date: Sat, 14 Dec 2024 15:25:13 -0600
Subject: [PATCH] proto: Refactor TokenDecodeError

---
 quinn-proto/src/endpoint.rs |  9 ++-----
 quinn-proto/src/token.rs    | 47 ++++++++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs
index a7de52eef..9f9820261 100644
--- a/quinn-proto/src/endpoint.rs
+++ b/quinn-proto/src/endpoint.rs
@@ -29,7 +29,7 @@ use crate::{
         ConnectionEvent, ConnectionEventInner, ConnectionId, DatagramConnectionEvent, EcnCodepoint,
         EndpointEvent, EndpointEventInner, IssuedCid,
     },
-    token::TokenDecodeError,
+    token,
     transport_parameters::{PreferredAddress, TransportParameters},
     Duration, Instant, ResetToken, RetryToken, Side, SystemTime, Transmit, TransportConfig,
     TransportError, INITIAL_MTU, MAX_CID_SIZE, MIN_INITIAL_SIZE, RESET_TOKEN_SIZE,
@@ -510,12 +510,7 @@ impl Endpoint {
                 {
                     (Some(header.dst_cid), token.orig_dst_cid)
                 }
-                Err(TokenDecodeError::UnknownToken) => {
-                    // Token may have been generated by an incompatible endpoint, e.g. a
-                    // different version or a neighbor behind the same load balancer. We
-                    // can't interpret it, so we proceed as if there was no token.
-                    (None, header.dst_cid)
-                }
+                Err(token::ValidationError::Unusable) => (None, header.dst_cid),
                 _ => {
                     debug!("rejecting invalid stateless retry token");
                     return Some(DatagramEvent::Response(self.initial_close(
diff --git a/quinn-proto/src/token.rs b/quinn-proto/src/token.rs
index d4de5200c..5face46a7 100644
--- a/quinn-proto/src/token.rs
+++ b/quinn-proto/src/token.rs
@@ -48,23 +48,21 @@ impl RetryToken {
         address: &SocketAddr,
         retry_src_cid: &ConnectionId,
         raw_token_bytes: &[u8],
-    ) -> Result<Self, TokenDecodeError> {
+    ) -> Result<Self, ValidationError> {
         let aead_key = key.aead_from_hkdf(retry_src_cid);
         let mut sealed_token = raw_token_bytes.to_vec();
 
         let data = aead_key.open(&mut sealed_token, &[])?;
         let mut reader = io::Cursor::new(data);
-        let token_addr = decode_addr(&mut reader).ok_or(TokenDecodeError::UnknownToken)?;
+        let token_addr = decode_addr(&mut reader).ok_or(ValidationError::Unusable)?;
         if token_addr != *address {
-            return Err(TokenDecodeError::WrongAddress);
+            return Err(ValidationError::InvalidRetry);
         }
         let orig_dst_cid =
-            ConnectionId::decode_long(&mut reader).ok_or(TokenDecodeError::UnknownToken)?;
+            ConnectionId::decode_long(&mut reader).ok_or(ValidationError::Unusable)?;
         let issued = UNIX_EPOCH
             + Duration::new(
-                reader
-                    .get::<u64>()
-                    .map_err(|_| TokenDecodeError::UnknownToken)?,
+                reader.get::<u64>().map_err(|_| ValidationError::Unusable)?,
                 0,
             );
 
@@ -99,19 +97,36 @@ fn decode_addr<B: Buf>(buf: &mut B) -> Option<SocketAddr> {
     Some(SocketAddr::new(ip, port))
 }
 
-/// Reasons why a retry token might fail to validate a client's address
+/// Error for a token failing to validate a client's address
 #[derive(Debug, Copy, Clone)]
-pub(crate) enum TokenDecodeError {
-    /// Token was not recognized. It should be silently ignored.
-    UnknownToken,
-    /// Token was well-formed but associated with an incorrect address. The connection cannot be
-    /// established.
-    WrongAddress,
+pub(crate) enum ValidationError {
+    /// Token may have come from a NEW_TOKEN frame (including from a different server or a previous
+    /// run of this server with different keys), and was not valid
+    ///
+    /// It should be silently ignored.
+    ///
+    /// In cases where a token cannot be decrypted/decoded, we must allow for the possibility that
+    /// this is caused not by client malfeasance, but by the token having been generated by an
+    /// incompatible endpoint, e.g. a different version or a neighbor behind the same load
+    /// balancer. In such cases we proceed as if there was no token.
+    ///
+    /// [_RFC 9000 § 8.1.3:_](https://www.rfc-editor.org/rfc/rfc9000.html#section-8.1.3-10)
+    ///
+    /// > If the token is invalid, then the server SHOULD proceed as if the client did not have a
+    /// > validated address, including potentially sending a Retry packet.
+    ///
+    /// That said, this may also be used when a token _can_ be unambiguously decrypted/decoded as a
+    /// token from a NEW_TOKEN frame, but is simply not valid.
+    Unusable,
+    /// Token was unambiguously from a Retry packet, and was not valid
+    ///
+    /// The connection cannot be established.
+    InvalidRetry,
 }
 
-impl From<CryptoError> for TokenDecodeError {
+impl From<CryptoError> for ValidationError {
     fn from(CryptoError: CryptoError) -> Self {
-        Self::UnknownToken
+        Self::Unusable
     }
 }