diff --git a/boringtun/src/noise/handshake.rs b/boringtun/src/noise/handshake.rs index ebc7ef2a..f885e42a 100644 --- a/boringtun/src/noise/handshake.rs +++ b/boringtun/src/noise/handshake.rs @@ -1,7 +1,7 @@ // Copyright (c) 2019 Cloudflare, Inc. All rights reserved. // SPDX-License-Identifier: BSD-3-Clause -use super::timers::COOKIE_EXPIRATION_TIME; +use super::timers::{COOKIE_EXPIRATION_TIME, REKEY_TIMEOUT}; use super::{HandshakeInit, HandshakeResponse, PacketCookieReply}; use crate::noise::errors::WireGuardError; use crate::noise::session::Session; @@ -442,15 +442,17 @@ impl Handshake { !matches!(self.state, HandshakeState::None | HandshakeState::Expired) } - pub(crate) fn timer(&self) -> Option<(Instant, u32)> { - match self.state { - HandshakeState::InitSent(HandshakeInitSentState { - time_sent, - local_index, - .. - }) => Some((time_sent, local_index)), - _ => None, - } + pub(crate) fn rekey_timeout(&self) -> Option<(Instant, u32)> { + let HandshakeState::InitSent(HandshakeInitSentState { + time_sent, + local_index, + .. + }) = self.state + else { + return None; + }; + + Some((time_sent + REKEY_TIMEOUT, local_index)) } pub(crate) fn set_expired(&mut self) { diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 07b007d9..104aa05b 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -46,6 +46,8 @@ pub enum TimerName { TimeLastDataPacketSent, /// Time we last sent persistent keepalive TimePersistentKeepalive, + /// Time we last updated our timers + TimeLastUpdate, Top, } @@ -56,14 +58,10 @@ pub struct Timers { /// Is the owner of the timer the initiator or the responder for the last handshake? is_initiator: bool, timers: [Instant; TimerName::Top as usize], - /// Did we receive data without sending anything back? - /// - /// If `Some`, tracks the timestamp when we want to send a keepalive. - want_passive_keepalive_at: Option, - /// Did we send data without hearing back? - /// - /// If `Some`, tracks the timestamp when we want to initiate the new handshake. - want_handshake_at: Option, + /// The last data packet we received without sending a reply. + last_data_received_without_reply: Option, + /// The earliest data packet we sent without receiving a reply. + first_data_sent_without_reply: Option, persistent_keepalive: usize, /// Should this timer call reset rr function (if not a shared rr instance) pub(super) should_reset_rr: bool, @@ -83,8 +81,8 @@ impl Timers { Timers { is_initiator: false, timers: [now; TimerName::Top as usize], - want_passive_keepalive_at: Default::default(), - want_handshake_at: Default::default(), + last_data_received_without_reply: Default::default(), + first_data_sent_without_reply: Default::default(), persistent_keepalive: usize::from(persistent_keepalive.unwrap_or(0)), should_reset_rr: reset_rr, send_handshake_at: None, @@ -92,6 +90,68 @@ impl Timers { } } + pub(crate) fn reject_after_time(&self) -> Instant { + self[TimeSessionEstablished] + REJECT_AFTER_TIME * 3 + } + + pub(crate) fn rekey_attempt_time(&self) -> Instant { + self[TimeLastHandshakeStarted] + REKEY_ATTEMPT_TIME + } + + pub(crate) fn rekey_after_time_on_send(&self) -> Option { + if !self.is_initiator { + // If we aren't the initiator of the current session, this timer does not matter. + return None; + } + + let session_established = self[TimeSessionEstablished]; + + if session_established >= self[TimeLastDataPacketSent] { + // If we haven't sent any data yet, this timer doesn't matter. + return None; + } + + Some(session_established + REKEY_AFTER_TIME) + } + + pub(crate) fn reject_after_time_on_receive(&self) -> Option { + if !self.is_initiator { + // If we aren't the initiator of the current session, this timer does not matter. + return None; + } + + let session_established = self[TimeSessionEstablished]; + + if session_established >= self[TimeLastDataPacketReceived] { + // If we haven't received any data yet, this timer doesn't matter. + return None; + } + + Some(session_established + REJECT_AFTER_TIME - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT) + } + + pub(crate) fn rekey_after_time_without_response(&self) -> Option { + let first_packet_without_reply = self.first_data_sent_without_reply?; + + Some(first_packet_without_reply + KEEPALIVE_TIMEOUT + REKEY_TIMEOUT) + } + + pub(crate) fn keepalive_after_time_without_send(&self) -> Option { + let last_data_received_without_reply = self.last_data_received_without_reply?; + + Some(last_data_received_without_reply + KEEPALIVE_TIMEOUT) + } + + pub(crate) fn next_persistent_keepalive(&self) -> Option { + let keepalive = Duration::from_secs(self.persistent_keepalive as u64); + + if keepalive.is_zero() { + return None; + } + + Some(self[TimerName::TimePersistentKeepalive] + keepalive) + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -106,8 +166,8 @@ impl Timers { for t in &mut self.timers[..] { *t = now; } - self.want_handshake_at = None; - self.want_passive_keepalive_at = None; + self.first_data_sent_without_reply = None; + self.last_data_received_without_reply = None; } } @@ -128,25 +188,23 @@ impl Tunn { pub(super) fn timer_tick(&mut self, timer_name: TimerName, now: Instant) { match timer_name { TimeLastPacketReceived => { - self.timers.want_handshake_at = None; + self.timers.first_data_sent_without_reply = None; } TimeLastPacketSent => { - self.timers.want_passive_keepalive_at = None; + self.timers.last_data_received_without_reply = None; } TimeLastDataPacketReceived => { - self.timers.want_passive_keepalive_at = Some(now + KEEPALIVE_TIMEOUT); + self.timers.last_data_received_without_reply = Some(now); } TimeLastDataPacketSent => { - match self.timers.want_handshake_at { + match self.timers.first_data_sent_without_reply { Some(_) => { // This isn't the first timer tick (i.e. not the first packet) // we haven't received a response to. } None => { // We sent a packet and haven't heard back yet. - // Start a timer for when we want to make a new handshake. - self.timers.want_handshake_at = - Some(now + KEEPALIVE_TIMEOUT + REKEY_TIMEOUT) + self.timers.first_data_sent_without_reply = Some(now) } } } @@ -189,12 +247,21 @@ impl Tunn { } } + fn next_expired_session(&self) -> Option { + self.sessions + .iter() + .flat_map(|s| Some(s.as_ref()?.established_at() + REJECT_AFTER_TIME)) + .min() + } + #[deprecated(note = "Prefer `Timers::update_timers_at` to avoid time-impurity")] pub fn update_timers<'a>(&mut self, dst: &'a mut [u8]) -> TunnResult<'a> { self.update_timers_at(dst, Instant::now()) } pub fn update_timers_at<'a>(&mut self, dst: &'a mut [u8], now: Instant) -> TunnResult<'a> { + self.timers[TimeLastUpdate] = now; + if let Some(scheduled_handshake_at) = self.timers.send_handshake_at { // If we have scheduled a handshake and the deadline expired, send it immediately. if now >= scheduled_handshake_at { @@ -232,13 +299,6 @@ impl Tunn { handshake_initiation_required = true; } - // Load timers only once: - let session_established = self.timers[TimeSessionEstablished]; - let handshake_started = self.timers[TimeLastHandshakeStarted]; - let data_packet_received = self.timers[TimeLastDataPacketReceived]; - let data_packet_sent = self.timers[TimeLastDataPacketSent]; - let persistent_keepalive = self.timers.persistent_keepalive; - if self.handshake.is_expired() { return TunnResult::Err(WireGuardError::ConnectionExpired); } @@ -255,16 +315,17 @@ impl Tunn { // All ephemeral private keys and symmetric session keys are zeroed out after // (REJECT_AFTER_TIME * 3) ms if no new keys have been exchanged. - if now - session_established >= REJECT_AFTER_TIME * 3 { + if now >= self.timers.reject_after_time() { tracing::debug!("CONNECTION_EXPIRED(REJECT_AFTER_TIME * 3)"); self.handshake.set_expired(); self.clear_all(); return TunnResult::Err(WireGuardError::ConnectionExpired); } - if let Some((time_init_sent, local_idx)) = self.handshake.timer() { + if let Some((rekey_timeout, local_idx)) = self.handshake.rekey_timeout() { // Handshake Initiation Retransmission - if now - handshake_started >= REKEY_ATTEMPT_TIME { + // Only applies if we initiated a handshake (and thus `rekey_timeout` is `Some`) + if now >= self.timers.rekey_attempt_time() { // After REKEY_ATTEMPT_TIME ms of trying to initiate a new handshake, // the retries give up and cease, and clear all existing packets queued // up to be sent. If a packet is explicitly queued up to be sent, then @@ -275,7 +336,7 @@ impl Tunn { return TunnResult::Err(WireGuardError::ConnectionExpired); } - if now.duration_since(time_init_sent) >= REKEY_TIMEOUT { + if now >= rekey_timeout { // We avoid using `time` here, because it can be earlier than `time_init_sent`. // Once `checked_duration_since` is stable we can use that. // A handshake initiation is retried after REKEY_TIMEOUT + jitter ms, @@ -285,32 +346,33 @@ impl Tunn { handshake_initiation_required = true; } } else { - if self.timers.is_initiator() { - // After sending a packet, if the sender was the original initiator - // of the handshake and if the current session key is REKEY_AFTER_TIME - // ms old, we initiate a new handshake. If the sender was the original - // responder of the handshake, it does not re-initiate a new handshake - // after REKEY_AFTER_TIME ms like the original initiator does. - if session_established < data_packet_sent - && now - session_established >= REKEY_AFTER_TIME - { - tracing::debug!("HANDSHAKE(REKEY_AFTER_TIME (on send))"); - handshake_initiation_required = true; - } + // After sending a packet, if the sender was the original initiator + // of the handshake and if the current session key is REKEY_AFTER_TIME + // ms old, we initiate a new handshake. If the sender was the original + // responder of the handshake, it does not re-initiate a new handshake + // after REKEY_AFTER_TIME ms like the original initiator does. + if self + .timers + .rekey_after_time_on_send() + .is_some_and(|deadline| now >= deadline) + { + tracing::debug!("HANDSHAKE(REKEY_AFTER_TIME (on send))"); + handshake_initiation_required = true; + } - // After receiving a packet, if the receiver was the original initiator - // of the handshake and if the current session key is REJECT_AFTER_TIME - // - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT ms old, we initiate a new - // handshake. - if session_established < data_packet_received - && now - session_established - >= REJECT_AFTER_TIME - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT - { - tracing::debug!( - "HANDSHAKE(REJECT_AFTER_TIME - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT (on receive))" - ); - handshake_initiation_required = true; - } + // After receiving a packet, if the receiver was the original initiator + // of the handshake and if the current session key is REJECT_AFTER_TIME + // - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT ms old, we initiate a new + // handshake. + if self + .timers + .reject_after_time_on_receive() + .is_some_and(|deadline| now >= deadline) + { + tracing::debug!( + "HANDSHAKE(REJECT_AFTER_TIME - KEEPALIVE_TIMEOUT - REKEY_TIMEOUT (on receive))" + ); + handshake_initiation_required = true; } // If we have sent a packet to a given peer but have not received a @@ -318,8 +380,8 @@ impl Tunn { // we initiate a new handshake. if self .timers - .want_handshake_at - .is_some_and(|handshake_at| now >= handshake_at) + .rekey_after_time_without_response() + .is_some_and(|deadline| now >= deadline) { tracing::debug!("HANDSHAKE(KEEPALIVE + REKEY_TIMEOUT)"); handshake_initiation_required = true; @@ -330,17 +392,18 @@ impl Tunn { // to the given peer in KEEPALIVE ms, we send an empty packet. if self .timers - .want_passive_keepalive_at - .is_some_and(|keepalive_at| now >= keepalive_at) + .keepalive_after_time_without_send() + .is_some_and(|deadline| now >= deadline) { tracing::debug!("KEEPALIVE(KEEPALIVE_TIMEOUT)"); keepalive_required = true; } // Persistent KEEPALIVE - if persistent_keepalive > 0 - && (now - self.timers[TimePersistentKeepalive] - >= Duration::from_secs(persistent_keepalive as _)) + if self + .timers + .next_persistent_keepalive() + .is_some_and(|deadline| now >= deadline) { tracing::debug!("KEEPALIVE(PERSISTENT_KEEPALIVE)"); self.timer_tick(TimePersistentKeepalive, now); @@ -360,7 +423,6 @@ impl Tunn { existing.is_none(), "Should never override existing handshake" ); - tracing::debug!(?jitter, "Scheduling new handshake"); return TunnResult::Done; @@ -375,12 +437,67 @@ impl Tunn { /// Returns an [`Instant`] when [`Tunn::update_timers_at`] should be called again. /// - /// If this returns `None`, you may call it at your usual desired precision (usually once a second is enough). - pub fn next_timer_update(&self) -> Option { - iter::empty() - .chain(self.timers.send_handshake_at) - .chain(self.timers.want_passive_keepalive_at) - .min() + /// Calling it earlier than the given [`Instant`] is safe but unlikely to have any effect. + pub fn next_timer_update(&self) -> Option<(Instant, &'static str)> { + let (next, reason) = self.next_timer_update_internal()?; + let last_update = self.timers[TimeLastUpdate]; + + Some((next.max(last_update), reason)) + } + + fn next_timer_update_internal(&self) -> Option<(Instant, &'static str)> { + // Mimic the `update_timers_at` function: If we have a handshake scheduled, other timers don't matter. + if let Some(scheduled_handshake) = self.timers.send_handshake_at { + return Some((scheduled_handshake, "scheduled handshake")); + } + + let common_timers = iter::empty() + .chain( + self.next_expired_session() + .map(|instant| (instant, "next expired session")), + ) + .chain( + self.handshake + .cookie_expiration() + .map(|instant| (instant, "cookie expiration")), + ) + .chain(Some((self.timers.reject_after_time(), "reject-after-time"))); + + if let Some((rekey_timeout, _)) = self.handshake.rekey_timeout() { + common_timers + .chain(Some((rekey_timeout, "rekey-timeout"))) + .chain(Some((self.timers.rekey_attempt_time(), "rekey-attempt"))) + .min_by_key(|(i, _)| *i) + } else { + // Persistent keep-alive only makes sense if the current session is active. + let persistent_keepalive = self.sessions[self.current % N_SESSIONS] + .as_ref() + .and_then(|_| self.timers.next_persistent_keepalive()); + + common_timers + .chain( + self.timers + .rekey_after_time_on_send() + .map(|instant| (instant, "rekey_after_time_on_send")), + ) + .chain( + self.timers + .reject_after_time_on_receive() + .map(|instant| (instant, "reject_after_time_on_receive")), + ) + .chain( + self.timers + .rekey_after_time_without_response() + .map(|instant| (instant, "rekey_after_time_without_response")), + ) + .chain( + self.timers + .keepalive_after_time_without_send() + .map(|instant| (instant, "keepalive_after_time_without_send")), + ) + .chain(persistent_keepalive.map(|instant| (instant, "persistent keep-alive"))) + .min_by_key(|(i, _)| *i) + } } #[deprecated(note = "Prefer `Tunn::time_since_last_handshake_at` to avoid time-impurity")]