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 | 131 ++++++++++++++++-- .../integration_tests__firewall.snap | 100 +++++++++++++ 2 files changed, 218 insertions(+), 13 deletions(-) diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index 2512537..570a172 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -283,12 +283,15 @@ impl ToNftRules for RuleMatch { } } +/// Handle matching on an ipset. +/// +/// This function adds statements to the `rules` that match on the ipset with the given name +/// `name`. It matches the IPs contained in the ipset with the field given in `field_name`. 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 +306,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 +317,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 +340,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 +351,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 +375,114 @@ fn handle_set( Ok(()) } +/// Handle matching on an ipfilter. +/// +/// This function adds statements to the `rules` that match if the IP in the field `field_name` +/// does not fulfill the criteria of the given 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::V6); + + 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::V6); + + 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 +558,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 +790,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..71285af 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -4279,6 +4279,31 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "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" + } + }, { "match": { "op": "==", @@ -4328,6 +4353,31 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "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" + } + }, { "match": { "op": "==", @@ -4579,6 +4629,31 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "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" + } + }, { "match": { "op": "==", @@ -4628,6 +4703,31 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "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" + } + }, { "match": { "op": "==", -- 2.47.3 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
