From 355420892e97f10aac43d6a716d3241f20300c0d Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Wed, 7 Jun 2023 16:06:17 +0200 Subject: [PATCH 01/16] bump version to 5.0.1 Signed-off-by: Wolfgang Bumiller --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index fa7c9f6..88d1a55 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +pve-firewall (5.0.1) bookworm; urgency=medium + + * fix #4556: support 'dc/' and 'guest/' prefix for aliases and ipsets + + -- Proxmox Support Team Wed, 07 Jun 2023 16:06:10 +0200 + pve-firewall (5.0.0) bookworm; urgency=medium * switch to native versioning scheme -- 2.39.2 From aaa87fbb8311e10a702e479e81f2a4e113789039 Mon Sep 17 00:00:00 2001 From: Leo Nunner Date: Tue, 13 Jun 2023 14:06:32 +0200 Subject: [PATCH 02/16] api: fix scoping for ipset endpoint in the cluster class, we save the cluster config into the 'fw_conf' variable, and not into 'cluster_conf', which in turns is set to 'undef' instead. Signed-off-by: Leo Nunner --- src/PVE/API2/Firewall/IPSet.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm index ed92d87..baa57ca 100644 --- a/src/PVE/API2/Firewall/IPSet.pm +++ b/src/PVE/API2/Firewall/IPSet.pm @@ -203,6 +203,8 @@ sub register_create_ip { if ($cidr =~ m@^(dc/|guest/)?(${PVE::Firewall::ip_alias_pattern})$@) { my $scope = $1 // ""; my $alias = $2; + # on the cluster level + $cluster_conf = $fw_conf if (!$cluster_conf); # make sure alias exists (if $cidr is an alias) PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $alias, $scope); } else { -- 2.39.2 From b06a8c2d0377758f3f7353a8829dbe1f909c6a12 Mon Sep 17 00:00:00 2001 From: Leo Nunner Date: Tue, 13 Jun 2023 14:06:33 +0200 Subject: [PATCH 03/16] fix #4556: api: return scoped IPSets and aliases Introduce a new 'scope' field in the return values for the /ref endpoints. Also add the 'ref' field in the VM endpoint, since it has been missing up until now. Signed-off-by: Leo Nunner --- src/PVE/API2/Firewall/Cluster.pm | 34 +++-------------------- src/PVE/API2/Firewall/VM.pm | 46 +++++++------------------------- src/PVE/Firewall/Helpers.pm | 39 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 67 deletions(-) diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm index c9c3e67..48ad90d 100644 --- a/src/PVE/API2/Firewall/Cluster.pm +++ b/src/PVE/API2/Firewall/Cluster.pm @@ -240,6 +240,9 @@ __PACKAGE__->register_method({ ref => { type => 'string', }, + scope => { + type => 'string', + }, comment => { type => 'string', optional => 1, @@ -252,36 +255,7 @@ __PACKAGE__->register_method({ my $conf = PVE::Firewall::load_clusterfw_conf(); - my $res = []; - - if (!$param->{type} || $param->{type} eq 'ipset') { - foreach my $name (keys %{$conf->{ipset}}) { - my $data = { - type => 'ipset', - name => $name, - ref => "+$name", - }; - if (my $comment = $conf->{ipset_comments}->{$name}) { - $data->{comment} = $comment; - } - push @$res, $data; - } - } - - if (!$param->{type} || $param->{type} eq 'alias') { - foreach my $name (keys %{$conf->{aliases}}) { - my $e = $conf->{aliases}->{$name}; - my $data = { - type => 'alias', - name => $name, - ref => $name, - }; - $data->{comment} = $e->{comment} if $e->{comment}; - push @$res, $data; - } - } - - return $res; + return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc"); }}); 1; diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm index fb255e0..4222103 100644 --- a/src/PVE/API2/Firewall/VM.pm +++ b/src/PVE/API2/Firewall/VM.pm @@ -262,6 +262,12 @@ sub register_handlers { name => { type => 'string', }, + ref => { + type => 'string', + }, + scope => { + type => 'string', + }, comment => { type => 'string', optional => 1, @@ -275,44 +281,10 @@ sub register_handlers { my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid}); - my $ipsets = {}; - my $aliases = {}; - - foreach my $conf (($cluster_conf, $fw_conf)) { - next if !$conf; - if (!$param->{type} || $param->{type} eq 'ipset') { - foreach my $name (keys %{$conf->{ipset}}) { - my $data = { - type => 'ipset', - name => $name, - ref => "+$name", - }; - if (my $comment = $conf->{ipset_comments}->{$name}) { - $data->{comment} = $comment; - } - $ipsets->{$name} = $data; - } - } - - if (!$param->{type} || $param->{type} eq 'alias') { - foreach my $name (keys %{$conf->{aliases}}) { - my $e = $conf->{aliases}->{$name}; - my $data = { - type => 'alias', - name => $name, - ref => $name, - }; - $data->{comment} = $e->{comment} if $e->{comment}; - $aliases->{$name} = $data; - } - } - } - - my $res = []; - foreach my $e (values %$ipsets) { push @$res, $e; }; - foreach my $e (values %$aliases) { push @$res, $e; }; + my $dc_refs = PVE::Firewall::Helpers::collect_refs($cluster_conf, $param->{type}, 'dc'); + my $vm_refs = PVE::Firewall::Helpers::collect_refs($fw_conf, $param->{type}, 'guest'); - return $res; + return [@$dc_refs, @$vm_refs]; }}); } diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm index a8e18e2..7dcbca3 100644 --- a/src/PVE/Firewall/Helpers.pm +++ b/src/PVE/Firewall/Helpers.pm @@ -15,6 +15,7 @@ our @EXPORT_OK = qw( lock_vmfw_conf remove_vmfw_conf clone_vmfw_conf +collect_refs ); my $pvefw_conf_dir = "/etc/pve/firewall"; @@ -130,4 +131,42 @@ sub dump_fw_logfile { return ($state{'count'}, $state{'lines'}); } +sub collect_refs { + my ($conf, $type, $scope) = @_; + + + my $res = []; + + if (!$type || $type eq 'ipset') { + foreach my $name (keys %{$conf->{ipset}}) { + my $data = { + type => 'ipset', + name => $name, + ref => "+$name", + scope => $scope, + }; + if (my $comment = $conf->{ipset_comments}->{$name}) { + $data->{comment} = $comment; + } + push @$res, $data; + } + } + + if (!$type || $type eq 'alias') { + foreach my $name (keys %{$conf->{aliases}}) { + my $e = $conf->{aliases}->{$name}; + my $data = { + type => 'alias', + name => $name, + ref => $name, + scope => $scope, + }; + $data->{comment} = $e->{comment} if $e->{comment}; + push @$res, $data; + } + } + + return $res; +} + 1; -- 2.39.2 From 0d28aa2abcf2d453504049388b71d27a7ba3259b Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 21 Jun 2023 19:17:27 +0200 Subject: [PATCH 04/16] bump version to 5.0.2 Signed-off-by: Thomas Lamprecht --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index 88d1a55..7acf849 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +pve-firewall (5.0.2) bookworm; urgency=medium + + * fix #4556: api: return scoped IPSets and aliases + + -- Proxmox Support Team Wed, 21 Jun 2023 19:17:19 +0200 + pve-firewall (5.0.1) bookworm; urgency=medium * fix #4556: support 'dc/' and 'guest/' prefix for aliases and ipsets -- 2.39.2 From 1f0303f82a7d661024a374ab2cf195835390b738 Mon Sep 17 00:00:00 2001 From: Leo Nunner Date: Tue, 11 Jul 2023 11:41:15 +0200 Subject: [PATCH 05/16] parser: fix scoped alias resolution We tried to resolve aliases in some places where the cluster configuration didn't get set. It's probably better to handle these cases directly in the function at hand, instead of at every place where this issues might arise. This seemingly fixes the issues reported on pve-user and the forum: * https://forum.proxmox.com/threads/pve-8-pve-firewall-status-no-such-alias.130202/ * https://forum.proxmox.com/threads/ipset-not-working-for-accepting-cluster-traffic.129599/ Signed-off-by: Leo Nunner --- src/PVE/API2/Firewall/IPSet.pm | 2 -- src/PVE/Firewall.pm | 18 ++++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm index baa57ca..ed92d87 100644 --- a/src/PVE/API2/Firewall/IPSet.pm +++ b/src/PVE/API2/Firewall/IPSet.pm @@ -203,8 +203,6 @@ sub register_create_ip { if ($cidr =~ m@^(dc/|guest/)?(${PVE::Firewall::ip_alias_pattern})$@) { my $scope = $1 // ""; my $alias = $2; - # on the cluster level - $cluster_conf = $fw_conf if (!$cluster_conf); # make sure alias exists (if $cidr is an alias) PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $alias, $scope); } else { diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 9bed8df..77cbaf4 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -2979,13 +2979,23 @@ sub parse_clusterfw_option { sub resolve_alias { my ($clusterfw_conf, $fw_conf, $cidr, $scope) = @_; + # When we're on the cluster level, the cluster config only gets + # saved into fw_conf, so we need some extra handling here (to + # stay consistent) + my ($cluster_config, $local_config); + if (!$clusterfw_conf) { + ($cluster_config, $local_config) = ($fw_conf, undef); + } else { + ($cluster_config, $local_config) = ($clusterfw_conf, $fw_conf); + } + my $alias = lc($cidr); my $e; - if ($scope ne 'dc/' && $fw_conf) { - $e = $fw_conf->{aliases}->{$alias}; + if ($scope ne 'dc/' && $local_config) { + $e = $local_config->{aliases}->{$alias}; } - if ($scope ne 'guest/' && !$e && $clusterfw_conf) { - $e = $clusterfw_conf->{aliases}->{$alias}; + if ($scope ne 'guest/' && !$e && $cluster_config) { + $e = $cluster_config->{aliases}->{$alias}; } die "no such alias '$cidr'\n" if !$e;; -- 2.39.2 From 372869e075d45997758834ff8283a6b332c3acc2 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Mon, 17 Jul 2023 10:40:17 +0200 Subject: [PATCH 06/16] bump version to 5.0.3 Signed-off-by: Wolfgang Bumiller --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index 7acf849..1c43aff 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +pve-firewall (5.0.3) bookworm; urgency=medium + + * fix resolution of scoped aliases in ipsets + + -- Proxmox Support Team Mon, 17 Jul 2023 10:39:28 +0200 + pve-firewall (5.0.2) bookworm; urgency=medium * fix #4556: api: return scoped IPSets and aliases -- 2.39.2 From 3640b561ed999e89f25701b244163cb4eff2d9b1 Mon Sep 17 00:00:00 2001 From: Daniel Krambrock via pve-devel Date: Tue, 16 Apr 2024 09:26:45 +0200 Subject: [PATCH 07/16] fix #5335: stable sorting in cluster.fw Stable sorting in cluster.fw config file allows tracking changes by checking into git or when using automation like ansible. Signed-off-by: Daniel Krambrock --- 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 77cbaf4..81a8798 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -3360,7 +3360,7 @@ my $format_aliases = sub { my $raw = ''; $raw .= "[ALIASES]\n\n"; - foreach my $k (keys %$aliases) { + foreach my $k (sort keys %$aliases) { my $e = $aliases->{$k}; $raw .= "$e->{name} $e->{cidr}"; $raw .= " # " . encode('utf8', $e->{comment}) -- 2.39.2 From 60abf9397265a366923aa17bdde0c5a9cd3ccd32 Mon Sep 17 00:00:00 2001 From: Stefan Hanreich Date: Fri, 19 Apr 2024 11:42:36 +0200 Subject: [PATCH 08/16] add configuration option for new nftables firewall Introduces new nftables configuration option that en/disables the new nftables firewall. pve-firewall reads this option and only generates iptables rules when nftables is set to `0` or if the proxmox-firewall package is not installed at all. Conversely, proxmox-firewall only generates rules when the option is set to `1`. Signed-off-by: Stefan Hanreich [ TL: mark as tech preview and clarify is_enabled method name ] Signed-off-by: Thomas Lamprecht --- src/PVE/Firewall.pm | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 81a8798..0abfecc 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -1408,6 +1408,12 @@ our $host_option_properties = { default => 0, optional => 1 }, + nftables => { + description => "Enable nftables based firewall (tech preview)", + type => 'boolean', + default => 0, + optional => 1, + }, }; our $vm_option_properties = { @@ -2929,7 +2935,7 @@ sub parse_hostfw_option { my $loglevels = "emerg|alert|crit|err|warning|notice|info|debug|nolog"; - if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood):\s*(0|1)\s*$/i) { + if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) { $opt = lc($1); $value = int($2); } elsif ($line =~ m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) { @@ -4673,12 +4679,30 @@ sub remove_pvefw_chains_ebtables { ebtables_restore_cmdlist(get_ebtables_cmdlist({})); } -sub init { - my $cluster_conf = load_clusterfw_conf(); - my $cluster_options = $cluster_conf->{options}; - my $enable = $cluster_options->{enable}; +sub is_nftables { + my ($cluster_conf, $host_conf) = @_; + + if (!-x "/usr/libexec/proxmox/proxmox-firewall") { + return 0; + } + + $cluster_conf = load_clusterfw_conf() if !defined($cluster_conf); + $host_conf = load_hostfw_conf($cluster_conf) if !defined($host_conf); - return if !$enable; + return $host_conf->{options}->{nftables}; +} + +sub is_enabled_and_not_nftables { + my ($cluster_conf, $host_conf) = @_; + + $cluster_conf = load_clusterfw_conf() if !defined($cluster_conf); + $host_conf = load_hostfw_conf($cluster_conf) if !defined($host_conf); + + return $cluster_conf->{options}->{enable} && !is_nftables($cluster_conf, $host_conf); +} + +sub init { + return if !is_enabled_and_not_nftables(); # load required modules here } @@ -4687,14 +4711,13 @@ sub update { my $code = sub { my $cluster_conf = load_clusterfw_conf(); - my $cluster_options = $cluster_conf->{options}; + my $hostfw_conf = load_hostfw_conf($cluster_conf); - if (!$cluster_options->{enable}) { + if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) { PVE::Firewall::remove_pvefw_chains(); return; } - my $hostfw_conf = load_hostfw_conf($cluster_conf); my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = compile($cluster_conf, $hostfw_conf); -- 2.39.2 From 50af7e0970c665072c03ff96aaec1e4621ae41d7 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 19 Apr 2024 20:04:25 +0200 Subject: [PATCH 09/16] bump version to 5.0.4 Signed-off-by: Thomas Lamprecht --- debian/changelog | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/debian/changelog b/debian/changelog index 1c43aff..0128bc0 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +pve-firewall (5.0.4) bookworm; urgency=medium + + * fix #5335: stable sorting in cluster.fw + + * add configuration option for new nftables firewall tech-preview + + -- Proxmox Support Team Fri, 19 Apr 2024 20:04:09 +0200 + pve-firewall (5.0.3) bookworm; urgency=medium * fix resolution of scoped aliases in ipsets -- 2.39.2 From b5c4c33632827f25b612e2c54b655d87e74fef82 Mon Sep 17 00:00:00 2001 From: Stefan Hanreich Date: Fri, 12 Apr 2024 10:07:36 +0200 Subject: [PATCH 10/16] simulator: use new bridge naming scheme MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We now allow bridges without the vmbr prefix, so we need to allow them here in the simulator as well. Reviewed-by: Fabian Grünbichler Signed-off-by: Stefan Hanreich --- src/PVE/FirewallSimulator.pm | 29 +++++++++++++++++++---------- src/PVE/Service/pve_firewall.pm | 5 +++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/PVE/FirewallSimulator.pm b/src/PVE/FirewallSimulator.pm index 140c46e..fa5ed0e 100644 --- a/src/PVE/FirewallSimulator.pm +++ b/src/PVE/FirewallSimulator.pm @@ -7,6 +7,12 @@ use PVE::Firewall; use File::Basename; use Net::IP; +use base 'Exporter'; +our @EXPORT_OK = qw( +$bridge_name_pattern +$bridge_interface_pattern +); + # dynamically include PVE::QemuServer and PVE::LXC # to avoid dependency problems my $have_qemu_server; @@ -27,6 +33,9 @@ my $debug = 0; my $NUMBER_RE = qr/0x[0-9a-fA-F]+|\d+/; +our $bridge_name_pattern = '[a-zA-Z][a-zA-Z0-9]{0,9}'; +our $bridge_interface_pattern = "($bridge_name_pattern)/(\\S+)"; + sub debug { my $new_value = shift; $debug = $new_value if defined($new_value); @@ -397,7 +406,7 @@ sub route_packet { $pkg->{physdev_in} = $target->{fwln} || die 'internal error'; $pkg->{physdev_out} = $target->{tapdev} || die 'internal error'; - } elsif ($route_state =~ m/^vmbr\d+$/) { + } elsif ($route_state =~ m/^$bridge_name_pattern$/) { die "missing physdev_in - internal error?" if !$physdev_in; $pkg->{physdev_in} = $physdev_in; @@ -531,11 +540,6 @@ sub simulate_firewall { $from_info->{type} = 'host'; $start_state = 'host'; $pkg->{source} = $host_ip if !defined($pkg->{source}); - } elsif ($from =~ m|^(vmbr\d+)/(\S+)$|) { - $from_info->{type} = 'bport'; - $from_info->{bridge} = $1; - $from_info->{iface} = $2; - $start_state = 'from-bport'; } elsif ($from eq 'outside') { $from_info->{type} = 'bport'; $from_info->{bridge} = 'vmbr0'; @@ -559,6 +563,11 @@ sub simulate_firewall { $from_info = extract_vm_info($vmdata, $vmid, $netnum); $start_state = 'fwbr-out'; $pkg->{mac_source} = $from_info->{macaddr}; + } elsif ($from =~ m|^$bridge_interface_pattern$|) { + $from_info->{type} = 'bport'; + $from_info->{bridge} = $1; + $from_info->{iface} = $2; + $start_state = 'from-bport'; } else { die "unable to parse \"from => '$from'\"\n"; } @@ -569,10 +578,6 @@ sub simulate_firewall { $target->{type} = 'host'; $target->{iface} = 'host'; $pkg->{dest} = $host_ip if !defined($pkg->{dest}); - } elsif ($to =~ m|^(vmbr\d+)/(\S+)$|) { - $target->{type} = 'bport'; - $target->{bridge} = $1; - $target->{iface} = $2; } elsif ($to eq 'outside') { $target->{type} = 'bport'; $target->{bridge} = 'vmbr0'; @@ -591,6 +596,10 @@ sub simulate_firewall { my $vmid = $1; $target = extract_vm_info($vmdata, $vmid, 0); $target->{iface} = $target->{tapdev}; + } elsif ($to =~ m|^$bridge_interface_pattern$|) { + $target->{type} = 'bport'; + $target->{bridge} = $1; + $target->{iface} = $2; } else { die "unable to parse \"to => '$to'\"\n"; } diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm index 30d14d9..65cb2b8 100755 --- a/src/PVE/Service/pve_firewall.pm +++ b/src/PVE/Service/pve_firewall.pm @@ -18,6 +18,7 @@ use PVE::Tools qw(dir_glob_foreach file_read_firstline); use PVE::Firewall; use PVE::FirewallSimulator; +use PVE::FirewallSimulator qw($bridge_interface_pattern); use base qw(PVE::Daemon); @@ -312,14 +313,14 @@ __PACKAGE__->register_method ({ from => { description => "Source zone.", type => 'string', - pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)', + pattern => "(host|outside|vm\\d+|ct\\d+|$bridge_interface_pattern)", optional => 1, default => 'outside', }, to => { description => "Destination zone.", type => 'string', - pattern => '(host|outside|vm\d+|ct\d+|vmbr\d+/\S+)', + pattern => "(host|outside|vm\\d+|ct\\d+|$bridge_interface_pattern)", optional => 1, default => 'host', }, -- 2.39.2 From 29b48c381d14bf425232dc65c9c0d18f95c8f222 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 23 Apr 2024 13:11:55 +0200 Subject: [PATCH 11/16] bump version to 5.0.5 Signed-off-by: Thomas Lamprecht --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index 0128bc0..72794fb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +pve-firewall (5.0.5) bookworm; urgency=medium + + * simulator: adapt to more flexible bridge naming scheme + + -- Proxmox Support Team Tue, 23 Apr 2024 13:11:43 +0200 + pve-firewall (5.0.4) bookworm; urgency=medium * fix #5335: stable sorting in cluster.fw -- 2.39.2 From e59c8fd1b5294b59837432c70787bf9bbd5ccc03 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 26 Apr 2024 15:43:27 +0200 Subject: [PATCH 12/16] buildsys: avoid that the dinstall target always triggers a rebuild Signed-off-by: Thomas Lamprecht --- Makefile | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 74be378..e5d1634 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,8 @@ DEBS=$(DEB) $(DEB2) all: $(DEBS) .PHONY: dinstall -dinstall: deb - dpkg -i $(DEBS) +dinstall: $(DEB) + dpkg -i $< $(BUILDDIR): rm -rf $(BUILDDIR) @@ -25,7 +25,7 @@ $(BUILDDIR): .PHONY: deb deb: $(DEBS) $(DEB2): $(DEB) -$(DEB): $(BUILDDIR) check +$(DEB): $(BUILDDIR) cd $(BUILDDIR); dpkg-buildpackage -b -us -uc lintian $(DEBS) @@ -41,7 +41,6 @@ $(DSC): $(BUILDDIR) sbuild: $(DSC) sbuild $(DSC) -.PHONY: check check: make -C test check -- 2.39.2 From 719bdfaab90a3b6b82779791ddd54df7beaa7a56 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 26 Apr 2024 16:04:51 +0200 Subject: [PATCH 13/16] service: create flag file to signal if nftables impl should not run The new nftables/rust based proxmox-firewall is still a WIP w.r.t. understanding all oddities the firewall config provides. This is not a problem in general, as it's released as tech-preview, but the new service needs to parse the config to check if it's enabled, so if that fails due to not recognizing some edge case, the users get some scary looking log-spam. So use a flag in the memory-backed /run as a side-channel that does not need any parsing to signal if the new implementation should be disabled. This can be removed again once proxmox-firewall covers all possible cases for sure and/or becomes the new default. Signed-off-by: Thomas Lamprecht Tested-by: Stefan Hanreich --- src/PVE/Firewall.pm | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 0abfecc..5dca91f 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -4707,6 +4707,9 @@ sub init { # load required modules here } +# This is checked in proxmox-firewall to avoid log-spam due to failing to parse the config +my $FORCE_NFT_DISABLE_FLAG_FILE = "/run/proxmox-nftables-firewall-force-disable"; + sub update { my $code = sub { @@ -4714,9 +4717,15 @@ sub update { my $hostfw_conf = load_hostfw_conf($cluster_conf); if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) { + unlink($FORCE_NFT_DISABLE_FLAG_FILE) + or $!{ENOENT} or warn "failed to unlink flag file '$FORCE_NFT_DISABLE_FLAG_FILE' - $!\n"; PVE::Firewall::remove_pvefw_chains(); return; } + if (! -e $FORCE_NFT_DISABLE_FLAG_FILE) { + open(my $_fh, '>', $FORCE_NFT_DISABLE_FLAG_FILE) + or warn "failed to create flag file '$FORCE_NFT_DISABLE_FLAG_FILE' – $!\n"; + } my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = compile($cluster_conf, $hostfw_conf); -- 2.39.2 From c71345969d59faa1925533121c64db63249b3a65 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 26 Apr 2024 17:19:57 +0200 Subject: [PATCH 14/16] bump version to 5.0.6 Signed-off-by: Thomas Lamprecht --- debian/changelog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/debian/changelog b/debian/changelog index 72794fb..44f5d82 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +pve-firewall (5.0.6) bookworm; urgency=medium + + * add flag to signal the new nftables-based proxmox-firewall that it's + disabled without the need to parse the config + + -- Proxmox Support Team Fri, 26 Apr 2024 17:19:50 +0200 + pve-firewall (5.0.5) bookworm; urgency=medium * simulator: adapt to more flexible bridge naming scheme -- 2.39.2 From 21e5d5223286983de0ec89d6635fa736e46a1928 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 30 Apr 2024 10:27:18 +0200 Subject: [PATCH 15/16] also signal force-disable nftables if FW is completely disabled If the FW is disabled on cluster level then touch the file flag to signal that the nftables FW should not run, to avoid that a config that uses some keys the new ipl doesn't yet understand causes log-spam there. Signed-off-by: Thomas Lamprecht --- src/PVE/Firewall.pm | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 5dca91f..09544ba 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -4692,13 +4692,34 @@ sub is_nftables { return $host_conf->{options}->{nftables}; } +my sub update_force_nftables_disable_flag { + my ($cluster_firewall_enabled, $is_nftables) = @_; + + # This is checked in proxmox-firewall to avoid log-spam due to failing to parse the config + my $FORCE_NFT_DISABLE_FLAG_FILE = "/run/proxmox-nftables-firewall-force-disable"; + + if (!($cluster_firewall_enabled && $is_nftables)) { + if (! -e $FORCE_NFT_DISABLE_FLAG_FILE) { + open(my $_fh, '>', $FORCE_NFT_DISABLE_FLAG_FILE) + or warn "failed to create flag file '$FORCE_NFT_DISABLE_FLAG_FILE' – $!\n"; + } + } else { + unlink($FORCE_NFT_DISABLE_FLAG_FILE) + or $!{ENOENT} or warn "failed to unlink flag file '$FORCE_NFT_DISABLE_FLAG_FILE' - $!\n"; + } +} + sub is_enabled_and_not_nftables { my ($cluster_conf, $host_conf) = @_; $cluster_conf = load_clusterfw_conf() if !defined($cluster_conf); $host_conf = load_hostfw_conf($cluster_conf) if !defined($host_conf); - return $cluster_conf->{options}->{enable} && !is_nftables($cluster_conf, $host_conf); + my $is_nftables = is_nftables($cluster_conf, $host_conf); + + update_force_nftables_disable_flag($cluster_conf->{options}->{enable}, $is_nftables); + + return $cluster_conf->{options}->{enable} && !$is_nftables; } sub init { @@ -4707,9 +4728,6 @@ sub init { # load required modules here } -# This is checked in proxmox-firewall to avoid log-spam due to failing to parse the config -my $FORCE_NFT_DISABLE_FLAG_FILE = "/run/proxmox-nftables-firewall-force-disable"; - sub update { my $code = sub { @@ -4717,16 +4735,9 @@ sub update { my $hostfw_conf = load_hostfw_conf($cluster_conf); if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) { - unlink($FORCE_NFT_DISABLE_FLAG_FILE) - or $!{ENOENT} or warn "failed to unlink flag file '$FORCE_NFT_DISABLE_FLAG_FILE' - $!\n"; PVE::Firewall::remove_pvefw_chains(); return; } - if (! -e $FORCE_NFT_DISABLE_FLAG_FILE) { - open(my $_fh, '>', $FORCE_NFT_DISABLE_FLAG_FILE) - or warn "failed to create flag file '$FORCE_NFT_DISABLE_FLAG_FILE' – $!\n"; - } - my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = compile($cluster_conf, $hostfw_conf); -- 2.39.2 From 4339ef1526fd482f800438fbdeec2f6b50133be2 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 30 Apr 2024 10:30:25 +0200 Subject: [PATCH 16/16] bump version to 5.0.7 Signed-off-by: Thomas Lamprecht --- debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/debian/changelog b/debian/changelog index 44f5d82..7d62a41 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +pve-firewall (5.0.7) bookworm; urgency=medium + + * also signal force-disable nftables if FW is completely disabled + + -- Proxmox Support Team Tue, 30 Apr 2024 10:30:16 +0200 + pve-firewall (5.0.6) bookworm; urgency=medium * add flag to signal the new nftables-based proxmox-firewall that it's -- 2.39.2