From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 0DE271FF165 for ; Thu, 25 Sep 2025 12:05:29 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 127B01D146; Thu, 25 Sep 2025 12:05:51 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Thu, 25 Sep 2025 12:05:09 +0200 Message-ID: <20250925100514.130484-5-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20250925100514.130484-1-s.hanreich@proxmox.com> References: <20250925100514.130484-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.183 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pve-devel] [PATCH proxmox-firewall 3/3] fix #6336: fix ipfilter matching logic X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Matching on ipsets in the firewall generally works by matching on two sets (one for match, one for nomatch): ip saddr @ipfilter ip saddr != @ipfilter-nomatch Ipfilters were created with the comparison operators simply inverted, which leads to ipfilters with empty nomatch sets never working, since the second expression always evaluates to false on empty sets: ip saddr != @ipfilter ip saddr = @ipfilter-nomatch drop In order to properly invert the logic, two statements need to be generated (analogous to negation in boolean logic): ip saddr != @ipfilter drop ip saddr = @ipfilter-nomatch drop To avoid cluttering the existing set handling function and to clearly separate the rule generation logic, introduce a completely new function that handles ipfilters separately. This also allows the set handling function to validate the name of ipsets in a later commit, since it only operates on ipsets that have to exist in the firewall config, whereas ipfilters do not necessarily exist in the firewall configuration. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/rule.rs | 125 ++++++++- .../integration_tests__firewall.snap | 238 +++++++++++++----- 2 files changed, 281 insertions(+), 82 deletions(-) diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index 2512537..6332a74 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -283,12 +283,12 @@ impl ToNftRules for RuleMatch { } } +/// Adds statements that evaluate to true if an element does match an IPSet fn handle_set( rules: &mut Vec, name: &IpsetName, field_name: &str, env: &NftRuleEnv, - contains: bool, ) -> Result<(), Error> { let mut new_rules = rules .drain(..) @@ -303,7 +303,7 @@ fn handle_set( rule.append(&mut vec![ Match::new( - if contains { Operator::Eq } else { Operator::Ne }, + Operator::Eq, field.clone(), Expression::set_name(&SetName::ipset_name( Family::V4, @@ -314,7 +314,7 @@ fn handle_set( ) .into(), Match::new( - if contains { Operator::Ne } else { Operator::Eq }, + Operator::Ne, field, Expression::set_name(&SetName::ipset_name( Family::V4, @@ -337,7 +337,7 @@ fn handle_set( rule.append(&mut vec![ Match::new( - if contains { Operator::Eq } else { Operator::Ne }, + Operator::Eq, field.clone(), Expression::set_name(&SetName::ipset_name( Family::V6, @@ -348,7 +348,7 @@ fn handle_set( ) .into(), Match::new( - if contains { Operator::Ne } else { Operator::Eq }, + Operator::Ne, field, Expression::set_name(&SetName::ipset_name( Family::V6, @@ -372,6 +372,111 @@ fn handle_set( Ok(()) } +/// Adds statements that evaluate to true if an element does *not* match an IPFilter +fn handle_ipfilter( + rules: &mut Vec, + name: &IpsetName, + field_name: &str, + env: &NftRuleEnv, +) -> Result<(), Error> { + let mut new_rules = rules + .drain(..) + .flat_map(|rule| { + let mut new_rules = Vec::new(); + + if matches!(rule.family(), Some(Family::V4) | None) && env.contains_family(Family::V4) { + let field = Payload::field("ip", field_name); + + let mut match_rule = rule.clone(); + match_rule.set_family(Family::V4); + + match_rule.push( + Match::new( + Operator::Ne, + field.clone(), + Expression::set_name(&SetName::ipset_name( + Family::V4, + name, + env.vmid, + false, + )), + ) + .into(), + ); + + new_rules.push(match_rule); + + let mut nomatch_rule = rule.clone(); + nomatch_rule.set_family(Family::V4); + + nomatch_rule.push( + Match::new( + Operator::Eq, + field, + Expression::set_name(&SetName::ipset_name( + Family::V4, + name, + env.vmid, + true, + )), + ) + .into(), + ); + + new_rules.push(nomatch_rule); + } + + if matches!(rule.family(), Some(Family::V6) | None) && env.contains_family(Family::V6) { + let field = Payload::field("ip6", field_name); + + let mut match_rule = rule.clone(); + match_rule.set_family(Family::V4); + + match_rule.push( + Match::new( + Operator::Ne, + field.clone(), + Expression::set_name(&SetName::ipset_name( + Family::V6, + name, + env.vmid, + false, + )), + ) + .into(), + ); + + new_rules.push(match_rule); + + let mut nomatch_rule = rule.clone(); + nomatch_rule.set_family(Family::V4); + + nomatch_rule.push( + Match::new( + Operator::Eq, + field, + Expression::set_name(&SetName::ipset_name( + Family::V6, + name, + env.vmid, + true, + )), + ) + .into(), + ); + + new_rules.push(nomatch_rule); + } + + new_rules + }) + .collect::>(); + + rules.append(&mut new_rules); + + Ok(()) +} + fn handle_match( rules: &mut Vec, ip: &IpAddrMatch, @@ -447,7 +552,7 @@ fn handle_match( Ok(()) } - IpAddrMatch::Set(name) => handle_set(rules, name, field_name, env, true), + IpAddrMatch::Set(name) => handle_set(rules, name, field_name, env), } } @@ -679,13 +784,7 @@ impl ToNftRules for Ipfilter<'_> { ); let mut ipfilter_rules = vec![base_rule.clone()]; - handle_set( - &mut ipfilter_rules, - self.ipset().name(), - "saddr", - env, - false, - )?; + handle_ipfilter(&mut ipfilter_rules, self.ipset().name(), "saddr", env)?; rules.append(&mut ipfilter_rules); if env.contains_family(Family::V4) { diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index feeda5b..c7d45f1 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -4112,7 +4112,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net1", + "name": "v4-guest-100/ipfilter-net3", "type": "ipv4_addr", "flags": [ "interval" @@ -4125,7 +4125,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net1" + "name": "v4-guest-100/ipfilter-net3" } } }, @@ -4134,7 +4134,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net1-nomatch", + "name": "v4-guest-100/ipfilter-net3-nomatch", "type": "ipv4_addr", "flags": [ "interval" @@ -4147,7 +4147,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net1-nomatch" + "name": "v4-guest-100/ipfilter-net3-nomatch" } } }, @@ -4156,12 +4156,12 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "element": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net1", + "name": "v4-guest-100/ipfilter-net3", "elem": [ { "prefix": { - "addr": "172.16.100.0", - "len": 24 + "addr": "192.0.2.10", + "len": 32 } } ] @@ -4173,7 +4173,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net1", + "name": "v6-guest-100/ipfilter-net3", "type": "ipv6_addr", "flags": [ "interval" @@ -4186,7 +4186,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net1" + "name": "v6-guest-100/ipfilter-net3" } } }, @@ -4195,7 +4195,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net1-nomatch", + "name": "v6-guest-100/ipfilter-net3-nomatch", "type": "ipv6_addr", "flags": [ "interval" @@ -4208,7 +4208,30 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net1-nomatch" + "name": "v6-guest-100/ipfilter-net3-nomatch" + } + } + }, + { + "add": { + "element": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "name": "v6-guest-100/ipfilter-net3", + "elem": [ + { + "prefix": { + "addr": "fe80::be24:11ff:fe4d:b0fe", + "len": 128 + } + }, + { + "prefix": { + "addr": "fd80::1235", + "len": 128 + } + } + ] } } }, @@ -4227,7 +4250,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "oifname" } }, - "right": "veth100i1" + "right": "veth100i3" } }, { @@ -4239,7 +4262,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "daddr ip" } }, - "right": "@v4-guest-100/ipfilter-net1" + "right": "@v4-guest-100/ipfilter-net3" } }, { @@ -4264,7 +4287,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i1" + "right": "veth100i3" } }, { @@ -4276,7 +4299,32 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v4-guest-100/ipfilter-net1" + "right": "@v4-guest-100/ipfilter-net3" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "iifname" + } + }, + "right": "veth100i3" } }, { @@ -4288,7 +4336,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v4-guest-100/ipfilter-net1-nomatch" + "right": "@v4-guest-100/ipfilter-net3-nomatch" } }, { @@ -4313,7 +4361,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i1" + "right": "veth100i3" } }, { @@ -4325,7 +4373,32 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v6-guest-100/ipfilter-net1" + "right": "@v6-guest-100/ipfilter-net3" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "iifname" + } + }, + "right": "veth100i3" } }, { @@ -4337,7 +4410,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v6-guest-100/ipfilter-net1-nomatch" + "right": "@v6-guest-100/ipfilter-net3-nomatch" } }, { @@ -4362,7 +4435,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i1" + "right": "veth100i3" } }, { @@ -4374,7 +4447,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr ip" } }, - "right": "@v4-guest-100/ipfilter-net1" + "right": "@v4-guest-100/ipfilter-net3" } }, { @@ -4389,7 +4462,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net3", + "name": "v4-guest-100/ipfilter-net1", "type": "ipv4_addr", "flags": [ "interval" @@ -4402,7 +4475,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net3" + "name": "v4-guest-100/ipfilter-net1" } } }, @@ -4411,7 +4484,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net3-nomatch", + "name": "v4-guest-100/ipfilter-net1-nomatch", "type": "ipv4_addr", "flags": [ "interval" @@ -4424,7 +4497,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net3-nomatch" + "name": "v4-guest-100/ipfilter-net1-nomatch" } } }, @@ -4433,12 +4506,12 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "element": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v4-guest-100/ipfilter-net3", + "name": "v4-guest-100/ipfilter-net1", "elem": [ { "prefix": { - "addr": "192.0.2.10", - "len": 32 + "addr": "172.16.100.0", + "len": 24 } } ] @@ -4450,7 +4523,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net3", + "name": "v6-guest-100/ipfilter-net1", "type": "ipv6_addr", "flags": [ "interval" @@ -4463,7 +4536,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net3" + "name": "v6-guest-100/ipfilter-net1" } } }, @@ -4472,7 +4545,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net3-nomatch", + "name": "v6-guest-100/ipfilter-net1-nomatch", "type": "ipv6_addr", "flags": [ "interval" @@ -4485,30 +4558,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "set": { "family": "bridge", "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net3-nomatch" - } - } - }, - { - "add": { - "element": { - "family": "bridge", - "table": "proxmox-firewall-guests", - "name": "v6-guest-100/ipfilter-net3", - "elem": [ - { - "prefix": { - "addr": "fe80::be24:11ff:fe4d:b0fe", - "len": 128 - } - }, - { - "prefix": { - "addr": "fd80::1235", - "len": 128 - } - } - ] + "name": "v6-guest-100/ipfilter-net1-nomatch" } } }, @@ -4527,7 +4577,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "oifname" } }, - "right": "veth100i3" + "right": "veth100i1" } }, { @@ -4539,7 +4589,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "daddr ip" } }, - "right": "@v4-guest-100/ipfilter-net3" + "right": "@v4-guest-100/ipfilter-net1" } }, { @@ -4564,7 +4614,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i3" + "right": "veth100i1" } }, { @@ -4576,7 +4626,32 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v4-guest-100/ipfilter-net3" + "right": "@v4-guest-100/ipfilter-net1" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "iifname" + } + }, + "right": "veth100i1" } }, { @@ -4588,7 +4663,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v4-guest-100/ipfilter-net3-nomatch" + "right": "@v4-guest-100/ipfilter-net1-nomatch" } }, { @@ -4613,7 +4688,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i3" + "right": "veth100i1" } }, { @@ -4625,7 +4700,32 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v6-guest-100/ipfilter-net3" + "right": "@v6-guest-100/ipfilter-net1" + } + }, + { + "drop": null + } + ] + } + } + }, + { + "add": { + "rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-out", + "expr": [ + { + "match": { + "op": "==", + "left": { + "meta": { + "key": "iifname" + } + }, + "right": "veth100i1" } }, { @@ -4637,7 +4737,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr" } }, - "right": "@v6-guest-100/ipfilter-net3-nomatch" + "right": "@v6-guest-100/ipfilter-net1-nomatch" } }, { @@ -4662,7 +4762,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "key": "iifname" } }, - "right": "veth100i3" + "right": "veth100i1" } }, { @@ -4674,7 +4774,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "field": "saddr ip" } }, - "right": "@v4-guest-100/ipfilter-net3" + "right": "@v4-guest-100/ipfilter-net1" } }, { @@ -5036,7 +5136,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "name": "vm-map-in", "elem": [ [ - "veth100i1", + "veth100i3", { "goto": { "target": "guest-100-in" @@ -5044,7 +5144,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } ], [ - "veth100i3", + "veth100i1", { "goto": { "target": "guest-100-in" @@ -5188,7 +5288,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "name": "vm-map-out", "elem": [ [ - "veth100i1", + "veth100i3", { "goto": { "target": "guest-100-out" @@ -5196,7 +5296,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } ], [ - "veth100i3", + "veth100i1", { "goto": { "target": "guest-100-out" -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel