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 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 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