mirror of https://github.com/ospab/ostp.git
fix: low-severity hardening (Karn RTT, 32-bit frame overflow, replay-cache DoS)
- Karn's algorithm: drop_acked_frames no longer samples RTT from frames that were retransmitted (last_sent is bumped on each retransmit, so an ACK for the original transmission would measure a spuriously small RTT and drag SRTT/RTO down). Added CongestionController::on_ack_no_rtt for the case where every acked frame was ambiguous, so the window still advances without polluting the RTT estimator. Refactored the shared window-growth into grow_window. - Frame decode: header+payload+pad length now uses checked_add. payload_len is a u32 from the header and on 32-bit targets (MIPS/ARMv7 routers are supported) the sum could wrap usize and slip past the truncation check. - Replay cache: a full cache used to reject ALL new handshakes globally until the next tick, letting one flooding key-holder deny service to everyone. Now it reclaims expired entries and, if still full, evicts the single oldest — new handshakes always get in. Fixed the mislabelled "100000" log (cap is 50000) and named it REPLAY_CACHE_MAX.
This commit is contained in:
parent
5754689e09
commit
a9509a235d
|
|
@ -144,6 +144,19 @@ impl CongestionController {
|
||||||
self.bytes_in_flight = self.bytes_in_flight.saturating_add(bytes);
|
self.bytes_in_flight = self.bytes_in_flight.saturating_add(bytes);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record that `bytes` were acknowledged but WITHOUT a usable RTT sample
|
||||||
|
/// (e.g. every acked frame was retransmitted, so Karn's algorithm forbids
|
||||||
|
/// measuring RTT from it). The window still advances; only the RTT estimator
|
||||||
|
/// is left untouched.
|
||||||
|
pub fn on_ack_no_rtt(&mut self, bytes: u64) {
|
||||||
|
let now = Instant::now();
|
||||||
|
self.bytes_in_flight = self.bytes_in_flight.saturating_sub(bytes);
|
||||||
|
self.total_acked = self.total_acked.saturating_add(bytes);
|
||||||
|
self.grow_window(bytes);
|
||||||
|
self.update_pacing_rate();
|
||||||
|
self.last_ack_time = now;
|
||||||
|
}
|
||||||
|
|
||||||
/// Record that `bytes` were acknowledged with the given RTT sample.
|
/// Record that `bytes` were acknowledged with the given RTT sample.
|
||||||
pub fn on_ack(&mut self, bytes: u64, rtt: Duration) {
|
pub fn on_ack(&mut self, bytes: u64, rtt: Duration) {
|
||||||
let now = Instant::now();
|
let now = Instant::now();
|
||||||
|
|
@ -153,6 +166,13 @@ impl CongestionController {
|
||||||
// Update RTT measurements
|
// Update RTT measurements
|
||||||
self.update_rtt(rtt, now);
|
self.update_rtt(rtt, now);
|
||||||
|
|
||||||
|
self.grow_window(bytes);
|
||||||
|
self.update_pacing_rate();
|
||||||
|
self.last_ack_time = now;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Congestion-window growth shared by both ACK paths (slow start / probe).
|
||||||
|
fn grow_window(&mut self, bytes: u64) {
|
||||||
// State machine
|
// State machine
|
||||||
match self.phase {
|
match self.phase {
|
||||||
Phase::SlowStart => {
|
Phase::SlowStart => {
|
||||||
|
|
@ -168,9 +188,6 @@ impl CongestionController {
|
||||||
self.cwnd = self.cwnd.saturating_add(bytes * self.mtu / self.cwnd.max(1));
|
self.cwnd = self.cwnd.saturating_add(bytes * self.mtu / self.cwnd.max(1));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.update_pacing_rate();
|
|
||||||
self.last_ack_time = now;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Record a loss event.
|
/// Record a loss event.
|
||||||
|
|
@ -313,6 +330,23 @@ mod tests {
|
||||||
assert_eq!(rto, Duration::from_millis(150));
|
assert_eq!(rto, Duration::from_millis(150));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_on_ack_no_rtt_grows_window_without_touching_srtt() {
|
||||||
|
let mut cc = CongestionController::new(1200);
|
||||||
|
// Establish a known SRTT with a real sample.
|
||||||
|
cc.on_send(1200);
|
||||||
|
cc.on_ack(1200, Duration::from_millis(40));
|
||||||
|
let srtt_before = cc.smoothed_rtt();
|
||||||
|
let cwnd_before = cc.cwnd();
|
||||||
|
|
||||||
|
// A Karn's-algorithm ACK (all acked frames were retransmitted): window
|
||||||
|
// must advance, RTT estimate must be untouched.
|
||||||
|
cc.on_send(1200);
|
||||||
|
cc.on_ack_no_rtt(1200);
|
||||||
|
assert!(cc.cwnd() > cwnd_before, "cwnd should still grow on a no-RTT ack");
|
||||||
|
assert_eq!(cc.smoothed_rtt(), srtt_before, "SRTT must not move on a no-RTT ack");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_rto_clamp_min() {
|
fn test_rto_clamp_min() {
|
||||||
let cc = CongestionController::new(1200);
|
let cc = CongestionController::new(1200);
|
||||||
|
|
|
||||||
|
|
@ -101,7 +101,15 @@ impl FramedPacket {
|
||||||
let payload_len = header.payload_len as usize;
|
let payload_len = header.payload_len as usize;
|
||||||
let pad_len = header.pad_len as usize;
|
let pad_len = header.pad_len as usize;
|
||||||
|
|
||||||
let expected = FRAME_HEADER_LEN + payload_len + pad_len;
|
// Use checked arithmetic: payload_len is a u32 from the (decrypted, but
|
||||||
|
// still to-be-trusted) header, and on 32-bit targets — MIPS/ARMv7
|
||||||
|
// routers are supported build targets — header+payload+pad can overflow
|
||||||
|
// usize and wrap to a small value that spuriously passes the length
|
||||||
|
// check, causing an out-of-range slice below.
|
||||||
|
let expected = FRAME_HEADER_LEN
|
||||||
|
.checked_add(payload_len)
|
||||||
|
.and_then(|v| v.checked_add(pad_len))
|
||||||
|
.ok_or_else(|| ProtocolError::Framing("frame length overflow".to_string()))?;
|
||||||
if buf.len() < expected {
|
if buf.len() < expected {
|
||||||
return Err(ProtocolError::Framing("frame body truncated".to_string()));
|
return Err(ProtocolError::Framing("frame body truncated".to_string()));
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -683,24 +683,34 @@ impl ProtocolMachine {
|
||||||
fn drop_acked_frames(&mut self, ranges: &[(u64, u64)]) {
|
fn drop_acked_frames(&mut self, ranges: &[(u64, u64)]) {
|
||||||
let now = Instant::now();
|
let now = Instant::now();
|
||||||
let mut acked_bytes = 0u64;
|
let mut acked_bytes = 0u64;
|
||||||
let mut min_rtt = Duration::from_secs(60);
|
let mut min_rtt: Option<Duration> = None;
|
||||||
|
|
||||||
// Compute RTT from the oldest acked frame's send timestamp
|
|
||||||
for frame in self.sent_history.iter() {
|
for frame in self.sent_history.iter() {
|
||||||
if nonce_in_ranges(frame.nonce, ranges) {
|
if nonce_in_ranges(frame.nonce, ranges) {
|
||||||
acked_bytes += frame.bytes.len() as u64;
|
acked_bytes += frame.bytes.len() as u64;
|
||||||
let rtt = now.duration_since(frame.last_sent);
|
// Karn's algorithm: never take an RTT sample from a frame that
|
||||||
if rtt < min_rtt {
|
// was retransmitted. `last_sent` is bumped on every retransmit,
|
||||||
min_rtt = rtt;
|
// so an ACK for the ORIGINAL transmission would be measured
|
||||||
|
// against the retransmit time, yielding a spuriously small RTT
|
||||||
|
// that drags SRTT/RTO down and triggers more spurious
|
||||||
|
// retransmits. Only unambiguous (never-retried) frames qualify.
|
||||||
|
if frame.retries == 0 {
|
||||||
|
let rtt = now.duration_since(frame.last_sent);
|
||||||
|
min_rtt = Some(min_rtt.map_or(rtt, |m| m.min(rtt)));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.sent_history.retain(|frame| !nonce_in_ranges(frame.nonce, ranges));
|
self.sent_history.retain(|frame| !nonce_in_ranges(frame.nonce, ranges));
|
||||||
|
|
||||||
// Notify congestion controller
|
// Notify congestion controller. Feed an RTT sample only when we had at
|
||||||
|
// least one unambiguous ACK; otherwise update the window without
|
||||||
|
// polluting the RTT estimator.
|
||||||
if acked_bytes > 0 {
|
if acked_bytes > 0 {
|
||||||
self.cc.on_ack(acked_bytes, min_rtt);
|
match min_rtt {
|
||||||
|
Some(rtt) => self.cc.on_ack(acked_bytes, rtt),
|
||||||
|
None => self.cc.on_ack_no_rtt(acked_bytes),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,11 @@ use portable_atomic::AtomicU64;
|
||||||
/// Excess handshake attempts are silently dropped -- no response, no state allocated.
|
/// Excess handshake attempts are silently dropped -- no response, no state allocated.
|
||||||
const MAX_SESSIONS: usize = 1024;
|
const MAX_SESSIONS: usize = 1024;
|
||||||
|
|
||||||
|
/// Cap on the anti-replay handshake cache. When reached, expired entries are
|
||||||
|
/// reclaimed (and if needed the oldest is evicted) rather than rejecting new
|
||||||
|
/// handshakes globally — see the eviction logic in on_datagram.
|
||||||
|
const REPLAY_CACHE_MAX: usize = 50_000;
|
||||||
|
|
||||||
pub enum DispatchOutcome {
|
pub enum DispatchOutcome {
|
||||||
Unauthorized,
|
Unauthorized,
|
||||||
/// Packet matched a registered key's per-key junk marker — drop silently.
|
/// Packet matched a registered key's per-key junk marker — drop silently.
|
||||||
|
|
@ -457,9 +462,30 @@ impl Dispatcher {
|
||||||
}
|
}
|
||||||
|
|
||||||
if !self.replay_cache.contains_key(&payload.to_vec()) {
|
if !self.replay_cache.contains_key(&payload.to_vec()) {
|
||||||
if self.replay_cache.len() >= 50_000 {
|
if self.replay_cache.len() >= REPLAY_CACHE_MAX {
|
||||||
tracing::warn!("Replay cache full (100000 entries), rejecting handshake from {}", peer);
|
// Don't globally reject new handshakes when full —
|
||||||
return Ok(DispatchOutcome::Unauthorized);
|
// that would let one flooding key-holder deny
|
||||||
|
// service to everyone. Reclaim space instead:
|
||||||
|
// first drop entries already past the drift
|
||||||
|
// window, then, if still full, evict the single
|
||||||
|
// oldest. A replay is still caught because it can
|
||||||
|
// only be accepted while within the 300s drift
|
||||||
|
// window, and an entry that young is never the
|
||||||
|
// one evicted before the cache genuinely holds
|
||||||
|
// 50k sub-300s handshakes.
|
||||||
|
self.replay_cache.retain(|_, &mut cached_ts| {
|
||||||
|
(now as i64 - cached_ts as i64).abs() <= 300
|
||||||
|
});
|
||||||
|
if self.replay_cache.len() >= REPLAY_CACHE_MAX {
|
||||||
|
if let Some(oldest) = self.replay_cache
|
||||||
|
.iter()
|
||||||
|
.min_by_key(|(_, &ts)| ts)
|
||||||
|
.map(|(k, _)| k.clone())
|
||||||
|
{
|
||||||
|
self.replay_cache.remove(&oldest);
|
||||||
|
}
|
||||||
|
tracing::warn!("Replay cache full ({} entries), evicting oldest", REPLAY_CACHE_MAX);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if self.peer_machines.len() >= MAX_SESSIONS {
|
if self.peer_machines.len() >= MAX_SESSIONS {
|
||||||
tracing::warn!("Max sessions reached ({}), rejecting handshake from {}", MAX_SESSIONS, peer);
|
tracing::warn!("Max sessions reached ({}), rejecting handshake from {}", MAX_SESSIONS, peer);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue