If a rule had both type and code set, then proxmox-firewall would only generate a rule that matched on the specified icmp code, but not the icmp type. This means that a code was blocked for every ICMP type.
Change up the order of type and code as well, such that the statement matching on the type is first. This makes the generated rules easier to read and more logical sense (semantics of codes are dependent on the type). Signed-off-by: Stefan Hanreich <[email protected]> --- proxmox-firewall/src/rule.rs | 32 +++++++++++++++---------- proxmox-firewall/tests/input/cluster.fw | 3 +++ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index 07f7924..0a4f110 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -651,17 +651,21 @@ impl ToNftRules for Icmp { fn to_nft_rules(&self, rules: &mut Vec<NftRule>, _env: &NftRuleEnv) -> Result<(), Error> { for rule in rules.iter_mut() { if matches!(rule.family(), Some(Family::V4) | None) { - if let Some(icmp_code) = self.code() { + if let Some(icmp_type) = self.ty() { rule.push( - Match::new_eq(Payload::field("icmp", "code"), Expression::from(icmp_code)) + Match::new_eq(Payload::field("icmp", "type"), Expression::from(icmp_type)) .into(), ); - } else if let Some(icmp_type) = self.ty() { + } + + if let Some(icmp_code) = self.code() { rule.push( - Match::new_eq(Payload::field("icmp", "type"), Expression::from(icmp_type)) + Match::new_eq(Payload::field("icmp", "code"), Expression::from(icmp_code)) .into(), ); - } else { + } + + if self.code().is_none() && self.ty().is_none() { rule.push(Match::new_eq(Meta::new("l4proto"), Expression::from("icmp")).into()); } @@ -679,23 +683,27 @@ impl ToNftRules for Icmpv6 { for rule in rules.iter_mut() { if matches!(rule.family(), Some(Family::V6) | None) { - if let Some(icmp_code) = self.code() { + if let Some(icmp_type) = self.ty() { rule.push( Match::new_eq( - Payload::field("icmpv6", "code"), - Expression::from(icmp_code), + Payload::field("icmpv6", "type"), + Expression::from(icmp_type), ) .into(), ); - } else if let Some(icmp_type) = self.ty() { + } + + if let Some(icmp_code) = self.code() { rule.push( Match::new_eq( - Payload::field("icmpv6", "type"), - Expression::from(icmp_type), + Payload::field("icmpv6", "code"), + Expression::from(icmp_code), ) .into(), ); - } else { + } + + if self.code().is_none() && self.ty().is_none() { rule.push( Match::new_eq(Meta::new("l4proto"), Expression::from("icmpv6")).into(), ); diff --git a/proxmox-firewall/tests/input/cluster.fw b/proxmox-firewall/tests/input/cluster.fw index 4f50cc1..c3460c8 100644 --- a/proxmox-firewall/tests/input/cluster.fw +++ b/proxmox-firewall/tests/input/cluster.fw @@ -22,6 +22,9 @@ dc/network1 GROUP network1 -i eth0 IN ACCEPT -log nolog IN ACCEPT -dest +network1 +FORWARD DROP -p ipv6-icmp -log nolog -icmp-type ttl-zero-during-transit +IN ACCEPT -p icmp -log nolog -icmp-type host-unreachable +OUT REJECT -p icmp -log nolog -icmp-type router-solicitation [group network1] -- 2.47.3 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
