fix(gateway): retry pending firewall updates

A failed NAT rebuild retained the requested change but waited for another mapping event before trying again. Track pending rebuilds and retry the latest desired state every ten seconds, clearing pending work only after success.

Cover encoding failure, kernel rejection, superseding changes, and recovery without another mapping event.

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

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

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 } => {

src/gateway/nat.rs

@@ -174,6 +174,8 @@ pub struct NatManager {
     mappings: HashMap<Ipv6Addr, NatMapping>,
     /// Inbound port-forward rules.
     port_forwards: Vec<PortForward>,
+    /// 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<bool, NatError> {
+        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();