diff --git a/CHANGELOG.md b/CHANGELOG.md index 5866b61e..8444ac8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +#### Gateway + +- 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. + #### Linux packages - The systemd tarball's `install.sh` restarts fips-dns and fips-gateway after diff --git a/docs/design/fips-gateway.md b/docs/design/fips-gateway.md index d3530135..34d15f70 100644 --- a/docs/design/fips-gateway.md +++ b/docs/design/fips-gateway.md @@ -458,8 +458,10 @@ sequence is: batch, and a batch too large for any send buffer is refused before it is sent. A batch the kernel refuses leaves the previous table in place; the failure is logged, and the gateway keeps its - record of the change, so the next rebuild that succeeds applies - it. + record of the change. Pending changes are retried every ten seconds, + even when no new mapping event arrives. Each retry rebuilds the latest + desired state, so a newer change supersedes an earlier failed one. + Successful rebuilds clear the pending flag; clean tables are not retried. The rustables crate does not expose rule-handle tracking, so incremental update of individual rules is not available. Atomic diff --git a/src/bin/fips-gateway.rs b/src/bin/fips-gateway.rs index 3d2b784d..60173841 100644 --- a/src/bin/fips-gateway.rs +++ b/src/bin/fips-gateway.rs @@ -545,8 +545,17 @@ async fn main() { info!("fips-gateway running"); let mut exit_code = 0; + let mut nat_retry = tokio::time::interval(std::time::Duration::from_secs(10)); + nat_retry.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); loop { tokio::select! { + _ = nat_retry.tick() => { + match nat_mgr.retry_pending() { + Ok(true) => info!("Applied pending NAT rules"), + Ok(false) => {}, + Err(e) => warn!(error = %e, "Failed to retry pending NAT rules"), + } + } Some(event) = event_rx.recv() => { match event { pool::PoolEvent::MappingCreated { virtual_ip, mesh_addr } => { diff --git a/src/gateway/nat.rs b/src/gateway/nat.rs index 7a0d2770..e7073c27 100644 --- a/src/gateway/nat.rs +++ b/src/gateway/nat.rs @@ -174,6 +174,8 @@ pub struct NatManager { mappings: HashMap, /// Inbound port-forward rules. port_forwards: Vec, + /// Desired state has not yet been acknowledged by the kernel. + rebuild_pending: bool, } impl NatManager { @@ -199,6 +201,7 @@ impl NatManager { lan_interface, mappings: HashMap::new(), port_forwards: Vec::new(), + rebuild_pending: false, } } @@ -222,7 +225,7 @@ impl NatManager { /// the nftables table atomically. Pass an empty slice to clear. pub fn set_port_forwards(&mut self, forwards: &[PortForward]) -> Result<(), NatError> { self.port_forwards = forwards.to_vec(); - self.rebuild()?; + self.rebuild_desired()?; info!( count = self.port_forwards.len(), "Applied inbound port forwards" @@ -307,6 +310,18 @@ impl NatManager { self.mappings.len() } + /// Retry a failed rebuild using the latest desired state. + /// + /// Returns whether a pending rebuild was applied. A clean manager does + /// not open a socket or rebuild the table. + pub fn retry_pending(&mut self) -> Result { + if !self.rebuild_pending { + return Ok(false); + } + self.rebuild_desired()?; + Ok(true) + } + /// The objects a rebuild sends, grouped into the batches that carry them. /// /// One batch, always. The kernel applies a batch as a single transaction, @@ -497,12 +512,19 @@ impl NatManager { /// Rebuild, returning the outcome with the time the rebuild took in /// microseconds, so a mapping change can log its cost on either path. - fn timed_rebuild(&self) -> (Result<(), NatError>, u64) { + fn timed_rebuild(&mut self) -> (Result<(), NatError>, u64) { let started = Instant::now(); - let result = self.rebuild(); + let result = self.rebuild_desired(); let elapsed_us = u64::try_from(started.elapsed().as_micros()).unwrap_or(u64::MAX); (result, elapsed_us) } + + fn rebuild_desired(&mut self) -> Result<(), NatError> { + self.rebuild_pending = true; + self.rebuild()?; + self.rebuild_pending = false; + Ok(()) + } } /// Largest batch, in bytes, that the kernel can admit in one send. @@ -883,6 +905,117 @@ mod tests { mgr } + #[test] + fn failed_rebuild_retains_latest_desired_state_for_retry() { + let mut mgr = manager_with_mappings(2); + // Make encoding fail before opening a socket. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(!mgr.retry_pending().unwrap(), "clean retry must not encode"); + + assert!(mgr.add_mapping(vip(3), mesh(3)).is_err()); + assert!(mgr.rebuild_pending); + assert!(mgr.retry_pending().is_err()); + assert!(mgr.rebuild_pending); + assert!(mgr.remove_mapping(vip(3)).is_err()); + assert!(!mgr.mappings.contains_key(&vip(3))); + assert!(mgr.add_mapping(vip(2), mesh(99)).is_err()); + assert_eq!(mgr.mappings[&vip(2)].mesh_addr, mesh(99)); + assert!(mgr.remove_mapping(vip(4)).is_err()); + assert!( + mgr.rebuild_pending, + "an absent mapping must not clear pending work" + ); + } + + #[test] + fn failed_port_forward_rebuild_remains_pending() { + let mut mgr = manager_with_mappings(1); + mgr.pre_chain = Chain::new(&mgr.table); + let forward = PortForward { + proto: Proto::Tcp, + listen_port: 8080, + target: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 80, 0, 0), + }; + assert!(mgr.set_port_forwards(&[forward]).is_err()); + assert_eq!(mgr.port_forwards.len(), 1); + assert!(mgr.rebuild_pending); + assert!(mgr.set_port_forwards(&[]).is_err()); + assert!(mgr.port_forwards.is_empty()); + assert!(mgr.rebuild_pending); + } + + #[test] + #[ignore = "requires CAP_NET_ADMIN and nft in an isolated network namespace"] + fn kernel_rejection_retries_latest_state_without_another_mapping_event() { + let mut mgr = manager_with_mappings(2); + let table_name = format!("{TABLE_NAME}_retry_test_{}", std::process::id()); + mgr.table = Table::new(ProtocolFamily::Inet).with_name(&table_name); + mgr.pre_chain = Chain::new(&mgr.table) + .with_name(PREROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PreRouting, DSTNAT_PRIORITY)); + mgr.post_chain = Chain::new(&mgr.table) + .with_name(POSTROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PostRouting, SRCNAT_PRIORITY)); + mgr.rebuild().unwrap(); + let listing = || { + let output = std::process::Command::new("nft") + .args(["-j", "list", "table", "inet", &table_name]) + .output() + .unwrap(); + assert!(output.status.success()); + output.stdout + }; + let before = listing(); + // Encoding succeeds, but the kernel rejects an overlong chain name. + let invalid = Chain::new(&mgr.table).with_name("x".repeat(300)); + let original = std::mem::replace(&mut mgr.pre_chain, invalid); + assert!(matches!( + mgr.add_mapping(vip(3), mesh(3)), + Err(NatError::Kernel { .. }) + )); + assert!(mgr.remove_mapping(vip(1)).is_err()); + assert!(mgr.remove_mapping(vip(3)).is_err()); + assert!(mgr.add_mapping(vip(2), mesh(99)).is_err()); + assert!(mgr.retry_pending().is_err()); + assert_eq!( + listing(), + before, + "rejection leaves the old kernel table intact" + ); + + mgr.pre_chain = original; + assert!(mgr.retry_pending().unwrap()); + assert!(!mgr.retry_pending().unwrap()); + assert_eq!(mgr.mapping_count(), 1); + assert_eq!(mgr.mappings[&vip(2)].mesh_addr, mesh(99)); + let after = listing(); + assert_ne!(after, before); + let table: serde_json::Value = serde_json::from_slice(&after).unwrap(); + assert_eq!( + table["nftables"] + .as_array() + .unwrap() + .iter() + .filter(|entry| entry.get("rule").is_some()) + .count(), + 3 + ); + assert!(String::from_utf8(after).unwrap().contains("fd02::63")); + + // A successful normal update also clears earlier pending work. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(mgr.add_mapping(vip(4), mesh(4)).is_err()); + mgr.pre_chain = Chain::new(&mgr.table) + .with_name(PREROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PreRouting, DSTNAT_PRIORITY)); + mgr.remove_mapping(vip(4)).unwrap(); + assert!(!mgr.retry_pending().unwrap()); + mgr.cleanup().unwrap(); + } + #[test] fn rebuild_deletes_and_recreates_the_table_inside_one_batch() { let batches = manager_with_mappings(3).rebuild_batches();