[PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

The firewall configuration files are stored on pmxcfs, which is only
available when pve-cluster is up. To be able to restore firewall rules
before the network comes up and close the boot-time window in which the
PVE host is unprotected, dump the required configuration files to a
local directory after every apply.

The last dumped version is cached for each file, the daemon only
actually writes to disk if there has been a change since the last write
or the dump file does not exist.

Ownership of the dumps is keyed on the nftables option in the host
config. When the firewall is disabled, the dumps are only removed if
nftables is enabled - otherwise that responsibility is on pve-firewall.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 debian/dirs                                  |  1 +
 proxmox-firewall/Cargo.toml                  |  2 +-
 proxmox-firewall/src/bin/proxmox-firewall.rs | 73 ++++++++++++++++++--
 proxmox-firewall/src/config.rs               | 56 +++++++++++----
 proxmox-firewall/src/firewall.rs             |  4 ++
 5 files changed, 117 insertions(+), 19 deletions(-)
 create mode 100644 debian/dirs

diff --git a/debian/dirs b/debian/dirs
new file mode 100644
index 0000000..34355af
--- /dev/null
+++ b/debian/dirs
@@ -0,0 +1 @@
+/var/lib/pve/firewall
diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml
index fa7fd34..77ca285 100644
--- a/proxmox-firewall/Cargo.toml
+++ b/proxmox-firewall/Cargo.toml
@@ -22,9 +22,9 @@ proxmox-log.workspace = true
 proxmox-network-types.workspace = true
 proxmox-network-api = { workspace = true, features = [ "impl" ] }
 proxmox-nftables = { workspace = true, features = [ "config-ext" ] }
+proxmox-sys.workspace = true
 proxmox-systemd.workspace = true
 proxmox-ve-config.workspace = true
 
 [dev-dependencies]
 insta = { workspace = true, features = [ "json" ] }
-proxmox-sys.workspace = true
diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 27fd67d..5b20ca0 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -1,3 +1,6 @@
+use std::collections::HashMap;
+use std::os::unix::fs::PermissionsExt;
+use std::path::Path;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::time::{Duration, Instant};
@@ -5,11 +8,14 @@ use std::time::{Duration, Instant};
 use anyhow::{Context, Error, bail, format_err};
 use pico_args::Arguments;
 
-use proxmox_firewall::config::{FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader};
+use proxmox_firewall::config::{
+    DUMP_DIR, FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader,
+};
 use proxmox_firewall::firewall::Firewall;
 use proxmox_log as log;
 use proxmox_log::{LevelFilter, Logger};
 use proxmox_nftables::{NftClient, client::NftError};
+use proxmox_sys::fs;
 use proxmox_systemd::systemctl;
 use proxmox_ve_config::firewall::host::Config as HostConfig;
 
@@ -42,14 +48,58 @@ fn remove_firewall() -> Result<(), std::io::Error> {
     Ok(())
 }
 
+fn dump_config(cfg: &FirewallConfig, cache: &mut HashMap<&str, Vec<u8>>) {
+    let dump_dir = Path::new(DUMP_DIR);
+    if !dump_dir.exists()
+        && let Err(e) = fs::create_path(dump_dir, None, None)
+    {
+        log::warn!("could not create {DUMP_DIR}: {e} - config will not be dumped");
+        return;
+    }
+
+    if let Err(e) = std::fs::set_permissions(dump_dir, std::fs::Permissions::from_mode(0o700)) {
+        log::warn!("could not set permissions on {DUMP_DIR}: {e}");
+    }
+
+    for (bytes, name) in [
+        (cfg.cluster_raw(), "cluster.fw"),
+        (cfg.host_raw(), "host.fw"),
+        (cfg.sdn_raw(), "sdn.json"),
+    ] {
+        let out = dump_dir.join(name);
+        match bytes {
+            Some(b) if !out.exists() || b != cache.get(name).map(Vec::as_slice).unwrap_or(&[]) => {
+                match fs::replace_file(&out, b, fs::CreateOptions::new(), true) {
+                    Err(e) => log::warn!("could not dump {name} to {out:?}: {e}"),
+                    Ok(()) => {
+                        log::info!("successfully dumped {out:?}");
+                        cache.insert(name, b.to_vec());
+                    }
+                }
+            }
+            Some(_) => log::info!("no changes to {name} since last dump to {out:?}"),
+            None => {
+                log::info!("nothing to dump for {name}");
+                cache.remove(name);
+                _ = std::fs::remove_file(&out);
+            }
+        }
+    }
+}
+
 fn create_firewall_instance() -> Result<Firewall, Error> {
     let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
     Ok(Firewall::new(config))
 }
 
-fn handle_firewall() -> Result<(), Error> {
-    let firewall = create_firewall_instance()?;
+fn delete_dumps(cache: &mut HashMap<&str, Vec<u8>>) {
+    cache.clear();
+    for f in ["cluster.fw", "host.fw", "sdn.json"] {
+        _ = std::fs::remove_file(Path::new(DUMP_DIR).join(f));
+    }
+}
 
+fn handle_firewall(firewall: &Firewall) -> Result<(), Error> {
     if !firewall.is_enabled() {
         return remove_firewall().with_context(|| "could not remove firewall tables".to_string());
     }
@@ -95,6 +145,8 @@ fn run_firewall() -> Result<(), Error> {
     // we're disabled here without the need to parse the config, avoiding log-spam errors from that
     let force_disable_flag = std::path::Path::new(FORCE_DISABLE_FLAG_FILE);
 
+    let mut cache = HashMap::new();
+
     while !term.load(Ordering::Relaxed) {
         if force_disable_flag.exists() {
             if let Err(error) = remove_firewall() {
@@ -106,9 +158,18 @@ fn run_firewall() -> Result<(), Error> {
         }
         let start = Instant::now();
 
-        if let Err(error) = handle_firewall() {
-            log::error!("error updating firewall rules: {error:#}");
-        }
+        match create_firewall_instance() {
+            Err(e) => log::error!("could not load firewall configuration: {e}"),
+            Ok(fw) => match handle_firewall(&fw) {
+                Err(e) => log::error!("error updating firewall rules: {e:#}"),
+                Ok(()) if fw.is_enabled() => dump_config(fw.config(), &mut cache),
+                // is_enabled() is false, nftables set here means the cluster firewall is disabled
+                // and the dumps are owned by us, so we clean them up. With nftables unset, they
+                // would be owned by pve-firewall.
+                Ok(()) if fw.config().host().nftables() => delete_dumps(&mut cache),
+                Ok(()) => (),
+            },
+        };
 
         let duration = start.elapsed();
         log::info!("firewall update time: {}ms", duration.as_millis());
diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index 341e05a..eb8dd21 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 use std::default::Default;
 use std::fs::{self, DirEntry, File, ReadDir};
-use std::io::{self, BufReader};
+use std::io::{self, BufRead, BufReader};
 
 use anyhow::{Context, Error, bail, format_err};
 
@@ -74,6 +74,16 @@ fn open_config_file(path: &str) -> Result<Option<File>, Error> {
     }
 }
 
+fn read_opt(reader: Option<Box<dyn BufRead>>) -> Result<Option<Vec<u8>>, Error> {
+    reader
+        .map(|mut r| {
+            let mut buf = Vec::new();
+            r.read_to_end(&mut buf)?;
+            Ok(buf)
+        })
+        .transpose()
+}
+
 fn open_config_folder(path: &str) -> Result<Option<ReadDir>, Error> {
     match fs::read_dir(path) {
         Ok(paths) => Ok(Some(paths)),
@@ -104,6 +114,8 @@ const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config";
 const SDN_IPAM_PATH: &str = "/etc/pve/sdn/pve-ipam-state.json";
 const SDN_IPAM_PATH_LEGACY: &str = "/etc/pve/priv/ipam.db"; // TODO: remove with PVE 9+
 
+pub const DUMP_DIR: &str = "/var/lib/pve/firewall";
+
 impl FirewallConfigLoader for PveFirewallConfigLoader {
     fn cluster(&self) -> Result<Option<Box<dyn io::BufRead>>, Error> {
         log::info!("loading cluster config");
@@ -298,11 +310,14 @@ pub struct FirewallConfig {
     sdn_config: Option<FirewallSdnConfig>,
     ipam_config: Option<FirewallIpamConfig>,
     interface_mapping: AltnameMapping,
+    cluster_raw: Option<Vec<u8>>,
+    host_raw: Option<Vec<u8>>,
+    sdn_raw: Option<Vec<u8>>,
 }
 
 impl FirewallConfig {
-    fn parse_cluster(firewall_loader: &dyn FirewallConfigLoader) -> Result<ClusterConfig, Error> {
-        match firewall_loader.cluster()? {
+    fn parse_cluster(raw: Option<&[u8]>) -> Result<ClusterConfig, Error> {
+        match raw {
             Some(data) => ClusterConfig::parse(data),
             None => {
                 log::info!("no cluster config found, falling back to default");
@@ -311,8 +326,8 @@ impl FirewallConfig {
         }
     }
 
-    fn parse_host(firewall_loader: &dyn FirewallConfigLoader) -> Result<HostConfig, Error> {
-        match firewall_loader.host()? {
+    fn parse_host(raw: Option<&[u8]>) -> Result<HostConfig, Error> {
+        match raw {
             Some(data) => HostConfig::parse(data),
             None => {
                 log::info!("no host config found, falling back to default");
@@ -355,10 +370,8 @@ impl FirewallConfig {
         Ok(guests)
     }
 
-    pub fn parse_sdn(
-        firewall_loader: &dyn FirewallConfigLoader,
-    ) -> Result<Option<FirewallSdnConfig>, Error> {
-        Ok(match firewall_loader.sdn_running_config()? {
+    pub fn parse_sdn(raw: Option<&[u8]>) -> Result<Option<FirewallSdnConfig>, Error> {
+        Ok(match raw {
             Some(data) => {
                 let running_config: RunningConfig = serde_json::from_reader(data)?;
                 let config = SdnConfig::try_from(running_config)?;
@@ -437,15 +450,22 @@ impl FirewallConfig {
         firewall_loader: &dyn FirewallConfigLoader,
         nft_loader: &dyn NftConfigLoader,
     ) -> Result<Self, Error> {
+        let cluster_raw = read_opt(firewall_loader.cluster()?)?;
+        let host_raw = read_opt(firewall_loader.host()?)?;
+        let sdn_raw = read_opt(firewall_loader.sdn_running_config()?)?;
+
         Ok(Self {
-            cluster_config: Self::parse_cluster(firewall_loader)?,
-            host_config: Self::parse_host(firewall_loader)?,
+            cluster_config: Self::parse_cluster(cluster_raw.as_deref())?,
+            host_config: Self::parse_host(host_raw.as_deref())?,
             guest_config: Self::parse_guests(firewall_loader)?,
             bridge_config: Self::parse_bridges(firewall_loader)?,
-            sdn_config: Self::parse_sdn(firewall_loader)?,
+            sdn_config: Self::parse_sdn(sdn_raw.as_deref())?,
             ipam_config: Self::parse_ipam(firewall_loader)?,
             nft_config: Self::parse_nft(nft_loader)?,
             interface_mapping: firewall_loader.interface_mapping()?,
+            cluster_raw,
+            host_raw,
+            sdn_raw,
         })
     }
 
@@ -453,10 +473,18 @@ impl FirewallConfig {
         &self.cluster_config
     }
 
+    pub fn cluster_raw(&self) -> Option<&[u8]> {
+        self.cluster_raw.as_deref()
+    }
+
     pub fn host(&self) -> &HostConfig {
         &self.host_config
     }
 
+    pub fn host_raw(&self) -> Option<&[u8]> {
+        self.host_raw.as_deref()
+    }
+
     pub fn guests(&self) -> &BTreeMap<Vmid, GuestConfig> {
         &self.guest_config
     }
@@ -473,6 +501,10 @@ impl FirewallConfig {
         self.sdn_config.as_ref()
     }
 
+    pub fn sdn_raw(&self) -> Option<&[u8]> {
+        self.sdn_raw.as_deref()
+    }
+
     pub fn ipam(&self) -> Option<&FirewallIpamConfig> {
         self.ipam_config.as_ref()
     }
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 477b69b..7f3e633 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -63,6 +63,10 @@ impl Firewall {
         Self { config }
     }
 
+    pub fn config(&self) -> &FirewallConfig {
+        &self.config
+    }
+
     pub fn is_enabled(&self) -> bool {
         self.config.is_enabled()
     }
-- 
2.47.3