From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 897081FF09B for ; Mon, 12 Oct 2026 11:40:26 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BE017213E2; Mon, 12 Oct 2026 11:40:22 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH pve-firewall 2/2] firewall: ebtables: honor nomatch in ipfilter ipsets Date: Mon, 12 Oct 2026 11:38:08 +0200 Message-ID: <20261012093956.285433-3-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261012093956.285433-1-a.bied-charreton@proxmox.com> References: <20261012093956.285433-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791798016802 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.038 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: MWYUX4R4QPUW2GZRCG556HQFJCQ7UJCF X-Message-ID-Hash: MWYUX4R4QPUW2GZRCG556HQFJCQ7UJCF X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The ARP filter turnes every ipfilter-netX entry into a RETURN rule, including nomatch entries. An address excluded from an allowed network with "!" is therefore still accepted as an ARP source. Emit DROP for nomatch entries and order the rules by prefix length. The rules are sorted most-specific first, and equal prefix lengths keep their original order. Signed-off-by: Arthur Bied-Charreton --- src/PVE/Firewall.pm | 18 ++++++++++--- src/test/test_ebtables_ipfilter.pl | 42 ++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index bb3350b..9630d58 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -4834,7 +4834,7 @@ my sub resolve_arpfilter { } next if $ver != 4; - push $arpfilter->@*, $ip; + push $arpfilter->@*, { cidr => $ip, nomatch => $entry->{nomatch} }; } return $arpfilter; @@ -4916,7 +4916,7 @@ sub compile_ebtables_filter { # want the address here, no network - see #2193 $ip =~ s|/(\d+)$||; if ($ip ne 'dhcp') { - push @$arpfilter, $ip; + push $arpfilter->@*, { cidr => $ip }; } } generate_tap_layer2filter( @@ -4958,8 +4958,18 @@ sub generate_tap_layer2filter { ruleset_addrule($ruleset, $tapchain, "-p ARP", "-j $arpchain"); ruleset_create_chain($ruleset, $arpchain); - foreach my $ip (@{$arpfilter}) { - ruleset_addrule($ruleset, $arpchain, "-p ARP --arp-ip-src $ip", '-j RETURN'); + my $prefix_len = sub { $arpfilter->[$_[0]]->{cidr} =~ m|/(\d+)$| ? $1 : 32 }; + my @order = + sort { $prefix_len->($b) <=> $prefix_len->($a) || $a <=> $b } 0 .. $arpfilter->$#*; + + for my $entry ($arpfilter->@[@order]) { + my $target = $entry->{nomatch} ? 'DROP' : 'RETURN'; + ruleset_addrule( + $ruleset, + $arpchain, + "-p ARP --arp-ip-src $entry->{cidr}", + "-j $target", + ); } ruleset_addrule($ruleset, $arpchain, '', '-j DROP'); } diff --git a/src/test/test_ebtables_ipfilter.pl b/src/test/test_ebtables_ipfilter.pl index 9716ac2..069ee6c 100755 --- a/src/test/test_ebtables_ipfilter.pl +++ b/src/test/test_ebtables_ipfilter.pl @@ -40,9 +40,11 @@ sub compile_arp_sources { PVE::Firewall::compile_ebtables_filter($cluster, {}, { 100 => $vmfw_conf }, $vmdata); ok(exists($ruleset->{'tap100i0-OUT'}), 'guest keeps its layer-2 filter'); - my @sources = map { /--arp-ip-src (\S+)/ ? $1 : () } ($ruleset->{'tap100i0-OUT-ARP'} // [])->@*; + my $arp_rules = $ruleset->{'tap100i0-OUT-ARP'} // []; + my @sources = map { /--arp-ip-src (\S+) -j RETURN/ ? $1 : () } $arp_rules->@*; + my @rules = map { /--arp-ip-src (\S+) -j (\S+)/ ? "$2 $1" : () } $arp_rules->@*; - return (\@sources, \@warnings); + return (\@sources, \@warnings, \@rules); } subtest 'aliases are resolved' => sub { @@ -82,4 +84,40 @@ subtest 'unresolvable entries are skipped with context' => sub { ); }; +subtest 'nomatch entries are excluded' => sub { + my (undef, $warnings, $rules) = compile_arp_sources([ + { cidr => '192.0.2.0/24' }, { cidr => '192.0.2.5', nomatch => 1 }, + ]); + is_deeply( + $rules, + ['DROP 192.0.2.5', 'RETURN 192.0.2.0/24'], + 'excluded address is dropped before the allowed network', + ); + is_deeply($warnings, [], 'no warnings'); +}; + +subtest 'more specific entries take precedence over nomatch networks' => sub { + my (undef, $warnings, $rules) = compile_arp_sources([ + { cidr => '192.0.2.0/24', nomatch => 1 }, { cidr => '192.0.2.5' }, + ]); + is_deeply( + $rules, + ['RETURN 192.0.2.5', 'DROP 192.0.2.0/24'], + 'allowed address is returned before the excluded network', + ); + is_deeply($warnings, [], 'no warnings'); +}; + +subtest 'entries with the same prefix length keep their order' => sub { + my (undef, $warnings, $rules) = compile_arp_sources([ + { cidr => '192.0.2.20' }, { cidr => '192.0.2.0/24' }, { cidr => '192.0.2.3' }, + ]); + is_deeply( + $rules, + ['RETURN 192.0.2.20', 'RETURN 192.0.2.3', 'RETURN 192.0.2.0/24'], + 'ties are not reordered', + ); + is_deeply($warnings, [], 'no warnings'); +}; + done_testing(); -- 2.47.3