all lists on 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 12/13] firewall: add restore command
Date: Tue, 21 Jul 2026 15:54:06 +0200	[thread overview]
Message-ID: <20260721135407.372150-13-a.bied-charreton@proxmox.com> (raw)
In-Reply-To: <20260721135407.372150-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 cluster and host config, a user-provided .override or a .new
written by the pve-network-interface-pinning tool take precedence over
the plain dump when present (.override -> .new -> plain).

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 proxmox-firewall/src/bin/proxmox-firewall.rs |  26 ++++-
 proxmox-firewall/src/config.rs               | 111 +++++++++++++++++--
 2 files changed, 128 insertions(+), 9 deletions(-)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 5b20ca0..e96083c 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, LocalNftConfigLoader,
+    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(),
+        &LocalNftConfigLoader::new(),
+    )?;
+    Ok(Firewall::new(config))
+}
+
 fn create_firewall_instance() -> Result<Firewall, Error> {
     let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
     Ok(Firewall::new(config))
@@ -189,6 +199,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,
@@ -196,6 +217,7 @@ pub enum Command {
     Skeleton,
     Start,
     Localnet,
+    Restore,
 }
 
 impl std::str::FromStr for Command {
@@ -208,6 +230,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")
             }
@@ -240,6 +263,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..83b9161 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.override", "cluster.fw"], "cluster")
+    }
+
+    fn host(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+        load_first_of(&["host.fw.override", "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>;
 }
@@ -271,6 +348,24 @@ impl NftConfigLoader for PveNftConfigLoader {
     }
 }
 
+#[derive(Debug, Default)]
+pub struct LocalNftConfigLoader {}
+
+impl LocalNftConfigLoader {
+    pub fn new() -> Self {
+        Self::default()
+    }
+}
+
+impl NftConfigLoader for LocalNftConfigLoader {
+    fn chains(&self) -> Result<Option<CommandOutput>, Error> {
+        let commands = Commands::new(vec![List::chains()]);
+
+        NftClient::run_json_commands(&commands)
+            .with_context(|| "unable to query nft chains".to_string())
+    }
+}
+
 pub struct FirewallSdnConfig {
     _config: SdnConfig,
     ipsets: BTreeMap<String, Ipset>,
-- 
2.47.3




  parent reply	other threads:[~2026-07-21 13:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-manager 01/13] network interface pinning: write new firewall config to local dir Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 02/13] firewall: config: sort OPTIONS when serializing Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 03/13] d/control: bump libpve-common-perl Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 04/13] firewall: dump configs locally after applying Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 05/13] fix #5759: firewall: do not remove chains when host is shutting down Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH pve-firewall 06/13] firewall: add restore command Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH pve-firewall 07/13] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox 08/13] systemd: systemctl: add is-system-running helper Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 09/13] firewall: fix clippy warnings Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 10/13] fix #5759: firewall: do not clear rules on system shutdown Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply Arthur Bied-Charreton
2026-07-21 13:54 ` Arthur Bied-Charreton [this message]
2026-07-21 13:54 ` [PATCH proxmox-firewall 13/13] 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=20260721135407.372150-13-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal