* [PATCH proxmox-firewall] nftables: move protocol match rendering into the lib
@ 2026-08-27 13:15 Hannes Laimer
0 siblings, 0 replies; only message in thread
From: Hannes Laimer @ 2026-08-27 13:15 UTC (permalink / raw)
To: pve-devel
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 <h.laimer@proxmox.com>
---
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<NftRule>, 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<NftRule>, 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<NftRule>, 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<NftRule>, 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<NftRule>, _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<NftRule>, _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<NftRule>, _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<Expression>) -> 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<Expression>, dport: Option<Expression>) -> Vec<Statement> {
+ 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<Expression>, code: Option<Expression>) -> Vec<Statement> {
+ 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<Statement> {
+ 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<Statement> {
+ let mut statements = vec![l4proto(protocol)];
+ statements.extend(ports(
+ config.sport().map(Expression::from),
+ config.dport().map(Expression::from),
+ ));
+ statements
+}
--
2.47.3
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-08-27 13:15 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 13:15 [PATCH proxmox-firewall] nftables: move protocol match rendering into the lib Hannes Laimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox