From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id B57751FF0E1 for ; Thu, 27 Aug 2026 15:15:33 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 15AC821440; Thu, 27 Aug 2026 15:15:33 +0200 (CEST) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-firewall] nftables: move protocol match rendering into the lib Date: Thu, 27 Aug 2026 15:15:24 +0200 Message-ID: <20260827131524.1411472-1-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787836518761 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.871 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_CSS_A 0.1 Contains URL's A record listed in the Spamhaus CSS blocklist [34.117.176.22] Message-ID-Hash: FUKILRW2CUSN2GRUVLDWS7NU3XN7DSRR X-Message-ID-Hash: FUKILRW2CUSN2GRUVLDWS7NU3XN7DSRR X-MailFrom: h.laimer@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The statements selecting a transport protocol, its ports or an ICMP type and code were generated in proxmox-firewall, although which statements select a protocol is nftables knowledge and does not depend on the firewall config. Move the rendering into proxmox-nftables, working on plain expressions, and add an entry point taking the firewall's protocol type to the config extension. The firewall keeps its IP family pinning on top. No functional change intended. Signed-off-by: Hannes Laimer --- spiritually depends/based on [1], but apply order doesnt really matter, and they are both good on their own [1] https://lore.proxmox.com/pve-devel/20260827115803.1331245-1-h.laimer@proxmox.com/ proxmox-firewall/src/rule.rs | 166 +++---------------------------- proxmox-nftables/src/lib.rs | 1 + proxmox-nftables/src/protocol.rs | 79 +++++++++++++++ 3 files changed, 93 insertions(+), 153 deletions(-) create mode 100644 proxmox-nftables/src/protocol.rs diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index 048c00e..ea63a08 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -6,6 +6,7 @@ use proxmox_log as log; use proxmox_nftables::{ Expression, Statement, expression::{Ct, IpFamily, Meta, Payload, Prefix}, + protocol, statement::{Log, LogLevel, Match, Operator}, types::{AddRule, ChainPart, SetName, TableFamily, TablePart}, }; @@ -19,9 +20,7 @@ use proxmox_ve_config::{ ipset::{Ipfilter, IpsetName, RuleIpsetName}, log::LogRateLimit, rule::{Direction, Kind, RuleGroup, Verdict as ConfigVerdict}, - rule_match::{ - Icmp, Icmpv6, IpAddrMatch, IpMatch, Ports, Protocol, RuleMatch, Sctp, Tcp, Udp, - }, + rule_match::{IpAddrMatch, IpMatch, Protocol, RuleMatch}, }, }, guest::types::Vmid, @@ -596,162 +595,23 @@ impl ToNftRules for IpMatch { } } -fn handle_protocol(rules: &mut [NftRule], _env: &NftRuleEnv, name: &str) -> Result<(), Error> { - for rule in rules.iter_mut() { - rule.push(Match::new_eq(Meta::new("l4proto"), Expression::from(name)).into()); - } - - Ok(()) -} - impl ToNftRules for Protocol { - fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { - log::trace!("adding protocol: {self:?}"); - - match self { - Protocol::Tcp(tcp) => tcp.to_nft_rules(rules, env), - Protocol::Udp(udp) => udp.to_nft_rules(rules, env), - Protocol::Dccp(ports) => { - handle_protocol(rules, env, "dccp")?; - ports.to_nft_rules(rules, env) - } - Protocol::UdpLite(ports) => { - handle_protocol(rules, env, "udplite")?; - ports.to_nft_rules(rules, env) - } - Protocol::Sctp(sctp) => sctp.to_nft_rules(rules, env), - Protocol::Icmp(icmp) => icmp.to_nft_rules(rules, env), - Protocol::Icmpv6(icmpv6) => icmpv6.to_nft_rules(rules, env), - Protocol::Named(name) => handle_protocol(rules, env, name), - Protocol::Numeric(id) => { - for rule in rules.iter_mut() { - rule.push(Match::new_eq(Meta::new("l4proto"), Expression::from(*id)).into()); - } - - Ok(()) - } - } - } -} - -impl ToNftRules for Tcp { - fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { - handle_protocol(rules, env, "tcp")?; - self.ports().to_nft_rules(rules, env) - } -} - -impl ToNftRules for Udp { - fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { - handle_protocol(rules, env, "udp")?; - self.ports().to_nft_rules(rules, env) - } -} - -impl ToNftRules for Sctp { - fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { - handle_protocol(rules, env, "sctp")?; - self.ports().to_nft_rules(rules, env) - } -} - -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_type) = self.ty() { - rule.push( - Match::new_eq(Payload::field("icmp", "type"), Expression::from(icmp_type)) - .into(), - ); - } - - if let Some(icmp_code) = self.code() { - rule.push( - Match::new_eq(Payload::field("icmp", "code"), Expression::from(icmp_code)) - .into(), - ); - } - - if self.code().is_none() && self.ty().is_none() { - rule.push(Match::new_eq(Meta::new("l4proto"), Expression::from("icmp")).into()); - } - - rule.set_family(Family::V4); - } - } - - Ok(()) - } -} - -impl ToNftRules for Icmpv6 { - fn to_nft_rules(&self, rules: &mut Vec, _env: &NftRuleEnv) -> Result<(), Error> { - log::trace!("applying icmpv6: {self:?}"); + log::trace!("adding protocol: {self:?}"); + let family = self.family(); for rule in rules.iter_mut() { - if matches!(rule.family(), Some(Family::V6) | None) { - if let Some(icmp_type) = self.ty() { - rule.push( - Match::new_eq( - Payload::field("icmpv6", "type"), - Expression::from(icmp_type), - ) - .into(), - ); - } - - if let Some(icmp_code) = self.code() { - rule.push( - Match::new_eq( - Payload::field("icmpv6", "code"), - Expression::from(icmp_code), - ) - .into(), - ); - } - - if self.code().is_none() && self.ty().is_none() { - rule.push( - Match::new_eq(Meta::new("l4proto"), Expression::from("icmpv6")).into(), - ); - } - - rule.set_family(Family::V6); + if family + .zip(rule.family()) + .is_some_and(|(wanted, pinned)| wanted != pinned) + { + continue; } - } - - Ok(()) - } -} - -impl ToNftRules for Ports { - fn to_nft_rules(&self, rules: &mut Vec, _env: &NftRuleEnv) -> Result<(), Error> { - log::trace!("applying ports: {self:?}"); - - for rule in rules { - if let Some(sport) = self.sport() { - log::trace!("applying sport: {sport:?}"); - - rule.push( - Match::new_eq( - Expression::from(Payload::field("th", "sport")), - Expression::from(sport), - ) - .into(), - ) + for statement in protocol::matches(self) { + rule.push(statement); } - - if let Some(dport) = self.dport() { - log::trace!("applying dport: {dport:?}"); - - rule.push( - Match::new_eq( - Expression::from(Payload::field("th", "dport")), - Expression::from(dport), - ) - .into(), - ) + if let Some(family) = family { + rule.set_family(family); } } diff --git a/proxmox-nftables/src/lib.rs b/proxmox-nftables/src/lib.rs index 2003e1b..c54905b 100644 --- a/proxmox-nftables/src/lib.rs +++ b/proxmox-nftables/src/lib.rs @@ -2,6 +2,7 @@ pub mod client; pub mod command; pub mod expression; pub mod helper; +pub mod protocol; pub mod statement; pub mod types; diff --git a/proxmox-nftables/src/protocol.rs b/proxmox-nftables/src/protocol.rs new file mode 100644 index 0000000..fcd657f --- /dev/null +++ b/proxmox-nftables/src/protocol.rs @@ -0,0 +1,79 @@ +//! Rendering of a transport protocol match into the statements that select it, which are +//! `meta l4proto` for the protocol, the transport-header ports, and the ICMP type and code. +//! The statements carry no IP family test of their own, so they work unchanged in the inet +//! and bridge families. An ICMP match still only applies to its own IP family, which is left +//! to the caller. + +#[cfg(feature = "config-ext")] +use proxmox_ve_config::firewall::types::rule_match::{Ports, Protocol}; + +use crate::expression::{Meta, Payload}; +use crate::statement::Match; +use crate::{Expression, Statement}; + +/// Selects a protocol by name or number. +pub fn l4proto(protocol: impl Into) -> Statement { + Match::new_eq(Meta::new("l4proto"), protocol.into()).into() +} + +/// Selects the transport-header source and destination ports that are given. +pub fn ports(sport: Option, dport: Option) -> Vec { + let mut statements = Vec::new(); + if let Some(sport) = sport { + statements.push(Match::new_eq(Payload::field("th", "sport"), sport).into()); + } + if let Some(dport) = dport { + statements.push(Match::new_eq(Payload::field("th", "dport"), dport).into()); + } + statements +} + +/// Selects an ICMP flavour by type and code. Matching either already implies the protocol, so +/// only a bare match needs the explicit `l4proto` test. +pub fn icmp(protocol: &str, ty: Option, code: Option) -> Vec { + if ty.is_none() && code.is_none() { + return vec![l4proto(protocol)]; + } + let mut statements = Vec::new(); + if let Some(ty) = ty { + statements.push(Match::new_eq(Payload::field(protocol, "type"), ty).into()); + } + if let Some(code) = code { + statements.push(Match::new_eq(Payload::field(protocol, "code"), code).into()); + } + statements +} + +/// The statements selecting `protocol`, in evaluation order. +#[cfg(feature = "config-ext")] +pub fn matches(protocol: &Protocol) -> Vec { + match protocol { + Protocol::Tcp(tcp) => with_ports("tcp", tcp.ports()), + Protocol::Udp(udp) => with_ports("udp", udp.ports()), + Protocol::Sctp(sctp) => with_ports("sctp", sctp.ports()), + Protocol::Dccp(config) => with_ports("dccp", config), + Protocol::UdpLite(config) => with_ports("udplite", config), + Protocol::Icmp(config) => icmp( + "icmp", + config.ty().map(Expression::from), + config.code().map(Expression::from), + ), + Protocol::Icmpv6(config) => icmp( + "icmpv6", + config.ty().map(Expression::from), + config.code().map(Expression::from), + ), + Protocol::Named(name) => vec![l4proto(name.as_str())], + Protocol::Numeric(id) => vec![l4proto(*id)], + } +} + +#[cfg(feature = "config-ext")] +fn with_ports(protocol: &str, config: &Ports) -> Vec { + let mut statements = vec![l4proto(protocol)]; + statements.extend(ports( + config.sport().map(Expression::from), + config.dport().map(Expression::from), + )); + statements +} -- 2.47.3