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 <[email protected]> --- 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 [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
