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 2108F1FF0E3 for ; Tue, 21 Jul 2026 15:55:15 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9BC33215F8; Tue, 21 Jul 2026 15:54:14 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-firewall 10/13] fix #5759: firewall: do not clear rules on system shutdown Date: Tue, 21 Jul 2026 15:54:04 +0200 Message-ID: <20260721135407.372150-11-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: XKM3Z5H5Q5HMIMMKDFWCAPXGIR455L5K X-Message-ID-Hash: XKM3Z5H5Q5HMIMMKDFWCAPXGIR455L5K 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 assumption until now was that the firewall rules should always be cleared when receiving SIGTERM. This is correct when SIGTERM is received as the result of `systemctl stop`, should however not be handled the same way in case of a system shutdown. Because of the way the proxmox-firewall service is ordered, it comes down before the network. Clearing the rules in that case creates a window where the network is up without firewall protection. Check the operational state of the system when receiving a stop signal and only clear the nftables rules if the system is not currently shutting down. Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5759 Signed-off-by: Arthur Bied-Charreton --- Cargo.toml | 1 + proxmox-firewall/Cargo.toml | 1 + proxmox-firewall/src/bin/proxmox-firewall.rs | 12 +++++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1dd2784..5f2fa1c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ proxmox-network-api = "1" proxmox-network-types = "1" proxmox-serde = "1" proxmox-sys = "1" +proxmox-systemd = "1.0.1" proxmox-ve-config = "0.10" # workspace crates diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml index 6ad7e79..fa7fd34 100644 --- a/proxmox-firewall/Cargo.toml +++ b/proxmox-firewall/Cargo.toml @@ -22,6 +22,7 @@ proxmox-log.workspace = true proxmox-network-types.workspace = true proxmox-network-api = { workspace = true, features = [ "impl" ] } proxmox-nftables = { workspace = true, features = [ "config-ext" ] } +proxmox-systemd.workspace = true proxmox-ve-config.workspace = true [dev-dependencies] diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs index e8ed477..27fd67d 100644 --- a/proxmox-firewall/src/bin/proxmox-firewall.rs +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs @@ -10,6 +10,7 @@ use proxmox_firewall::firewall::Firewall; use proxmox_log as log; use proxmox_log::{LevelFilter, Logger}; use proxmox_nftables::{NftClient, client::NftError}; +use proxmox_systemd::systemctl; use proxmox_ve_config::firewall::host::Config as HostConfig; const HELP: &str = r#" @@ -115,7 +116,16 @@ fn run_firewall() -> Result<(), Error> { std::thread::sleep(Duration::from_secs(5)); } - remove_firewall().with_context(|| "Could not remove firewall rules") + match systemctl::is_system_running() { + // Got SIGTERM as the result of a shutdown, do not remove rules. + Ok(systemctl::SystemState::Stopping) => { + log::info!("system is stopping, not removing firewall rules"); + Ok(()) + } + Err(e) => bail!("{e}"), + // System is not stopping, firewall was shut down explicitly, remove rules. + _ => remove_firewall().with_context(|| "could not remove firewall rules"), + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -- 2.47.3