diff --git a/CHANGELOG.md b/CHANGELOG.md index d715396..b686088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Nostr NAT traversal no longer breaks after the host suspends. The traversal + clock cached a Unix timestamp once at startup and advanced it with a + monotonic `Instant`, which does not tick while a machine is asleep, so after + a suspend the daemon's idea of the time trailed real time by the suspend + duration for the rest of the process lifetime. Every NIP-40 expiration it + computed was therefore published already in the past: relays dropped the + offers as expired, the initiator logged a signal timeout waiting for an + answer, and traversal stayed broken until the daemon was restarted. The + clock now reads the wall clock on every call. This is not macOS-specific, + though a laptop that sleeps is where it is easiest to hit; any host that + suspends or hibernates was affected. Reported in + [#128](https://github.com/jmcorgan/fips/issues/128). + - `SessionDatagram` hop-limit handling now follows IP semantics. Delivery to the addressed node is no longer TTL-gated, and a forwarder decrements before deciding rather than after, so a datagram that would leave with a TTL of zero diff --git a/src/discovery/nostr/tests.rs b/src/discovery/nostr/tests.rs index 355666f..8ec3f3a 100644 --- a/src/discovery/nostr/tests.rs +++ b/src/discovery/nostr/tests.rs @@ -7,7 +7,7 @@ use super::signal::{ }; use super::stun::{parse_stun_binding_success, parse_stun_url}; use super::traversal::{ - PunchStrategy, build_punch_packet, parse_punch_packet, plan_punch_targets, + PunchStrategy, build_punch_packet, now_ms, parse_punch_packet, plan_punch_targets, planned_remote_endpoints, session_hash, }; use super::{ @@ -670,3 +670,40 @@ fn responder_suppression_election() { &smaller, &smaller, true )); } + +#[test] +fn now_ms_tracks_the_wall_clock() { + use std::time::{SystemTime, UNIX_EPOCH}; + + fn wall_ms() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock is after the Unix epoch") + .as_millis() as u64 + } + + // Bracket a sample between two independent wall-clock reads taken either + // side of it. This is the property the traversal clock has to hold for the + // NIP-40 expiration tags it computes to be in the future when published. + // + // Read this for what it is: it pins the contract (Unix epoch, milliseconds, + // tracking real time) and it fires on a host that has actually suspended, + // where the sample falls below `before` by the suspend duration. It is NOT a + // regression guard for the anchored-clock defect. Nothing reachable from a + // unit test can simulate a suspend, so on a machine that has not slept, an + // anchored implementation passes this -- deterministically when this is the + // first caller of `now_ms()` in the binary, and otherwise with a probability + // set by the fractional millisecond the anchor happened to capture. + let before = wall_ms(); + let sampled = now_ms(); + let after = wall_ms(); + + assert!( + sampled >= before, + "now_ms() is behind the wall clock: {sampled} < {before}" + ); + assert!( + sampled <= after, + "now_ms() is ahead of the wall clock: {sampled} > {after}" + ); +} diff --git a/src/discovery/nostr/traversal.rs b/src/discovery/nostr/traversal.rs index 49f875a..cbb8531 100644 --- a/src/discovery/nostr/traversal.rs +++ b/src/discovery/nostr/traversal.rs @@ -1,5 +1,5 @@ use std::net::SocketAddr; -use std::sync::{Arc, OnceLock}; +use std::sync::Arc; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use tokio::net::UdpSocket; @@ -197,25 +197,35 @@ pub(super) fn nonce() -> String { format!("{}-{:016x}", now_ms(), rand::random::()) } +/// Current Unix time in milliseconds, read from the wall clock on every call. +/// +/// This deliberately does not cache a start-of-process anchor and advance it +/// with a monotonic `Instant`. A monotonic clock does not advance while the host +/// is suspended, so an anchored value trails real time by the suspend duration +/// for the remaining life of the process. Every expiry computed from it is then +/// published already in the past, the relay drops the event as expired, and +/// traversal signalling fails until the daemon is restarted. +/// +/// About half the consumers publish or serialize the value as an absolute +/// timestamp: the NIP-40 expiration tags on adverts and traversal signals, and +/// the `issuedAt`/`expiresAt` fields of offers and answers. The rest compare it +/// against timestamps on the same basis, including the peer-authored, signed +/// `created_at` of a received advert, so they need it to track real time too. +/// +/// The interval-shaped consumers survive a step in the wall clock. A forward +/// step, which is what a resume produces, saturates the punch start delay to +/// zero so punching begins immediately; the attempt's own bounds are monotonic +/// `Instant` deadlines, so its length is unaffected. A backward step lengthens +/// that delay instead and can cost a single punch attempt, which retries. Early +/// eviction from the replay window cannot admit a replay under the shipped +/// defaults, because the freshness window a replayed offer would also have to +/// satisfy (`signal_ttl_secs` plus `FRESHNESS_SKEW_TOLERANCE_MS`, 180s) is +/// strictly narrower than the replay window itself (`replay_window_secs`, 300s). pub(super) fn now_ms() -> u64 { - struct ClockAnchor { - started_at: Instant, - started_unix_ms: u64, - } - - static ANCHOR: OnceLock = OnceLock::new(); - - let anchor = ANCHOR.get_or_init(|| ClockAnchor { - started_at: Instant::now(), - started_unix_ms: SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_millis() as u64) - .unwrap_or(0), - }); - - anchor - .started_unix_ms - .saturating_add(anchor.started_at.elapsed().as_millis() as u64) + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_millis() as u64) + .unwrap_or(0) } pub(super) fn session_hash(session_id: &str) -> [u8; 16] {