[RFC firewall/manager/proxmox{,-firewall} 00/13] 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`.

Customizability:

{cluster,host}.fw.override configurations can be written to
/var/lib/pve/firewall to allow custom rulesets for the boot-time window.

Open questions:

1. Boot-time firewall config overrides:

I'm honestly not sure if the .override hack is the right design.
host.fw.override may drift away from the config after network interface
pinning changes. This can/will be fixed if we decide to go with this
approach, but I am not sure how I feel about overwriting a config file
manually edited by users.

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 (deliberately out of scope for this series):

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.

Proper wait conditions:

We currently have no way to query the firewall status. This leads to
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, which may be running for up to 5
seconds (i.e. until the next apply, assuming nftables firewall) without
protection.

Firewall status reporting:

As discussed off-list with Stefan H., 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. We should be
able to get this for free with the wait conditions.

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


pve-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 (6):
  firewall: config: sort OPTIONS when serializing
  d/control: bump libpve-common-perl
  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/control                          |   2 +-
 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                        |  10 ++-
 7 files changed, 176 insertions(+), 21 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 | 109 +++++++++++-
 proxmox-firewall/src/config.rs               | 167 ++++++++++++++++---
 proxmox-firewall/src/firewall.rs             |   8 +-
 proxmox-firewall/src/rule.rs                 |  38 ++---
 9 files changed, 295 insertions(+), 50 deletions(-)
 create mode 100644 debian/dirs
 create mode 100644 debian/proxmox-firewall-pre-network.service


Summary over all repositories:
  22 files changed, 580 insertions(+), 72 deletions(-)

-- 
Generated by murpp 0.12.0