public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH proxmox-firewall v2 11/12] firewall: add restore command
Date: Thu,  1 Oct 2026 14:26:24 +0200	[thread overview]
Message-ID: <20261001122625.348730-12-a.bied-charreton@proxmox.com> (raw)
In-Reply-To: <20261001122625.348730-1-a.bied-charreton@proxmox.com>

Add a 'restore' command that recompiles and applies the firewall from
the config dumped to /var/lib/pve/firewall, to bridge the boot window
before pmxcfs (and thus the real config) is available.

It reuses handle_firewall() along with a local firewall config loader.
The config files are loaded from the local dump instead of pmxcfs and
nothing is dumped back. A missing or unreadable SDN dump is treated as
empty and rules referencing an unavailable IPSet are skipped, so a
partial dump still restores the rest on a best-effort basis.

For the host config, a host.fw.new written by the
pve-network-interface-pinning tool takes precedence over the plain dump
when present. No .new file is ever created for the cluster config, so
that is only loaded from cluster.fw.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
Tested-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 proxmox-firewall/src/bin/proxmox-firewall.rs | 26 +++++-
 proxmox-firewall/src/config.rs               | 93 ++++++++++++++++++--
 2 files changed, 110 insertions(+), 9 deletions(-)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 1ac7c6c..423dfc7 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -9,7 +9,8 @@ use anyhow::{Context, Error, bail, format_err};
 use pico_args::Arguments;
 
 use proxmox_firewall::config::{
-    DUMP_DIR, FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader,
+    DUMP_DIR, FirewallConfig, LocalFirewallConfigLoader, PveFirewallConfigLoader,
+    PveNftConfigLoader,
 };
 use proxmox_firewall::firewall::Firewall;
 use proxmox_log as log;
@@ -29,6 +30,7 @@ COMMANDS:
   compile           Compile and print firewall rules as accepted by 'nft -j -f -'
   start             Execute proxmox-firewall service in foreground
   localnet          Print the contents of the management ipset
+  restore           Attempt to restore the last remembered ruleset
 "#;
 
 const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft");
@@ -87,6 +89,14 @@ fn dump_config(cfg: &FirewallConfig, cache: &mut HashMap<&str, Vec<u8>>) {
     }
 }
 
+fn create_local_firewall_instance() -> Result<Firewall, Error> {
+    let config = FirewallConfig::new(
+        &LocalFirewallConfigLoader::new(),
+        &PveNftConfigLoader::new(),
+    )?;
+    Ok(Firewall::new(config))
+}
+
 fn create_firewall_instance() -> Result<Firewall, Error> {
     let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
     Ok(Firewall::new(config))
@@ -194,6 +204,17 @@ fn run_firewall() -> Result<(), Error> {
     }
 }
 
+fn restore() -> Result<(), Error> {
+    let fw = create_local_firewall_instance()?;
+    match fw.is_enabled() {
+        true => handle_firewall(&fw),
+        false => {
+            log::info!("nftables firewall is not enabled, not restoring rules");
+            Ok(())
+        }
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Command {
     Compile,
@@ -201,6 +222,7 @@ pub enum Command {
     Skeleton,
     Start,
     Localnet,
+    Restore,
 }
 
 impl std::str::FromStr for Command {
@@ -213,6 +235,7 @@ impl std::str::FromStr for Command {
             "skeleton" => Command::Skeleton,
             "start" => Command::Start,
             "localnet" => Command::Localnet,
+            "restore" => Command::Restore,
             cmd => {
                 bail!("{cmd} is not a valid command")
             }
@@ -245,6 +268,7 @@ fn run_command(command: Command) -> Result<(), Error> {
                 println!("{ip}");
             }
         }
+        Command::Restore => restore()?,
     };
 
     Ok(())
diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index eb8dd21..d6fe166 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -1,7 +1,8 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
 use std::default::Default;
 use std::fs::{self, DirEntry, File, ReadDir};
 use std::io::{self, BufRead, BufReader};
+use std::path::{Path, PathBuf};
 
 use anyhow::{Context, Error, bail, format_err};
 
@@ -60,15 +61,16 @@ impl PveFirewallConfigLoader {
 /// opens a configuration file
 ///
 /// It returns a file handle to the file or [`None`] if it doesn't exist.
-fn open_config_file(path: &str) -> Result<Option<File>, Error> {
-    match File::open(path) {
+fn open_config_file<P: Into<PathBuf>>(path: P) -> Result<Option<File>, Error> {
+    let path: PathBuf = path.into();
+    match File::open(&path) {
         Ok(data) => Ok(Some(data)),
         Err(err) if err.kind() == io::ErrorKind::NotFound => {
-            log::info!("config file does not exist: {path}");
+            log::info!("config file does not exist: {path:?}");
             Ok(None)
         }
         Err(err) => {
-            let context = format!("unable to open configuration file at {path}");
+            let context = format!("unable to open configuration file at {path:?}");
             Err(anyhow::Error::new(err).context(context))
         }
     }
@@ -155,7 +157,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
     ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
         log::info!("loading guest #{vmid} config");
 
-        let fd = open_config_file(&GuestMap::config_path(vmid, entry))?;
+        let fd = open_config_file(GuestMap::config_path(vmid, entry))?;
 
         if let Some(file) = fd {
             let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -168,7 +170,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
     fn guest_firewall_config(&self, vmid: &Vmid) -> Result<Option<Box<dyn io::BufRead>>, Error> {
         log::info!("loading guest #{vmid} firewall config");
 
-        let fd = open_config_file(&GuestMap::firewall_config_path(vmid))?;
+        let fd = open_config_file(GuestMap::firewall_config_path(vmid))?;
 
         if let Some(file) = fd {
             let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -230,7 +232,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
     ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
         log::info!("loading firewall config for bridge {bridge_name}");
 
-        let fd = open_config_file(&format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?;
+        let fd = open_config_file(format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?;
 
         if let Some(file) = fd {
             let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -247,6 +249,81 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
     }
 }
 
+#[derive(Debug, Default)]
+pub struct LocalFirewallConfigLoader {}
+
+impl LocalFirewallConfigLoader {
+    pub fn new() -> Self {
+        Self::default()
+    }
+}
+
+fn load_first_of(candidates: &[&str], what: &str) -> Result<Option<Box<dyn BufRead>>, Error> {
+    for p in candidates.iter().map(|p| Path::new(DUMP_DIR).join(p)) {
+        if let Some(fd) = open_config_file(&p)? {
+            log::info!("loaded {what} config from {p:?}");
+            let reader = Box::new(BufReader::new(fd)) as Box<dyn BufRead>;
+            return Ok(Some(reader));
+        }
+    }
+    Ok(None)
+}
+
+impl FirewallConfigLoader for LocalFirewallConfigLoader {
+    fn cluster(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+        load_first_of(&["cluster.fw"], "cluster")
+    }
+
+    fn host(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+        load_first_of(&["host.fw.new", "host.fw"], "host")
+    }
+
+    fn sdn_running_config(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+        let path = Path::new(DUMP_DIR).join("sdn.json");
+        if let Some(fd) = open_config_file(&path)? {
+            let reader = Box::new(BufReader::new(fd)) as Box<dyn BufRead>;
+            return Ok(Some(reader));
+        }
+
+        Ok(None)
+    }
+
+    fn guest_config(
+        &self,
+        _: &Vmid,
+        _: &GuestEntry,
+    ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+        Ok(None)
+    }
+
+    fn guest_firewall_config(&self, _: &Vmid) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+        Ok(None)
+    }
+
+    fn guest_list(&self) -> Result<GuestMap, Error> {
+        Ok(GuestMap::from(HashMap::new()))
+    }
+
+    fn bridge_firewall_config(
+        &self,
+        _: &BridgeName,
+    ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+        Ok(None)
+    }
+
+    fn bridge_list(&self) -> Result<Vec<BridgeName>, Error> {
+        Ok(vec![])
+    }
+
+    fn ipam(&self) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+        Ok(None)
+    }
+
+    fn interface_mapping(&self) -> Result<AltnameMapping, Error> {
+        Ok(AltnameMapping::from_iter([]))
+    }
+}
+
 pub trait NftConfigLoader {
     fn chains(&self) -> Result<Option<CommandOutput>, Error>;
 }
-- 
2.47.3





  parent reply	other threads:[~2026-10-01 12:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01 12:26 [RFC firewall/manager/proxmox{,-firewall} v2 00/12] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-manager v2 01/12] network interface pinning: write new firewall config to local dir Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-firewall v2 02/12] firewall: config: sort OPTIONS when serializing Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-firewall v2 03/12] firewall: dump configs locally after applying Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-firewall v2 04/12] fix #5759: firewall: do not remove chains when host is shutting down Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-firewall v2 05/12] firewall: add restore command Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH pve-firewall v2 06/12] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH proxmox v2 07/12] systemd: systemctl: add is-system-running helper Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH proxmox-firewall v2 08/12] firewall: fix clippy warnings Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH proxmox-firewall v2 09/12] fix #5759: firewall: do not clear rules on system shutdown Arthur Bied-Charreton
2026-10-01 12:26 ` [PATCH proxmox-firewall v2 10/12] firewall: dump config to local directory after apply Arthur Bied-Charreton
2026-10-01 12:26 ` Arthur Bied-Charreton [this message]
2026-10-01 12:26 ` [PATCH proxmox-firewall v2 12/12] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261001122625.348730-12-a.bied-charreton@proxmox.com \
    --to=a.bied-charreton@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal