* [PATCH pve-manager 01/13] network interface pinning: write new firewall config to local dir
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
@ 2026-07-21 13:53 ` Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 02/13] firewall: config: sort OPTIONS when serializing Arthur Bied-Charreton
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:53 UTC (permalink / raw)
To: pve-devel
Preparatory step for restoring firewall rules before pve-cluster is up.
The boot-time restore recompiles the ruleset from the firewall config
dumped to local disk. Interface name pinnings only take effect on the
next boot, so a host.fw dumped before a pinning change still carries the
old interface names, and recompiling from it would produce rules
matching interfaces that no longer exist.
pve-network-interface-pinning already writes the updated config to
host.fw.new. Also write it to the local dump directory so the boot-time
restore prefers it over the stale host.fw; the local copy is removed
again in pve-firewall-commit once the pinning has been committed.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
PVE/CLI/pve_network_interface_pinning.pm | 6 +++++-
bin/pve-firewall-commit | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/PVE/CLI/pve_network_interface_pinning.pm b/PVE/CLI/pve_network_interface_pinning.pm
index 9dff181d..758b2107 100644
--- a/PVE/CLI/pve_network_interface_pinning.pm
+++ b/PVE/CLI/pve_network_interface_pinning.pm
@@ -22,6 +22,8 @@ use base qw(PVE::CLIHandler);
my $PVEETH_LOCK = "/run/lock/proxmox-network-interface-pinning.lck";
+my $local_dump_dir = "/var/lib/pve/firewall";
+
sub setup_environment {
PVE::RPCEnvironment->setup_default_cli_env();
}
@@ -120,7 +122,7 @@ my sub update_host_fw_config {
my ($mapping) = @_;
my $local_node = PVE::INotify::nodename();
- print "Updating /etc/pve/nodes/$local_node/host.fw.new\n";
+ print "Updating /etc/pve/nodes/$local_node/host.fw.new and $local_dump_dir/host.fw.new\n";
my $code = sub {
my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
@@ -143,6 +145,8 @@ my sub update_host_fw_config {
}
PVE::Firewall::save_hostfw_conf($host_conf, "/etc/pve/nodes/$local_node/host.fw.new");
+ make_path($local_dump_dir, { mode => 0700 });
+ PVE::Firewall::save_hostfw_conf($host_conf, "$local_dump_dir/host.fw.new");
};
PVE::Firewall::run_locked($code);
diff --git a/bin/pve-firewall-commit b/bin/pve-firewall-commit
index 3d208f67..cbb71f58 100644
--- a/bin/pve-firewall-commit
+++ b/bin/pve-firewall-commit
@@ -23,5 +23,6 @@ if (-e $new_fw_config_file) {
rename($new_fw_config_file, $current_fw_config_file)
or die "failed to commit new local node firewall config '$new_fw_config_file' - $!\n";
}
+unlink "/var/lib/pve/firewall/host.fw.new";
exit 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 02/13] firewall: config: sort OPTIONS when serializing
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-manager 01/13] network interface pinning: write new firewall config to local dir Arthur Bied-Charreton
@ 2026-07-21 13:53 ` Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 03/13] d/control: bump libpve-common-perl Arthur Bied-Charreton
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:53 UTC (permalink / raw)
To: pve-devel
Until now, firewall config serialization was iterating over the options
hash without sorting the keys, making the output non-deterministic.
Iterate over the sorted keys instead in format_options.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Firewall.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 894a87b..6270771 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -3754,7 +3754,7 @@ my $format_options = sub {
my $raw = '';
$raw .= "[OPTIONS]\n\n";
- foreach my $opt (keys %$options) {
+ foreach my $opt (sort keys %$options) {
$raw .= "$opt: $options->{$opt}\n";
}
$raw .= "\n";
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 03/13] d/control: bump libpve-common-perl
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-manager 01/13] network interface pinning: write new firewall config to local dir Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 02/13] firewall: config: sort OPTIONS when serializing Arthur Bied-Charreton
@ 2026-07-21 13:53 ` Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 04/13] firewall: dump configs locally after applying Arthur Bied-Charreton
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:53 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
debian/control | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/debian/control b/debian/control
index a420016..85b6a3b 100644
--- a/debian/control
+++ b/debian/control
@@ -23,7 +23,7 @@ Depends: conntrack,
iptables,
libpve-access-control,
libpve-cluster-perl,
- libpve-common-perl (>= 9.0.2),
+ libpve-common-perl (>= 9.1.12),
libpve-network-perl (>= 0.9.9~),
libpve-rs-perl (>= 0.8.13),
pve-cluster (>= 6.1-6),
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 04/13] firewall: dump configs locally after applying
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (2 preceding siblings ...)
2026-07-21 13:53 ` [PATCH pve-firewall 03/13] d/control: bump libpve-common-perl Arthur Bied-Charreton
@ 2026-07-21 13:53 ` Arthur Bied-Charreton
2026-07-21 13:53 ` [PATCH pve-firewall 05/13] fix #5759: firewall: do not remove chains when host is shutting down Arthur Bied-Charreton
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:53 UTC (permalink / raw)
To: pve-devel
... and remove them when pve-firewall is disabled in the config.
The firewall config lives on pmxcfs, which is only available once
pve-cluster is up, i.e. well after the network. To be able to restore
rules before that, dump the config files needed to recompile the ruleset
(cluster.fw, host.fw and the SDN config as sdn.json) to a local
directory after every successful apply. The guest configs are not
dumped, since pve-guests does not start until the actual firewall daemon
is running.
The last dumped content is cached per file, so the daemon only writes
when something changed since the last dump.
Ownership of the dumps is keyed on the nftables option in the host
config. When the firewall is disabled, the dumps are only removed if
nftables is disabled - otherwise that responsibility is on
proxmox-firewall.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
debian/dirs | 1 +
src/PVE/Firewall.pm | 79 +++++++++++++++++++++++++++++----
src/PVE/Service/pve_firewall.pm | 5 ++-
3 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/debian/dirs b/debian/dirs
index c9e3b54..e472f56 100644
--- a/debian/dirs
+++ b/debian/dirs
@@ -1 +1,2 @@
/var/lib/pve-firewall
+/var/lib/pve/firewall
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 6270771..ada8c1d 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -8,6 +8,7 @@ use Encode;
use File::Basename;
use File::Path;
use IO::File;
+use JSON;
use Net::IP;
use POSIX;
use Socket qw(AF_INET AF_INET6 inet_ntop inet_pton);
@@ -16,6 +17,7 @@ use Storable qw(dclone);
use PVE::Cluster;
use PVE::Corosync;
use PVE::Exception qw(raise raise_param_exc);
+use PVE::File;
use PVE::INotify;
use PVE::JSONSchema qw(register_standard_option get_standard_option);
use PVE::Network;
@@ -31,6 +33,8 @@ my $pvefw_conf_dir = "/etc/pve/firewall";
my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
my $vnetfw_conf_dir = "/etc/pve/sdn/firewall";
+my $dump_dir = '/var/lib/pve/firewall';
+
# dynamically include PVE::QemuServer and PVE::LXC
# to avoid dependency problems
my $have_qemu_server;
@@ -4084,7 +4088,7 @@ sub load_sdn_conf {
return $sdn_config // $empty_sdn_config;
}
-sub save_clusterfw_conf {
+sub serialize_clusterfw_conf {
my ($cluster_conf) = @_;
my $raw = '';
@@ -4119,11 +4123,21 @@ sub save_clusterfw_conf {
}
}
+ return $raw;
+}
+
+sub save_clusterfw_conf {
+ my ($cluster_conf, $filename) = @_;
+
+ $filename = $clusterfw_conf_filename if !defined($filename);
+
+ my $raw = serialize_clusterfw_conf($cluster_conf);
+
if ($raw) {
mkdir $pvefw_conf_dir;
- PVE::Tools::file_set_contents($clusterfw_conf_filename, $raw);
+ PVE::Tools::file_set_contents($filename, $raw);
} else {
- unlink $clusterfw_conf_filename;
+ unlink($filename);
}
}
@@ -4147,11 +4161,8 @@ sub load_hostfw_conf {
return generic_fw_config_parser($filename, $cluster_conf, $empty_conf, 'host');
}
-sub save_hostfw_conf {
- my ($hostfw_conf, $filename) = @_;
-
- $filename = $hostfw_conf_filename if !defined($filename);
-
+sub serialize_hostfw_conf {
+ my ($hostfw_conf) = @_;
my $raw = '';
my $options = $hostfw_conf->{options};
@@ -4164,6 +4175,16 @@ sub save_hostfw_conf {
$raw .= "\n";
}
+ return $raw;
+}
+
+sub save_hostfw_conf {
+ my ($hostfw_conf, $filename) = @_;
+
+ $filename = $hostfw_conf_filename if !defined($filename);
+
+ my $raw = serialize_hostfw_conf($hostfw_conf);
+
if ($raw) {
PVE::Tools::file_set_contents($filename, $raw);
} else {
@@ -5234,6 +5255,14 @@ sub remove_pvefw_chains {
}
+sub delete_dumps {
+
+ unlink "$dump_dir/cluster.fw";
+ unlink "$dump_dir/host.fw";
+ unlink "$dump_dir/sdn.json";
+
+}
+
sub remove_pvefw_chains_iptables {
my ($iptablescmd, $table) = @_;
@@ -5332,21 +5361,55 @@ sub init {
# load required modules here
}
+my $dump_cache = {};
+
+sub dump_if_changed {
+ my ($f, $data) = @_;
+ if (-f $f && defined($dump_cache->{$f}) && $dump_cache->{$f} eq $data) {
+ syslog('info', "no changes to $f since last dump\n");
+ return;
+ }
+ PVE::File::file_set_contents($f, $data);
+ $dump_cache->{$f} = $data;
+ syslog('info', "dumped $f\n");
+}
+
sub update {
my $code = sub {
+ my $cfw_dump_path = "$dump_dir/cluster.fw";
+ my $hfw_dump_path = "$dump_dir/host.fw";
+ my $sdn_dump_path = "$dump_dir/sdn.json";
my $cluster_conf = load_clusterfw_conf();
my $hostfw_conf = load_hostfw_conf($cluster_conf);
if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) {
PVE::Firewall::remove_pvefw_chains();
+ $dump_cache = {};
+ # Disabled in config: drop stale dumps so the pre-network restore does not resurrect a
+ # disabled ruleset. Leave them in nftables mode, since in that case proxmox-firewall
+ # owns the dump directory.
+ delete_dumps() if !$cluster_conf->{options}->{enable};
return;
}
+ # compile() rewrites rule actions in-place. Snapshot the untouched configs now for the
+ # post-apply boot-time dump.
+ my ($cfw_dump, $hfw_dump) = (dclone($cluster_conf), dclone($hostfw_conf));
+
my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) =
compile($cluster_conf, $hostfw_conf);
apply_ruleset($ruleset, $hostfw_conf, $ipset_ruleset, $rulesetv6, $ebtables_ruleset);
+
+ eval {
+ mkdir($dump_dir);
+ chmod(0700, $dump_dir);
+ dump_if_changed($cfw_dump_path, serialize_clusterfw_conf($cfw_dump));
+ dump_if_changed($hfw_dump_path, serialize_hostfw_conf($hfw_dump));
+ dump_if_changed($sdn_dump_path, JSON->new->utf8->canonical->encode($cfw_dump->{sdn}));
+ };
+ syslog('err', "could not persist firewall configs at $dump_dir: $@\n") if $@;
};
run_locked($code);
diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
index 95901a5..9c64442 100755
--- a/src/PVE/Service/pve_firewall.pm
+++ b/src/PVE/Service/pve_firewall.pm
@@ -51,7 +51,10 @@ sub shutdown {
syslog('info', "clear PVE-generated firewall rules");
- eval { PVE::Firewall::remove_pvefw_chains(); };
+ eval {
+ PVE::Firewall::remove_pvefw_chains();
+ PVE::Firewall::delete_dumps();
+ };
warn $@ if $@;
$self->exit_daemon(0);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 05/13] fix #5759: firewall: do not remove chains when host is shutting down
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (3 preceding siblings ...)
2026-07-21 13:53 ` [PATCH pve-firewall 04/13] firewall: dump configs locally after applying Arthur Bied-Charreton
@ 2026-07-21 13:53 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH pve-firewall 06/13] firewall: add restore command Arthur Bied-Charreton
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:53 UTC (permalink / raw)
To: pve-devel
The assumption until now was that the firewall rules should always be
cleared when the daemon is shutting down. This is correct when the
shutdown happens as the result of `systemctl stop`, should however not
be handled the same way in case of a system shutdown.
Because of the way pve-firewall is ordered, it comes down before the
network does. Clearing the rules in the host shutdown case creates a
window where the network is up without firewall protection.
When shutting down, check the operational state of the system and only
remove the chains if the system is _not_ currently shutting down.
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5759
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Service/pve_firewall.pm | 40 +++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 7 deletions(-)
diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
index 9c64442..2fe68cc 100755
--- a/src/PVE/Service/pve_firewall.pm
+++ b/src/PVE/Service/pve_firewall.pm
@@ -41,6 +41,29 @@ my $updatetime = 10;
my $initial_memory_usage;
+=head3 is_host_shutdown()
+
+Query C<systemctl is-system-running>, returning C<1> if the system's running state is C<stopping>,
+C<0> otherwise.
+
+Note that C<1> is also returned if the running state of the system could not be determined, in order
+to fail closed.
+
+=cut
+
+sub is_host_shutdown {
+ my $state = undef;
+
+ PVE::Tools::run_command(
+ ['systemctl', 'is-system-running'],
+ outfunc => sub { $state = PVE::Tools::trim(shift) },
+ noerr => 1,
+ );
+
+ return 1 if !defined($state) || $state eq '';
+ return $state eq 'stopping';
+}
+
sub shutdown {
my ($self) = @_;
@@ -49,13 +72,16 @@ sub shutdown {
# wait for children
1 while (waitpid(-1, POSIX::WNOHANG()) > 0);
- syslog('info', "clear PVE-generated firewall rules");
-
- eval {
- PVE::Firewall::remove_pvefw_chains();
- PVE::Firewall::delete_dumps();
- };
- warn $@ if $@;
+ if (is_host_shutdown()) {
+ syslog('info', "system is stopping, not removing firewall rules\n");
+ } else {
+ syslog('info', "clear PVE-generated firewall rules");
+ eval {
+ PVE::Firewall::remove_pvefw_chains();
+ PVE::Firewall::delete_dumps();
+ };
+ warn $@ if $@;
+ }
$self->exit_daemon(0);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 06/13] firewall: add restore command
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (4 preceding siblings ...)
2026-07-21 13:53 ` [PATCH pve-firewall 05/13] fix #5759: firewall: do not remove chains when host is shutting down Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH pve-firewall 07/13] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
Add a 'restore' command that recompiles and applies the firewall from
the config dumped to /var/lib/pve/firewall, to bridge the boot window
before pmxcfs (and thus the real config) is available.
It reuses update() with a local flag: configs are loaded from the local
dumps instead of pmxcfs and nothing is dumped back. A missing or
unreadable SDN dump is treated as empty, and rules referencing an
unavailable IPSet are skipped, so a partial dump still restores the rest
on a best-effort basis.
For the cluster and host config, a user-provided .override or a .new
written by the pve-network-interface-pinning take precedence over the
plain dump when present (.override -> .new -> plain).
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Firewall.pm | 33 ++++++++++++++++++++++++++++-----
src/PVE/Service/pve_firewall.pm | 18 ++++++++++++++++++
src/pve-firewall | 10 +++++++---
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ada8c1d..76dbec1 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -4050,11 +4050,11 @@ sub lock_clusterfw_conf {
}
sub load_clusterfw_conf {
- my ($filename) = @_;
+ my ($filename, $sdn_conf) = @_;
$filename = $clusterfw_conf_filename if !defined($filename);
- my $sdn_conf = load_sdn_conf();
+ $sdn_conf //= load_sdn_conf();
my $empty_conf = {
rules => [],
@@ -5374,14 +5374,32 @@ sub dump_if_changed {
syslog('info', "dumped $f\n");
}
+my sub get_local_config_dump_path {
+ my ($filename) = @_;
+ for my $p (qw(.override .new)) {
+ my $f = $filename . $p;
+ return $f if -f $f;
+ }
+ return $filename;
+}
+
sub update {
+ my ($local) = @_;
+
my $code = sub {
my $cfw_dump_path = "$dump_dir/cluster.fw";
my $hfw_dump_path = "$dump_dir/host.fw";
my $sdn_dump_path = "$dump_dir/sdn.json";
- my $cluster_conf = load_clusterfw_conf();
- my $hostfw_conf = load_hostfw_conf($cluster_conf);
+ my $cfw_conf_path = $local ? get_local_config_dump_path($cfw_dump_path) : undef;
+ my $hfw_conf_path = $local ? get_local_config_dump_path($hfw_dump_path) : undef;
+ my $sdn_conf = undef;
+ $sdn_conf = eval { decode_json(PVE::File::file_get_contents($sdn_dump_path)) }
+ if $local;
+ $sdn_conf = undef if $local && (!ref($sdn_conf) || ref($sdn_conf->{ipset}) ne 'HASH');
+
+ my $cluster_conf = load_clusterfw_conf($cfw_conf_path, $sdn_conf);
+ my $hostfw_conf = load_hostfw_conf($cluster_conf, $hfw_conf_path);
if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) {
PVE::Firewall::remove_pvefw_chains();
@@ -5389,10 +5407,13 @@ sub update {
# Disabled in config: drop stale dumps so the pre-network restore does not resurrect a
# disabled ruleset. Leave them in nftables mode, since in that case proxmox-firewall
# owns the dump directory.
- delete_dumps() if !$cluster_conf->{options}->{enable};
+ delete_dumps() if !$local && !$cluster_conf->{options}->{enable};
+ syslog('info', "iptables firewall is not enabled, not restoring rules\n") if $local;
return;
}
+ syslog('info', "restoring last remembered ruleset from $dump_dir\n") if $local;
+
# compile() rewrites rule actions in-place. Snapshot the untouched configs now for the
# post-apply boot-time dump.
my ($cfw_dump, $hfw_dump) = (dclone($cluster_conf), dclone($hostfw_conf));
@@ -5402,6 +5423,8 @@ sub update {
apply_ruleset($ruleset, $hostfw_conf, $ipset_ruleset, $rulesetv6, $ebtables_ruleset);
+ return if $local;
+
eval {
mkdir($dump_dir);
chmod(0700, $dump_dir);
diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
index 2fe68cc..8cba523 100755
--- a/src/PVE/Service/pve_firewall.pm
+++ b/src/PVE/Service/pve_firewall.pm
@@ -10,6 +10,7 @@ use PVE::CLIHandler;
use PVE::Cluster qw(cfs_read_file);
use PVE::Corosync;
use PVE::Daemon;
+use PVE::File;
use PVE::INotify;
use PVE::ProcFSTools;
use PVE::RPCEnvironment;
@@ -487,6 +488,22 @@ __PACKAGE__->register_method({
},
});
+__PACKAGE__->register_method({
+ name => 'restore',
+ path => 'restore',
+ method => 'GET',
+ description => 'Attempt to restore the last remembered ruleset',
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => { type => 'null' },
+ code => sub {
+ PVE::Firewall::update(1);
+ return undef;
+ },
+});
+
our $cmddef = {
start => [__PACKAGE__, 'start', []],
restart => [__PACKAGE__, 'restart', []],
@@ -494,6 +511,7 @@ our $cmddef = {
compile => [__PACKAGE__, 'compile', []],
simulate => [__PACKAGE__, 'simulate', []],
localnet => [__PACKAGE__, 'localnet', []],
+ restore => [__PACKAGE__, 'restore', []],
status => [
__PACKAGE__,
'status',
diff --git a/src/pve-firewall b/src/pve-firewall
index 5b62430..6343adc 100755
--- a/src/pve-firewall
+++ b/src/pve-firewall
@@ -14,12 +14,16 @@ $SIG{'__WARN__'} = sub {
$@ = $err;
};
+my $is_restore = ($ARGV[0] // '') eq 'restore';
+
my $prepare = sub {
my $rpcenv = PVE::RPCEnvironment->init('cli');
- $rpcenv->init_request();
- $rpcenv->set_language($ENV{LANG});
- $rpcenv->set_user('root@pam');
+
+ # 'restore' runs at pre-network, before pmxcfs is up, and needs no ACL
+ $rpcenv->init_request() if !$is_restore;
+ $rpcenv->set_language($ENV{LANG}) if !$is_restore;
+ $rpcenv->set_user('root@pam') if !$is_restore;
};
PVE::Service::pve_firewall->run_cli_handler(prepare => $prepare);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH pve-firewall 07/13] fix #5759: firewall: restore from dumped config before network-pre
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (5 preceding siblings ...)
2026-07-21 13:54 ` [PATCH pve-firewall 06/13] firewall: add restore command Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox 08/13] systemd: systemctl: add is-system-running helper Arthur Bied-Charreton
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
Since pve-firewall depends on pve-cluster for its config, pve-cluster
depends on corosync, which itself depends on network-online. This
creates a window at boot where the network is up with no protection.
To address this, add a simple oneshot service depending on
network-pre [0] that applies the last remembered rules to bridge the gap
until the configured firewall daemon takes over.
[0] https://systemd.io/NETWORK_ONLINE/
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5759
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
debian/pve-firewall-pre-network.service | 16 ++++++++++++++++
debian/rules | 1 +
2 files changed, 17 insertions(+)
create mode 100644 debian/pve-firewall-pre-network.service
diff --git a/debian/pve-firewall-pre-network.service b/debian/pve-firewall-pre-network.service
new file mode 100644
index 0000000..e1127e4
--- /dev/null
+++ b/debian/pve-firewall-pre-network.service
@@ -0,0 +1,16 @@
+[Unit]
+Description=Proxmox VE Pre-Network iptables Firewall
+Wants=network-pre.target
+Before=network-pre.target shutdown.target
+Conflicts=shutdown.target
+DefaultDependencies=no
+After=local-fs.target
+
+[Service]
+Type=oneshot
+ExecStart=/usr/sbin/pve-firewall restore
+RemainAfterExit=true
+Environment=PVE_LOG=info
+
+[Install]
+WantedBy=sysinit.target
diff --git a/debian/rules b/debian/rules
index 9bd53bc..170a90d 100755
--- a/debian/rules
+++ b/debian/rules
@@ -8,6 +8,7 @@
override_dh_installsystemd:
dh_installsystemd --name pvefw-logger pvefw-logger.service
+ dh_installsystemd --no-start --name pve-firewall-pre-network pve-firewall-pre-network.service
dh_installsystemd --name pve-firewall --no-stop-on-upgrade --no-restart-after-upgrade pve-firewall.service
override_dh_installinit:
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox 08/13] systemd: systemctl: add is-system-running helper
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (6 preceding siblings ...)
2026-07-21 13:54 ` [PATCH pve-firewall 07/13] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 09/13] firewall: fix clippy warnings Arthur Bied-Charreton
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
Introduce new `systemctl` module with `is_system_running` helper.
In some cases, it can be important to recognize whether a service is
being stopped as the result of a system shutdown, as opposed to an
explicit `systemctl stop`.
For example, firewalls must differentiate between a manual stop and a
shutdown. A manual stop should clear the ruleset, while a shutdown
should ideally not touch it.
`is_system_running` runs `systemctl is-system-running` and parses its
output to allow differentiating between those cases. While the `is_`
prefix implies a boolean return value by convention, the name mirrors
the systemctl subcommand, which seems less confusing than inventing a
new name for a wrapper.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
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
diff --git a/proxmox-systemd/Cargo.toml b/proxmox-systemd/Cargo.toml
index 74ec03d3..387aa3c7 100644
--- a/proxmox-systemd/Cargo.toml
+++ b/proxmox-systemd/Cargo.toml
@@ -14,3 +14,4 @@ repository.workspace = true
[dependencies]
libc.workspace = true
+thiserror.workspace = true
diff --git a/proxmox-systemd/debian/control b/proxmox-systemd/debian/control
index 766c9c78..d9e5d3a3 100644
--- a/proxmox-systemd/debian/control
+++ b/proxmox-systemd/debian/control
@@ -7,6 +7,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
rustc:native <!nocheck>,
libstd-rust-dev <!nocheck>,
librust-libc-0.2+default-dev (>= 0.2.107-~~) <!nocheck>,
+ librust-thiserror-2+default-dev <!nocheck>,
libsystemd-dev <!nocheck>
Maintainer: Proxmox Support Team <support@proxmox.com>
Standards-Version: 4.7.2
@@ -21,6 +22,7 @@ Multi-Arch: same
Depends:
${misc:Depends},
librust-libc-0.2+default-dev (>= 0.2.107-~~),
+ librust-thiserror-2+default-dev,
libsystemd-dev
Provides:
librust-proxmox-systemd+default-dev (= ${binary:Version}),
diff --git a/proxmox-systemd/src/lib.rs b/proxmox-systemd/src/lib.rs
index eff61d58..a0d8de2c 100644
--- a/proxmox-systemd/src/lib.rs
+++ b/proxmox-systemd/src/lib.rs
@@ -9,3 +9,5 @@ pub mod journal;
pub mod notify;
pub mod sd_id128;
+
+pub mod systemctl;
diff --git a/proxmox-systemd/src/systemctl.rs b/proxmox-systemd/src/systemctl.rs
new file mode 100644
index 00000000..e03820bc
--- /dev/null
+++ b/proxmox-systemd/src/systemctl.rs
@@ -0,0 +1,98 @@
+use std::{str::FromStr, string::FromUtf8Error};
+
+#[derive(thiserror::Error, Debug)]
+pub enum SystemctlError {
+ #[error("could not run systemctl: {0}")]
+ Io(#[from] std::io::Error),
+ #[error("unexpected output: {0}")]
+ UnexpectedOutput(String),
+}
+
+impl From<FromUtf8Error> for SystemctlError {
+ fn from(value: FromUtf8Error) -> Self {
+ Self::UnexpectedOutput(format!("output is not valid UTF-8: {value}"))
+ }
+}
+
+/// Possible operational states of the system as returned by `systemctl is-system-running` [0].
+///
+/// [0]: https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#is-system-running
+#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
+pub enum SystemState {
+ /// Early bootup, before `basic.target` is reached or the [`SystemState::Maintenance`]
+ /// is entered.
+ Initializing,
+ /// Late bootup, before the job queue becomes idle for the first time, or one of the
+ /// rescue targets are reached.
+ Starting,
+ /// The system is fully operational.
+ Running,
+ /// The system is operational but one or more units failed.
+ Degraded,
+ /// The rescue or emergency target is active.
+ Maintenance,
+ /// The manager is shutting down.
+ Stopping,
+ /// The manager is not running. Specifically, this is the operational state if an
+ /// incompatible program is running as system manager (PID 1).
+ Offline,
+ /// The operational state could not be determined, due to lack of resources or another
+ /// error case.
+ Unknown,
+}
+
+impl std::fmt::Display for SystemState {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ Self::Initializing => write!(f, "initializing"),
+ Self::Starting => write!(f, "starting"),
+ Self::Running => write!(f, "running"),
+ Self::Degraded => write!(f, "degraded"),
+ Self::Maintenance => write!(f, "maintenance"),
+ Self::Stopping => write!(f, "stopping"),
+ Self::Offline => write!(f, "offline"),
+ Self::Unknown => write!(f, "unknown"),
+ }
+ }
+}
+
+impl FromStr for SystemState {
+ type Err = SystemctlError;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ match s {
+ "initializing" => Ok(Self::Initializing),
+ "starting" => Ok(Self::Starting),
+ "running" => Ok(Self::Running),
+ "degraded" => Ok(Self::Degraded),
+ "maintenance" => Ok(Self::Maintenance),
+ "stopping" => Ok(Self::Stopping),
+ "offline" => Ok(Self::Offline),
+ "unknown" => Ok(Self::Unknown),
+ other => Err(SystemctlError::UnexpectedOutput(other.into())),
+ }
+ }
+}
+
+/// Get the current operational state of the system.
+///
+/// This runs `systemctl is-system-running` [0] and parses the state printed to stdout. While the
+/// command exits non-zero whenever the state is anything other than [`SystemState::Running`], this
+/// function returns any recognized state wrapped in `Ok` instead.
+///
+/// ## Errors
+///
+/// Returns an error if the command could not be spawned, its output was not valid UTF-8, or if the
+/// printed state was not one of the known [`SystemState`] variants.
+///
+/// [0]: https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#is-system-running
+pub fn is_system_running() -> Result<SystemState, SystemctlError> {
+ let output = std::process::Command::new("systemctl")
+ .arg("is-system-running")
+ .output()?;
+
+ // Current system state is always printed to stdout.
+ let stdout = String::from_utf8(output.stdout)?;
+
+ SystemState::from_str(stdout.trim())
+}
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox-firewall 09/13] firewall: fix clippy warnings
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (7 preceding siblings ...)
2026-07-21 13:54 ` [PATCH proxmox 08/13] systemd: systemctl: add is-system-running helper Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 10/13] fix #5759: firewall: do not clear rules on system shutdown Arthur Bied-Charreton
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
No functional changes intended.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-firewall/src/firewall.rs | 4 ++--
proxmox-firewall/src/rule.rs | 38 ++++++++++++++++----------------
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index f105fa8..477b69b 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -251,8 +251,8 @@ impl Firewall {
if let Some(ipam_config) = self.config.ipam() {
let ipsets = ipam_config.ipsets();
- self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?;
- self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?;
+ self.create_ipsets(&mut commands, ipsets, &cluster_host_table, None)?;
+ self.create_ipsets(&mut commands, ipsets, &guest_table, None)?;
}
if self.config.host().is_enabled() {
diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index 048c00e..d33e31a 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -250,24 +250,24 @@ impl ToNftRules for RuleMatch {
return Ok(());
}
- if let Some(log) = self.log() {
- if let Ok(log_level) = LogLevel::try_from(log) {
- let mut terminal_statements = Vec::new();
+ if let Some(log) = self.log()
+ && let Ok(log_level) = LogLevel::try_from(log)
+ {
+ let mut terminal_statements = Vec::new();
- if let Some(limit) = env.default_log_limit() {
- terminal_statements.push(Statement::from(limit));
- }
+ if let Some(limit) = env.default_log_limit() {
+ terminal_statements.push(Statement::from(limit));
+ }
- terminal_statements.push(
- Log::new_nflog(
- Log::generate_prefix(env.vmid, log_level, env.chain.name(), self.verdict()),
- 0,
- )
- .into(),
- );
+ terminal_statements.push(
+ Log::new_nflog(
+ Log::generate_prefix(env.vmid, log_level, env.chain.name(), self.verdict()),
+ 0,
+ )
+ .into(),
+ );
- rules.push(NftRule::from_terminal_statements(terminal_statements));
- }
+ rules.push(NftRule::from_terminal_statements(terminal_statements));
}
rules.push(NftRule::new(generate_verdict(self.verdict(), env)));
@@ -844,10 +844,10 @@ impl ToNftRules for Ipfilter<'_> {
impl ToNftRules for CtHelperMacro {
fn to_nft_rules(&self, rules: &mut Vec<NftRule>, env: &NftRuleEnv) -> Result<(), Error> {
- if let Some(family) = self.family() {
- if !env.contains_family(family) {
- return Ok(());
- }
+ if let Some(family) = self.family()
+ && !env.contains_family(family)
+ {
+ return Ok(());
}
if self.tcp().is_none() && self.udp().is_none() {
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox-firewall 10/13] fix #5759: firewall: do not clear rules on system shutdown
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (8 preceding siblings ...)
2026-07-21 13:54 ` [PATCH proxmox-firewall 09/13] firewall: fix clippy warnings Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply Arthur Bied-Charreton
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
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 <a.bied-charreton@proxmox.com>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (9 preceding siblings ...)
2026-07-21 13:54 ` [PATCH proxmox-firewall 10/13] fix #5759: firewall: do not clear rules on system shutdown Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 12/13] firewall: add restore command Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 13/13] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
The firewall configuration files are stored on pmxcfs, which is only
available when pve-cluster is up. To be able to restore firewall rules
before the network comes up and close the boot-time window in which the
PVE host is unprotected, dump the required configuration files to a
local directory after every apply.
The last dumped version is cached for each file, the daemon only
actually writes to disk if there has been a change since the last write
or the dump file does not exist.
Ownership of the dumps is keyed on the nftables option in the host
config. When the firewall is disabled, the dumps are only removed if
nftables is enabled - otherwise that responsibility is on pve-firewall.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
debian/dirs | 1 +
proxmox-firewall/Cargo.toml | 2 +-
proxmox-firewall/src/bin/proxmox-firewall.rs | 73 ++++++++++++++++++--
proxmox-firewall/src/config.rs | 56 +++++++++++----
proxmox-firewall/src/firewall.rs | 4 ++
5 files changed, 117 insertions(+), 19 deletions(-)
create mode 100644 debian/dirs
diff --git a/debian/dirs b/debian/dirs
new file mode 100644
index 0000000..34355af
--- /dev/null
+++ b/debian/dirs
@@ -0,0 +1 @@
+/var/lib/pve/firewall
diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml
index fa7fd34..77ca285 100644
--- a/proxmox-firewall/Cargo.toml
+++ b/proxmox-firewall/Cargo.toml
@@ -22,9 +22,9 @@ proxmox-log.workspace = true
proxmox-network-types.workspace = true
proxmox-network-api = { workspace = true, features = [ "impl" ] }
proxmox-nftables = { workspace = true, features = [ "config-ext" ] }
+proxmox-sys.workspace = true
proxmox-systemd.workspace = true
proxmox-ve-config.workspace = true
[dev-dependencies]
insta = { workspace = true, features = [ "json" ] }
-proxmox-sys.workspace = true
diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 27fd67d..5b20ca0 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -1,3 +1,6 @@
+use std::collections::HashMap;
+use std::os::unix::fs::PermissionsExt;
+use std::path::Path;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::{Duration, Instant};
@@ -5,11 +8,14 @@ use std::time::{Duration, Instant};
use anyhow::{Context, Error, bail, format_err};
use pico_args::Arguments;
-use proxmox_firewall::config::{FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader};
+use proxmox_firewall::config::{
+ DUMP_DIR, FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader,
+};
use proxmox_firewall::firewall::Firewall;
use proxmox_log as log;
use proxmox_log::{LevelFilter, Logger};
use proxmox_nftables::{NftClient, client::NftError};
+use proxmox_sys::fs;
use proxmox_systemd::systemctl;
use proxmox_ve_config::firewall::host::Config as HostConfig;
@@ -42,14 +48,58 @@ fn remove_firewall() -> Result<(), std::io::Error> {
Ok(())
}
+fn dump_config(cfg: &FirewallConfig, cache: &mut HashMap<&str, Vec<u8>>) {
+ let dump_dir = Path::new(DUMP_DIR);
+ if !dump_dir.exists()
+ && let Err(e) = fs::create_path(dump_dir, None, None)
+ {
+ log::warn!("could not create {DUMP_DIR}: {e} - config will not be dumped");
+ return;
+ }
+
+ if let Err(e) = std::fs::set_permissions(dump_dir, std::fs::Permissions::from_mode(0o700)) {
+ log::warn!("could not set permissions on {DUMP_DIR}: {e}");
+ }
+
+ for (bytes, name) in [
+ (cfg.cluster_raw(), "cluster.fw"),
+ (cfg.host_raw(), "host.fw"),
+ (cfg.sdn_raw(), "sdn.json"),
+ ] {
+ let out = dump_dir.join(name);
+ match bytes {
+ Some(b) if !out.exists() || b != cache.get(name).map(Vec::as_slice).unwrap_or(&[]) => {
+ match fs::replace_file(&out, b, fs::CreateOptions::new(), true) {
+ Err(e) => log::warn!("could not dump {name} to {out:?}: {e}"),
+ Ok(()) => {
+ log::info!("successfully dumped {out:?}");
+ cache.insert(name, b.to_vec());
+ }
+ }
+ }
+ Some(_) => log::info!("no changes to {name} since last dump to {out:?}"),
+ None => {
+ log::info!("nothing to dump for {name}");
+ cache.remove(name);
+ _ = std::fs::remove_file(&out);
+ }
+ }
+ }
+}
+
fn create_firewall_instance() -> Result<Firewall, Error> {
let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
Ok(Firewall::new(config))
}
-fn handle_firewall() -> Result<(), Error> {
- let firewall = create_firewall_instance()?;
+fn delete_dumps(cache: &mut HashMap<&str, Vec<u8>>) {
+ cache.clear();
+ for f in ["cluster.fw", "host.fw", "sdn.json"] {
+ _ = std::fs::remove_file(Path::new(DUMP_DIR).join(f));
+ }
+}
+fn handle_firewall(firewall: &Firewall) -> Result<(), Error> {
if !firewall.is_enabled() {
return remove_firewall().with_context(|| "could not remove firewall tables".to_string());
}
@@ -95,6 +145,8 @@ fn run_firewall() -> Result<(), Error> {
// we're disabled here without the need to parse the config, avoiding log-spam errors from that
let force_disable_flag = std::path::Path::new(FORCE_DISABLE_FLAG_FILE);
+ let mut cache = HashMap::new();
+
while !term.load(Ordering::Relaxed) {
if force_disable_flag.exists() {
if let Err(error) = remove_firewall() {
@@ -106,9 +158,18 @@ fn run_firewall() -> Result<(), Error> {
}
let start = Instant::now();
- if let Err(error) = handle_firewall() {
- log::error!("error updating firewall rules: {error:#}");
- }
+ match create_firewall_instance() {
+ Err(e) => log::error!("could not load firewall configuration: {e}"),
+ Ok(fw) => match handle_firewall(&fw) {
+ Err(e) => log::error!("error updating firewall rules: {e:#}"),
+ Ok(()) if fw.is_enabled() => dump_config(fw.config(), &mut cache),
+ // is_enabled() is false, nftables set here means the cluster firewall is disabled
+ // and the dumps are owned by us, so we clean them up. With nftables unset, they
+ // would be owned by pve-firewall.
+ Ok(()) if fw.config().host().nftables() => delete_dumps(&mut cache),
+ Ok(()) => (),
+ },
+ };
let duration = start.elapsed();
log::info!("firewall update time: {}ms", duration.as_millis());
diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index 341e05a..eb8dd21 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -1,7 +1,7 @@
use std::collections::BTreeMap;
use std::default::Default;
use std::fs::{self, DirEntry, File, ReadDir};
-use std::io::{self, BufReader};
+use std::io::{self, BufRead, BufReader};
use anyhow::{Context, Error, bail, format_err};
@@ -74,6 +74,16 @@ fn open_config_file(path: &str) -> Result<Option<File>, Error> {
}
}
+fn read_opt(reader: Option<Box<dyn BufRead>>) -> Result<Option<Vec<u8>>, Error> {
+ reader
+ .map(|mut r| {
+ let mut buf = Vec::new();
+ r.read_to_end(&mut buf)?;
+ Ok(buf)
+ })
+ .transpose()
+}
+
fn open_config_folder(path: &str) -> Result<Option<ReadDir>, Error> {
match fs::read_dir(path) {
Ok(paths) => Ok(Some(paths)),
@@ -104,6 +114,8 @@ const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config";
const SDN_IPAM_PATH: &str = "/etc/pve/sdn/pve-ipam-state.json";
const SDN_IPAM_PATH_LEGACY: &str = "/etc/pve/priv/ipam.db"; // TODO: remove with PVE 9+
+pub const DUMP_DIR: &str = "/var/lib/pve/firewall";
+
impl FirewallConfigLoader for PveFirewallConfigLoader {
fn cluster(&self) -> Result<Option<Box<dyn io::BufRead>>, Error> {
log::info!("loading cluster config");
@@ -298,11 +310,14 @@ pub struct FirewallConfig {
sdn_config: Option<FirewallSdnConfig>,
ipam_config: Option<FirewallIpamConfig>,
interface_mapping: AltnameMapping,
+ cluster_raw: Option<Vec<u8>>,
+ host_raw: Option<Vec<u8>>,
+ sdn_raw: Option<Vec<u8>>,
}
impl FirewallConfig {
- fn parse_cluster(firewall_loader: &dyn FirewallConfigLoader) -> Result<ClusterConfig, Error> {
- match firewall_loader.cluster()? {
+ fn parse_cluster(raw: Option<&[u8]>) -> Result<ClusterConfig, Error> {
+ match raw {
Some(data) => ClusterConfig::parse(data),
None => {
log::info!("no cluster config found, falling back to default");
@@ -311,8 +326,8 @@ impl FirewallConfig {
}
}
- fn parse_host(firewall_loader: &dyn FirewallConfigLoader) -> Result<HostConfig, Error> {
- match firewall_loader.host()? {
+ fn parse_host(raw: Option<&[u8]>) -> Result<HostConfig, Error> {
+ match raw {
Some(data) => HostConfig::parse(data),
None => {
log::info!("no host config found, falling back to default");
@@ -355,10 +370,8 @@ impl FirewallConfig {
Ok(guests)
}
- pub fn parse_sdn(
- firewall_loader: &dyn FirewallConfigLoader,
- ) -> Result<Option<FirewallSdnConfig>, Error> {
- Ok(match firewall_loader.sdn_running_config()? {
+ pub fn parse_sdn(raw: Option<&[u8]>) -> Result<Option<FirewallSdnConfig>, Error> {
+ Ok(match raw {
Some(data) => {
let running_config: RunningConfig = serde_json::from_reader(data)?;
let config = SdnConfig::try_from(running_config)?;
@@ -437,15 +450,22 @@ impl FirewallConfig {
firewall_loader: &dyn FirewallConfigLoader,
nft_loader: &dyn NftConfigLoader,
) -> Result<Self, Error> {
+ let cluster_raw = read_opt(firewall_loader.cluster()?)?;
+ let host_raw = read_opt(firewall_loader.host()?)?;
+ let sdn_raw = read_opt(firewall_loader.sdn_running_config()?)?;
+
Ok(Self {
- cluster_config: Self::parse_cluster(firewall_loader)?,
- host_config: Self::parse_host(firewall_loader)?,
+ cluster_config: Self::parse_cluster(cluster_raw.as_deref())?,
+ host_config: Self::parse_host(host_raw.as_deref())?,
guest_config: Self::parse_guests(firewall_loader)?,
bridge_config: Self::parse_bridges(firewall_loader)?,
- sdn_config: Self::parse_sdn(firewall_loader)?,
+ sdn_config: Self::parse_sdn(sdn_raw.as_deref())?,
ipam_config: Self::parse_ipam(firewall_loader)?,
nft_config: Self::parse_nft(nft_loader)?,
interface_mapping: firewall_loader.interface_mapping()?,
+ cluster_raw,
+ host_raw,
+ sdn_raw,
})
}
@@ -453,10 +473,18 @@ impl FirewallConfig {
&self.cluster_config
}
+ pub fn cluster_raw(&self) -> Option<&[u8]> {
+ self.cluster_raw.as_deref()
+ }
+
pub fn host(&self) -> &HostConfig {
&self.host_config
}
+ pub fn host_raw(&self) -> Option<&[u8]> {
+ self.host_raw.as_deref()
+ }
+
pub fn guests(&self) -> &BTreeMap<Vmid, GuestConfig> {
&self.guest_config
}
@@ -473,6 +501,10 @@ impl FirewallConfig {
self.sdn_config.as_ref()
}
+ pub fn sdn_raw(&self) -> Option<&[u8]> {
+ self.sdn_raw.as_deref()
+ }
+
pub fn ipam(&self) -> Option<&FirewallIpamConfig> {
self.ipam_config.as_ref()
}
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 477b69b..7f3e633 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -63,6 +63,10 @@ impl Firewall {
Self { config }
}
+ pub fn config(&self) -> &FirewallConfig {
+ &self.config
+ }
+
pub fn is_enabled(&self) -> bool {
self.config.is_enabled()
}
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox-firewall 12/13] firewall: add restore command
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (10 preceding siblings ...)
2026-07-21 13:54 ` [PATCH proxmox-firewall 11/13] firewall: dump config to local directory after apply Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
2026-07-21 13:54 ` [PATCH proxmox-firewall 13/13] fix #5759: firewall: restore from dumped config before network-pre Arthur Bied-Charreton
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
Add a 'restore' command that recompiles and applies the firewall from
the config dumped to /var/lib/pve/firewall, to bridge the boot window
before pmxcfs (and thus the real config) is available.
It reuses handle_firewall() along with a local firewall config loader.
The config files are loaded from the local dump instead of pmxcfs and
nothing is dumped back. A missing or unreadable SDN dump is treated as
empty and rules referencing an unavailable IPSet are skipped, so a
partial dump still restores the rest on a best-effort basis.
For the cluster and host config, a user-provided .override or a .new
written by the pve-network-interface-pinning tool take precedence over
the plain dump when present (.override -> .new -> plain).
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
proxmox-firewall/src/bin/proxmox-firewall.rs | 26 ++++-
proxmox-firewall/src/config.rs | 111 +++++++++++++++++--
2 files changed, 128 insertions(+), 9 deletions(-)
diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 5b20ca0..e96083c 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -9,7 +9,8 @@ use anyhow::{Context, Error, bail, format_err};
use pico_args::Arguments;
use proxmox_firewall::config::{
- DUMP_DIR, FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader,
+ DUMP_DIR, FirewallConfig, LocalFirewallConfigLoader, LocalNftConfigLoader,
+ PveFirewallConfigLoader, PveNftConfigLoader,
};
use proxmox_firewall::firewall::Firewall;
use proxmox_log as log;
@@ -29,6 +30,7 @@ COMMANDS:
compile Compile and print firewall rules as accepted by 'nft -j -f -'
start Execute proxmox-firewall service in foreground
localnet Print the contents of the management ipset
+ restore Attempt to restore the last remembered ruleset
"#;
const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft");
@@ -87,6 +89,14 @@ fn dump_config(cfg: &FirewallConfig, cache: &mut HashMap<&str, Vec<u8>>) {
}
}
+fn create_local_firewall_instance() -> Result<Firewall, Error> {
+ let config = FirewallConfig::new(
+ &LocalFirewallConfigLoader::new(),
+ &LocalNftConfigLoader::new(),
+ )?;
+ Ok(Firewall::new(config))
+}
+
fn create_firewall_instance() -> Result<Firewall, Error> {
let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
Ok(Firewall::new(config))
@@ -189,6 +199,17 @@ fn run_firewall() -> Result<(), Error> {
}
}
+fn restore() -> Result<(), Error> {
+ let fw = create_local_firewall_instance()?;
+ match fw.is_enabled() {
+ true => handle_firewall(&fw),
+ false => {
+ log::info!("nftables firewall is not enabled, not restoring rules");
+ Ok(())
+ }
+ }
+}
+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Command {
Compile,
@@ -196,6 +217,7 @@ pub enum Command {
Skeleton,
Start,
Localnet,
+ Restore,
}
impl std::str::FromStr for Command {
@@ -208,6 +230,7 @@ impl std::str::FromStr for Command {
"skeleton" => Command::Skeleton,
"start" => Command::Start,
"localnet" => Command::Localnet,
+ "restore" => Command::Restore,
cmd => {
bail!("{cmd} is not a valid command")
}
@@ -240,6 +263,7 @@ fn run_command(command: Command) -> Result<(), Error> {
println!("{ip}");
}
}
+ Command::Restore => restore()?,
};
Ok(())
diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index eb8dd21..83b9161 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -1,7 +1,8 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
use std::default::Default;
use std::fs::{self, DirEntry, File, ReadDir};
use std::io::{self, BufRead, BufReader};
+use std::path::{Path, PathBuf};
use anyhow::{Context, Error, bail, format_err};
@@ -60,15 +61,16 @@ impl PveFirewallConfigLoader {
/// opens a configuration file
///
/// It returns a file handle to the file or [`None`] if it doesn't exist.
-fn open_config_file(path: &str) -> Result<Option<File>, Error> {
- match File::open(path) {
+fn open_config_file<P: Into<PathBuf>>(path: P) -> Result<Option<File>, Error> {
+ let path: PathBuf = path.into();
+ match File::open(&path) {
Ok(data) => Ok(Some(data)),
Err(err) if err.kind() == io::ErrorKind::NotFound => {
- log::info!("config file does not exist: {path}");
+ log::info!("config file does not exist: {path:?}");
Ok(None)
}
Err(err) => {
- let context = format!("unable to open configuration file at {path}");
+ let context = format!("unable to open configuration file at {path:?}");
Err(anyhow::Error::new(err).context(context))
}
}
@@ -155,7 +157,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
) -> Result<Option<Box<dyn io::BufRead>>, Error> {
log::info!("loading guest #{vmid} config");
- let fd = open_config_file(&GuestMap::config_path(vmid, entry))?;
+ let fd = open_config_file(GuestMap::config_path(vmid, entry))?;
if let Some(file) = fd {
let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -168,7 +170,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
fn guest_firewall_config(&self, vmid: &Vmid) -> Result<Option<Box<dyn io::BufRead>>, Error> {
log::info!("loading guest #{vmid} firewall config");
- let fd = open_config_file(&GuestMap::firewall_config_path(vmid))?;
+ let fd = open_config_file(GuestMap::firewall_config_path(vmid))?;
if let Some(file) = fd {
let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -230,7 +232,7 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
) -> Result<Option<Box<dyn io::BufRead>>, Error> {
log::info!("loading firewall config for bridge {bridge_name}");
- let fd = open_config_file(&format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?;
+ let fd = open_config_file(format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?;
if let Some(file) = fd {
let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
@@ -247,6 +249,81 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
}
}
+#[derive(Debug, Default)]
+pub struct LocalFirewallConfigLoader {}
+
+impl LocalFirewallConfigLoader {
+ pub fn new() -> Self {
+ Self::default()
+ }
+}
+
+fn load_first_of(candidates: &[&str], what: &str) -> Result<Option<Box<dyn BufRead>>, Error> {
+ for p in candidates.iter().map(|p| Path::new(DUMP_DIR).join(p)) {
+ if let Some(fd) = open_config_file(&p)? {
+ log::info!("loaded {what} config from {p:?}");
+ let reader = Box::new(BufReader::new(fd)) as Box<dyn BufRead>;
+ return Ok(Some(reader));
+ }
+ }
+ Ok(None)
+}
+
+impl FirewallConfigLoader for LocalFirewallConfigLoader {
+ fn cluster(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+ load_first_of(&["cluster.fw.override", "cluster.fw"], "cluster")
+ }
+
+ fn host(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+ load_first_of(&["host.fw.override", "host.fw.new", "host.fw"], "host")
+ }
+
+ fn sdn_running_config(&self) -> Result<Option<Box<dyn BufRead>>, Error> {
+ let path = Path::new(DUMP_DIR).join("sdn.json");
+ if let Some(fd) = open_config_file(&path)? {
+ let reader = Box::new(BufReader::new(fd)) as Box<dyn BufRead>;
+ return Ok(Some(reader));
+ }
+
+ Ok(None)
+ }
+
+ fn guest_config(
+ &self,
+ _: &Vmid,
+ _: &GuestEntry,
+ ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+ Ok(None)
+ }
+
+ fn guest_firewall_config(&self, _: &Vmid) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+ Ok(None)
+ }
+
+ fn guest_list(&self) -> Result<GuestMap, Error> {
+ Ok(GuestMap::from(HashMap::new()))
+ }
+
+ fn bridge_firewall_config(
+ &self,
+ _: &BridgeName,
+ ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+ Ok(None)
+ }
+
+ fn bridge_list(&self) -> Result<Vec<BridgeName>, Error> {
+ Ok(vec![])
+ }
+
+ fn ipam(&self) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+ Ok(None)
+ }
+
+ fn interface_mapping(&self) -> Result<AltnameMapping, Error> {
+ Ok(AltnameMapping::from_iter([]))
+ }
+}
+
pub trait NftConfigLoader {
fn chains(&self) -> Result<Option<CommandOutput>, Error>;
}
@@ -271,6 +348,24 @@ impl NftConfigLoader for PveNftConfigLoader {
}
}
+#[derive(Debug, Default)]
+pub struct LocalNftConfigLoader {}
+
+impl LocalNftConfigLoader {
+ pub fn new() -> Self {
+ Self::default()
+ }
+}
+
+impl NftConfigLoader for LocalNftConfigLoader {
+ fn chains(&self) -> Result<Option<CommandOutput>, Error> {
+ let commands = Commands::new(vec![List::chains()]);
+
+ NftClient::run_json_commands(&commands)
+ .with_context(|| "unable to query nft chains".to_string())
+ }
+}
+
pub struct FirewallSdnConfig {
_config: SdnConfig,
ipsets: BTreeMap<String, Ipset>,
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH proxmox-firewall 13/13] fix #5759: firewall: restore from dumped config before network-pre
2026-07-21 13:53 [RFC firewall/manager/proxmox{,-firewall} 00/13] fix #5759: keep firewall rules up across boot and shutdown Arthur Bied-Charreton
` (11 preceding siblings ...)
2026-07-21 13:54 ` [PATCH proxmox-firewall 12/13] firewall: add restore command Arthur Bied-Charreton
@ 2026-07-21 13:54 ` Arthur Bied-Charreton
12 siblings, 0 replies; 14+ messages in thread
From: Arthur Bied-Charreton @ 2026-07-21 13:54 UTC (permalink / raw)
To: pve-devel
proxmox-firewall depends on pve-cluster, pve-cluster depends on
corosync, which itself depends on network-online. This creates a
boot-time window where the network is up on the PVE host without it
having any firewall protection.
To address this, add a simple oneshot service depending on
network-pre [0] that applies the last remembered rules to bridge the
gap until the configured firewall daemon takes over.
[0] https://systemd.io/NETWORK_ONLINE/
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5759
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
debian/proxmox-firewall-pre-network.service | 16 ++++++++++++++++
debian/rules | 2 +-
2 files changed, 17 insertions(+), 1 deletion(-)
create mode 100644 debian/proxmox-firewall-pre-network.service
diff --git a/debian/proxmox-firewall-pre-network.service b/debian/proxmox-firewall-pre-network.service
new file mode 100644
index 0000000..83c686f
--- /dev/null
+++ b/debian/proxmox-firewall-pre-network.service
@@ -0,0 +1,16 @@
+[Unit]
+Description=Proxmox VE Pre-Network NFT Firewall
+Wants=network-pre.target
+Before=network-pre.target shutdown.target
+Conflicts=shutdown.target
+DefaultDependencies=no
+After=local-fs.target
+
+[Service]
+Type=oneshot
+ExecStart=/usr/libexec/proxmox/proxmox-firewall restore
+RemainAfterExit=true
+Environment=PVE_LOG=info
+
+[Install]
+WantedBy=sysinit.target
diff --git a/debian/rules b/debian/rules
index 0dc4e0f..99a6797 100755
--- a/debian/rules
+++ b/debian/rules
@@ -28,4 +28,4 @@ override_dh_auto_configure:
override_dh_installsystemd:
dh_installsystemd proxmox-firewall.service
-
+ dh_installsystemd --no-start --name proxmox-firewall-pre-network proxmox-firewall-pre-network.service
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread