Skip to content

Commit

Permalink
recovery: reject ACK frames that have a largest ACKed that is too large
Browse files Browse the repository at this point in the history
This is somewhat of an optimization to help reject obviously invalid ACK
frames. In order to do this we track the largest packet number that has
been sent, and compare if against the largest ACK packet from ACK
frames.

The spec says:

  An endpoint SHOULD treat receipt of an acknowledgment for a packet it
  did not send as a connection error of type PROTOCOL_VIOLATION

So we are free to close the connection as well.
  • Loading branch information
ghedo committed Sep 22, 2019
1 parent e739f55 commit d646a60
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2445,7 +2445,7 @@ impl Connection {
epoch,
now,
&self.trace_id,
);
)?;

// When we receive an ACK for a 1-RTT packet after handshake
// completion, it means the handshake has been confirmed.
Expand Down
23 changes: 21 additions & 2 deletions src/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ use std::time::Instant;

use std::collections::BTreeMap;

use crate::Error;
use crate::Result;

use crate::frame;
use crate::packet;
use crate::ranges;
Expand Down Expand Up @@ -83,6 +86,8 @@ pub struct Recovery {

largest_acked_pkt: [u64; packet::EPOCH_COUNT],

largest_sent_pkt: [u64; packet::EPOCH_COUNT],

latest_rtt: Duration,

smoothed_rtt: Option<Duration>,
Expand Down Expand Up @@ -133,6 +138,8 @@ impl Default for Recovery {

largest_acked_pkt: [std::u64::MAX; packet::EPOCH_COUNT],

largest_sent_pkt: [0; packet::EPOCH_COUNT],

latest_rtt: Duration::new(0, 0),

smoothed_rtt: None,
Expand Down Expand Up @@ -178,6 +185,9 @@ impl Recovery {
let is_crypto = pkt.is_crypto;
let sent_bytes = pkt.size;

self.largest_sent_pkt[epoch] =
cmp::max(self.largest_sent_pkt[epoch], pkt.pkt_num);

self.sent[epoch].insert(pkt_num, pkt);

if in_flight {
Expand All @@ -203,9 +213,16 @@ impl Recovery {
pub fn on_ack_received(
&mut self, ranges: &ranges::RangeSet, ack_delay: u64,
epoch: packet::Epoch, now: Instant, trace_id: &str,
) {
) -> Result<()> {
let largest_acked = ranges.largest().unwrap();

// If the largest packet number ACKed exceeds any packet number we have
// sent, then the ACK is obviously invalid, so there's no need to
// continue further.
if largest_acked > self.largest_sent_pkt[epoch] {
return Err(Error::InvalidPacket);
}

if self.largest_acked_pkt[epoch] == std::u64::MAX {
self.largest_acked_pkt[epoch] = largest_acked;
} else {
Expand Down Expand Up @@ -241,7 +258,7 @@ impl Recovery {
}

if !has_newly_acked {
return;
return Ok(());
}

self.detect_lost_packets(epoch, now, trace_id);
Expand All @@ -252,6 +269,8 @@ impl Recovery {
self.set_loss_detection_timer();

trace!("{} {:?}", trace_id, self);

Ok(())
}

pub fn on_loss_detection_timeout(&mut self, now: Instant, trace_id: &str) {
Expand Down

0 comments on commit d646a60

Please sign in to comment.