[RFC firewall/manager/proxmox{,-firewall} v2 00/12] fix #5759: keep firewall rules up across boot and shutdown

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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