diff --git a/CHANGELOG.md b/CHANGELOG.md index ef445ad3..fffbbe9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -773,6 +773,12 @@ with v0.5.x or earlier peers. - The gateway retries failed firewall rebuilds every ten seconds without waiting for another mapping change. Retries apply the latest desired mappings and port forwards, preserving changes across transient failures. +- An inbound port-forwarded connection from a mesh peer that also has a live + `.fips` mapping on the gateway now reaches the LAN target from the + gateway's LAN address, as connections from every other peer do. The + mapping's source rewrite used to take it first, so the target saw the + peer's pool address instead, which changed as mappings came and went and + could be answered only by a host that routes the pool to the gateway. #### Packaging @@ -780,6 +786,17 @@ with v0.5.x or earlier peers. an upgrade when they were running. It stopped fips first, which stopped both through `Requires=fips.service`, then restarted only fips, so `.fips` resolution stayed down and the gateway stayed stopped until started by hand. +- Removing the `.deb` without purging it now removes the `.fips` DNS routing + when `fips-dns` was not running at the time, as purging already did. The + routing stayed behind, so the host kept sending `.fips` queries to + `[::1]:5354`, where nothing listens once the package is removed, and + `.fips` lookups timed out until the file was deleted by hand. +- When no supported DNS resolver is found, `fips-dns-setup` no longer tells + the host to run `apt install systemd-resolved`. The script is not + Debian-only, so the hint now names no package manager. It says to start + systemd-resolved (`systemctl enable --now`) and then restart + `fips-dns.service`, since setup uses systemd-resolved only when it is + running. - A node no longer relays away the answer to its own lookup. A request is flooded to every tree peer whose bloom filter claims the target, so a false @@ -962,6 +979,11 @@ with v0.5.x or earlier peers. closed, losing the peer's first datagram, when the kernel's descriptor garbage collector ran before the client read the arrival message. The same exposure on connect and listen replies is closed too. +- A native API flow can now reach a node that is not a peer, has no session, + was not resolved through DNS and has no known route. Discovery ran for it, + but its answer was discarded, and the datagram was held without being + sent. The node now records the flow's key before starting discovery, and + retries the session for held native datagrams once discovery answers. #### OpenWrt @@ -983,6 +1005,14 @@ with v0.5.x or earlier peers. edited config is kept across upgrades, along with its old "physical port names, NOT bridge names" comment, so an upgrade alone will not correct it. +#### Routing and discovery + +- A backward step of the system clock, from an NTP correction, a manual set + or a VM resume, no longer holds a node's bloom filter announces for the + size of the step. The spacing between announces to a peer + (`node.bloom.update_debounce_ms`, 500 ms by default) is now measured on a + monotonic clock. + #### Sessions and rekey - Link quality estimates no longer freeze after a link rekey. Frames the peer diff --git a/docs/design/fips-gateway.md b/docs/design/fips-gateway.md index 34d15f70..a123ea74 100644 --- a/docs/design/fips-gateway.md +++ b/docs/design/fips-gateway.md @@ -419,6 +419,13 @@ masquerade in the outbound pipeline; the two have disjoint match clauses (different `iifname`/`oifname` combinations) and coexist without interaction when both directions are active. +The per-mapping SNAT rules match on source address alone, so an +inbound forwarded connection from a mesh peer that also holds a live +mapping matches both its SNAT and the LAN-side masquerade. NAT +statements are terminal, and the rebuild places the LAN-side +masquerade ahead of every per-mapping SNAT, so that peer's connection +reaches the LAN target from the gateway's LAN address like any other. + ### Independence From Outbound The inbound half does not require: @@ -449,9 +456,9 @@ sequence is: 1. Add the table (which succeeds whether or not it exists), delete it, and add it again, so the delete always has a target. 2. Add the `prerouting` and `postrouting` chains; the always-on - `oifname fips0` masquerade; per-mapping DNAT/SNAT rules for - every live pool entry; per-port-forward DNAT rules; the LAN-side - masquerade if any port-forwards exist. + `oifname fips0` masquerade; the LAN-side masquerade if any + port-forwards exist; per-mapping DNAT/SNAT rules for every live + pool entry; per-port-forward DNAT rules. 3. Send all of it as one batch, which the kernel applies as a single transaction. Only the last message before the batch end requests an acknowledgement, the socket's send buffer is sized to the diff --git a/packaging/common/fips-dns-setup b/packaging/common/fips-dns-setup index 6617060f..fe26b5c0 100755 --- a/packaging/common/fips-dns-setup +++ b/packaging/common/fips-dns-setup @@ -224,7 +224,7 @@ log "To resolve .fips domains, configure your DNS resolver to forward" log "the .fips domain to [${FIPS_DNS_LOOPBACK_V6}]:${FIPS_DNS_PORT} (matches the daemon's default bind)." log "" log "Examples:" -log " systemd-resolved: sudo apt install systemd-resolved" +log " systemd-resolved: install it if needed (a separate package on some distributions), run 'sudo systemctl enable --now systemd-resolved', then 'sudo systemctl restart fips-dns.service'" log " dnsmasq: echo 'server=/fips/${FIPS_DNS_LOOPBACK_V6}#${FIPS_DNS_PORT}' | sudo tee /etc/dnsmasq.d/fips.conf" save_backend "none" exit 0 diff --git a/packaging/debian/postrm b/packaging/debian/postrm index 8f1afd5d..123e48aa 100755 --- a/packaging/debian/postrm +++ b/packaging/debian/postrm @@ -3,21 +3,16 @@ set -e case "$1" in - purge) - # Remove configuration and identity keys - rm -rf /etc/fips/ - - # Remove tmpfiles.d entry - rm -f /usr/lib/tmpfiles.d/fips.conf - - # Remove runtime directory - rm -rf /run/fips/ - + remove|purge) # Remove the DNS routing fips-dns-setup may have written, in case # fips-dns-teardown did not run (prerm's stop runs it only when - # fips-dns.service was active), and make the resolver drop it. The - # paths match packaging/common/fips-dns-teardown, which dpkg has - # already removed, so it cannot be called from here. + # fips-dns.service was active), and make the resolver drop it. This + # runs on remove as well as purge: a removed package leaves nothing + # listening behind the routing, and purging a package already removed + # runs only postrm purge. On a purge of an installed package the + # purge pass finds nothing left and restarts nothing. The paths match + # packaging/common/fips-dns-teardown, which dpkg has already removed, + # so it cannot be called from here. restart_resolved=0 if [ -f /etc/systemd/dns-delegate.d/fips.dns-delegate ]; then rm -f /etc/systemd/dns-delegate.d/fips.dns-delegate @@ -52,6 +47,19 @@ case "$1" in || echo "fips: warning: could not reload NetworkManager; reload it to drop the .fips route" fi fi + ;; +esac + +case "$1" in + purge) + # Remove configuration and identity keys + rm -rf /etc/fips/ + + # Remove tmpfiles.d entry + rm -f /usr/lib/tmpfiles.d/fips.conf + + # Remove runtime directory + rm -rf /run/fips/ # Remove fips system group if getent group fips >/dev/null 2>&1; then diff --git a/src/gateway/nat.rs b/src/gateway/nat.rs index 72de3a89..517814ca 100644 --- a/src/gateway/nat.rs +++ b/src/gateway/nat.rs @@ -341,6 +341,16 @@ impl NatManager { NatOp::FipsMasquerade, ]; + // When any port forwards are configured, one LAN-side masquerade in + // postrouting gives the LAN target the gateway's LAN address as the + // source, so replies flow back through conntrack. It goes ahead of + // every per-mapping SNAT: those match on source address alone, NAT + // statements are terminal, and a SNAT listed first would take an + // inbound forwarded flow from a peer that holds a live mapping. + if !self.port_forwards.is_empty() { + ops.push(NatOp::LanMasquerade); + } + for mapping in self.mappings.values() { ops.push(NatOp::Dnat(mapping.virtual_ip)); ops.push(NatOp::Snat(mapping.virtual_ip)); @@ -348,15 +358,9 @@ impl NatManager { // Inbound port-forward rules. Each forward is one DNAT rule in // prerouting keyed on (iif fips0, nfproto ipv6, l4proto, th dport). - // When any forwards are configured, emit a single LAN-side masquerade - // in postrouting so the LAN target host sees the gateway's LAN address - // as source and replies flow back through conntrack. for index in 0..self.port_forwards.len() { ops.push(NatOp::PortForward(index)); } - if !self.port_forwards.is_empty() { - ops.push(NatOp::LanMasquerade); - } vec![ops] } @@ -1246,6 +1250,39 @@ mod tests { ); } + #[test] + fn rebuild_places_the_lan_masquerade_before_every_mapping_snat() { + let mut mgr = manager_with_mappings(3); + mgr.port_forwards = vec![PortForward { + proto: Proto::Tcp, + listen_port: 8080, + target: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 80, 0, 0), + }]; + + let ops = mgr.rebuild_batches().remove(0); + + let masquerade = ops + .iter() + .position(|op| matches!(op, NatOp::LanMasquerade)) + .expect("a port forward emits the LAN masquerade"); + let snats: Vec = ops + .iter() + .enumerate() + .filter(|(_, op)| matches!(op, NatOp::Snat(_))) + .map(|(i, _)| i) + .collect(); + assert_eq!(snats.len(), 3, "one SNAT per mapping: {ops:?}"); + for snat in snats { + assert!( + masquerade < snat, + "the LAN masquerade at index {masquerade} follows the SNAT at \ + index {snat}, so an inbound forwarded flow from a peer with a \ + live mapping takes the SNAT and bypasses the masquerade: \ + {ops:?}" + ); + } + } + /// The encoded rebuild of a manager holding `count` mappings. fn encoded_rebuild(count: u16) -> Vec { let mgr = manager_with_mappings(count); diff --git a/src/node/bloom.rs b/src/node/bloom.rs index 48c08415..58dad566 100644 --- a/src/node/bloom.rs +++ b/src/node/bloom.rs @@ -44,10 +44,9 @@ impl Node { peer_addr: &NodeAddr, filter: BloomFilter, ) -> Result<(), NodeError> { - let now_ms = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_millis() as u64) - .unwrap_or(0); + // Monotonic: the debounce compares two reads, and a wall-clock step + // back would hold announces for the size of the step. + let now_ms = crate::time::mono_ms(); // Check debounce if !self.bloom_state.should_send_update(peer_addr, now_ms) { @@ -137,10 +136,9 @@ impl Node { /// Send pending rate-limited filter announces whose debounce has expired. pub(super) async fn send_pending_filter_announces(&mut self) { - let now_ms = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_millis() as u64) - .unwrap_or(0); + // Monotonic: the debounce compares two reads, and a wall-clock step + // back would hold announces for the size of the step. + let now_ms = crate::time::mono_ms(); let ready: Vec = self .peers diff --git a/src/node/handlers/lookup.rs b/src/node/handlers/lookup.rs index e86d3846..d7569134 100644 --- a/src/node/handlers/lookup.rs +++ b/src/node/handlers/lookup.rs @@ -504,13 +504,17 @@ impl Node { } } LookupAction::RetryQueuedPackets { target } => { - // If we have pending TUN packets for this target, retry session - // initiation. The coord_cache now has coords, so find_next_hop() - // should succeed. - if let Some(packets) = self.pending_tun_packets.get(&target) { + // If we hold TUN packets or native datagrams for this target, + // retry session initiation. The coord_cache now has coords, so + // find_next_hop() should succeed. Native datagrams are held in + // their own queue, and nothing else re-initiates for them. + let packets = self.pending_tun_packets.get(&target).map(|q| q.len()); + let native = self.pending_native.get(&target).map(|q| q.len()); + if packets.is_some() || native.is_some() { debug!( dest = %self.peer_display_name(&target), - queued_packets = packets.len(), + queued_packets = packets.unwrap_or(0), + queued_native = native.unwrap_or(0), "Retrying queued packets after discovery" ); self.retry_session_after_discovery(target).await; diff --git a/src/node/handlers/native.rs b/src/node/handlers/native.rs index 93a0ebd9..c995d33b 100644 --- a/src/node/handlers/native.rs +++ b/src/node/handlers/native.rs @@ -157,6 +157,21 @@ impl Node { // hashes the DH output x-only and forces even parity in the XK // premessage, so both parities derive the same material. let pubkey = peer.public_key(secp256k1::Parity::Even); + + // Cache the key the client supplied before trying the route. If there + // is none, the lookup below verifies its answer against this cache, and + // `initiate_session` registers the key only after a send succeeds, so + // without this the answer is dropped. Only on a miss, the same rule + // `cache_session_identity` follows: when no route is known, a key + // already cached from DNS, a handshake or a configured peer keeps its + // own parity. The presence check refreshes that entry, so it is not + // the oldest one when the answer arrives. + let mut prefix = [0u8; 15]; + prefix.copy_from_slice(&key.peer.as_bytes()[0..15]); + if self.lookup_by_fips_prefix(&prefix).is_none() { + self.register_identity(key.peer, pubkey); + } + if let Err(error) = self.initiate_session(key.peer, pubkey).await { debug!( peer = %self.peer_display_name(&key.peer), diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 703edffc..66915366 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -3557,8 +3557,9 @@ impl Node { /// Retry session initiation after discovery provided coordinates. /// /// Called when a LookupResponse arrives and we have pending TUN packets - /// for the discovered target. The coord_cache now has coords, so - /// `find_next_hop()` should succeed and the SessionSetup can be sent. + /// or native datagrams for the discovered target. The coord_cache now + /// has coords, so `find_next_hop()` should succeed and the SessionSetup + /// can be sent. pub(in crate::node) async fn retry_session_after_discovery(&mut self, dest_addr: NodeAddr) { // Look up the destination's public key from the identity cache let mut prefix = [0u8; 15]; diff --git a/src/node/tests/bloom.rs b/src/node/tests/bloom.rs index 0c32848b..94c4fdc1 100644 --- a/src/node/tests/bloom.rs +++ b/src/node/tests/bloom.rs @@ -2113,3 +2113,45 @@ async fn test_tree_lost_announce_is_resent_after_the_fallback_when_no_receiver_r ); cleanup_nodes(&mut line.nodes).await; } + +/// The filter-announce debounce is stamped on the monotonic clock, so a step +/// back of the wall clock cannot hold announces for the size of the step. +#[tokio::test] +async fn filter_announce_debounce_stamps_the_monotonic_clock() { + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + let peer = *nodes[1].node.node_addr(); + let node = &mut nodes[0].node; + + node.bloom_state.set_update_debounce_ms(0); + node.bloom_state.mark_update_needed(peer); + let sent = node.metrics().bloom.sent.get(); + let before = crate::time::mono_ms(); + node.send_pending_filter_announces().await; + let after = crate::time::mono_ms(); + + assert_eq!( + node.metrics().bloom.sent.get(), + sent + 1, + "setup: the pending announce must be sent" + ); + let stamp = node.bloom_state.last_update_sent(&peer); + assert!( + stamp.is_some_and(|t| before <= t && t <= after), + "stamp {stamp:?} not in [{before}, {after}]" + ); + + // Regression guard for the ready-set read: with the debounce window + // still open, the peer must not even reach the send path, where the + // re-check would count it as suppressed. + node.bloom_state.set_update_debounce_ms(60_000); + node.bloom_state.mark_update_needed(peer); + let suppressed = node.metrics().bloom.debounce_suppressed.get(); + node.send_pending_filter_announces().await; + assert_eq!( + node.metrics().bloom.debounce_suppressed.get(), + suppressed, + "a peer inside the debounce window must not enter the ready set" + ); + + cleanup_nodes(&mut nodes).await; +} diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 6b4bef13..e2e56299 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -5,12 +5,15 @@ //! response routing. use super::*; +use crate::native::link::NativeMessage; +use crate::native::registry::FlowKey; use crate::proto::lookup::{LookupRequest, LookupResponse, RecentRequest}; use crate::proto::stp::TreeCoordinate; use spanning_tree::{ cleanup_nodes, generate_random_edges, lock_large_network_test, process_available_packets, run_tree_test, run_tree_test_with_mtus, verify_tree_convergence, }; +use tokio::sync::{mpsc, oneshot}; // ============================================================================ // Unit Tests — LookupRequest Handler @@ -2200,3 +2203,205 @@ async fn test_check_pending_lookups_default_sequence_unreachable() { assert_eq!(icmp_frame[6], 58, "next_header must be IPPROTO_ICMPV6 (58)"); assert_eq!(icmp_frame[40], 1, "ICMPv6 type 1 = Destination Unreachable"); } + +// ============================================================================ +// Native API — first send to an uncached, unrouted key +// ============================================================================ + +/// The registry's port for the listener the native tests bind. +const NATIVE_PORT: u16 = 7000; + +#[tokio::test] +async fn a_native_send_to_an_unrouted_key_caches_that_key_so_discovery_can_verify_the_answer() { + let mut node = make_node(); + let dest = crate::Identity::generate(); + let dest_addr = *dest.node_addr(); + let key = FlowKey { + peer: dest_addr, + remote: NATIVE_PORT, + local: 7001, + }; + + assert!( + !node.has_cached_identity(&dest_addr), + "precondition: the destination's key must not be cached" + ); + + node.handle_native_outbound(key, dest.pubkey(), b"hello".to_vec()) + .await; + + assert!( + node.has_cached_identity(&dest_addr), + "a native send with no route must cache the flow's key, or the lookup it \ + starts cannot verify its answer" + ); +} + +#[tokio::test] +async fn a_native_send_does_not_replace_a_key_already_cached_for_that_destination() { + let mut node = make_node(); + + // A fixed key with odd parity, so the native path's even-parity lift would + // be visible if it replaced the cached entry. Searched over a bounded set + // of fixed secrets so the test is deterministic. + let dest = (1u8..=64) + .map(|n| { + let mut secret = [0u8; 32]; + secret[31] = n; + crate::Identity::from_secret_bytes(&secret).expect("a small non-zero secret is valid") + }) + .find(|id| id.pubkey_full().x_only_public_key().1 == secp256k1::Parity::Odd) + .expect("one of 64 fixed secrets has an odd-parity public key"); + let dest_addr = *dest.node_addr(); + let original = dest.pubkey_full(); + node.register_identity(dest_addr, original); + + let key = FlowKey { + peer: dest_addr, + remote: NATIVE_PORT, + local: 7001, + }; + node.handle_native_outbound(key, dest.pubkey(), b"hello".to_vec()) + .await; + + let mut prefix = [0u8; 15]; + prefix.copy_from_slice(&dest_addr.as_bytes()[0..15]); + let (_, cached) = node + .lookup_by_fips_prefix(&prefix) + .expect("the destination's key must still be cached"); + assert_eq!( + cached, original, + "a native send must not replace a cached key with its even-parity lift" + ); +} + +#[tokio::test] +async fn a_native_first_send_to_an_uncached_unrouted_key_is_delivered_after_discovery() { + // Topology: node0 — node1 — node2. Node 0 knows nothing of node 2: no + // peer link, no cached key, no cached coordinates, no session. + let edges = vec![(0, 1), (1, 2)]; + let mut nodes = run_tree_test(3, &edges, false).await; + + let node0_xonly = nodes[0].node.identity().pubkey(); + let node2_addr = *nodes[2].node.node_addr(); + let node2_xonly = nodes[2].node.identity().pubkey(); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0); + assert!( + nodes[0].node.get_peer(&node2_addr).is_none(), + "precondition: node 2 must not be a peer of node 0" + ); + assert!( + !nodes[0].node.has_cached_identity(&node2_addr), + "precondition: node 0 must not have node 2's key cached" + ); + assert!( + !nodes[0].node.coord_cache().contains(&node2_addr, now_ms), + "precondition: node 0 must not have node 2's coordinates cached" + ); + assert!( + nodes[0].node.get_session(&node2_addr).is_none(), + "precondition: node 0 must have no session to node 2" + ); + + // Node 2 listens on the port node 0 will send to. + let (arrivals_tx, mut arrivals) = mpsc::channel(8); + let (reply_tx, mut reply_rx) = oneshot::channel(); + nodes[2].node.handle_native(NativeMessage::Listen { + port: Some(NATIVE_PORT), + arrivals: arrivals_tx, + reply: reply_tx, + }); + assert_eq!( + reply_rx + .try_recv() + .expect("Listen must answer synchronously"), + Ok(NATIVE_PORT), + "node 2 must hold the listener port" + ); + + let lookup = &nodes[0].node.metrics().lookup; + let initiated_before = lookup.req_initiated.get(); + let accepted_before = lookup.resp_accepted.get(); + let identity_miss_before = lookup.resp_identity_miss.get(); + + let payload = b"first native datagram".to_vec(); + let flow = FlowKey { + peer: node2_addr, + remote: NATIVE_PORT, + local: 7001, + }; + nodes[0] + .node + .handle_native_outbound(flow, node2_xonly, payload.clone()) + .await; + + // Bound the drive on the arrival itself: the initiator flushes the held + // datagram in the same call that sends msg3, and packets move one hop per + // pass, so the datagram is still in transit when node 0 first shows its + // session established. Never await `recv()` here: node 2's registry holds + // the sender, so a broken fix would hang rather than fail. + let mut arrival = None; + for _ in 0..40 { + tokio::time::sleep(Duration::from_millis(50)).await; + process_available_packets(&mut nodes).await; + if let Ok(seen) = arrivals.try_recv() { + arrival = Some(seen); + break; + } + } + + let lookup = &nodes[0].node.metrics().lookup; + assert!( + lookup.req_initiated.get() > initiated_before, + "the unrouted native send must start discovery" + ); + assert_eq!( + lookup.resp_identity_miss.get(), + identity_miss_before, + "discovery's answer must not be dropped for want of the target's key" + ); + assert!( + lookup.resp_accepted.get() > accepted_before, + "discovery's answer must be verified and accepted" + ); + assert!( + nodes[0] + .node + .get_session(&node2_addr) + .is_some_and(|entry| entry.is_established()), + "discovery must lead to an established session for the held native datagram" + ); + assert_eq!( + nodes[0].node.metrics().native.sent_datagrams.get(), + 1, + "node 0 must send the held native datagram once the session is up" + ); + + let arrival = arrival.expect("node 2's listener must see the native flow arrive"); + assert_eq!( + arrival.pubkey, node0_xonly, + "the arrival must carry node 0's authenticated key" + ); + + let (sink_tx, _sink_rx) = mpsc::channel(8); + let (accept_tx, mut accept_rx) = oneshot::channel(); + nodes[2].node.handle_native(NativeMessage::Accept { + flow: arrival.flow, + sink: sink_tx, + reply: accept_tx, + }); + let accepted = accept_rx + .try_recv() + .expect("Accept must answer synchronously") + .expect("node 2 must accept the announced flow"); + assert_eq!( + accepted.held, + vec![payload], + "the accepted flow must hold exactly the datagram node 0 sent" + ); + + cleanup_nodes(&mut nodes).await; +} diff --git a/src/nostr/tests.rs b/src/nostr/tests.rs index 4f3ec05f..cf1d8cbb 100644 --- a/src/nostr/tests.rs +++ b/src/nostr/tests.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; use std::net::{IpAddr, SocketAddr}; +#[cfg(target_os = "linux")] use std::time::Duration; use nostr::nips::nip17; @@ -14,10 +15,12 @@ use super::signal::{ estimate_clock_skew, validate_offer_freshness, validate_traversal_answer_for_offer, }; use super::stun::{parse_stun_binding_success, parse_stun_url}; +#[cfg(target_os = "linux")] +use super::traversal::run_punch_attempt; use super::traversal::{ PunchStrategy, SourceRank, build_punch_packet, is_doc_ip, is_never_punchable_ip, is_private_ip, now_ms, parse_punch_packet, plan_punch_targets, planned_remote_endpoints, rank_punch_source, - run_punch_attempt, session_hash, + session_hash, }; use super::traversal_machine::suppress_responder_for_own_initiator; use super::types::BootstrapError; @@ -1404,6 +1407,7 @@ fn punch_socket(host: &str) -> std::net::UdpSocket { /// A hint that starts punching immediately. `start_at_ms` is absolute wall /// clock, so anything plausible-looking in the future would sleep out the test. +#[cfg(target_os = "linux")] fn immediate_punch_hint(duration_ms: u64) -> PunchHint { PunchHint { start_at_ms: 0, diff --git a/src/packaging_tests.rs b/src/packaging_tests.rs index e68df0a9..10338974 100644 --- a/src/packaging_tests.rs +++ b/src/packaging_tests.rs @@ -391,17 +391,19 @@ fn freebsd_newsyslog_entry_signals_the_daemon8_supervisor_started_with_sighup_re ); } -/// Pins the DNS cleanup in `postrm purge` and `uninstall.sh` to the files -/// `fips-dns-setup` writes, so a purge after a `fips-dns` that never ran its -/// teardown does not leave the resolver sending `.fips` to a dead responder. +/// Pins the DNS cleanup in `postrm remove` and `postrm purge` and in +/// `uninstall.sh` to the files `fips-dns-setup` writes, so a remove or purge +/// after a `fips-dns` that never ran its teardown does not leave the resolver +/// sending `.fips` to a dead responder. /// /// This is a text test. Each path must appear on an `rm -f` line, but a /// resolver command passes wherever it appears on a code line, including in a -/// message. What `postrm` actually does is covered by the deb-install purge -/// check. No suite runs `uninstall.sh`: its two resolved paths were run once, -/// by hand in a container, and its dnsmasq and NetworkManager paths by nothing. +/// message. What `postrm` actually does is covered by the deb-install remove +/// and purge checks. No suite runs `uninstall.sh`: its two resolved paths were +/// run once, by hand in a container, and its dnsmasq and NetworkManager paths +/// by nothing. #[test] -fn dns_cleanup_in_postrm_purge_and_uninstall_removes_every_file_fips_dns_setup_writes_and_restarts_its_resolver() +fn dns_cleanup_in_postrm_remove_and_purge_and_uninstall_removes_every_file_fips_dns_setup_writes_and_restarts_its_resolver() { let setup = rc_vars(&repo_file("packaging/common/fips-dns-setup")); let teardown = rc_vars(&repo_file("packaging/common/fips-dns-teardown")); @@ -433,8 +435,8 @@ fn dns_cleanup_in_postrm_purge_and_uninstall_removes_every_file_fips_dns_setup_w let scripts = [ ( - "packaging/debian/postrm purge)", - case_branch(&repo_file("packaging/debian/postrm"), "purge"), + "packaging/debian/postrm remove|purge)", + case_branch(&repo_file("packaging/debian/postrm"), "remove|purge"), ), ( "packaging/systemd/uninstall.sh", @@ -1256,3 +1258,60 @@ fn openwrt_config_offers_no_ble_block_because_musl_builds_have_no_ble() { where the BLE transport is not compiled: {found:?}" ); } + +/// `fips-dns-setup` is shared by the Debian package, the Arch packages, the +/// systemd tarball and, where it is packaged, the RPM, so the hint it prints +/// when it finds no DNS resolver must not tell the host to use one +/// distribution's package manager. +/// +/// Every systemd-resolved backend in the script gates on the unit being active +/// (`is_active`), so the hint must say to start it (`enable --now`), not only to +/// install or enable it, and then to restart `fips-dns.service` so detection +/// runs again. `test_no_resolver` in `testing/dns-resolver/test.sh` asserts the +/// same on the script's real output. +#[test] +fn fips_dns_setup_no_resolver_hint_starts_resolved_and_names_no_package_manager_because_the_script_is_not_debian_only() + { + const SETUP: &str = "packaging/common/fips-dns-setup"; + const BANNED: [&str; 6] = [ + "apt install", + "apt-get install", + "dnf install", + "yum install", + "zypper install", + "pacman -S", + ]; + let lines = code_lines(&repo_file(SETUP)); + let mut problems = Vec::new(); + + let hints: Vec<&String> = lines + .iter() + .filter(|l| l.starts_with("log \" systemd-resolved:")) + .collect(); + match hints.as_slice() { + [hint] => { + for needed in ["enable --now", "restart fips-dns.service"] { + if !hint.contains(needed) { + problems.push(format!("hint lacks '{needed}': {hint}")); + } + } + } + _ => problems.push(format!( + "expected one systemd-resolved hint line, found {}: {hints:?}", + hints.len() + )), + } + for line in &lines { + for banned in BANNED { + if line.contains(banned) { + problems.push(format!("names a package manager ('{banned}'): {line}")); + } + } + } + + assert!( + problems.is_empty(), + "{SETUP}'s no-resolver hint is wrong:\n {}", + problems.join("\n ") + ); +} diff --git a/src/proto/bloom/state.rs b/src/proto/bloom/state.rs index bdebfb03..b2c5c1ab 100644 --- a/src/proto/bloom/state.rs +++ b/src/proto/bloom/state.rs @@ -128,13 +128,20 @@ impl BloomState { } /// Check if we should send an update to a peer (respecting debounce). + /// + /// A time earlier than the last send permits the send rather than holding + /// the update until the clock catches up; the send then restamps and the + /// ordinary debounce resumes. The comparison takes a difference rather + /// than adding the debounce, so a very large debounce cannot overflow. pub fn should_send_update(&self, peer_id: &NodeAddr, current_time_ms: u64) -> bool { if !self.pending_updates.contains(peer_id) { return false; } match self.last_update_sent.get(peer_id) { - Some(&last_time) => current_time_ms >= last_time + self.update_debounce_ms, + Some(&last_time) => current_time_ms + .checked_sub(last_time) + .is_none_or(|elapsed| elapsed >= self.update_debounce_ms), None => true, } } @@ -145,6 +152,12 @@ impl BloomState { self.pending_updates.remove(&peer_id); } + /// Read back the time of the last update sent to a peer, if any. + #[cfg(test)] + pub(crate) fn last_update_sent(&self, peer_id: &NodeAddr) -> Option { + self.last_update_sent.get(peer_id).copied() + } + /// Clear all pending updates. pub fn clear_pending_updates(&mut self) { self.pending_updates.clear(); diff --git a/src/proto/bloom/tests/state.rs b/src/proto/bloom/tests/state.rs index 9f8c2418..f1e19ee8 100644 --- a/src/proto/bloom/tests/state.rs +++ b/src/proto/bloom/tests/state.rs @@ -65,6 +65,31 @@ fn test_bloom_state_debounce() { assert!(state.should_send_update(&peer, 1600)); } +#[test] +fn should_send_update_after_backward_clock_step_waits_at_most_the_debounce() { + let node = make_node_addr(0); + let peer = make_node_addr(1); + let mut state = BloomState::new(node); + state.set_update_debounce_ms(500); + + state.mark_update_needed(peer); + state.record_update_sent(peer, 100_000); + state.mark_update_needed(peer); + + // A clock reading 60 s before the last send must not hold the update + // until the clock regains the 60 s. + assert!( + state.should_send_update(&peer, 40_000), + "a time before the last send must permit the send" + ); + + // Once restamped at the earlier time, the ordinary window applies again. + state.record_update_sent(peer, 40_000); + state.mark_update_needed(peer); + assert!(!state.should_send_update(&peer, 40_200)); + assert!(state.should_send_update(&peer, 40_500)); +} + #[test] fn test_bloom_state_sequence() { let node = make_node_addr(0); @@ -215,9 +240,10 @@ fn test_bloom_state_remove_peer_state() { // Pending updates cleared assert!(!state.needs_update(&peer)); - // Debounce state cleared — should be able to send immediately + // Debounce state cleared — should be able to send inside the window a + // surviving stamp of 1000 would still impose state.mark_update_needed(peer); - assert!(state.should_send_update(&peer, 0)); + assert!(state.should_send_update(&peer, 1200)); // Sent filter cleared — mark_changed_peers should treat as "never sent" state.clear_pending_updates(); diff --git a/src/proto/lookup/core.rs b/src/proto/lookup/core.rs index 437d6dd3..fc7f97f4 100644 --- a/src/proto/lookup/core.rs +++ b/src/proto/lookup/core.rs @@ -60,7 +60,8 @@ pub(crate) enum LookupAction { }, /// Reset the coords-warmup counter if an established session exists. ResetWarmupIfEstablished { target: NodeAddr }, - /// Retry queued TUN packets for the target if any are pending. + /// Retry queued TUN packets or native datagrams for the target if any are + /// pending. RetryQueuedPackets { target: NodeAddr }, } diff --git a/testing/deb-install/test.sh b/testing/deb-install/test.sh index 63977bd4..6e73d154 100755 --- a/testing/deb-install/test.sh +++ b/testing/deb-install/test.sh @@ -10,8 +10,10 @@ # through the resolver backend that fips-dns-setup configured. Then # exercises fips-gateway against the same daemon to verify the # gateway/daemon default-pairing. Finally it -# purges the package with the DNS routing file planted and fips-dns -# stopped, and checks the file is removed and systemd-resolved restarted. +# removes the package with the DNS routing file planted and fips-dns +# stopped, and checks the file is removed and systemd-resolved restarted, +# then plants the file again and purges the package from config-files +# state, with the same checks. # # This is the most thorough test surface — it exercises: # - cargo deb packaging (binary stripping, dependency declaration) @@ -19,7 +21,8 @@ # of /etc/fips/fips.yaml # - postinst maintainer scripts (systemd unit enablement, # fips-dns.service running fips-dns-setup) -# - postrm purge (removing the DNS routing fips-dns-setup wrote) +# - postrm remove and postrm purge (removing the DNS routing +# fips-dns-setup wrote) # - The fips, fips-dns, and (optionally) fips-gateway systemd units # - End-to-end .fips resolution as a real user would experience it # @@ -342,19 +345,155 @@ check_gateway_default_listener() { fi } -# Purge the package with the DNS routing file planted and fips-dns stopped, and +# Put the saved DNS routing file back at and restart systemd-resolved, +# then check the state in which removal leaves the file behind: the file in +# place, fips-dns.service not active, and the resolver routing .fips to +# [::1]:5354. Every failure is recorded with