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 55e3e62 commit 0555b91
Showing 1 changed file with 56 additions and 1 deletion.
57 changes: 56 additions & 1 deletion quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6820,7 +6820,20 @@ impl Connection {
crypto::Level::Initial => packet::Epoch::Initial,
crypto::Level::ZeroRTT => unreachable!(),
crypto::Level::Handshake => packet::Epoch::Handshake,
crypto::Level::OneRTT => packet::Epoch::Application,
crypto::Level::OneRTT => {
if self.handshake_confirmed {
packet::Epoch::Application
} else {
// 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-
packet::Epoch::Handshake
}
},
};

if !self.is_established() {
Expand Down Expand Up @@ -14922,6 +14935,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 0555b91

Please sign in to comment.