From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 29D361FF0E3 for ; Tue, 21 Jul 2026 15:54:52 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 87E5C2158E; Tue, 21 Jul 2026 15:54:13 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply Date: Tue, 21 Jul 2026 15:54:05 +0200 Message-ID: <20260721135407.372150-12-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260721135407.372150-1-a.bied-charreton@proxmox.com> References: <20260721135407.372150-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 2 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: IY7FGECHVLDLF7OR6R4WJENUOWIHM5TK X-Message-ID-Hash: IY7FGECHVLDLF7OR6R4WJENUOWIHM5TK X-MailFrom: abied-charreton@jett.proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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>) { + 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 { 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>) { + 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, Error> { } } +fn read_opt(reader: Option>) -> Result>, 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, 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>, Error> { log::info!("loading cluster config"); @@ -298,11 +310,14 @@ pub struct FirewallConfig { sdn_config: Option, ipam_config: Option, interface_mapping: AltnameMapping, + cluster_raw: Option>, + host_raw: Option>, + sdn_raw: Option>, } impl FirewallConfig { - fn parse_cluster(firewall_loader: &dyn FirewallConfigLoader) -> Result { - match firewall_loader.cluster()? { + fn parse_cluster(raw: Option<&[u8]>) -> Result { + 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 { - match firewall_loader.host()? { + fn parse_host(raw: Option<&[u8]>) -> Result { + 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, Error> { - Ok(match firewall_loader.sdn_running_config()? { + pub fn parse_sdn(raw: Option<&[u8]>) -> Result, 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 { + 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 { &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