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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.