From dfe9176b3c9a5320b52ae034da0c79cae0cccfb2 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Wed, 23 Aug 2023 13:21:58 -0700 Subject: [PATCH] proto: detect stateless resets in authed and unprotected packets RFC9000 says: endpoints MUST treat any packet ending in a valid stateless reset token as a Stateless Reset Previously, we did not detect stateless resets that appeared to be unprotected packets (e.g. Retry or Version Negotiation) or which were successfully decrypted (astronomically unlikely with TLS, but possible with custom cryptographic layers). (cherry picked from commit 7f260292848a93d615eb43e6e88114a97e64daf1) --- quinn-proto/src/connection/mod.rs | 41 ++++++++++++++----------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 04fe28ded..3f0c0e7cc 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -2007,29 +2007,28 @@ impl Connection { .map(move |number| (packet, number)), }; let result = match decrypted { + _ if stateless_reset => { + debug!("got stateless reset"); + Err(ConnectionError::Reset) + } Err(Some(e)) => { warn!("illegal packet: {}", e); Err(e.into()) } Err(None) => { - if stateless_reset { - debug!("got stateless reset"); - Err(ConnectionError::Reset) + debug!("failed to authenticate packet"); + self.authentication_failures += 1; + let integrity_limit = self.spaces[self.highest_space] + .crypto + .as_ref() + .unwrap() + .packet + .local + .integrity_limit(); + if self.authentication_failures > integrity_limit { + Err(TransportError::AEAD_LIMIT_REACHED("integrity limit violated").into()) } else { - debug!("failed to authenticate packet"); - self.authentication_failures += 1; - let integrity_limit = self.spaces[self.highest_space] - .crypto - .as_ref() - .unwrap() - .packet - .local - .integrity_limit(); - if self.authentication_failures > integrity_limit { - Err(TransportError::AEAD_LIMIT_REACHED("integrity limit violated").into()) - } else { - return; - } + return; } } Ok((packet, number)) => { @@ -2041,12 +2040,8 @@ impl Connection { let is_duplicate = |n| self.spaces[packet.header.space()].dedup.insert(n); if number.map_or(false, is_duplicate) { - if stateless_reset { - Err(ConnectionError::Reset) - } else { - warn!("discarding possible duplicate packet"); - return; - } + warn!("discarding possible duplicate packet"); + return; } else if self.state.is_handshake() && packet.header.is_short() { // TODO: SHOULD buffer these to improve reordering tolerance. trace!("dropping short packet during handshake");