From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 601B61FF0A3 for ; Thu, 01 Oct 2026 14:27:42 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id ACA4321814; Thu, 01 Oct 2026 14:26:47 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [RFC firewall/manager/proxmox{,-firewall} v2 00/12] fix #5759: keep firewall rules up across boot and shutdown Date: Thu, 1 Oct 2026 14:26:13 +0200 Message-ID: <20261001122625.348730-1-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790857599484 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.146 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ITS4R6IPLAWPZONI5IAV23UM6ZPZD5TB X-Message-ID-Hash: ITS4R6IPLAWPZONI5IAV23UM6ZPZD5TB X-MailFrom: a.bied-charreton@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: Both the nftables and iptables firewall services depend on pve-cluster, which depends on corosync, which itself depends on network-online. This means that, as reported in bug #5759, there currently are windows during both system boot and shutdown where PVE hosts have no firewall protection. Approach: Both windows need to be closed without reordering the firewall services against pve-cluster (not feasible given the pmxcfs dependency). 1. Boot: After every successful apply, each daemon dumps the config files it compiled from (cluster.fw, host.fw, sdn.json) to a local directory on the root filesystem (/var/lib/pve/firewall), available before pmxcfs. The guest configurations are not dumped, since pve-guests starts after the firewall daemon anyway. A new oneshot service ordered before network-pre.target runs the newly added restore command, which recompiles and applies rules from those local dumps. 2. Shutdown On SIGTERM the daemons now query `systemctl is-system-running`. If the system is shutting down they leave the ruleset in place (the service otherwise comes down before the network does, which reopens the window), and only clear the rules on an explicit `systemctl stop`. An explicit stop also removes the dumps, so a ruleset the admin deliberately took down is not restored at the next boot, where no daemon might be around to clear it again. Each daemon only removes the dumps if it wrote them itself, so stopping the inactive backend leaves the active one's dumps alone. Open questions: 1. Customizability: Currently, the boot-time config mirrors the runtime config. Should the boot-time config be customizable? This would mean we would have to manage node-local configuration files, which can be okay but I am not quite sure this is needed. 2. Is this a breaking change? The fact that the firewall rulesets are not up during boot/shutdown is not documented, however it has been a property of PVE for a while and changing this might break some systems. Is this a change that needs to wait or at least be made opt-in until PVE 10? 3. SDN config pve-firewall and proxmox-firewall dump sdn.json in different formats: pve-firewall writes the parsed SDN IPSets, proxmox-firewall writes the raw running-config it reads from pmxcfs. Only the active backend ever writes the dumps, so the formats only clash right after switching backends, before the now-active daemon has rewritten sdn.json in its own format. A reboot in this window makes the restore read the previous backend's format. Neither side errors on the foreign format; both just read it as an empty SDN config, so the restore drops only the rules that reference SDN IPSets and applies the rest. For a best-effort boot-time restore, and given the plan to sunset pve-firewall, I think that is tolerable, so I left the proper fix out of this series: unifying the format would mean teaching pve-firewall to parse the raw running-config through a new proxmox-ve-rs binding. Happy to revisit if deemed necessary. Future work (out of scope for this series): Firewall status reporting: The firewall currently has no status reporting. Rules that fail to parse are logged to the journal and skipped, those failures are not visible anywhere in the UI. Proper wait conditions: There are currently windows where a guest may run without its firewall rules being applied. For example, when migrating a guest, we currently have no way to know whether its firewall rules have been applied on the target host already. The same applies to starting a guest, where it could be running for up to 5 seconds (i.e. until the next apply) without protection. This would build on the status reporting to query the firewall's status upfront before starting/migrating/... a guest. ExecReload: A remaining "edge case" is restarting the service, since systemctl restart just enqueues a stop and a start job, it is a little trickier to figure out that the service is being restarted as opposed to just stopped. One could technically query systemd for a queued start job on SIGTERM, but as far as I can tell this would be racy, and I believe a firewall not being up while the service is being restarted is a defensible position. The better direction, in my opinion, is a proper ExecReload implementation. Stefan H. and I discussed adding a proper runtime to proxmox-firewall and reworking the signal handling, this would fit well together with such a change. Build dependencies: - proxmox-firewall requires bumped proxmox-systemd Runtime dependencies: - proxmox-firewall requires bumped pve-manager - pve-firewall requires bumped pve-manager - pve-firewall requires bumped libpve-common-perl (for PVE::File) Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5759 Changes since [v1]: - Rebase - Remove .override logic in favor of an open question in the cover letter - this should be integrated into the UI if we decide to let users configure the boot-time firewall differently - Adapt log levels (promote some `warn`s to `error`s and demote some `info`s to `debug`s), and drop pve-firewall's "no changes since last dump" message, since journald stores debug messages by default and it would be logged three times per update cycle - Clarify .new handling in commit messages - Remove the dumps on an explicit stop in proxmox-firewall too, and only if the stopping daemon owns them (pve-firewall previously removed them unconditionally, including proxmox-firewall's) - Drop libpve-common-perl bump commit [v1] https://lore.proxmox.com/pve-devel/20260721135407.372150-1-a.bied-charreton@proxmox.com/ manager: Arthur Bied-Charreton (1): network interface pinning: write new firewall config to local dir PVE/CLI/pve_network_interface_pinning.pm | 6 +++++- bin/pve-firewall-commit | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) pve-firewall: Arthur Bied-Charreton (5): firewall: config: sort OPTIONS when serializing firewall: dump configs locally after applying fix #5759: firewall: do not remove chains when host is shutting down firewall: add restore command fix #5759: firewall: restore from dumped config before network-pre debian/dirs | 1 + debian/pve-firewall-pre-network.service | 16 ++++ debian/rules | 1 + src/PVE/Firewall.pm | 112 +++++++++++++++++++++--- src/PVE/Service/pve_firewall.pm | 55 +++++++++++- src/pve-firewall | 9 +- 6 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 debian/pve-firewall-pre-network.service proxmox: Arthur Bied-Charreton (1): systemd: systemctl: add is-system-running helper proxmox-systemd/Cargo.toml | 1 + proxmox-systemd/debian/control | 2 + proxmox-systemd/src/lib.rs | 2 + proxmox-systemd/src/systemctl.rs | 98 ++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 proxmox-systemd/src/systemctl.rs proxmox-firewall: Arthur Bied-Charreton (5): firewall: fix clippy warnings fix #5759: firewall: do not clear rules on system shutdown firewall: dump config to local directory after apply firewall: add restore command fix #5759: firewall: restore from dumped config before network-pre Cargo.toml | 1 + debian/dirs | 1 + debian/proxmox-firewall-pre-network.service | 16 ++ debian/rules | 2 +- proxmox-firewall/Cargo.toml | 3 +- proxmox-firewall/src/bin/proxmox-firewall.rs | 114 +++++++++++++- proxmox-firewall/src/config.rs | 149 ++++++++++++++++--- proxmox-firewall/src/firewall.rs | 8 +- proxmox-firewall/src/rule.rs | 38 ++--- 9 files changed, 282 insertions(+), 50 deletions(-) create mode 100644 debian/dirs create mode 100644 debian/proxmox-firewall-pre-network.service Summary over all repositories: 21 files changed, 565 insertions(+), 71 deletions(-) -- Generated by murpp 0.12.1