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 870931FF0BC for ; Thu, 15 Oct 2026 14:46:12 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 68AA321666; Thu, 15 Oct 2026 14:45:39 +0200 (CEST) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [RFC firewall/manager/proxmox{,-firewall} v2 00/12] fix #5759: keep firewall rules up across boot and shutdown From: Stefan Hanreich To: Arthur Bied-Charreton In-Reply-To: <20261001122625.348730-1-a.bied-charreton@proxmox.com> References: <20261001122625.348730-1-a.bied-charreton@proxmox.com> Date: Thu, 15 Oct 2026 14:45:23 +0200 Message-Id: <179206832307.200960.4122756112896374851.b4-review@b4> X-Mailer: b4 0.16-dev-fbf84 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792068330963 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.480 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: Q2JIJYNT4SLO4H2XMYNTDCIJ4FZCXP7H X-Message-ID-Hash: Q2JIJYNT4SLO4H2XMYNTDCIJ4FZCXP7H X-MailFrom: s.hanreich@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 CC: pve-devel@lists.proxmox.com 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. What about the ipam state? For pve-firewall this works since we dump the generated IPSets, for proxmox-firewall the IPAM state is required as well. > > 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. I think for a start just having this feature alone is alright, adding node-local override files specifically for startup could be done in a follow-up as well if the need for it arises. > > 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? People wanting to opt out could simply mask the service I'd say? > > 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. The firewall basically reloads every cycle anyway? The main problem I see are updates where the service gets re-started everytime so there's still a small window during updates where the host is left unprotected. -- Stefan Hanreich