diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4decfe9..2d27397 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,28 @@ jobs: components: rustfmt - run: cargo fmt --check + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install system dependencies + run: sudo apt-get update && sudo apt-get install -y libdbus-1-dev + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + - name: Cache Cargo registry + build + uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-clippy-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo- + - run: cargo clippy --all-targets --all-features -- -D warnings + build: name: Build (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/CHANGELOG.md b/CHANGELOG.md index bd1e6f8..f0b09fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -270,6 +270,21 @@ with v0.2.x peers. pre-`0.3.0` so a tagged release supersedes any prior dev .deb. Tagged release builds (no `-dev` in Cargo.toml) keep the clean `-1` form. Operator override via `--version` still wins +- One-shot startup advert sweep for Nostr open-discovery. On daemon + startup under `node.discovery.nostr.policy: open`, after a short + settle delay (`startup_sweep_delay_secs`, default 5s) the cached + overlay-advert table is iterated once and recent adverts (newer + than `startup_sweep_max_age_secs`, default 3600s) are queued for + outbound retry, modulo the same skip-filters as the per-tick sweep + (configured peer, already connected, retry-pending, connecting). + Closes the gap where peers learned only through relay backlog at + startup were not dialed until they republished. +- Diagnostic logging on the open-discovery sweep. Each `queued retry` + now logs at info-level with the peer short-npub and advert age, + and a one-line summary (cached count, queued count, per-reason + skip counts) is emitted on every startup sweep and on any per-tick + sweep that queues at least one retry. Operator-facing visibility + into what the auto-dial path is doing. ### Changed @@ -335,6 +350,16 @@ with v0.2.x peers. ### Fixed +- Tor onion adverts published over Nostr overlay discovery now + include the public-facing port (`.onion:`) instead of + just the bare onion hostname. The publisher previously emitted a + bare onion that the parser refused (`expected host:port`), + producing a persistent retry-fail loop on any peer whose Tor + advert was the only entry in the discovery cache. New + `transports.tor.advertised_port` config field (default `443`, + matching the Tor `HiddenServicePort` convention) controls the + advertised port; operators with non-default virtual ports can + override. - Control socket path detection in fipsctl and fipstop now checks for the `/run/fips/` directory instead of the socket file inside it, so users not yet in the `fips` group get a clear "Permission denied" diff --git a/docs/design/fips-configuration.md b/docs/design/fips-configuration.md index 9e586d5..1e5d541 100644 --- a/docs/design/fips-configuration.md +++ b/docs/design/fips-configuration.md @@ -206,6 +206,8 @@ without that feature ignore `udp:nat` bootstrap configuration. | `node.discovery.nostr.punch_duration_ms` | u64 | `10000` | How long to keep punching before failure | | `node.discovery.nostr.advert_ttl_secs` | u64 | `3600` | Advert TTL in seconds | | `node.discovery.nostr.advert_refresh_secs` | u64 | `1800` | How often adverts are refreshed in seconds | +| `node.discovery.nostr.startup_sweep_delay_secs` | u64 | `5` | Settle delay after Nostr discovery starts before the one-shot startup advert sweep runs (only used under `policy: open`). Allows the relay subscription backlog to populate the in-memory advert cache before the sweep fires | +| `node.discovery.nostr.startup_sweep_max_age_secs` | u64 | `3600` | Maximum advert age (`now - created_at`) considered by the one-shot startup sweep (only used under `policy: open`). Adverts older than this are skipped on startup; the per-tick sweep still considers them up to `valid_until_ms` | If `stun_servers` is omitted, the built-in default list above is used. If it is specified in YAML, the configured list fully overrides the defaults. @@ -468,6 +470,7 @@ Requires an external Tor daemon providing a SOCKS5 proxy. Three modes: | `transports.tor.max_inbound_connections` | usize | `64` | Maximum inbound connections via onion service. | | `transports.tor.directory_service.hostname_file` | string | `"/var/lib/tor/fips_onion_service/hostname"` | Path to Tor-managed hostname file containing the `.onion` address. | | `transports.tor.directory_service.bind_addr` | string | `"127.0.0.1:8443"` | Local bind address for the listener that Tor forwards inbound connections to. Must match `HiddenServicePort` target in `torrc`. | +| `transports.tor.advertised_port` | u16 | `443` | Public-facing onion port published in Nostr overlay adverts. Must match the virtual port in torrc's `HiddenServicePort 127.0.0.1:` directive — that is the port other peers will use to reach this onion. | **Named instances.** Like other transports, multiple Tor instances can be configured with named sub-keys for different SOCKS5 proxy endpoints. diff --git a/src/config/mod.rs b/src/config/mod.rs index f67222f..a9f5862 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1309,12 +1309,14 @@ peers: #[test] #[allow(clippy::field_reassign_with_default)] fn test_validate_peer_via_nostr_requires_nostr_enabled() { - let mut config = Config::default(); - config.peers = vec![PeerConfig { - npub: "npub1peer".to_string(), - via_nostr: true, + let mut config = Config { + peers: vec![PeerConfig { + npub: "npub1peer".to_string(), + via_nostr: true, + ..Default::default() + }], ..Default::default() - }]; + }; config.node.discovery.nostr.enabled = false; let err = config.validate().expect_err("validation should fail"); @@ -1325,11 +1327,13 @@ peers: #[allow(clippy::field_reassign_with_default)] fn test_validate_peer_addresses_required_unless_via_nostr() { // Empty addresses + via_nostr=false → error. - let mut config = Config::default(); - config.peers = vec![PeerConfig { - npub: "npub1peer".to_string(), + let mut config = Config { + peers: vec![PeerConfig { + npub: "npub1peer".to_string(), + ..Default::default() + }], ..Default::default() - }]; + }; let err = config.validate().expect_err("validation should fail"); assert!(err.to_string().contains("at least one address")); diff --git a/src/config/node.rs b/src/config/node.rs index 7e98fbf..b6cea3e 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -353,6 +353,18 @@ pub struct NostrDiscoveryConfig { /// How often adverts are refreshed in seconds. #[serde(default = "NostrDiscoveryConfig::default_advert_refresh_secs")] pub advert_refresh_secs: u64, + /// Settle delay in seconds after Nostr discovery starts before the + /// one-shot startup sweep of cached adverts runs. Allows the relay + /// subscription backlog to populate the in-memory advert cache. + /// Only used under `policy: open`. Default: 5. + #[serde(default = "NostrDiscoveryConfig::default_startup_sweep_delay_secs")] + pub startup_sweep_delay_secs: u64, + /// Maximum age in seconds for cached adverts considered by the + /// one-shot startup sweep. Adverts whose `created_at` is older than + /// `now - startup_sweep_max_age_secs` are skipped. Only used under + /// `policy: open`. Default: 3600 (1 hour). + #[serde(default = "NostrDiscoveryConfig::default_startup_sweep_max_age_secs")] + pub startup_sweep_max_age_secs: u64, } impl Default for NostrDiscoveryConfig { @@ -378,6 +390,8 @@ impl Default for NostrDiscoveryConfig { punch_duration_ms: Self::default_punch_duration_ms(), advert_ttl_secs: Self::default_advert_ttl_secs(), advert_refresh_secs: Self::default_advert_refresh_secs(), + startup_sweep_delay_secs: Self::default_startup_sweep_delay_secs(), + startup_sweep_max_age_secs: Self::default_startup_sweep_max_age_secs(), } } } @@ -462,6 +476,14 @@ impl NostrDiscoveryConfig { fn default_advert_refresh_secs() -> u64 { 1_800 } + + fn default_startup_sweep_delay_secs() -> u64 { + 5 + } + + fn default_startup_sweep_max_age_secs() -> u64 { + 3_600 + } } /// Spanning tree (`node.tree.*`). @@ -1048,6 +1070,32 @@ mod tests { assert!((c.etx_threshold - 3.0).abs() < 1e-9); // default } + #[test] + fn test_nostr_discovery_startup_sweep_defaults() { + let c = NostrDiscoveryConfig::default(); + assert_eq!(c.startup_sweep_delay_secs, 5); + assert_eq!(c.startup_sweep_max_age_secs, 3_600); + } + + #[test] + fn test_nostr_discovery_startup_sweep_yaml_override() { + let yaml = "enabled: true\npolicy: open\nstartup_sweep_delay_secs: 10\nstartup_sweep_max_age_secs: 1800\n"; + let c: NostrDiscoveryConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(c.enabled); + assert_eq!(c.policy, NostrDiscoveryPolicy::Open); + assert_eq!(c.startup_sweep_delay_secs, 10); + assert_eq!(c.startup_sweep_max_age_secs, 1_800); + } + + #[test] + fn test_nostr_discovery_startup_sweep_partial_yaml_uses_defaults() { + // Only override delay; max_age should fall back to default. + let yaml = "enabled: true\nstartup_sweep_delay_secs: 30\n"; + let c: NostrDiscoveryConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(c.startup_sweep_delay_secs, 30); + assert_eq!(c.startup_sweep_max_age_secs, 3_600); + } + #[cfg(windows)] #[test] fn test_default_socket_path_windows() { diff --git a/src/config/transport.rs b/src/config/transport.rs index ac2a150..af996f6 100644 --- a/src/config/transport.rs +++ b/src/config/transport.rs @@ -441,6 +441,10 @@ const DEFAULT_HOSTNAME_FILE: &str = "/var/lib/tor/fips_onion_service/hostname"; /// Default directory mode bind address. const DEFAULT_DIRECTORY_BIND_ADDR: &str = "127.0.0.1:8443"; +/// Default advertised onion port for Nostr overlay discovery. Matches the +/// Tor convention of `HiddenServicePort 443 127.0.0.1:` in torrc. +const DEFAULT_TOR_ADVERTISED_PORT: u16 = 443; + /// Tor transport instance configuration. /// /// Supports three modes: @@ -503,6 +507,13 @@ pub struct TorConfig { /// Default: false. #[serde(default, skip_serializing_if = "Option::is_none")] pub advertise_on_nostr: Option, + + /// Public-facing onion port published in Nostr overlay adverts. Must + /// match the virtual port in torrc's `HiddenServicePort + /// 127.0.0.1:` directive — that is the port other peers + /// will use to reach this onion. Default: 443. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub advertised_port: Option, } /// Directory-mode onion service configuration. @@ -595,6 +606,12 @@ impl TorConfig { pub fn advertise_on_nostr(&self) -> bool { self.advertise_on_nostr.unwrap_or(false) } + + /// Public-facing onion port published in Nostr overlay adverts. + /// Default: 443. + pub fn advertised_port(&self) -> u16 { + self.advertised_port.unwrap_or(DEFAULT_TOR_ADVERTISED_PORT) + } } // ============================================================================ diff --git a/src/discovery/nostr/runtime.rs b/src/discovery/nostr/runtime.rs index 7aa1245..cba7b83 100644 --- a/src/discovery/nostr/runtime.rs +++ b/src/discovery/nostr/runtime.rs @@ -198,7 +198,7 @@ impl NostrDiscovery { pub async fn cached_open_discovery_candidates( &self, max: usize, - ) -> Vec<(String, Vec)> { + ) -> Vec<(String, Vec, u64)> { self.prune_advert_cache().await; let now = now_ms(); let cache = self.advert_cache.read().await; @@ -206,7 +206,13 @@ impl NostrDiscovery { .values() .filter(|entry| entry.author_npub != self.npub) .filter(|entry| entry.valid_until_ms > now) - .map(|entry| (entry.author_npub.clone(), entry.advert.endpoints.clone())) + .map(|entry| { + ( + entry.author_npub.clone(), + entry.advert.endpoints.clone(), + entry.created_at, + ) + }) .take(max) .collect() } diff --git a/src/node/lifecycle.rs b/src/node/lifecycle.rs index bc6761e..ad51971 100644 --- a/src/node/lifecycle.rs +++ b/src/node/lifecycle.rs @@ -467,6 +467,8 @@ impl Node { } } + self.maybe_run_startup_open_discovery_sweep(&bootstrap) + .await; self.queue_open_discovery_retries(&bootstrap).await; } @@ -617,6 +619,7 @@ impl Node { warn!(error = %err, "Failed to publish initial Nostr overlay advert"); } self.nostr_discovery = Some(runtime); + self.nostr_discovery_started_at_ms = Some(Self::now_ms()); info!("Nostr overlay discovery enabled"); } Err(err) => { @@ -1184,6 +1187,26 @@ impl Node { } async fn queue_open_discovery_retries(&mut self, bootstrap: &std::sync::Arc) { + self.run_open_discovery_sweep(bootstrap, None, "per-tick") + .await; + } + + /// Open-discovery cache sweep. Iterates the cached overlay adverts and + /// queues retries for non-configured, not-yet-connected peers. + /// + /// `max_age_secs`, if set, filters out adverts whose `created_at` is + /// older than `now - max_age_secs`. The per-tick sweep passes `None` + /// (relies on the cache's own `valid_until_ms` filter); the one-shot + /// startup sweep passes `Some(startup_sweep_max_age_secs)`. + /// + /// `caller` is a short label included in log lines so per-tick and + /// startup sweeps are distinguishable in operator-facing logs. + async fn run_open_discovery_sweep( + &mut self, + bootstrap: &std::sync::Arc, + max_age_secs: Option, + caller: &'static str, + ) { if !self.config.node.discovery.nostr.enabled || self.config.node.discovery.nostr.policy != crate::config::NostrDiscoveryPolicy::Open { @@ -1197,28 +1220,63 @@ impl Node { .map(|peer| peer.npub.clone()) .collect::>(); let now_ms = Self::now_ms(); + let now_secs = now_ms / 1000; let mut enqueue_budget = self.open_discovery_enqueue_budget(&configured_npubs); if enqueue_budget == 0 { + debug!( + caller = %caller, + "open-discovery sweep: enqueue budget is 0, skipping" + ); return; } - for (npub, endpoints) in bootstrap.cached_open_discovery_candidates(64).await { + let candidates = bootstrap.cached_open_discovery_candidates(64).await; + let cached_count = candidates.len(); + let mut enqueued = 0usize; + let mut skipped_age = 0usize; + let mut skipped_configured = 0usize; + let mut skipped_self = 0usize; + let mut skipped_connected = 0usize; + let mut skipped_retry_pending = 0usize; + let mut skipped_connecting = 0usize; + let mut skipped_no_endpoints = 0usize; + let mut skipped_invalid_npub = 0usize; + + for (npub, endpoints, created_at_secs) in candidates { if enqueue_budget == 0 { break; } + + if let Some(max_age) = max_age_secs + && now_secs.saturating_sub(created_at_secs) > max_age + { + skipped_age = skipped_age.saturating_add(1); + continue; + } + if configured_npubs.contains(&npub) { + skipped_configured = skipped_configured.saturating_add(1); continue; } let peer_identity = match PeerIdentity::from_npub(&npub) { Ok(identity) => identity, - Err(_) => continue, + Err(_) => { + skipped_invalid_npub = skipped_invalid_npub.saturating_add(1); + continue; + } }; let node_addr = *peer_identity.node_addr(); - if node_addr == *self.identity.node_addr() || self.peers.contains_key(&node_addr) { + if node_addr == *self.identity.node_addr() { + skipped_self = skipped_self.saturating_add(1); + continue; + } + if self.peers.contains_key(&node_addr) { + skipped_connected = skipped_connected.saturating_add(1); continue; } if self.retry_pending.contains_key(&node_addr) { + skipped_retry_pending = skipped_retry_pending.saturating_add(1); continue; } let connecting = self.connections.values().any(|conn| { @@ -1227,6 +1285,7 @@ impl Node { .unwrap_or(false) }); if connecting { + skipped_connecting = skipped_connecting.saturating_add(1); continue; } @@ -1246,6 +1305,7 @@ impl Node { priority = priority.saturating_add(1); } if addresses.is_empty() { + skipped_no_endpoints = skipped_no_endpoints.saturating_add(1); continue; } @@ -1266,8 +1326,87 @@ impl Node { state.retry_after_ms = now_ms; state.expires_at_ms = Some(self.open_discovery_retry_expires_at_ms(now_ms)); self.retry_pending.insert(node_addr, state); + info!( + caller = %caller, + peer = %peer_identity.short_npub(), + advert_age_secs = now_secs.saturating_sub(created_at_secs), + "open-discovery sweep: queued retry for cached advert" + ); enqueue_budget = enqueue_budget.saturating_sub(1); + enqueued = enqueued.saturating_add(1); } + + // Always log a one-line summary on the startup sweep so operators + // can verify it ran. Per-tick sweeps are noisier; only summarize + // when something happened. + let total_skipped = skipped_age + + skipped_configured + + skipped_self + + skipped_connected + + skipped_retry_pending + + skipped_connecting + + skipped_no_endpoints + + skipped_invalid_npub; + let should_summarize = caller == "startup" || enqueued > 0; + if should_summarize { + info!( + caller = %caller, + cached = cached_count, + queued = enqueued, + skipped_age = skipped_age, + skipped_configured = skipped_configured, + skipped_self = skipped_self, + skipped_connected = skipped_connected, + skipped_retry_pending = skipped_retry_pending, + skipped_connecting = skipped_connecting, + skipped_no_endpoints = skipped_no_endpoints, + skipped_invalid_npub = skipped_invalid_npub, + skipped_total = total_skipped, + "open-discovery sweep complete" + ); + } + } + + /// One-shot startup sweep: runs once after the configured settle + /// delay, iterating the cached overlay adverts and queueing retries + /// for any peer with a recent enough advert that we haven't already + /// configured statically or established a link to. + /// + /// Gated identically to [`run_open_discovery_sweep`]: requires + /// `node.discovery.nostr.enabled` and `policy == open`. + async fn maybe_run_startup_open_discovery_sweep( + &mut self, + bootstrap: &std::sync::Arc, + ) { + if self.startup_open_discovery_sweep_done { + return; + } + if !self.config.node.discovery.nostr.enabled + || self.config.node.discovery.nostr.policy != crate::config::NostrDiscoveryPolicy::Open + { + // Mark done so we don't keep re-checking on every tick. + self.startup_open_discovery_sweep_done = true; + return; + } + let Some(started_at_ms) = self.nostr_discovery_started_at_ms else { + return; + }; + let now_ms = Self::now_ms(); + let delay_ms = self + .config + .node + .discovery + .nostr + .startup_sweep_delay_secs + .saturating_mul(1000); + if now_ms < started_at_ms.saturating_add(delay_ms) { + return; + } + + let max_age_secs = self.config.node.discovery.nostr.startup_sweep_max_age_secs; + self.run_open_discovery_sweep(bootstrap, Some(max_age_secs), "startup") + .await; + self.startup_open_discovery_sweep_done = true; } fn available_outbound_slots(&self) -> usize { @@ -1384,7 +1523,7 @@ impl Node { if let Some(addr) = handle.onion_address() { endpoints.push(OverlayEndpointAdvert { transport: OverlayTransportKind::Tor, - addr: addr.to_string(), + addr: format!("{}:{}", addr, cfg.advertised_port()), }); } } @@ -1593,6 +1732,13 @@ impl Node { self.register_identity(peer_node_addr, peer_identity.pubkey_full()); let transport_id = self.allocate_transport_id(); + // Adopted ephemeral UDP transports use UdpConfig::default() when the + // bootstrap runtime doesn't pass an override. Default MTU resolves to + // 1280 (IPv6 minimum), which is the only value guaranteed to survive + // arbitrary NAT-traversal middlebox paths. Inheriting from the named + // [transports.udp] config (Option 3 in ISSUE-2026-0013) would track + // operator config more closely but risks regressions on hostile paths; + // accepted as-is until a concrete use case justifies the change. let mut transport = crate::transport::udp::UdpTransport::new( transport_id, traversal.transport_name.clone(), diff --git a/src/node/mod.rs b/src/node/mod.rs index 8e0d2bc..4564e89 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -441,6 +441,15 @@ pub struct Node { /// Optional Nostr/STUN overlay discovery coordinator for `udp:nat` peers. nostr_discovery: Option>, + /// Wall-clock ms when Nostr discovery successfully started, used to + /// schedule the one-shot startup advert sweep after a settle delay. + /// `None` until discovery comes up; remains `None` if discovery is + /// disabled or failed to start. + nostr_discovery_started_at_ms: Option, + /// Whether the one-shot startup advert sweep has run. Set to true + /// after the first sweep fires (under `policy: open`); thereafter + /// only the per-tick `queue_open_discovery_retries` continues. + startup_open_discovery_sweep_done: bool, /// Per-peer UDP transports adopted from NAT traversal handoff. bootstrap_transports: HashSet, @@ -608,6 +617,8 @@ impl Node { pending_connects: Vec::new(), retry_pending: HashMap::new(), nostr_discovery: None, + nostr_discovery_started_at_ms: None, + startup_open_discovery_sweep_done: false, bootstrap_transports: HashSet::new(), last_parent_reeval: None, last_congestion_log: None, @@ -737,6 +748,8 @@ impl Node { pending_connects: Vec::new(), retry_pending: HashMap::new(), nostr_discovery: None, + nostr_discovery_started_at_ms: None, + startup_open_discovery_sweep_done: false, bootstrap_transports: HashSet::new(), last_parent_reeval: None, last_congestion_log: None, @@ -1023,18 +1036,31 @@ impl Node { crate::upper::icmp::effective_ipv6_mtu(self.transport_mtu()) } - /// Get the transport MTU for a specific transport. + /// Get the transport MTU governing the global TUN-boundary MSS clamp. /// - /// When called without a specific transport context, returns the MTU - /// of the first operational transport, or 1280 (IPv6 minimum) as - /// fallback. This is used for initial TUN configuration where a - /// specific transport isn't yet known. + /// Returns the **minimum** MTU across all operational transports, or + /// 1280 (IPv6 minimum) as fallback. Used for initial TUN configuration + /// where a specific egress transport isn't yet known: the resulting + /// `effective_ipv6_mtu` (transport_mtu - 77) and `max_mss` + /// (effective_mtu - 60) form a conservative ceiling that fits ANY + /// configured-transport's egress, eliminating PMTU-D black holes that + /// would otherwise occur when a flow's actual egress is smaller than + /// the clamp ceiling assumed at TUN init. + /// + /// Returning the smallest (rather than the first-iterated, which used + /// to vary across HashMap iteration order + async-startup race) makes + /// the clamp deterministic across daemon restarts. + /// + /// See `ISSUE-2026-0011` for the empirical investigation. pub fn transport_mtu(&self) -> u16 { - // Prefer the MTU from the first operational transport - for handle in self.transports.values() { - if handle.is_operational() { - return handle.mtu(); - } + let min_operational = self + .transports + .values() + .filter(|h| h.is_operational()) + .map(|h| h.mtu()) + .min(); + if let Some(mtu) = min_operational { + return mtu; } // Fallback to config: try UDP first, then Ethernet if let Some((_, cfg)) = self.config.transports.udp.iter().next() { diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 282b70e..5eda3d1 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -954,3 +954,82 @@ fn test_promote_clears_retry_pending() { "retry_pending should be cleared on successful promotion" ); } + +// ============================================================================ +// transport_mtu() — ISSUE-2026-0011 regression coverage +// ============================================================================ + +/// Helper: spawn a UdpTransport with the given mtu, started and operational. +async fn make_udp_transport_with_mtu(id: u32, mtu: u16) -> TransportHandle { + let (packet_tx, _packet_rx) = packet_channel(64); + let transport_id = TransportId::new(id); + let mut udp = UdpTransport::new( + transport_id, + Some(format!("udp{}", id)), + crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + mtu: Some(mtu), + ..Default::default() + }, + packet_tx, + ); + udp.start_async().await.unwrap(); + TransportHandle::Udp(udp) +} + +#[tokio::test] +async fn test_transport_mtu_returns_min_across_operational() { + // Multiple operational transports with varied MTUs. The picker must + // return the smallest, deterministically, regardless of HashMap + // iteration order. This is the core ISSUE-2026-0011 regression test. + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp1 = make_udp_transport_with_mtu(1, 1497).await; + let udp2 = make_udp_transport_with_mtu(2, 1280).await; + let udp3 = make_udp_transport_with_mtu(3, 1400).await; + + node.transports.insert(TransportId::new(1), udp1); + node.transports.insert(TransportId::new(2), udp2); + node.transports.insert(TransportId::new(3), udp3); + + // Expect the smallest (UDP-1280), not whichever HashMap iterates first. + assert_eq!(node.transport_mtu(), 1280); + + // effective_ipv6_mtu = 1280 - 77 = 1203, max_mss = 1203 - 60 = 1143 + // (verifies the downstream clamp value). + assert_eq!(node.effective_ipv6_mtu(), 1203); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +#[tokio::test] +async fn test_transport_mtu_fallback_when_no_operational_transports() { + // No transports configured at all → falls back to 1280 (IPv6 minimum). + let node = make_node(); + assert_eq!(node.transport_mtu(), 1280); +} + +#[tokio::test] +async fn test_transport_mtu_min_with_single_operational() { + // Single transport: trivially returns its MTU. Pins the picker doesn't + // accidentally drop down to a smaller fallback when one transport is + // operational. + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + assert_eq!(node.transport_mtu(), 1452); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} diff --git a/src/transport/tor/mod.rs b/src/transport/tor/mod.rs index 4fbf0c3..06d4019 100644 --- a/src/transport/tor/mod.rs +++ b/src/transport/tor/mod.rs @@ -1446,6 +1446,41 @@ mod tests { assert_eq!(config.socks5_addr(), "127.0.0.1:9050"); assert_eq!(config.connect_timeout_ms(), 120000); assert_eq!(config.mtu(), 1400); + assert_eq!(config.advertised_port(), 443); + } + + #[test] + fn test_advertised_port_override() { + let config = TorConfig { + advertised_port: Some(9001), + ..Default::default() + }; + assert_eq!(config.advertised_port(), 9001); + } + + /// Pins the publisher/parser contract for Tor overlay adverts. + /// `build_overlay_advert` formats Tor endpoints as `:`; + /// `parse_tor_addr` must accept that exact form back. A bare onion + /// (no port) was the production bug — assert it does not parse. + #[test] + fn test_advert_address_round_trips_through_parser() { + let onion = "mwvj6q3pnsiaky7i6wg5s42xlfurt5uqr3qzckrlw2graa2ugcgwhiqd.onion"; + let cfg = TorConfig::default(); + let advertised = format!("{}:{}", onion, cfg.advertised_port()); + + let parsed = parse_tor_addr(&TransportAddr::from_string(&advertised)).unwrap(); + match parsed { + TorAddr::Onion(host, port) => { + assert_eq!(host, onion); + assert_eq!(port, 443); + } + other => panic!("expected Onion variant, got {:?}", other), + } + + // Sanity-check the inverse: the bare-onion form (the bug) must + // not parse, so any future regression in the publisher will be + // caught by the round-trip test above. + assert!(parse_tor_addr(&TransportAddr::from_string(onion)).is_err()); } #[tokio::test] diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 5d81e1a..1f8f7e9 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -193,8 +193,8 @@ run_build() { return 1 fi - info "cargo clippy --all -- -D warnings" - if cargo clippy --all -- -D warnings 2>&1; then + info "cargo clippy --all-targets --all-features -- -D warnings" + if cargo clippy --all-targets --all-features -- -D warnings 2>&1; then record "clippy" 0 else record "clippy" 1 @@ -235,8 +235,10 @@ install_binaries() { cp target/release/fips "$dest/fips" cp target/release/fipsctl "$dest/fipsctl" [[ -f target/release/fipstop ]] && cp target/release/fipstop "$dest/fipstop" || true + [[ -f target/release/fips-gateway ]] && cp target/release/fips-gateway "$dest/fips-gateway" || true chmod +x "$dest/fips" "$dest/fipsctl" [[ -f "$dest/fipstop" ]] && chmod +x "$dest/fipstop" || true + [[ -f "$dest/fips-gateway" ]] && chmod +x "$dest/fips-gateway" || true } # Run a static topology test (mesh, chain)