Skip to content

Commit

Permalink
Force Handshake packet epoch for CC frame if handshake hasn't been co…
Browse files Browse the repository at this point in the history
…nfirmed

The RFC states that an endpoint can immediately close a connection by
sending a CONNECTION_CLOSE frame during the handshake:
> However, prior to confirming the handshake, it is possible that more
  advanced packet protection keys are not available to the peer, so
  another CONNECTION_CLOSE frame MAY be sent in a packet that uses a lower
  packet protection level.

Currently, when we decide the packet epoch when a CC frame is present,
we always choose Application. This commit downgrades the epoch to
Handshake if the handshake wasn't confirmed.

The test added in this PR fails without this patch - since the packet is
sent in the Application space without keys, the server can't decrypt it,
so its `peer_error` is None.
  • Loading branch information
evanrittenhouse committed Aug 28, 2024
1 parent 0ba4a74 commit decb16b
Showing 1 changed file with 57 additions and 1 deletion.
58 changes: 57 additions & 1 deletion quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6823,7 +6823,14 @@ impl Connection {
crypto::Level::OneRTT => packet::Epoch::Application,
};

if !self.is_established() {
// If the handshake hasn't been confirmed, we can't send
// 1-RTT packets since the peer
// may not have the correct keys. Downgrade the status to
// Handshake - if required, we'll downgrade further to
// Initial later on.
//
// https://datatracker.ietf.org/doc/html/rfc9000#name-immediate-close-during-the-
if !self.handshake_confirmed {
match epoch {
// Downgrade the epoch to Handshake as the handshake is not
// completed yet.
Expand All @@ -6832,6 +6839,13 @@ impl Connection {

// Downgrade the epoch to Initial as the remote peer might
// not be able to decrypt handshake packets yet.
//
// We don't downgrade if we forcibly downgraded an Application
// packet to a Handshake one since the
// client drops the keys for the Initial epoch upon
// sending a Handshake packet. In that scenario, we need the
// packet to remain in the Handshake
// epoch.
packet::Epoch::Handshake
if self.pkt_num_spaces[packet::Epoch::Initial]
.has_keys() =>
Expand Down Expand Up @@ -14922,6 +14936,48 @@ mod tests {
);
}

#[test]
fn transport_close_by_client_during_handshake_established() {
let mut pipe = testing::Pipe::new().unwrap();

// Client sends initial flight.
let flight = testing::emit_flight(&mut pipe.client).unwrap();
testing::process_flight(&mut pipe.server, flight).unwrap();

let flight = testing::emit_flight(&mut pipe.server).unwrap();

// Both connections are not established.
assert!(!pipe.client.is_established() && !pipe.server.is_established());

testing::process_flight(&mut pipe.client, flight).unwrap();

// Connection is established on the client.
assert!(pipe.client.is_established());

// Client sends after connection is established.
pipe.client.close(false, 123, b"connection close").unwrap();

let flight = testing::emit_flight(&mut pipe.client).unwrap();
testing::process_flight(&mut pipe.server, flight).unwrap();

assert_eq!(
pipe.server.peer_error(),
Some(&ConnectionError {
is_app: false,
error_code: 123,
reason: b"connection close".to_vec()
})
);
assert_eq!(
pipe.client.local_error(),
Some(&ConnectionError {
is_app: false,
error_code: 123,
reason: b"connection close".to_vec()
})
);
}

#[test]
fn peer_error() {
let mut pipe = testing::Pipe::new().unwrap();
Expand Down

0 comments on commit decb16b

Please sign in to comment.