From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 3DC3B1FF183 for ; Wed, 8 Oct 2025 15:15:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AD91B8EB0; Wed, 8 Oct 2025 15:15:30 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Wed, 8 Oct 2025 15:14:53 +0200 Message-ID: <20251008131457.178090-2-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251008131457.178090-1-s.hanreich@proxmox.com> References: <20251008131457.178090-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.234 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pve-devel] [PATCH proxmox-firewall 1/2] firewall: fix generating rules with both type and code set X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 --- 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, _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 pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel