From 7c95ce373116f89a58684a5e3fbeaf154416d130 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:27:47 +0100 Subject: [PATCH 01/14] Extract handshake-rekey timer --- boringtun/src/noise/handshake.rs | 22 ++++++++++++---------- boringtun/src/noise/timers.rs | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/boringtun/src/noise/handshake.rs b/boringtun/src/noise/handshake.rs index ebc7ef2a2..f885e42aa 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 07b007d9f..cb421baa7 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -262,7 +262,7 @@ impl Tunn { 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 { // After REKEY_ATTEMPT_TIME ms of trying to initiate a new handshake, @@ -275,7 +275,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, From 7887b5f8ad959e666f6979e033f077eb61c973a6 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:30:09 +0100 Subject: [PATCH 02/14] Extract reject-after-time timer --- boringtun/src/noise/timers.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index cb421baa7..9da72e6f9 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -92,6 +92,10 @@ impl Timers { } } + pub(crate) fn reject_after_time(&self) -> Instant { + self[TimeSessionEstablished] + REJECT_AFTER_TIME * 3 + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -255,7 +259,7 @@ 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(); From 8408328febfccec7bccdf28bae9797284da19f89 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 13 Jan 2025 16:43:06 +0100 Subject: [PATCH 03/14] Extract rekey-attempt timer --- boringtun/src/noise/timers.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 9da72e6f9..0dfe70e98 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -96,6 +96,10 @@ impl Timers { self[TimeSessionEstablished] + REJECT_AFTER_TIME * 3 } + pub(crate) fn rekey_attempt_time(&self) -> Instant { + self[TimeLastHandshakeStarted] + REKEY_ATTEMPT_TIME + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -268,7 +272,8 @@ impl Tunn { 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 From 82b4ce107ffa04906e737788d93b01dbf02fabf5 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:34:24 +0100 Subject: [PATCH 04/14] Extract rekey-after-time-on-send timer --- boringtun/src/noise/timers.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 0dfe70e98..57bb1afa2 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -100,6 +100,17 @@ impl Timers { self[TimeLastHandshakeStarted] + REKEY_ATTEMPT_TIME } + pub(crate) fn rekey_after_time_on_send(&self) -> Option { + 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) + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -300,8 +311,10 @@ impl Tunn { // 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 + 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; From d655af9b5de9f0ad8a31fc8da41a4bdde34fe336 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:36:10 +0100 Subject: [PATCH 05/14] Extract reject-after-time-on-receive timer --- boringtun/src/noise/timers.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 57bb1afa2..735c4b30f 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -111,6 +111,17 @@ impl Timers { Some(session_established + REKEY_AFTER_TIME) } + pub(crate) fn reject_after_time_on_receive(&self) -> Option { + 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) + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -324,9 +335,10 @@ impl Tunn { // 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 + 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))" From ff0b50377c84016352fca01f834707f779377e04 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:36:50 +0100 Subject: [PATCH 06/14] Extract rekey-after-time-without-response timer --- boringtun/src/noise/timers.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 735c4b30f..d1fc1ef38 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -60,10 +60,8 @@ pub struct Timers { /// /// 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 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, @@ -84,7 +82,7 @@ impl Timers { is_initiator: false, timers: [now; TimerName::Top as usize], want_passive_keepalive_at: Default::default(), - want_handshake_at: 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, @@ -122,6 +120,12 @@ impl Timers { 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) + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -136,7 +140,7 @@ impl Timers { for t in &mut self.timers[..] { *t = now; } - self.want_handshake_at = None; + self.first_data_sent_without_reply = None; self.want_passive_keepalive_at = None; } } @@ -158,7 +162,7 @@ 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; @@ -167,16 +171,14 @@ impl Tunn { self.timers.want_passive_keepalive_at = Some(now + KEEPALIVE_TIMEOUT); } 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) } } } @@ -352,8 +354,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; From d2d2b8601aa1ebf9d993752811eb42a2ac6e1cd6 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:37:34 +0100 Subject: [PATCH 07/14] Extract keepalive-after-time-without-send timer --- boringtun/src/noise/timers.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index d1fc1ef38..03737495c 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -56,10 +56,8 @@ 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, + /// 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, @@ -81,7 +79,7 @@ impl Timers { Timers { is_initiator: false, timers: [now; TimerName::Top as usize], - want_passive_keepalive_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, @@ -126,6 +124,12 @@ impl Timers { 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) + } + fn is_initiator(&self) -> bool { self.is_initiator } @@ -141,7 +145,7 @@ impl Timers { *t = now; } self.first_data_sent_without_reply = None; - self.want_passive_keepalive_at = None; + self.last_data_received_without_reply = None; } } @@ -165,10 +169,10 @@ impl Tunn { 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.first_data_sent_without_reply { @@ -366,8 +370,8 @@ 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; @@ -415,7 +419,7 @@ impl Tunn { pub fn next_timer_update(&self) -> Option { iter::empty() .chain(self.timers.send_handshake_at) - .chain(self.timers.want_passive_keepalive_at) + .chain(self.timers.last_data_received_without_reply) .min() } From 7c84938e0bce4f4ddb20b4e9cc223d164ee62a23 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:38:12 +0100 Subject: [PATCH 08/14] Extract persistent keep-alive timer --- boringtun/src/noise/timers.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 03737495c..62272f5c9 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -130,6 +130,16 @@ impl Timers { 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 } @@ -378,9 +388,10 @@ impl Tunn { } // 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); @@ -400,7 +411,6 @@ impl Tunn { existing.is_none(), "Should never override existing handshake" ); - tracing::debug!(?jitter, "Scheduling new handshake"); return TunnResult::Done; From 938461a104891cf8fec5785a0ff9afb2b37f2f27 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 14 Jan 2025 23:34:53 +0100 Subject: [PATCH 09/14] Delete old timers --- boringtun/src/noise/timers.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 62272f5c9..3ba3aaf53 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -278,13 +278,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); } From 0179410135700a4645040f10c57d3355374bfc4d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 13 Jan 2025 16:56:20 +0100 Subject: [PATCH 10/14] Move initiator condition into timer fn --- boringtun/src/noise/timers.rs | 64 ++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 3ba3aaf53..faf1d1d6d 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -97,6 +97,11 @@ impl Timers { } 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] { @@ -108,6 +113,11 @@ impl Timers { } 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] { @@ -325,35 +335,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 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 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 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; - } + // 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 From 442429382663e461331ecee99ca4c733f035e68f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 13 Jan 2025 19:56:22 +0100 Subject: [PATCH 11/14] Return next scheduled update from `next_timer_update` --- boringtun/src/noise/timers.rs | 40 +++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index faf1d1d6d..f29ebbf7c 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -245,6 +245,13 @@ 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()) @@ -428,10 +435,35 @@ impl Tunn { /// /// 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.last_data_received_without_reply) - .min() + // 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); + } + + let common_timers = iter::empty() + .chain(self.next_expired_session()) + .chain(self.handshake.cookie_expiration()) + .chain(Some(self.timers.reject_after_time())); + + if let Some((rekey_timeout, _)) = self.handshake.rekey_timeout() { + common_timers + .chain(Some(rekey_timeout)) + .chain(Some(self.timers.rekey_attempt_time())) + .min() + } 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()) + .chain(self.timers.reject_after_time_on_receive()) + .chain(self.timers.rekey_after_time_without_response()) + .chain(self.timers.keepalive_after_time_without_send()) + .chain(persistent_keepalive) + .min() + } } #[deprecated(note = "Prefer `Tunn::time_since_last_handshake_at` to avoid time-impurity")] From 533419a4ac4d3e0afe84eda5d35721eb2a2a7647 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 15 Jul 2025 05:08:47 -0700 Subject: [PATCH 12/14] Update docs --- boringtun/src/noise/timers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index f29ebbf7c..7ef317b55 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -433,7 +433,7 @@ 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). + /// Calling it earlier than the given [`Instant`] is safe but unlikely to have any effect. pub fn next_timer_update(&self) -> Option { // 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 { From ff091adf910a4a1cb77273ceb503bfb8e5557fab Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 15 Jul 2025 06:30:19 -0700 Subject: [PATCH 13/14] Add wake reason --- boringtun/src/noise/timers.rs | 51 +++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 7ef317b55..432007666 100644 --- a/boringtun/src/noise/timers.rs +++ b/boringtun/src/noise/timers.rs @@ -434,22 +434,29 @@ impl Tunn { /// Returns an [`Instant`] when [`Tunn::update_timers_at`] should be called again. /// /// Calling it earlier than the given [`Instant`] is safe but unlikely to have any effect. - pub fn next_timer_update(&self) -> Option { + pub fn next_timer_update(&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); + return Some((scheduled_handshake, "scheduled handshake")); } let common_timers = iter::empty() - .chain(self.next_expired_session()) - .chain(self.handshake.cookie_expiration()) - .chain(Some(self.timers.reject_after_time())); + .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)) - .chain(Some(self.timers.rekey_attempt_time())) - .min() + .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] @@ -457,12 +464,28 @@ impl Tunn { .and_then(|_| self.timers.next_persistent_keepalive()); common_timers - .chain(self.timers.rekey_after_time_on_send()) - .chain(self.timers.reject_after_time_on_receive()) - .chain(self.timers.rekey_after_time_without_response()) - .chain(self.timers.keepalive_after_time_without_send()) - .chain(persistent_keepalive) - .min() + .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) } } From c405263b2a472b30570bb29d936c9546347990a9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 15 Jul 2025 06:42:28 -0700 Subject: [PATCH 14/14] Return at least most recent timer update --- boringtun/src/noise/timers.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs index 432007666..104aa05b3 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, } @@ -258,6 +260,8 @@ impl Tunn { } 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 { @@ -435,6 +439,13 @@ impl Tunn { /// /// 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"));