proto: Change how tokens are encrypted

Previously, retry tokens were encrypted using the retry src cid as the
key derivation input. This has been described by a reputable individual
as "cheeky" (who, coincidentially, wrote that code in the first place).
More importantly, this presents obstacles to using NEW_TOKEN frames.

With this commit, tokens carry a random 128-bit value, which is used to
derive the key for encrypting the rest of the token.

quinn-proto/src/endpoint.rs

@@ -735,7 +735,7 @@ impl Endpoint {
             orig_dst_cid: incoming.packet.header.dst_cid,
             issued: server_config.time_source.now(),
         };
-        let token = Token::new(payload).encode(&*server_config.token_key, loc_cid);
+        let token = Token::new(payload, &mut self.rng).encode(&*server_config.token_key);
 
         let header = Header::Retry {
             src_cid: loc_cid,

quinn-proto/src/token.rs

@@ -1,9 +1,11 @@
 use std::{
     fmt,
+    mem::size_of,
     net::{IpAddr, SocketAddr},
 };
 
 use bytes::{Buf, BufMut};
+use rand::Rng;
 
 use crate::{
     coding::{BufExt, BufMutExt},
@@ -47,8 +49,7 @@ impl IncomingToken {
         //
         // > 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.
-        let Some(retry) = Token::decode(&*server_config.token_key, header.dst_cid, &header.token)
-        else {
+        let Some(retry) = Token::decode(&*server_config.token_key, &header.token) else {
             return Ok(unvalidated);
         };
 
@@ -79,18 +80,21 @@ pub(crate) struct InvalidRetryTokenError;
 pub(crate) struct Token {
     /// Content that is encrypted from the client
     pub(crate) payload: TokenPayload,
+    /// Randomly generated value, which must be unique, and is visible to the client
+    nonce: u128,
 }
 
 impl Token {
-    pub(crate) fn new(payload: TokenPayload) -> Self {
-        Self { payload }
+    /// Construct with newly sampled randomness
+    pub(crate) fn new(payload: TokenPayload, rng: &mut impl Rng) -> Self {
+        Self {
+            nonce: rng.gen(),
+            payload,
+        }
     }
 
-    pub(crate) fn encode(
-        &self,
-        key: &dyn HandshakeTokenKey,
-        retry_src_cid: ConnectionId,
-    ) -> Vec<u8> {
+    /// Encode and encrypt
+    pub(crate) fn encode(&self, key: &dyn HandshakeTokenKey) -> Vec<u8> {
         let mut buf = Vec::new();
 
         // Encode payload
@@ -99,20 +103,25 @@ impl Token {
         encode_unix_secs(&mut buf, self.payload.issued);
 
         // Encrypt
-        let aead_key = key.aead_from_hkdf(&retry_src_cid);
+        let aead_key = key.aead_from_hkdf(&self.nonce.to_le_bytes());
         aead_key.seal(&mut buf, &[]).unwrap();
+        buf.extend(&self.nonce.to_le_bytes());
 
         buf
     }
 
-    fn decode(
-        key: &dyn HandshakeTokenKey,
-        retry_src_cid: ConnectionId,
-        raw_token_bytes: &[u8],
-    ) -> Option<Self> {
+    /// Decode and decrypt
+    fn decode(key: &dyn HandshakeTokenKey, raw_token_bytes: &[u8]) -> Option<Self> {
         // Decrypt
-        let aead_key = key.aead_from_hkdf(&retry_src_cid);
-        let mut sealed_token = raw_token_bytes.to_vec();
+
+        // MSRV: split_at_checked requires 1.80.0
+        let nonce_slice_start = raw_token_bytes.len().checked_sub(size_of::<u128>())?;
+        let (sealed_token, nonce_bytes) = raw_token_bytes.split_at(nonce_slice_start);
+
+        let nonce = u128::from_le_bytes(nonce_bytes.try_into().unwrap());
+
+        let aead_key = key.aead_from_hkdf(nonce_bytes);
+        let mut sealed_token = sealed_token.to_vec();
         let data = aead_key.open(&mut sealed_token, &[]).ok()?;
 
         // Decode payload
@@ -127,6 +136,7 @@ impl Token {
         }
 
         Some(Self {
+            nonce,
             payload: TokenPayload {
                 address,
                 orig_dst_cid,
@@ -263,17 +273,17 @@ mod test {
         let prk = hkdf::Salt::new(hkdf::HKDF_SHA256, &[]).extract(&master_key);
 
         let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 4433);
-        let retry_src_cid = RandomConnectionIdGenerator::new(MAX_CID_SIZE).generate_cid();
         let token = Token {
+            nonce: rng.gen(),
             payload: TokenPayload {
                 address,
                 orig_dst_cid: RandomConnectionIdGenerator::new(MAX_CID_SIZE).generate_cid(),
                 issued: UNIX_EPOCH + Duration::from_secs(42), // Fractional seconds would be lost
             },
         };
-        let encoded = token.encode(&prk, retry_src_cid);
+        let encoded = token.encode(&prk);
 
-        let decoded = Token::decode(&prk, retry_src_cid, &encoded).expect("token didn't validate");
+        let decoded = Token::decode(&prk, &encoded).expect("token didn't validate");
         assert_eq!(token.payload.address, decoded.payload.address);
         assert_eq!(token.payload.orig_dst_cid, decoded.payload.orig_dst_cid);
         assert_eq!(token.payload.issued, decoded.payload.issued);
@@ -282,8 +292,6 @@ mod test {
     #[test]
     fn invalid_token_returns_err() {
         use super::*;
-        use crate::cid_generator::{ConnectionIdGenerator, RandomConnectionIdGenerator};
-        use crate::MAX_CID_SIZE;
         use rand::RngCore;
 
         let rng = &mut rand::thread_rng();
@@ -293,15 +301,13 @@ mod test {
 
         let prk = hkdf::Salt::new(hkdf::HKDF_SHA256, &[]).extract(&master_key);
 
-        let retry_src_cid = RandomConnectionIdGenerator::new(MAX_CID_SIZE).generate_cid();
-
         let mut invalid_token = Vec::new();
 
         let mut random_data = [0; 32];
         rand::thread_rng().fill_bytes(&mut random_data);
         invalid_token.put_slice(&random_data);
 
         // Assert: garbage sealed data returns err
-        assert!(Token::decode(&prk, retry_src_cid, &invalid_token).is_none());
+        assert!(Token::decode(&prk, &invalid_token).is_none());
     }
 }