mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-22 07:48:26 +00:00
Session idle timeout now based on application data only
MMP reports (SenderReport, ReceiverReport, PathMtuNotification) were resetting the idle timer on both RX and TX paths, preventing sessions from ever timing out when MMP traffic kept flowing. Changed touch() to only be called for DataPacket send/receive and session establishment, so sessions with no application data tear down after the idle timeout even with active MMP measurement traffic.
This commit is contained in:
@@ -224,7 +224,8 @@ pub struct SessionConfig {
|
||||
#[serde(default = "SessionConfig::default_pending_max_destinations")]
|
||||
pub pending_max_destinations: usize,
|
||||
/// Idle session timeout in seconds (`node.session.idle_timeout_secs`).
|
||||
/// Established sessions with no activity for this duration are removed.
|
||||
/// Established sessions with no application data for this duration are
|
||||
/// removed. MMP reports do not count as activity for this timer.
|
||||
#[serde(default = "SessionConfig::default_idle_timeout_secs")]
|
||||
pub idle_timeout_secs: u64,
|
||||
/// Number of initial data packets per session that include COORDS_PRESENT
|
||||
|
||||
@@ -189,7 +189,6 @@ impl Node {
|
||||
}
|
||||
};
|
||||
|
||||
entry.touch(Self::now_ms());
|
||||
self.sessions.insert(*src_addr, entry);
|
||||
|
||||
// Strip FSP inner header (6 bytes)
|
||||
@@ -256,6 +255,14 @@ impl Node {
|
||||
}
|
||||
}
|
||||
|
||||
// Only application data resets the idle timer — MMP reports
|
||||
// (SenderReport, ReceiverReport, PathMtuNotification) do not.
|
||||
if msg_type == SessionMessageType::DataPacket.to_byte()
|
||||
&& let Some(entry) = self.sessions.get_mut(src_addr)
|
||||
{
|
||||
entry.touch(Self::now_ms());
|
||||
}
|
||||
|
||||
// Flush any pending outbound packets (e.g., simultaneous initiation
|
||||
// where responder also had queued outbound packets)
|
||||
self.flush_pending_packets(src_addr).await;
|
||||
@@ -854,12 +861,11 @@ impl Node {
|
||||
|
||||
self.send_session_datagram(&mut datagram).await?;
|
||||
|
||||
// Record in MMP sender state and touch
|
||||
if let Some(entry) = self.sessions.get_mut(dest_addr) {
|
||||
if let Some(mmp) = entry.mmp_mut() {
|
||||
mmp.sender.record_sent(counter, timestamp, ciphertext.len());
|
||||
}
|
||||
entry.touch(now_ms);
|
||||
// Record in MMP sender state (no touch — MMP reports don't reset idle timer)
|
||||
if let Some(entry) = self.sessions.get_mut(dest_addr)
|
||||
&& let Some(mmp) = entry.mmp_mut()
|
||||
{
|
||||
mmp.sender.record_sent(counter, timestamp, ciphertext.len());
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -109,7 +109,7 @@ impl Node {
|
||||
debug!(
|
||||
dest = %addr,
|
||||
idle_secs = timeout_ms / 1000,
|
||||
"Idle session removed"
|
||||
"Idle session removed (no application data)"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,7 +54,9 @@ pub(crate) struct SessionEntry {
|
||||
/// When the session was created (Unix milliseconds).
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
created_at: u64,
|
||||
/// Last activity timestamp (Unix milliseconds).
|
||||
/// Last application data activity timestamp (Unix milliseconds).
|
||||
/// Only updated for DataPacket send/receive and session establishment.
|
||||
/// MMP reports do not update this field. Used for idle session timeout.
|
||||
last_activity: u64,
|
||||
/// When the session transitioned to Established (Unix milliseconds).
|
||||
/// Used to compute session-relative timestamps for the FSP inner header.
|
||||
@@ -116,7 +118,10 @@ impl SessionEntry {
|
||||
self.state.take()
|
||||
}
|
||||
|
||||
/// Update the last activity timestamp.
|
||||
/// Update the last application data activity timestamp.
|
||||
///
|
||||
/// Only call for DataPacket send/receive and session establishment,
|
||||
/// not for MMP reports. Used by the idle session timeout.
|
||||
pub(crate) fn touch(&mut self, now_ms: u64) {
|
||||
self.last_activity = now_ms;
|
||||
}
|
||||
|
||||
@@ -1307,6 +1307,36 @@ fn test_purge_idle_sessions_disabled_when_zero() {
|
||||
assert_eq!(node.session_count(), 1, "Sessions should not be purged when idle timeout is disabled");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_purge_idle_sessions_mmp_activity_does_not_prevent_purge() {
|
||||
let mut node = make_node();
|
||||
let remote = Identity::generate();
|
||||
let remote_addr = *remote.node_addr();
|
||||
|
||||
let session = make_noise_session(node.identity(), &remote);
|
||||
let entry = crate::node::session::SessionEntry::new(
|
||||
remote_addr,
|
||||
remote.pubkey_full(),
|
||||
EndToEndState::Established(session),
|
||||
1000, // created at t=1s
|
||||
true,
|
||||
);
|
||||
|
||||
// Do NOT call entry.touch() — simulates a session where only MMP
|
||||
// reports have flowed (MMP no longer calls touch). last_activity
|
||||
// remains at creation time (1000ms).
|
||||
node.sessions.insert(remote_addr, entry);
|
||||
|
||||
// Purge at t=92s — 91s since creation, exceeds 90s idle timeout.
|
||||
// Even though MMP reports would have been flowing, they no longer
|
||||
// reset the idle timer.
|
||||
let now_ms = 92_000;
|
||||
node.purge_idle_sessions(now_ms);
|
||||
|
||||
assert_eq!(node.session_count(), 0,
|
||||
"Session with MMP-only activity should be purged");
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Unit tests: COORDS_PRESENT warmup counter
|
||||
// ============================================================================
|
||||
|
||||
Reference in New Issue
Block a user