From d246f84da5ca9ced1d0812c96dabb58c2c146ba5 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 18:09:44 +0000 Subject: [PATCH] Rebuild alias name resolution and ACL alias entries when the peer list is replaced at runtime Node::update_peers replaced the configured peer list but left every map that reads peer aliases as it was at startup. Each hosts map (the display map, the peer ACL's alias resolution and the .fips DNS responder's map) kept the startup aliases as a fixed base and only re-merged the hosts file over it. After an update, a new peer's alias did not resolve as a .fips name or match an ACL entry naming it, a removed or moved alias kept resolving to the old npub, and a deny entry written as an alias that moved to another key kept denying the old key while admitting the new one. A kept peer whose alias was removed also kept showing the old alias as its display name. update_peers now rebuilds the alias base from the new peer list and hands it to all three maps: the display map directly, the peer ACL through a forced rebuild that runs before any added peer is dialed, and the running DNS responder through a watch channel it checks before each answer. Each map keeps the hosts file as last read, so the file stays merged on top and still wins, including edits picked up at runtime. The kept peer's display name falls back to its short npub when its alias is removed. run_dns_responder keeps its signature. This reaches only embedders that call Node::update_peers. --- src/control/snapshot.rs | 11 +- src/node/acl.rs | 65 ++++- src/node/lifecycle/mod.rs | 30 ++- src/node/lifecycle/supervisor.rs | 5 + src/node/mod.rs | 25 +- src/node/reloadable.rs | 90 ++++++- src/node/tests/mod.rs | 1 + src/node/tests/update_peers.rs | 397 +++++++++++++++++++++++++++++++ src/upper/dns.rs | 83 ++++++- src/upper/hosts.rs | 98 +++++++- 10 files changed, 769 insertions(+), 36 deletions(-) create mode 100644 src/node/tests/update_peers.rs diff --git a/src/control/snapshot.rs b/src/control/snapshot.rs index bea9d8d7..c7263b07 100644 --- a/src/control/snapshot.rs +++ b/src/control/snapshot.rs @@ -10,7 +10,8 @@ //! - the cheap scalar gauges `show_status` needs (`estimated_mesh_size`, //! node `state`, `tun_state`, `tun_name`, `effective_ipv6_mtu`, and the //! peer / session / link / connection / transport counts), plus -//! `peer_aliases` (effectively immutable after construction). +//! `peer_aliases` (copied from the node's display-name map on each tick; +//! it changes when `update_peers` replaces the peer list). //! //! The snapshot holds *data*, not rendered `Response` envelopes: //! rendering happens in the control task off the rx_loop. Staleness is bounded @@ -61,11 +62,13 @@ pub(crate) struct StatsSnapshot { /// transport type. Configured-but-idle types appear with a zero count. /// Keyed by the transport type name (`"udp"`, `"tcp"`, `"tor"`, ...). pub transport_peer_counts: std::collections::BTreeMap, - /// Configured peer aliases, keyed by `NodeAddr`. Effectively immutable - /// after construction; shared to avoid a per-tick map clone. + /// Configured peer aliases, keyed by `NodeAddr`. Copied from the node's + /// display-name map on each stats tick; it changes when `update_peers` + /// replaces the peer list. pub peer_aliases: Arc>, /// Loaded peer-ACL status (`show_acl`). The ACL itself is an - /// `arc_swap::ArcSwap` mutated only by the tick's `reload_peer_acl`; + /// `arc_swap::ArcSwap` mutated only by the tick's `reload_peer_acl` + /// and by `update_peers`; /// the human-readable status is a cheap projection of it. pub acl_status: PeerAclStatus, /// Per-stats-history-peer metadata resolved against the live peer/session diff --git a/src/node/acl.rs b/src/node/acl.rs index d079cf3a..3ba32472 100644 --- a/src/node/acl.rs +++ b/src/node/acl.rs @@ -529,7 +529,8 @@ impl PeerAcl { /// [`arc_swap::ArcSwap`] so the authorization hot path reads it without /// locking, while the reloader's change-detection state (file mtimes, the /// embedded hosts reloader) is touched only by [`Reloadable::reload`] on the -/// single node tick task. +/// node tick task and by [`PeerAclReloader::rebase`], which `update_peers` +/// reaches through `&mut Node`, so there is still one writer at a time. pub struct PeerAclReloader { /// Reader-facing effective ACL snapshot. acl: arc_swap::ArcSwap, @@ -551,6 +552,10 @@ pub struct PeerAclReloader { retry_pending: bool, /// Consecutive reloads held back by the empty-snapshot guard. empty_holds: u32, + /// Set when the alias base changed, forcing the next reload to rebuild + /// although no file changed. Cleared only when a rebuilt ACL is + /// published, so a held reload retries the rebuild on later ticks. + rebased: bool, } impl PeerAclReloader { @@ -646,9 +651,22 @@ impl PeerAclReloader { last_deny_mtime, retry_pending, empty_holds: 0, + rebased: false, } } + /// Replace the peer-alias base the ACL's alias entries resolve through, + /// and rebuild the ACL from it now. + /// + /// Goes through [`Reloadable::reload`], so an unreadable input holds the + /// last good ACL and the empty-ACL guard applies exactly as on a tick. + /// Returns `true` if a rebuilt ACL was published. + pub(crate) async fn rebase(&mut self, base: HostMap) -> bool { + self.hosts.set_base(base); + self.rebased = true; + self.reload().await + } + /// Keep the published snapshot after a reload input failed to read. /// /// Leaves the recorded mtimes and the ACL in force untouched, arms the @@ -714,6 +732,7 @@ impl Reloadable for PeerAclReloader { && deny_mtime == self.last_deny_mtime && !hosts_changed && !self.retry_pending + && !self.rebased && !allow_moved && !deny_moved { @@ -764,6 +783,7 @@ impl Reloadable for PeerAclReloader { ); } self.retry_pending = false; + self.rebased = false; self.empty_holds = 0; self.last_allow_mtime = allow_mtime; self.last_deny_mtime = deny_mtime; @@ -1800,4 +1820,47 @@ mod tests { PeerAclDecision::DefaultAllow ); } + + /// Rebasing the alias map rebuilds and publishes the ACL although no ACL + /// or hosts file changed, and the forced rebuild does not repeat on the + /// next reload. + #[tokio::test] + async fn rebase_republishes_alias_entries_without_any_file_change() { + let dir = tempfile::tempdir().unwrap(); + let allow = dir.path().join("peers.allow"); + let deny = dir.path().join("peers.deny"); + let hosts = dir.path().join("hosts"); + let (x, y) = (test_npub(), test_npub()); + write_file(&allow, "node-a\n"); + + let mut base = HostMap::new(); + base.insert("node-a", &x).unwrap(); + let mut reloader = PeerAclReloader::with_alias_sources(allow, deny, base, hosts); + assert_eq!( + reloader.acl().check(&test_peer(&x)), + PeerAclDecision::AllowList, + "alias resolves to X at startup" + ); + + let mut moved = HostMap::new(); + moved.insert("node-a", &y).unwrap(); + assert!( + reloader.rebase(moved).await, + "rebase publishes a rebuilt ACL" + ); + assert_eq!( + reloader.acl().check(&test_peer(&y)), + PeerAclDecision::AllowList, + "alias entry follows the new base to Y" + ); + assert_eq!( + reloader.acl().check(&test_peer(&x)), + PeerAclDecision::DefaultAllow, + "X is no longer on the allow list" + ); + assert!( + !reloader.reload().await, + "a reload with nothing changed does not rebuild again" + ); + } } diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index 6d16427f..cca08fd2 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -89,6 +89,11 @@ impl Node { /// is already connected and a new concrete candidate appears, FIPS starts /// an alternate handshake in parallel; promotion switches only after that /// handshake authenticates. + /// + /// Peer aliases follow the new list: `.fips` names, display names and + /// peer ACL entries written as an alias are rebuilt from it, with the + /// hosts file still taking precedence. Rebuilding the ACL re-reads the + /// ACL files and checks the hosts file before any added peer is dialed. pub async fn update_peers( &mut self, new_peers: Vec, @@ -164,8 +169,12 @@ impl Node { state.peer_config = new_peer.clone(); state.retry_after_ms = Self::now_ms(); } - if let Some(alias) = new_peer.alias.clone() { - self.peer_aliases.insert(*node_addr, alias); + if let Ok(identity) = PeerIdentity::from_npub(&new_peer.npub) { + let name = new_peer + .alias + .clone() + .unwrap_or_else(|| identity.short_npub()); + self.peer_aliases.insert(*node_addr, name); } } else { outcome.unchanged += 1; @@ -185,6 +194,7 @@ impl Node { let mut new_config = (*self.context.config).clone(); new_config.peers = new_by_addr.into_values().collect(); self.replace_context(|ctx| ctx.config = std::sync::Arc::new(new_config)); + self.rebase_aliases().await; for peer_config in added_configs { outcome.added += 1; @@ -1843,6 +1853,8 @@ impl Node { let hosts_path = std::path::PathBuf::from( crate::upper::hosts::DEFAULT_HOSTS_PATH, ); + let (aliases_tx, aliases_rx) = + tokio::sync::watch::channel(base_hosts.clone()); let reloader = crate::upper::hosts::HostMapReloader::new( base_hosts, hosts_path, ); @@ -1880,17 +1892,19 @@ impl Node { let dns_child_tx = self.child_exit_tx.clone(); let handle = tokio::spawn(report_exit( Child::Dns, - crate::upper::dns::run_dns_responder( + crate::upper::dns::run_responder( socket, identity_tx, dns_ttl, reloader, + Some(aliases_rx), mesh_ifindex, ), dns_child_tx, )); self.supervisor.dns_identity_rx = Some(identity_rx); self.supervisor.dns_task = Some(handle); + self.supervisor.dns_aliases = Some(aliases_tx); self.supervisor.dns_local_addr = Some(local_addr); Event::SubstrateUp { child } } @@ -2161,6 +2175,7 @@ impl Node { handle.abort(); debug!("DNS responder stopped"); } + self.supervisor.dns_aliases.take(); // Retract the published address in the same step that kills // the listener, so an embedder polling `dns_local_addr()` // never dials a socket that is already gone. @@ -2276,10 +2291,11 @@ impl Node { /// reads it to rebuild the teardown set, and aborting an already-finished /// handle there is harmless. /// - /// For `Dns` the event comes only from a panic. `run_dns_responder` is an - /// unconditional loop whose every failure arm continues, so it has no - /// ordinary exit; [`report_exit`] catches a panic in it and reports - /// `Child::Dns`, which is what reaches this. + /// For `Dns` the event comes only from a panic. `run_responder`, the loop + /// the node spawns and `run_dns_responder` wraps, is an unconditional + /// loop whose every failure arm continues, so it has no ordinary exit; + /// [`report_exit`] catches a panic in it and reports `Child::Dns`, which + /// is what reaches this. pub(in crate::node) fn retract_child_publications(&mut self, child: Child) { if matches!(child, Child::Dns) { self.supervisor.dns_local_addr.take(); diff --git a/src/node/lifecycle/supervisor.rs b/src/node/lifecycle/supervisor.rs index af5197da..e62a399b 100644 --- a/src/node/lifecycle/supervisor.rs +++ b/src/node/lifecycle/supervisor.rs @@ -697,6 +697,10 @@ pub(crate) struct Supervisor { /// only while the responder is up; published to embedders through /// [`Node::dns_local_addr`](crate::Node::dns_local_addr). pub(in crate::node) dns_local_addr: Option, + /// Sends a new peer-alias base to the running DNS responder; `Some` only + /// while it runs. + pub(in crate::node) dns_aliases: + Option>, /// Sender for each UDP listen socket the transport spawn binds — its raw /// fd and the instance name it was configured under — armed by @@ -751,6 +755,7 @@ impl Supervisor { dns_identity_rx: None, dns_task: None, dns_local_addr: None, + dns_aliases: None, #[cfg(unix)] udp_fd_tx: None, nostr_rendezvous: crate::nostr::RendezvousDriver::default(), diff --git a/src/node/mod.rs b/src/node/mod.rs index 8debb4e6..588c7c16 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -637,7 +637,7 @@ pub struct Node { // === Display Names === /// Human-readable names for configured peers (alias or short npub). - /// Populated at startup from peer config. + /// Populated at startup from peer config and updated by `update_peers`. peer_aliases: HashMap, /// Reloadable peer ACL state from standard allow/deny files. @@ -645,8 +645,9 @@ pub struct Node { // === Host Map === /// Static hostname → npub mapping for DNS resolution. - /// Built at construction from peer aliases and /etc/fips/hosts, and - /// published through a lock-free snapshot for the display path. + /// Built at construction from peer aliases and /etc/fips/hosts, with the + /// peer aliases replaced by `update_peers`, and published through a + /// lock-free snapshot for the display path. host_map: reloadable::HostMapReloadable, /// Sessions whose recv cipher + replay window have been handed @@ -1386,11 +1387,27 @@ impl Node { self.host_map.reload().await } + /// Rebuild every peer-alias map from the current peer list. + /// + /// Replaces the alias base under the display host map, the peer ACL's + /// alias resolution (rebuilding the ACL now) and the running DNS + /// responder's map. The hosts file stays merged over each and still wins. + pub(crate) async fn rebase_aliases(&mut self) { + let base = HostMap::from_peer_configs(self.config().peers()); + tracing::debug!(entries = base.len(), "Rebuilding peer alias maps"); + self.host_map.set_base(base.clone()); + self.peer_acl.rebase(base.clone()).await; + if let Some(tx) = &self.supervisor.dns_aliases { + tx.send_replace(base); + } + } + /// Return a human-readable display name for a NodeAddr. /// /// Lookup order: /// 1. Host map hostname (from peer aliases + /etc/fips/hosts) - /// 2. Configured peer alias or short npub (from startup map) + /// 2. Configured peer alias or short npub (from startup map, updated by + /// `update_peers`) /// 3. Active peer's short npub (e.g., inbound peer not in config) /// 4. Session endpoint's short npub (end-to-end, may not be direct peer) /// 5. Truncated NodeAddr hex (unknown address) diff --git a/src/node/reloadable.rs b/src/node/reloadable.rs index 3fc58b2c..c05c4e0e 100644 --- a/src/node/reloadable.rs +++ b/src/node/reloadable.rs @@ -8,17 +8,19 @@ //! //! # Canonical Arc-wrapper template //! -//! These resources follow a single-writer / many-reader pattern: the node -//! tick task is the only writer, while the hot path reads the current value -//! frequently and must never block. +//! These resources follow a single-writer / many-reader pattern: every write +//! goes through `&mut Node`, from the node tick task or from +//! `Node::update_peers`, so there is one writer at a time, while the hot path +//! reads the current value frequently and must never block. //! //! - The reader-facing immutable snapshot lives in an //! [`arc_swap::ArcSwap`]. Readers call [`Reloadable::load`], which yields //! a lock-free [`arc_swap::Guard>`] that derefs straight to the //! snapshot — no mutex, no clone on the read path. //! - The owning struct also holds the change-detection state (file mtime, -//! immutable base data, source path). That state is touched only by -//! [`Reloadable::reload`], which runs on the single writer task. +//! base data, source path). That state is touched only by +//! [`Reloadable::reload`] and, for the host map's peer-alias base, by +//! `HostMapReloadable::set_base`, both reached only through `&mut Node`. //! - `reload` builds a brand-new `T` and then stores `Arc::new(new)` into the //! `ArcSwap`, so a reader either sees the entire old snapshot or the entire //! new one — never a partial update. @@ -92,16 +94,20 @@ pub trait Reloadable: Send { /// Reloadable hostname → npub map (base peer aliases merged with the operator /// hosts file). /// -/// Holds the immutable base map (from peer-config aliases) plus the -/// change-detection state for the hosts file. The effective map (base merged +/// Holds the base map (from peer-config aliases, replaced when the peer list +/// is replaced at runtime) plus the change-detection state for the hosts +/// file. The effective map (base merged /// with the hosts file) is published through an [`arc_swap::ArcSwap`] so the /// display path can read it without locking. pub struct HostMapReloadable { /// Reader-facing effective snapshot (base merged with hosts file). snapshot: arc_swap::ArcSwap, - /// Base map from peer-config aliases (never changes). Read only by - /// `reload` on the tick task. + /// Base map from peer-config aliases. Read by `reload` and replaced by + /// `set_base`, both reached only through `&mut Node`. base: HostMap, + /// The hosts file as last read, kept so a new base can be merged under + /// it without reading the file again. Written by `new` and `reload`. + file: HostMap, /// Path to the operator hosts file. Read only by `reload`. path: std::path::PathBuf, /// Last observed modification time of the hosts file (`None` if absent). @@ -118,15 +124,29 @@ impl HostMapReloadable { let last_mtime = file_mtime(&path); let hosts_file = HostMap::load_hosts_file(&path); let mut effective = base.clone(); - effective.merge(hosts_file); + effective.merge(hosts_file.clone()); Self { snapshot: arc_swap::ArcSwap::from(Arc::new(effective)), base, + file: hosts_file, path, last_mtime, } } + + /// Replace the peer-alias base and publish it merged with the hosts file + /// as last read, which still wins on conflicts. + /// + /// Reads no file and leaves the recorded mtime alone. Like `reload`, it is + /// reached only through `&mut Node`, which keeps one writer at a time; + /// readers see either the whole old snapshot or the whole new one. + pub(crate) fn set_base(&mut self, base: HostMap) { + let mut effective = base.clone(); + effective.merge(self.file.clone()); + self.base = base; + self.snapshot.store(Arc::new(effective)); + } } impl Reloadable for HostMapReloadable { @@ -143,7 +163,8 @@ impl Reloadable for HostMapReloadable { self.last_mtime = current_mtime; let hosts_file = HostMap::load_hosts_file(&self.path); let mut new_effective = self.base.clone(); - new_effective.merge(hosts_file); + new_effective.merge(hosts_file.clone()); + self.file = hosts_file; let count = new_effective.len(); self.snapshot.store(Arc::new(new_effective)); @@ -329,4 +350,51 @@ mod tests { assert_eq!(snapshot.lookup_npub(key), expected.lookup_npub(key)); } } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// Replacing the base swaps the peer aliases while the hosts file, as it + /// was last re-read at runtime, stays merged on top and still wins. + #[tokio::test] + async fn set_base_replaces_peer_aliases_and_keeps_the_last_reloaded_hosts_file_on_top() { + let [x, y, z, v, w] = std::array::from_fn(|_| Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + std::fs::write(&path, format!("f {}\na {}\n", y.npub(), z.npub())).unwrap(); + + let mut reloadable = HostMapReloadable::new(one_entry("a", &x), path.clone()); + let npub = |r: &HostMapReloadable, name: &str| r.load().lookup_npub(name).map(String::from); + assert_eq!(npub(&reloadable, "f"), Some(y.npub()), "startup file entry"); + + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("g {}\na {}\n", v.npub(), z.npub())).unwrap(); + assert!(reloadable.reload().await, "reload sees the rewrite"); + reloadable.set_base(one_entry("b", &w)); + + assert_eq!(npub(&reloadable, "b"), Some(w.npub()), "new base alias"); + assert_eq!( + npub(&reloadable, "g"), + Some(v.npub()), + "file entry added at runtime survives set_base" + ); + assert_eq!(npub(&reloadable, "a"), Some(z.npub()), "file still wins"); + assert_eq!( + npub(&reloadable, "f"), + None, + "file entry removed at runtime stays removed" + ); + let x_addr = *crate::PeerIdentity::from_npub(&x.npub()) + .unwrap() + .node_addr(); + assert_eq!( + reloadable.load().lookup_hostname(&x_addr), + None, + "old base npub no longer reverse-resolves" + ); + } } diff --git a/src/node/tests/mod.rs b/src/node/tests/mod.rs index 9b793f78..b31c75aa 100644 --- a/src/node/tests/mod.rs +++ b/src/node/tests/mod.rs @@ -27,6 +27,7 @@ mod session; mod spanning_tree; mod tcp; mod unit; +mod update_peers; pub(super) fn make_node() -> Node { make_node_with(Config::new()) diff --git a/src/node/tests/update_peers.rs b/src/node/tests/update_peers.rs new file mode 100644 index 00000000..7a195a29 --- /dev/null +++ b/src/node/tests/update_peers.rs @@ -0,0 +1,397 @@ +//! Replacing the peer list at runtime must carry peer aliases into every map +//! that reads them: the display host map, the peer ACL's alias entries and +//! the running `.fips` DNS responder. + +use super::*; +use crate::config::{ConnectPolicy, PeerAddress, PeerConfig}; +use crate::node::acl::{PeerAclDecision, PeerAclReloader}; +use crate::node::reloadable::HostMapReloadable; +use crate::upper::hosts::HostMap; +use std::net::Ipv6Addr; + +/// A suffix taken from a fresh npub's data part, so alias names cannot +/// collide with a hosts file on the build host. +fn suffix() -> String { + Identity::generate().npub()[5..17].to_string() +} + +/// A peer entry with an explicit address and connect policy. +fn peer_at(id: &Identity, alias: Option<&str>, addr: &str, policy: ConnectPolicy) -> PeerConfig { + PeerConfig { + npub: id.npub(), + alias: alias.map(String::from), + addresses: vec![PeerAddress::new("udp", addr)], + connect_policy: policy, + auto_reconnect: false, + via_nostr: false, + } +} + +/// An on-demand peer entry on the placeholder address, which nothing dials. +fn peer(id: &Identity, alias: Option<&str>) -> PeerConfig { + peer_at(id, alias, "127.0.0.1:9", ConnectPolicy::OnDemand) +} + +/// The identity form of a test identity, as the ACL and display paths see it. +fn ident(id: &Identity) -> PeerIdentity { + PeerIdentity::from_npub(&id.npub()).unwrap() +} + +/// The node address of a test identity. +fn addr(id: &Identity) -> NodeAddr { + *ident(id).node_addr() +} + +/// Build a node from `peers`, with its host map and peer ACL rebuilt exactly +/// as construction builds them but on temp paths, and its display-name map +/// seeded as `start()` seeds it (alias or else short npub). +/// +/// `allow`, `deny` and `hosts` are file contents; `None` leaves the file +/// absent. The returned directory must outlive the node. +fn alias_node( + peers: Vec, + allow: Option<&str>, + deny: Option<&str>, + hosts: Option<&str>, +) -> (tempfile::TempDir, Node) { + let dir = tempfile::tempdir().unwrap(); + let allow_path = dir.path().join("peers.allow"); + let deny_path = dir.path().join("peers.deny"); + let hosts_path = dir.path().join("hosts"); + for (path, contents) in [ + (&allow_path, allow), + (&deny_path, deny), + (&hosts_path, hosts), + ] { + if let Some(contents) = contents { + std::fs::write(path, contents).unwrap(); + } + } + + let mut config = Config::new(); + config.peers = peers; + let mut node = make_node_with(config); + let base = || HostMap::from_peer_configs(node.config().peers()); + let host_map = HostMapReloadable::new(base(), hosts_path.clone()); + let peer_acl = PeerAclReloader::with_alias_sources(allow_path, deny_path, base(), hosts_path); + node.host_map = host_map; + node.peer_acl = peer_acl; + + let seeded: Vec<_> = node + .config() + .peers() + .iter() + .map(|pc| { + let id = PeerIdentity::from_npub(&pc.npub).unwrap(); + let name = pc.alias.clone().unwrap_or_else(|| id.short_npub()); + (*id.node_addr(), name) + }) + .collect(); + node.peer_aliases.extend(seeded); + (dir, node) +} + +/// The npub a name resolves to in the node's display host map. +fn resolved(node: &Node, name: &str) -> Option { + node.host_map.load().lookup_npub(name).map(String::from) +} + +/// The ACL decision the node's authorization path would make for `id`. +fn decision(node: &Node, id: &Identity) -> PeerAclDecision { + node.peer_acl.acl().check(&ident(id)) +} + +/// Aliases renamed, removed and moved by `update_peers` are reflected in the +/// display host map, in display names and in the stats snapshot, with the +/// hosts file still overlaid and winning as at startup. +#[tokio::test] +async fn update_peers_rebuilds_host_map_display_names_and_the_stats_snapshot_from_the_new_aliases() +{ + let s = suffix(); + let [a, b, c, d, e, f, g] = std::array::from_fn(|_| Identity::generate()); + let (alpha, alpha2, bravo) = ( + format!("alpha-{s}"), + format!("alpha2-{s}"), + format!("bravo-{s}"), + ); + let (charlie, delta, echo) = ( + format!("charlie-{s}"), + format!("delta-{s}"), + format!("echo-{s}"), + ); + let hosts = format!("{echo} {}\n{charlie} {}\n", f.npub(), g.npub()); + let (_dir, mut node) = alias_node( + vec![ + peer(&a, Some(&alpha)), + peer(&b, Some(&bravo)), + peer(&d, Some(&delta)), + ], + None, + None, + Some(&hosts), + ); + + assert_eq!(resolved(&node, &alpha), Some(a.npub()), "before: alpha"); + assert_eq!(resolved(&node, &bravo), Some(b.npub()), "before: bravo"); + assert_eq!( + resolved(&node, &echo), + Some(f.npub()), + "before: echo from file" + ); + assert_eq!(node.peer_display_name(&addr(&a)), alpha, "before: A's name"); + assert_eq!( + node.peer_aliases.get(&addr(&d)), + Some(&delta), + "before: D's seeded display entry" + ); + assert_eq!(node.peer_display_name(&addr(&d)), delta, "before: D's name"); + + node.update_peers(vec![ + peer(&a, Some(&alpha2)), + peer(&c, Some(&charlie)), + peer(&d, None), + peer(&e, Some(&delta)), + ]) + .await + .unwrap(); + + assert_eq!( + resolved(&node, &alpha2), + Some(a.npub()), + "renamed alias resolves" + ); + assert_eq!(resolved(&node, &alpha), None, "old name of a renamed alias"); + assert_eq!(resolved(&node, &bravo), None, "alias of a removed peer"); + assert_eq!(resolved(&node, &delta), Some(e.npub()), "moved alias"); + assert_eq!(resolved(&node, &echo), Some(f.npub()), "file entry kept"); + assert_eq!( + resolved(&node, &charlie), + Some(g.npub()), + "file entry still wins over a peer alias" + ); + + let d_short = ident(&d).short_npub(); + assert_eq!(node.peer_display_name(&addr(&a)), alpha2, "renamed display"); + assert_eq!(node.peer_display_name(&addr(&e)), delta, "moved display"); + assert_eq!( + node.peer_display_name(&addr(&d)), + d_short, + "a peer whose alias was removed shows its short npub" + ); + + node.record_stats_history(); + let snapshot = node.stats_snapshot.load(); + assert_eq!( + snapshot.peer_aliases.get(&addr(&d)), + Some(&d_short), + "snapshot: removed alias" + ); + assert_eq!( + snapshot.peer_aliases.get(&addr(&a)), + Some(&alpha2), + "snapshot: renamed alias" + ); +} + +/// A deny entry written as an alias follows the alias to its new npub: the +/// new key is refused and the old one is no longer denied. +#[tokio::test] +async fn update_peers_moves_a_deny_entry_written_as_an_alias_onto_the_new_npub() { + let blocked = format!("blocked-{}", suffix()); + let [x, y] = std::array::from_fn(|_| Identity::generate()); + let (_dir, mut node) = alias_node( + vec![peer(&x, Some(&blocked))], + None, + Some(&format!("{blocked}\n")), + None, + ); + assert_eq!(decision(&node, &x), PeerAclDecision::DenyList, "before: X"); + assert_eq!( + decision(&node, &y), + PeerAclDecision::DefaultAllow, + "before: Y" + ); + + node.update_peers(vec![peer(&x, None), peer(&y, Some(&blocked))]) + .await + .unwrap(); + + assert_eq!(decision(&node, &y), PeerAclDecision::DenyList, "after: Y"); + assert_eq!( + decision(&node, &x), + PeerAclDecision::DefaultAllow, + "after: X" + ); +} + +/// Under deny `ALL`, an allow entry written as an alias follows the alias: +/// the new key is admitted and the old key falls to the deny. +#[tokio::test] +async fn update_peers_moves_an_allow_entry_written_as_an_alias_under_deny_all() { + let trusted = format!("trusted-{}", suffix()); + let [a, b] = std::array::from_fn(|_| Identity::generate()); + let (_dir, mut node) = alias_node( + vec![peer(&a, Some(&trusted))], + Some(&format!("{trusted}\n")), + Some("ALL\n"), + None, + ); + assert_eq!(decision(&node, &a), PeerAclDecision::AllowList, "before: A"); + assert_eq!(decision(&node, &b), PeerAclDecision::DenyList, "before: B"); + + node.update_peers(vec![peer(&a, None), peer(&b, Some(&trusted))]) + .await + .unwrap(); + + assert_eq!(decision(&node, &b), PeerAclDecision::AllowList, "after: B"); + assert_eq!(decision(&node, &a), PeerAclDecision::DenyList, "after: A"); +} + +/// The rebuilt ACL is in force before `update_peers` dials an added peer: a +/// deny entry moved onto the added peer Y stops its dial, while a control +/// peer Z added in the same call is dialed. +#[tokio::test] +async fn update_peers_applies_the_rebuilt_acl_before_dialing_an_added_peer() { + let blocked = format!("blocked-{}", suffix()); + let [x, y, z] = std::array::from_fn(|_| Identity::generate()); + let x_at = |alias| peer_at(&x, alias, "127.0.0.1:29", ConnectPolicy::OnDemand); + let (_dir, mut node) = alias_node( + vec![x_at(Some(&blocked))], + None, + Some(&format!("{blocked}\n")), + None, + ); + let (packet_tx, packet_rx) = packet_channel(64); + node.supervisor.packet_tx = Some(packet_tx.clone()); + node.packet_rx = Some(packet_rx); + let transport_id = TransportId::new(1); + let mut udp = UdpTransport::new( + transport_id, + Some("main".to_string()), + crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }, + packet_tx, + ); + udp.start_async().await.unwrap(); + node.transports + .insert(transport_id, TransportHandle::Udp(udp)); + + node.update_peers(vec![ + x_at(None), + peer_at( + &y, + Some(&blocked), + "127.0.0.1:9", + ConnectPolicy::AutoConnect, + ), + peer_at(&z, None, "127.0.0.1:19", ConnectPolicy::AutoConnect), + ]) + .await + .unwrap(); + + let dialed: Vec<_> = node + .connections() + .filter_map(|(_, machine)| machine.conn_expected_identity().copied()) + .map(|id| *id.node_addr()) + .collect(); + assert_eq!( + dialed.len(), + 1, + "only the control peer is dialed; Y's dial must meet the moved deny entry" + ); + assert_eq!(dialed[0], addr(&z), "the one dial is the control peer Z"); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +/// Send an AAAA query for `name` to the responder at `dns` and return the +/// address answered, or `None` for a reply without an answer. Fails if no +/// reply arrives within two seconds. +async fn query_aaaa(dns: std::net::SocketAddr, name: &str) -> Option { + use simple_dns::{CLASS, Name, Packet, QCLASS, QTYPE, Question, TYPE}; + let mut packet = Packet::new_query(0x4242); + packet.questions.push(Question::new( + Name::new_unchecked(name).into_owned(), + QTYPE::TYPE(TYPE::AAAA), + QCLASS::CLASS(CLASS::IN), + false, + )); + let query = packet.build_bytes_vec().unwrap(); + let client = tokio::net::UdpSocket::bind("[::1]:0").await.unwrap(); + client.send_to(&query, dns).await.unwrap(); + + let mut buf = [0u8; 512]; + let (len, _) = tokio::time::timeout(Duration::from_secs(2), client.recv_from(&mut buf)) + .await + .unwrap_or_else(|_| panic!("no DNS reply for {name} within the timeout")) + .unwrap(); + let reply = Packet::parse(&buf[..len]).expect("well-formed DNS response"); + reply.answers.first().map(|answer| match &answer.rdata { + simple_dns::rdata::RData::AAAA(aaaa) => Ipv6Addr::from(aaaa.address), + other => panic!("expected an AAAA record for {name}, got {other:?}"), + }) +} + +/// A running DNS responder answers `.fips` alias names from the new peer +/// list after `update_peers`, through the node's real DNS start path. +#[tokio::test] +async fn update_peers_republishes_aliases_to_the_running_dns_responder() { + let s = suffix(); + let (alpha, charlie, delta) = ( + format!("alpha-{s}"), + format!("charlie-{s}"), + format!("delta-{s}"), + ); + let [a, b, c, d] = std::array::from_fn(|_| Identity::generate()); + let mut config = Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = true; + config.dns.bind_addr = Some("::1".to_string()); + config.dns.port = Some(0); + config.peers = vec![peer(&a, Some(&alpha)), peer(&d, Some(&delta))]; + let mut node = make_node_with(config); + node.start().await.unwrap(); + let dns = node.dns_local_addr().expect("responder is up"); + let fips = |name: &str| format!("{name}.fips"); + let v6 = |id: &Identity| id.address().to_ipv6(); + + assert_eq!( + query_aaaa(dns, &fips(&alpha)).await, + Some(v6(&a)), + "before: alpha" + ); + assert_eq!( + query_aaaa(dns, &fips(&charlie)).await, + None, + "before: charlie" + ); + + node.update_peers(vec![peer(&b, Some(&alpha)), peer(&c, Some(&charlie))]) + .await + .unwrap(); + + assert_eq!( + query_aaaa(dns, &fips(&alpha)).await, + Some(v6(&b)), + "after: alpha moved to B" + ); + assert_eq!( + query_aaaa(dns, &fips(&charlie)).await, + Some(v6(&c)), + "after: charlie added" + ); + assert_eq!( + query_aaaa(dns, &fips(&delta)).await, + None, + "after: delta removed" + ); + + node.stop().await.unwrap(); +} diff --git a/src/upper/dns.rs b/src/upper/dns.rs index 2fca5e70..69500c21 100644 --- a/src/upper/dns.rs +++ b/src/upper/dns.rs @@ -167,10 +167,27 @@ fn is_mesh_interface_query(arrival_ifindex: Option, mesh_ifindex: Option, +) { + run_responder(socket, identity_tx, ttl, reloader, None, mesh_ifindex).await +} + +/// Run the DNS responder UDP server loop, taking peer-alias base updates. +/// +/// Behaves as [`run_dns_responder`], and in addition, when `aliases` is +/// `Some`, applies the newest peer-alias base sent on it before answering +/// each query, so aliases follow the node's peer list when it is replaced +/// at runtime. The hosts file stays merged over the new base and still wins. +pub(crate) async fn run_responder( socket: tokio::net::UdpSocket, identity_tx: DnsIdentityTx, ttl: u32, mut reloader: HostMapReloader, + mut aliases: Option>, mesh_ifindex: Option, ) { let mut buf = [0u8; 512]; // Standard DNS UDP max @@ -195,8 +212,9 @@ pub async fn run_dns_responder( let query_bytes = &buf[..len]; - // Check for hosts file changes on each request (cheap stat call) - reloader.check_reload(); + // Apply any new peer-alias base, then check for hosts file changes + // (cheap stat call). + refresh_hosts(&mut reloader, aliases.as_mut()); match handle_dns_packet(query_bytes, ttl, reloader.hosts()) { Some((response_bytes, identity)) => { @@ -219,6 +237,29 @@ pub async fn run_dns_responder( } } +/// Bring the responder's host map up to date before answering a query. +/// +/// Applies the newest peer-alias base from `aliases` if it has not been seen +/// yet, then re-reads the hosts file if its mtime changed. The change test is +/// made on the borrowed value rather than the receiver, so a value sent just +/// before the sender closed is still applied. +fn refresh_hosts( + reloader: &mut HostMapReloader, + aliases: Option<&mut tokio::sync::watch::Receiver>, +) { + if let Some(rx) = aliases { + // Take the value out so the watch lock is released before the merge. + let next = { + let seen = rx.borrow_and_update(); + seen.has_changed().then(|| seen.clone()) + }; + if let Some(base) = next { + reloader.set_base(base); + } + } + reloader.check_reload(); +} + /// Receive a UDP datagram with arrival-interface info via `IPV6_PKTINFO`. /// /// Returns `(len, src, arrival_ifindex)`. The ifindex is `Some` when the @@ -940,4 +981,42 @@ mod tests { packet.questions.push(question); packet.build_bytes_vec().unwrap() } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// A base sent on the alias channel is applied before the next answer, + /// and is kept once the sender is gone. + #[test] + fn refresh_hosts_applies_the_latest_base_before_answering() { + let (x, y) = (Identity::generate(), Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("absent-hosts"); + let npub = |r: &HostMapReloader| r.hosts().lookup_npub("a").map(String::from); + + let mut reloader = HostMapReloader::new(one_entry("a", &x), path.clone()); + let (tx, mut rx) = tokio::sync::watch::channel(one_entry("a", &x)); + tx.send_replace(one_entry("a", &y)); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!(npub(&reloader), Some(y.npub()), "new base applied"); + drop(tx); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!(npub(&reloader), Some(y.npub()), "base kept after close"); + + // A value sent just before the sender closed is still applied. + let mut reloader = HostMapReloader::new(one_entry("a", &x), path); + let (tx, mut rx) = tokio::sync::watch::channel(one_entry("a", &x)); + tx.send_replace(one_entry("a", &y)); + drop(tx); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!( + npub(&reloader), + Some(y.npub()), + "pending base applied although the sender is closed" + ); + } } diff --git a/src/upper/hosts.rs b/src/upper/hosts.rs index f9e9f3ff..c41034ba 100644 --- a/src/upper/hosts.rs +++ b/src/upper/hosts.rs @@ -222,12 +222,17 @@ pub fn file_mtime(path: &Path) -> Option { /// Tracks a hosts file and reloads it when the modification time changes. /// -/// Holds the base host map (from peer config aliases) and the current -/// effective map (base + hosts file). On each `check_reload()`, stats the -/// hosts file and rebuilds the effective map if the mtime has changed. +/// Holds the base host map (from peer config aliases), the hosts file as last +/// read, and the current effective map (base + hosts file). On each +/// `check_reload()`, stats the hosts file and rebuilds the effective map if +/// the mtime has changed. The base is replaced when the node's peer list is +/// replaced at runtime. pub struct HostMapReloader { - /// Base map from peer config aliases (never changes). + /// Base map from peer config aliases, replaced by `set_base`. base: HostMap, + /// The hosts file as last applied, kept so a new base can be merged under + /// it without reading the file again. + file: HostMap, /// Current effective map (base merged with hosts file). effective: HostMap, /// Path to the hosts file. @@ -252,10 +257,11 @@ impl HostMapReloader { } }; let mut effective = base.clone(); - effective.merge(hosts_file); + effective.merge(hosts_file.clone()); Self { base, + file: hosts_file, effective, path, last_mtime, @@ -307,11 +313,24 @@ impl HostMapReloader { Ok(true) } + /// Replace the peer-alias base and rebuild the effective map over the + /// hosts file as last read, which still wins on conflicts. + /// + /// Reads no file and leaves the recorded mtime alone, so a hosts-file + /// change is still picked up by the next reload check. + pub(crate) fn set_base(&mut self, base: HostMap) { + let mut effective = base.clone(); + effective.merge(self.file.clone()); + self.base = base; + self.effective = effective; + } + /// Replace the effective map with the base merged with a freshly read - /// hosts file. + /// hosts file, and keep that file as the one last applied. fn apply(&mut self, hosts_file: HostMap) { let mut new_effective = self.base.clone(); - new_effective.merge(hosts_file); + new_effective.merge(hosts_file.clone()); + self.file = hosts_file; let count = new_effective.len(); self.effective = new_effective; @@ -763,4 +782,69 @@ mod tests { assert!(reloader.hosts().lookup_npub("core").is_some()); assert!(reloader.hosts().lookup_npub("gateway").is_none()); } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// Replacing the base swaps the peer aliases while the hosts file, as it + /// was last re-read at runtime through either reload path, stays merged on + /// top and still wins on conflicts. + #[test] + fn set_base_replaces_peer_aliases_and_keeps_the_last_reloaded_hosts_file_on_top() { + let [x, y, z, v, w, u, t] = std::array::from_fn(|_| Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + std::fs::write(&path, format!("f {}\na {}\n", y.npub(), z.npub())).unwrap(); + + let mut reloader = HostMapReloader::new(one_entry("a", &x), path.clone()); + let npub = |r: &HostMapReloader, name: &str| r.hosts().lookup_npub(name).map(String::from); + assert_eq!(npub(&reloader, "f"), Some(y.npub()), "startup file entry"); + + // Step 1: the file changes at runtime and is re-read by check_reload. + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("g {}\na {}\n", v.npub(), z.npub())).unwrap(); + assert!(reloader.check_reload(), "check_reload sees the rewrite"); + reloader.set_base(one_entry("b", &w)); + assert_eq!(npub(&reloader, "b"), Some(w.npub()), "new base alias"); + assert_eq!( + npub(&reloader, "g"), + Some(v.npub()), + "file entry added at runtime survives set_base" + ); + assert_eq!(npub(&reloader, "a"), Some(z.npub()), "file still wins"); + assert_eq!( + npub(&reloader, "f"), + None, + "file entry removed at runtime stays removed" + ); + let x_addr = *PeerIdentity::from_npub(&x.npub()).unwrap().node_addr(); + assert_eq!( + reloader.hosts().lookup_hostname(&x_addr), + None, + "old base npub no longer reverse-resolves" + ); + + // Step 2: the file changes again and is re-read by try_check_reload. + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("h {}\na {}\n", u.npub(), z.npub())).unwrap(); + assert!( + reloader.try_check_reload().unwrap(), + "try_check_reload sees the rewrite" + ); + reloader.set_base(one_entry("c", &t)); + assert_eq!(npub(&reloader, "c"), Some(t.npub()), "second base alias"); + assert_eq!( + npub(&reloader, "h"), + Some(u.npub()), + "file entry re-read by try_check_reload survives set_base" + ); + assert_eq!(npub(&reloader, "a"), Some(z.npub()), "file still wins"); + for gone in ["g", "f", "b"] { + assert_eq!(npub(&reloader, gone), None, "{gone} no longer resolves"); + } + } }