mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
node: migrate peer ACL to the Reloadable trait and hot-reload the host map
Move PeerAclReloader onto the Reloadable trait: its ACL snapshot is now published through an arc_swap::ArcSwap so the authorization hot path reads it without locking, and the former check_reload becomes the trait's reload(). The node tick calls self.peer_acl.reload().await. Wire the host map into the tick as well. The host map snapshot was previously taken once at construction and never polled; it now hot-reloads on /etc/fips/hosts mtime changes once per tick, alongside the ACL, so hostname display reflects edits without a restart. The path_mtu_lookup cache (event-driven, populated from observed traffic) and the nostr_discovery subsystem (an async spawned task) are deliberately left off the trait: neither reloads from a backing file, so a no-op reload() would be misleading. The rationale is documented on the trait module. The host map and the ACL's embedded alias reloader still stat /etc/fips/hosts independently each tick. A single small-file stat per tick is cheap, so the duplicate is left in place; sharing one mtime observation between the two is a possible future cleanup. Tests: a node-level test exercises the host-map tick reload end to end through peer_display_name; the ACL reloader tests are updated to drive the async reload().
This commit is contained in:
+55
-36
@@ -9,6 +9,7 @@
|
||||
//! evaluated first, an allowlist match overrides a denylist match for the
|
||||
//! same peer.
|
||||
|
||||
use crate::node::reloadable::Reloadable;
|
||||
use crate::node::{Node, NodeError};
|
||||
use crate::transport::{TransportAddr, TransportId};
|
||||
use crate::upper::hosts::{DEFAULT_HOSTS_PATH, HostMap, HostMapReloader, file_mtime};
|
||||
@@ -17,6 +18,7 @@ use serde::Serialize;
|
||||
use std::collections::{BTreeSet, HashSet};
|
||||
use std::fmt;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
use std::time::SystemTime;
|
||||
use tracing::{debug, info, warn};
|
||||
|
||||
@@ -283,8 +285,16 @@ impl PeerAcl {
|
||||
}
|
||||
|
||||
/// Tracks peer ACL files and reloads them on mtime changes.
|
||||
///
|
||||
/// Follows the canonical Arc-wrapper template from [`Reloadable`]: the
|
||||
/// reader-facing [`PeerAcl`] snapshot is published through an
|
||||
/// [`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.
|
||||
pub struct PeerAclReloader {
|
||||
acl: PeerAcl,
|
||||
/// Reader-facing effective ACL snapshot.
|
||||
acl: arc_swap::ArcSwap<PeerAcl>,
|
||||
hosts: HostMapReloader,
|
||||
allow_path: PathBuf,
|
||||
deny_path: PathBuf,
|
||||
@@ -328,7 +338,7 @@ impl PeerAclReloader {
|
||||
let acl = PeerAcl::load_files_with_hosts(&allow_path, &deny_path, hosts.hosts());
|
||||
|
||||
Self {
|
||||
acl,
|
||||
acl: arc_swap::ArcSwap::from(Arc::new(acl)),
|
||||
hosts,
|
||||
allow_path,
|
||||
deny_path,
|
||||
@@ -337,30 +347,34 @@ impl PeerAclReloader {
|
||||
}
|
||||
}
|
||||
|
||||
/// Get the current ACL.
|
||||
pub fn acl(&self) -> &PeerAcl {
|
||||
&self.acl
|
||||
/// Acquire a lock-free guard over the current ACL snapshot.
|
||||
pub fn acl(&self) -> arc_swap::Guard<Arc<PeerAcl>> {
|
||||
self.load()
|
||||
}
|
||||
|
||||
/// Return a human-readable snapshot of the loaded ACL state.
|
||||
pub fn status(&self) -> PeerAclStatus {
|
||||
let acl = self.acl.load();
|
||||
PeerAclStatus {
|
||||
allow_file: self.allow_path.display().to_string(),
|
||||
deny_file: self.deny_path.display().to_string(),
|
||||
enforcement_active: !self.acl.is_empty(),
|
||||
effective_mode: self.acl.effective_mode().to_string(),
|
||||
default_decision: self.acl.default_decision().to_string(),
|
||||
allow_all: self.acl.allow_all,
|
||||
deny_all: self.acl.deny_all,
|
||||
allow_file_entries: self.acl.allow_file_entries(),
|
||||
deny_file_entries: self.acl.deny_file_entries(),
|
||||
allow_entries: self.acl.allow_entries(),
|
||||
deny_entries: self.acl.deny_entries(),
|
||||
enforcement_active: !acl.is_empty(),
|
||||
effective_mode: acl.effective_mode().to_string(),
|
||||
default_decision: acl.default_decision().to_string(),
|
||||
allow_all: acl.allow_all,
|
||||
deny_all: acl.deny_all,
|
||||
allow_file_entries: acl.allow_file_entries(),
|
||||
deny_file_entries: acl.deny_file_entries(),
|
||||
allow_entries: acl.allow_entries(),
|
||||
deny_entries: acl.deny_entries(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check whether ACL or hosts alias sources changed and reload if needed.
|
||||
pub fn check_reload(&mut self) -> bool {
|
||||
impl Reloadable for PeerAclReloader {
|
||||
type Snapshot = PeerAcl;
|
||||
|
||||
async fn reload(&mut self) -> bool {
|
||||
let allow_mtime = file_mtime(&self.allow_path);
|
||||
let deny_mtime = file_mtime(&self.deny_path);
|
||||
let hosts_changed = self.hosts.check_reload();
|
||||
@@ -374,27 +388,32 @@ impl PeerAclReloader {
|
||||
|
||||
self.last_allow_mtime = allow_mtime;
|
||||
self.last_deny_mtime = deny_mtime;
|
||||
self.acl =
|
||||
let new_acl =
|
||||
PeerAcl::load_files_with_hosts(&self.allow_path, &self.deny_path, self.hosts.hosts());
|
||||
|
||||
info!(
|
||||
allow_file = %self.allow_path.display(),
|
||||
deny_file = %self.deny_path.display(),
|
||||
allow_entries = self.acl.allow.len(),
|
||||
deny_entries = self.acl.deny.len(),
|
||||
allow_entries = new_acl.allow.len(),
|
||||
deny_entries = new_acl.deny.len(),
|
||||
alias_entries = self.hosts.hosts().len(),
|
||||
allow_all = self.acl.allow_all,
|
||||
deny_all = self.acl.deny_all,
|
||||
allow_all = new_acl.allow_all,
|
||||
deny_all = new_acl.deny_all,
|
||||
"Reloaded peer ACL files"
|
||||
);
|
||||
self.acl.store(Arc::new(new_acl));
|
||||
true
|
||||
}
|
||||
|
||||
fn load(&self) -> arc_swap::Guard<Arc<PeerAcl>> {
|
||||
self.acl.load()
|
||||
}
|
||||
}
|
||||
|
||||
impl Node {
|
||||
/// Reload the peer ACL if the ACL or hosts files changed.
|
||||
pub(crate) fn reload_peer_acl(&mut self) -> bool {
|
||||
self.peer_acl.check_reload()
|
||||
pub(crate) async fn reload_peer_acl(&mut self) -> bool {
|
||||
self.peer_acl.reload().await
|
||||
}
|
||||
|
||||
/// Return a control-plane snapshot of the current peer ACL.
|
||||
@@ -747,20 +766,20 @@ mod tests {
|
||||
assert_eq!(acl.check(&test_peer(&npub)), PeerAclDecision::AllowList);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_acl_reloader_detects_change() {
|
||||
#[tokio::test]
|
||||
async fn test_acl_reloader_detects_change() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let allow = dir.path().join("peers.allow");
|
||||
let deny = dir.path().join("peers.deny");
|
||||
let denied = test_npub();
|
||||
|
||||
let mut reloader = PeerAclReloader::with_paths(allow.clone(), deny.clone());
|
||||
assert!(!reloader.check_reload());
|
||||
assert!(!reloader.reload().await);
|
||||
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
std::fs::write(&deny, format!("{denied}\n")).unwrap();
|
||||
|
||||
assert!(reloader.check_reload());
|
||||
assert!(reloader.reload().await);
|
||||
assert_eq!(
|
||||
reloader
|
||||
.acl()
|
||||
@@ -769,8 +788,8 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_acl_reloader_detects_allow_file_removal() {
|
||||
#[tokio::test]
|
||||
async fn test_acl_reloader_detects_allow_file_removal() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let allow = dir.path().join("peers.allow");
|
||||
let deny = dir.path().join("peers.deny");
|
||||
@@ -786,7 +805,7 @@ mod tests {
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
std::fs::remove_file(&allow).unwrap();
|
||||
|
||||
assert!(reloader.check_reload());
|
||||
assert!(reloader.reload().await);
|
||||
assert!(reloader.acl().is_empty());
|
||||
assert_eq!(
|
||||
reloader.acl().check(&test_peer(&allowed)),
|
||||
@@ -892,8 +911,8 @@ mod tests {
|
||||
assert_eq!(acl.check(&peer), PeerAclDecision::AllowList);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_acl_reloader_detects_hosts_change_for_alias_entry() {
|
||||
#[tokio::test]
|
||||
async fn test_acl_reloader_detects_hosts_change_for_alias_entry() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let allow = dir.path().join("peers.allow");
|
||||
let deny = dir.path().join("peers.deny");
|
||||
@@ -909,7 +928,7 @@ mod tests {
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
std::fs::write(&hosts, format!("node-a {npub}\n")).unwrap();
|
||||
|
||||
assert!(reloader.check_reload());
|
||||
assert!(reloader.reload().await);
|
||||
assert_eq!(
|
||||
reloader.acl().allow_file_entries(),
|
||||
vec!["node-a".to_string()]
|
||||
@@ -923,8 +942,8 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_acl_reloader_detects_hosts_removal_for_alias_entry() {
|
||||
#[tokio::test]
|
||||
async fn test_acl_reloader_detects_hosts_removal_for_alias_entry() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let allow = dir.path().join("peers.allow");
|
||||
let deny = dir.path().join("peers.deny");
|
||||
@@ -944,7 +963,7 @@ mod tests {
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
std::fs::remove_file(&hosts).unwrap();
|
||||
|
||||
assert!(reloader.check_reload());
|
||||
assert!(reloader.reload().await);
|
||||
assert!(reloader.acl().is_empty());
|
||||
assert_eq!(
|
||||
reloader.acl().check(&test_peer(&npub)),
|
||||
|
||||
@@ -249,7 +249,15 @@ impl Node {
|
||||
_ = tick.tick() => {
|
||||
self.check_timeouts();
|
||||
let now_ms = Self::now_ms();
|
||||
self.reload_peer_acl();
|
||||
self.reload_peer_acl().await;
|
||||
// The host map hot-reloads on the same tick as the ACL. It
|
||||
// is polled separately from `reload_peer_acl` because the
|
||||
// ACL's embedded alias reloader and this snapshot are
|
||||
// distinct resources; the `path_mtu_lookup` cache and the
|
||||
// `nostr_discovery` subsystem are deliberately excluded
|
||||
// from `Reloadable` since neither reloads from a backing
|
||||
// file (see `node::reloadable`).
|
||||
self.reload_host_map().await;
|
||||
self.poll_pending_connects().await;
|
||||
self.poll_nostr_discovery().await;
|
||||
self.resend_pending_handshakes(now_ms).await;
|
||||
|
||||
@@ -1072,6 +1072,13 @@ impl Node {
|
||||
self.identity.npub()
|
||||
}
|
||||
|
||||
/// Reload the host map if the backing `/etc/fips/hosts` file changed.
|
||||
///
|
||||
/// Returns `true` if a new snapshot was published.
|
||||
pub(crate) async fn reload_host_map(&mut self) -> bool {
|
||||
self.host_map.reload().await
|
||||
}
|
||||
|
||||
/// Return a human-readable display name for a NodeAddr.
|
||||
///
|
||||
/// Lookup order:
|
||||
|
||||
+22
-8
@@ -30,6 +30,25 @@
|
||||
//! source degrades to an empty/base snapshot plus a warning), so callers have
|
||||
//! nothing to handle. The return flag reports only whether the snapshot was
|
||||
//! replaced, which is all the tick loop needs for logging.
|
||||
//!
|
||||
//! # What is and isn't `Reloadable`
|
||||
//!
|
||||
//! Two node-owned resources are deliberately *not* `Reloadable` because
|
||||
//! neither has a "re-read from a backing source" concept:
|
||||
//!
|
||||
//! - `path_mtu_lookup` is an event-driven cache (`Arc<RwLock<HashMap>>`)
|
||||
//! populated from observed path-MTU discovery traffic, not loaded from a
|
||||
//! file. There is nothing to poll. (Its read side could adopt the same
|
||||
//! lock-free `ArcSwap` shape in the future, but that is an optimization, not
|
||||
//! a reload.)
|
||||
//! - `nostr_discovery` is an async spawned subsystem, not a snapshot of disk
|
||||
//! state.
|
||||
//!
|
||||
//! Both [`HostMapReloadable`] and the peer ACL reloader currently stat
|
||||
//! `/etc/fips/hosts` independently each tick (the ACL reloader embeds its own
|
||||
//! hosts reloader for alias resolution). A single small-file `stat` per tick
|
||||
//! is cheap, so the duplicate is left in place; collapsing the two onto a
|
||||
//! single shared mtime observation is a possible future cleanup.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -50,10 +69,8 @@ pub trait Reloadable: Send {
|
||||
/// changed. I/O errors are absorbed internally (degrading to an
|
||||
/// empty/base snapshot with a warning) rather than surfaced.
|
||||
///
|
||||
/// Not yet driven from the node tick — the host map snapshot is still
|
||||
/// taken once at construction and not polled. Wiring the periodic poll
|
||||
/// into the tick is a follow-up.
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
/// Driven once per node tick from the rx loop, alongside the other
|
||||
/// reloadable resources.
|
||||
async fn reload(&mut self) -> bool;
|
||||
|
||||
/// Acquire a lock-free guard over the current snapshot.
|
||||
@@ -73,15 +90,12 @@ pub struct HostMapReloadable {
|
||||
/// Reader-facing effective snapshot (base merged with hosts file).
|
||||
snapshot: arc_swap::ArcSwap<HostMap>,
|
||||
/// Base map from peer-config aliases (never changes). Read only by
|
||||
/// `reload`, which is not yet driven from the node tick.
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
/// `reload` on the tick task.
|
||||
base: HostMap,
|
||||
/// Path to the operator hosts file. Read only by `reload`.
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
path: std::path::PathBuf,
|
||||
/// Last observed modification time of the hosts file (`None` if absent).
|
||||
/// Read only by `reload`.
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
last_mtime: Option<std::time::SystemTime>,
|
||||
}
|
||||
|
||||
|
||||
+32
-4
@@ -1,7 +1,9 @@
|
||||
use super::*;
|
||||
use crate::ReceivedPacket;
|
||||
use crate::node::acl::PeerAclReloader;
|
||||
use crate::node::reloadable::HostMapReloadable;
|
||||
use crate::node::wire::{build_msg1, build_msg2};
|
||||
use crate::upper::hosts::HostMap;
|
||||
use crate::utils::index::SessionIndex;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
@@ -29,7 +31,7 @@ async fn test_outbound_connect_denied_by_denylist() {
|
||||
let (dir, mut node) = make_acl_node();
|
||||
let denied = Identity::generate();
|
||||
std::fs::write(deny_path(&dir), format!("{}\n", denied.npub())).unwrap();
|
||||
node.reload_peer_acl();
|
||||
node.reload_peer_acl().await;
|
||||
|
||||
let result = node
|
||||
.initiate_connection(
|
||||
@@ -51,7 +53,7 @@ async fn test_inbound_msg1_denied_by_acl() {
|
||||
let node_a = make_node();
|
||||
|
||||
std::fs::write(deny_path(&dir), format!("{}\n", node_a.npub())).unwrap();
|
||||
node_b.reload_peer_acl();
|
||||
node_b.reload_peer_acl().await;
|
||||
|
||||
let peer_b_identity = PeerIdentity::from_pubkey_full(node_b.identity.pubkey_full());
|
||||
let mut conn_a = PeerConnection::outbound(LinkId::new(1), peer_b_identity, 1000);
|
||||
@@ -121,7 +123,7 @@ async fn test_outbound_msg2_denied_after_acl_reload() {
|
||||
let wire_msg2 = build_msg2(our_index_b, our_index_a, &noise_msg2);
|
||||
|
||||
std::fs::write(deny_path(&dir), format!("{}\n", node_b.npub())).unwrap();
|
||||
assert!(node_a.reload_peer_acl());
|
||||
assert!(node_a.reload_peer_acl().await);
|
||||
|
||||
let packet = ReceivedPacket::with_timestamp(transport_id, remote_addr, wire_msg2, 1100);
|
||||
node_a.handle_msg2(packet).await;
|
||||
@@ -132,13 +134,39 @@ async fn test_outbound_msg2_denied_after_acl_reload() {
|
||||
assert!(node_a.pending_outbound.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_host_map_hot_reloads_from_tick() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let hosts_path = dir.path().join("hosts");
|
||||
|
||||
let mut node = Node::new(Config::new()).unwrap();
|
||||
node.host_map = HostMapReloadable::new(HostMap::new(), hosts_path.clone());
|
||||
|
||||
let peer = Identity::generate();
|
||||
let peer_addr = *PeerIdentity::from_pubkey_full(peer.pubkey_full()).node_addr();
|
||||
|
||||
// No hosts file yet: the display name is not the alias.
|
||||
assert_ne!(node.peer_display_name(&peer_addr), "gateway");
|
||||
assert!(!node.reload_host_map().await);
|
||||
|
||||
// Write a hosts entry and let the tick-driven reload pick it up.
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
std::fs::write(&hosts_path, format!("gateway {}\n", peer.npub())).unwrap();
|
||||
|
||||
assert!(node.reload_host_map().await);
|
||||
assert_eq!(node.peer_display_name(&peer_addr), "gateway");
|
||||
|
||||
// No further change: reload reports nothing replaced.
|
||||
assert!(!node.reload_host_map().await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_outbound_connect_not_denied_by_allowlist_miss() {
|
||||
let (dir, mut node) = make_acl_node();
|
||||
let denied = Identity::generate();
|
||||
let allowed = Identity::generate();
|
||||
std::fs::write(allow_path(&dir), format!("{}\n", allowed.npub())).unwrap();
|
||||
node.reload_peer_acl();
|
||||
node.reload_peer_acl().await;
|
||||
|
||||
let result = node
|
||||
.initiate_connection(
|
||||
|
||||
Reference in New Issue
Block a user