diff --git a/CHANGELOG.md b/CHANGELOG.md index 10ade41..e7426f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 unavailable on OpenWrt targets - IPv6 routing policy rule added at TUN setup to protect `fd00::/8` from interception by Tailscale's table 52 default route +- Bloom filter routing no longer swallows traffic when no bloom + candidate is strictly closer than the current node. `find_next_hop` + now falls through to greedy tree routing in that case instead of + returning `NoRoute`, which previously caused dropped packets in + topologies where the tree parent was closer but not a bloom + candidate +- Auto-connect peers now reconnect after a graceful `Disconnect` + notification from the remote side. `handle_disconnect` previously + removed the peer without scheduling a reconnect, orphaning the + entry on a clean upstream shutdown; the other removal paths + (link-dead, decrypt failure, peer restart) already scheduled + reconnect ([#60](https://github.com/jmcorgan/fips/issues/60), + reported by [@SwapMarket](https://github.com/SwapMarket)) +- `fipsctl connect` now rejects FIPS mesh (`fd00::/8`) addresses for + `udp`, `tcp`, and `ethernet` transports with a clear error message + instead of echoing success while the daemon silently failed the + bind with `EAFNOSUPPORT` + ([#61](https://github.com/jmcorgan/fips/issues/61), + reported by [@SwapMarket](https://github.com/SwapMarket)) ## [0.2.0] - 2026-03-22 diff --git a/src/bin/fipsctl.rs b/src/bin/fipsctl.rs index f649932..fb28146 100644 --- a/src/bin/fipsctl.rs +++ b/src/bin/fipsctl.rs @@ -12,6 +12,7 @@ use fips::upper::hosts::HostMap; use fips::version; use fips::{Identity, encode_nsec}; use std::io::{BufRead, BufReader, Write}; +use std::net::{Ipv6Addr, SocketAddrV6}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -249,6 +250,50 @@ fn default_key_dir() -> PathBuf { } } +/// Check if `address` is an IPv6 literal in `fd00::/8` (FIPS mesh ULA range). +/// +/// Handles three common syntaxes: +/// - bare IPv6: `fd9d:...` +/// - bracketed + port: `[fd9d:...]:2121` +/// - bare IPv6 + port: `fd9d:...:2121` (ambiguous; accepted if tail is numeric) +fn is_fips_mesh_address(address: &str) -> bool { + let is_ula = |a: &Ipv6Addr| a.octets()[0] == 0xfd; + + if let Ok(a) = address.parse::() { + return is_ula(&a); + } + if let Ok(sa) = address.parse::() { + return is_ula(sa.ip()); + } + if let Some((host, port)) = address.rsplit_once(':') + && port.chars().all(|c| c.is_ascii_digit()) + && !port.is_empty() + { + let host = host.trim_start_matches('[').trim_end_matches(']'); + if let Ok(a) = host.parse::() { + return is_ula(&a); + } + } + false +} + +/// Reject `fd00::/8` addresses for transports that expect a reachable network endpoint. +/// +/// FIPS mesh ULAs are derived from npubs and only make sense as destinations +/// inside an already-established mesh — they are not valid udp/tcp/ethernet +/// transport endpoints. Without this check the CLI echoes success while the +/// daemon rejects the bind with EAFNOSUPPORT (issue #61). +fn validate_connect_address(address: &str, transport: &str) -> Result<(), String> { + let checked = matches!(transport, "udp" | "tcp" | "ethernet"); + if checked && is_fips_mesh_address(address) { + return Err(format!( + "'{address}' is a FIPS mesh address (fd00::/8), not a reachable {transport} endpoint.\n\ + Provide the peer's routable IP/hostname and port (e.g., '192.0.2.1:2121' or 'peer.example.com:2121')." + )); + } + Ok(()) +} + /// Resolve a peer identifier to an npub. /// /// If the identifier starts with "npub1", it's returned as-is. @@ -328,6 +373,10 @@ fn main() { address, transport, } => { + if let Err(e) = validate_connect_address(address, transport) { + eprintln!("error: {e}"); + std::process::exit(1); + } let npub = resolve_peer(peer); build_command( "connect", @@ -353,3 +402,62 @@ fn main() { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_bare_ula_literal() { + assert!(is_fips_mesh_address("fd9d:abcd::1")); + assert!(is_fips_mesh_address("fd00::")); + assert!(is_fips_mesh_address( + "fdff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" + )); + } + + #[test] + fn detects_bracketed_ula_with_port() { + assert!(is_fips_mesh_address("[fd9d:abcd::1]:2121")); + assert!(is_fips_mesh_address("[fd00::1]:8443")); + } + + #[test] + fn detects_bare_ula_with_port() { + assert!(is_fips_mesh_address("fd9d:abcd::1:2121")); + } + + #[test] + fn rejects_non_ula_ipv6() { + // fc00::/7 other half (fcXX:) is also ULA but not fd00::/8 — we only + // block the fd-prefixed half that FIPS actually uses. + assert!(!is_fips_mesh_address("fc00::1")); + assert!(!is_fips_mesh_address("::1")); + assert!(!is_fips_mesh_address("2001:db8::1")); + assert!(!is_fips_mesh_address("[2001:db8::1]:2121")); + } + + #[test] + fn ignores_ipv4_and_hostnames() { + assert!(!is_fips_mesh_address("192.0.2.1:2121")); + assert!(!is_fips_mesh_address("peer.example.com:2121")); + assert!(!is_fips_mesh_address("coinos.pro:2121")); + } + + #[test] + fn validates_only_target_transports() { + assert!(validate_connect_address("fd9d::1:2121", "udp").is_err()); + assert!(validate_connect_address("fd9d::1:2121", "tcp").is_err()); + assert!(validate_connect_address("fd9d::1:2121", "ethernet").is_err()); + // Other transports are not inspected — they may legitimately accept + // non-IP endpoints (tor onion, etc.). + assert!(validate_connect_address("fd9d::1:2121", "tor").is_ok()); + } + + #[test] + fn allows_valid_endpoints() { + assert!(validate_connect_address("192.0.2.1:2121", "udp").is_ok()); + assert!(validate_connect_address("peer.example.com:2121", "tcp").is_ok()); + assert!(validate_connect_address("[2001:db8::1]:2121", "udp").is_ok()); + } +} diff --git a/src/node/handlers/dispatch.rs b/src/node/handlers/dispatch.rs index c441b5e..44707ce 100644 --- a/src/node/handlers/dispatch.rs +++ b/src/node/handlers/dispatch.rs @@ -67,8 +67,12 @@ impl Node { /// Handle a Disconnect notification from a peer. /// /// The peer is signaling an orderly departure. We immediately remove - /// them from all state rather than waiting for timeout detection. - fn handle_disconnect(&mut self, from: &NodeAddr, payload: &[u8]) { + /// them from all state rather than waiting for timeout detection, and + /// schedule a reconnect if the peer is configured as auto-connect. + /// Without this, a graceful upstream shutdown orphans auto-connect + /// entries — other removal paths (link-dead, decrypt failure, peer + /// restart) all schedule reconnect. + pub(in crate::node) fn handle_disconnect(&mut self, from: &NodeAddr, payload: &[u8]) { let disconnect = match crate::protocol::Disconnect::decode(payload) { Ok(msg) => msg, Err(e) => { @@ -83,7 +87,13 @@ impl Node { "Peer sent disconnect notification" ); + let addr = *from; self.remove_active_peer(from); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + self.schedule_reconnect(addr, now_ms); } /// Remove an active peer and clean up all associated state. diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 015762e..97d1ac1 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -801,6 +801,43 @@ fn test_schedule_reconnect_fresh_state() { assert_eq!(state.retry_after_ms, 1_000 + expected_delay); } +/// Test that a graceful Disconnect from an auto-connect peer schedules reconnect. +/// +/// Regression test for issue #60: `handle_disconnect` previously called +/// `remove_active_peer` without `schedule_reconnect`, orphaning auto-connect +/// entries on a clean upstream shutdown. Other peer-removal paths (link-dead, +/// decrypt failure, peer restart) all schedule reconnect. +#[test] +fn test_disconnect_schedules_reconnect() { + use crate::protocol::{Disconnect, DisconnectReason}; + + let peer_identity = Identity::generate(); + let peer_npub = peer_identity.npub(); + let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr(); + + let mut config = Config::new(); + config.peers.push(crate::config::PeerConfig::new( + peer_npub, + "udp", + "10.0.0.2:2121", + )); + + let mut node = Node::new(config).unwrap(); + + let payload = Disconnect::new(DisconnectReason::Shutdown).encode(); + node.handle_disconnect(&peer_node_addr, &payload); + + let state = node + .retry_pending + .get(&peer_node_addr) + .expect("handle_disconnect should schedule reconnect for auto-connect peer"); + assert!(state.reconnect, "Entry should be marked as reconnect"); + assert_eq!( + state.retry_count, 0, + "Fresh reconnect after disconnect should start at count=0" + ); +} + /// Test that promote_connection clears retry_pending. #[test] fn test_promote_clears_retry_pending() {