From: Stefan Hanreich <s.hanreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH proxmox-firewall 3/3] fix #6336: fix ipfilter matching logic
Date: Thu, 25 Sep 2025 12:05:09 +0200 [thread overview]
Message-ID: <20250925100514.130484-5-s.hanreich@proxmox.com> (raw)
In-Reply-To: <20250925100514.130484-1-s.hanreich@proxmox.com>
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 <verdict>
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 <s.hanreich@proxmox.com>
---
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<NftRule>,
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<NftRule>,
+ 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::<Vec<NftRule>>();
+
+ rules.append(&mut new_rules);
+
+ Ok(())
+}
+
fn handle_match(
rules: &mut Vec<NftRule>,
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
next prev parent reply other threads:[~2025-09-25 10:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-25 10:05 [pve-devel] [PATCH proxmox{-ve-rs, -firewall} 0/4] Fix ipfilters in proxmox-firewall Stefan Hanreich
2025-09-25 10:05 ` [pve-devel] [PATCH proxmox-ve-rs 1/1] config: guest: store network devices in BTreeMap Stefan Hanreich
2025-09-25 10:05 ` [pve-devel] [PATCH proxmox-firewall 1/3] run rustfmt Stefan Hanreich
2025-09-25 10:05 ` [pve-devel] [PATCH proxmox-firewall 2/3] ipfilter: fix wrong entries for containers Stefan Hanreich
2025-09-25 10:05 ` Stefan Hanreich [this message]
2025-09-25 12:22 ` [pve-devel] superseded: [PATCH proxmox{-ve-rs, -firewall} 0/4] Fix ipfilters in proxmox-firewall Stefan Hanreich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250925100514.130484-5-s.hanreich@proxmox.com \
--to=s.hanreich@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox