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 EEE131FF16B for ; Fri, 12 Sep 2025 18:11:26 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 72C5334A; Fri, 12 Sep 2025 18:11:26 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Fri, 12 Sep 2025 18:11:16 +0200 Message-ID: <20250912161121.229819-2-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20250912161121.229819-1-s.hanreich@proxmox.com> References: <20250912161121.229819-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.183 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [alias.rs, cluster.rs, guest.rs, ipset.rs, rule.rs] Subject: [pve-devel] [PATCH proxmox-ve-rs 1/2] config: firewall: add support for legacy alias names 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" Proxmox VE 8.0 introduced scopes for ipsets and aliases in the firewall configuration. Since existing firewall configuration was not converted to this new syntax, there are still many systems that have at least one alias / ipset name in their firewall ruleset that does not contain any scope. proxmox-firewall failed to parse such rules and in turn failed to create a new ruleset, which was a common occurence when users switched from pve-firewall to proxmox-firewall. Since aliases can also occur in ipset definitions, add support for parsing non-scoped alias names there as well. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/cluster.rs | 12 ++- proxmox-ve-config/src/firewall/guest.rs | 6 +- proxmox-ve-config/src/firewall/types/alias.rs | 98 +++++++++++++++++-- proxmox-ve-config/src/firewall/types/ipset.rs | 7 +- proxmox-ve-config/src/firewall/types/rule.rs | 7 +- .../src/firewall/types/rule_match.rs | 10 +- 6 files changed, 118 insertions(+), 22 deletions(-) diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs index d588302..a477a53 100644 --- a/proxmox-ve-config/src/firewall/cluster.rs +++ b/proxmox-ve-config/src/firewall/cluster.rs @@ -138,7 +138,7 @@ mod tests { use crate::firewall::types::{ address::IpList, - alias::{AliasName, AliasScope}, + alias::{AliasName, AliasScope, RuleAliasName}, ipset::{IpsetAddress, IpsetEntry}, log::{LogLevel, LogRateLimitTimescale}, rule::{Kind, RuleGroup}, @@ -250,12 +250,18 @@ IN BGP(REJECT) -log crit -source 1.2.3.4 }, IpsetEntry { nomatch: false, - address: IpsetAddress::Alias(AliasName::new(AliasScope::Datacenter, "analias")), + address: IpsetAddress::Alias(RuleAliasName::Scoped(AliasName::new( + AliasScope::Datacenter, + "analias", + ))), comment: Some("a comment".to_string()), }, IpsetEntry { nomatch: false, - address: IpsetAddress::Alias(AliasName::new(AliasScope::Datacenter, "wide")), + address: IpsetAddress::Alias(RuleAliasName::Scoped(AliasName::new( + AliasScope::Datacenter, + "wide", + ))), comment: None, }, IpsetEntry { diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index 4428a75..349fdb4 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -4,7 +4,7 @@ use std::io; use crate::guest::types::Vmid; use crate::guest::vm::NetworkConfig; -use crate::firewall::types::alias::{Alias, AliasName}; +use crate::firewall::types::alias::Alias; use crate::firewall::types::ipset::IpsetScope; use crate::firewall::types::log::LogLevel; use crate::firewall::types::rule::{Direction, Rule, Verdict}; @@ -108,8 +108,8 @@ impl Config { self.vmid } - pub fn alias(&self, name: &AliasName) -> Option<&Alias> { - self.config.alias(name.name()) + pub fn alias(&self, name: &str) -> Option<&Alias> { + self.config.alias(name) } pub fn iface_name_by_key(&self, key: &str) -> Result { diff --git a/proxmox-ve-config/src/firewall/types/alias.rs b/proxmox-ve-config/src/firewall/types/alias.rs index c7a58c1..0d89bb9 100644 --- a/proxmox-ve-config/src/firewall/types/alias.rs +++ b/proxmox-ve-config/src/firewall/types/alias.rs @@ -6,8 +6,7 @@ use proxmox_network_types::ip_address::Cidr; use crate::firewall::parse::{match_name, match_non_whitespace}; -#[derive(Debug, Clone)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive(Debug, Clone, Eq, PartialEq)] pub enum AliasScope { Datacenter, Guest, @@ -34,10 +33,51 @@ impl Display for AliasScope { } } -/// Represents the name of an alias in a firewall rule in the RULES section of the firewall -/// configuration. -#[derive(Debug, Clone)] -#[cfg_attr(test, derive(Eq, PartialEq))] +/// An alias name in the firewall rules without any scope identifier. +/// +/// It starts with an alphabetic character, followed by alphanumeric characters or +/// hyphens/underscores. +/// +/// When parsing the name, this will convert any ASCII characters contained in the name into +/// lowercase. This is for maintaining backwards-compatibility with pve-firewall, where all aliases +/// are lowercased when reading from the config. +#[derive(Debug, Clone, Eq, PartialEq)] +#[repr(transparent)] +pub struct LegacyAliasName(String); + +impl AsRef for LegacyAliasName { + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl FromStr for LegacyAliasName { + type Err = Error; + + fn from_str(s: &str) -> Result { + if let Some((name, "")) = match_name(s) { + return Ok(Self(name.to_lowercase())); + } + + bail!("not a valid alias name: {s}"); + } +} + +impl Display for LegacyAliasName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// An alias name in the firewall rules with scope identifier +/// +/// It starts with a scope ([`AliasScope`]), followed by a slash, and then has the same format as +/// [`LegacyAliasName`] +/// +/// When parsing the name, this will convert any ASCII characters contained in the name into +/// lowercase. This is for maintaining backwards-compatibility with pve-firewall, where all aliases +/// are lowercased when reading from the config. +#[derive(Debug, Clone, Eq, PartialEq)] pub struct AliasName { scope: AliasScope, name: String, @@ -90,6 +130,40 @@ impl AliasName { } } +/// The name of an alias in a firewall rule. +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum RuleAliasName { + Scoped(AliasName), + Legacy(LegacyAliasName), +} + +proxmox_serde::forward_deserialize_to_from_str!(RuleAliasName); + +impl FromStr for RuleAliasName { + type Err = Error; + + fn from_str(s: &str) -> Result { + if let Ok(scoped_name) = s.parse() { + return Ok(Self::Scoped(scoped_name)); + } + + if let Ok(legacy_name) = s.parse() { + return Ok(Self::Legacy(legacy_name)); + } + + bail!("invalid alias name: {s}"); + } +} + +impl Display for RuleAliasName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Scoped(alias_name) => alias_name.fmt(f), + Self::Legacy(legacy_name) => legacy_name.fmt(f), + } + } +} + /// Represents an Alias stored in the ALIASES section of the firewall configuration. /// /// Since they contain no scope in the firewall configuration itself, this struct also does not @@ -189,6 +263,18 @@ mod tests { assert_eq!(alias.comment(), Some("a comment")); } + #[test] + fn test_parse_legacy_alias_name() { + for name in ["proxmox_123", "proxmox-123"] { + name.parse::().expect("valid alias name"); + } + + for name in ["1proxmox_123", "-proxmox-123"] { + name.parse::() + .expect_err("invalid alias name"); + } + } + #[test] fn test_parse_alias_name() { for name in ["dc/proxmox_123", "guest/proxmox-123"] { diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 8a51e38..2df6943 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -5,10 +5,11 @@ use std::str::FromStr; use anyhow::{bail, format_err, Error}; use proxmox_network_types::ip_address::{Cidr, IpRange}; -use crate::firewall::parse::match_non_whitespace; -use crate::firewall::types::alias::AliasName; +use crate::firewall::parse::{match_name, match_non_whitespace}; use crate::guest::vm::NetworkConfig; +use super::alias::RuleAliasName; + #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum IpsetScope { Datacenter, @@ -92,7 +93,7 @@ impl Display for IpsetName { #[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpsetAddress { - Alias(AliasName), + Alias(RuleAliasName), Cidr(Cidr), Range(IpRange), } diff --git a/proxmox-ve-config/src/firewall/types/rule.rs b/proxmox-ve-config/src/firewall/types/rule.rs index 3ad8cf0..049f270 100644 --- a/proxmox-ve-config/src/firewall/types/rule.rs +++ b/proxmox-ve-config/src/firewall/types/rule.rs @@ -251,8 +251,8 @@ mod tests { use crate::firewall::types::{ address::{IpEntry, IpList}, - alias::{AliasName, AliasScope}, ipset::{IpsetName, IpsetScope}, + alias::{AliasName, AliasScope, RuleAliasName}, log::LogLevel, rule_match::{Icmp, IcmpCode, IpAddrMatch, IpMatch, Ports, Protocol, Udp}, }; @@ -381,7 +381,10 @@ mod tests { verdict: Verdict::Accept, ip: Some( IpMatch::new( - IpAddrMatch::Alias(AliasName::new(AliasScope::Datacenter, "test")), + IpAddrMatch::Alias(RuleAliasName::Scoped(AliasName::new( + AliasScope::Datacenter, + "test" + ))), IpAddrMatch::Set(IpsetName::new(IpsetScope::Datacenter, "test"),), ) .unwrap() diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs index 7fcd35c..2be84c5 100644 --- a/proxmox-ve-config/src/firewall/types/rule_match.rs +++ b/proxmox-ve-config/src/firewall/types/rule_match.rs @@ -12,7 +12,7 @@ use proxmox_sortable_macro::sortable; use crate::firewall::parse::{match_name, match_non_whitespace, SomeStr}; use crate::firewall::types::address::IpList; -use crate::firewall::types::alias::AliasName; +use crate::firewall::types::alias::RuleAliasName; use crate::firewall::types::ipset::IpsetName; use crate::firewall::types::log::LogLevel; use crate::firewall::types::port::PortList; @@ -254,7 +254,7 @@ impl IpMatch { pub enum IpAddrMatch { Ip(IpList), Set(IpsetName), - Alias(AliasName), + Alias(RuleAliasName), } impl IpAddrMatch { @@ -771,7 +771,7 @@ impl fmt::Display for Icmpv6Code { #[cfg(test)] mod tests { - use crate::firewall::types::alias::AliasScope::Guest; + use crate::firewall::types::alias::{AliasName, AliasScope::Guest}; use proxmox_network_types::ip_address::Cidr; use super::*; @@ -844,7 +844,7 @@ mod tests { IpMatch::new( IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0, 0, 0], 8).unwrap())), - IpAddrMatch::Alias(AliasName::new(Guest, "test")), + IpAddrMatch::Alias(RuleAliasName::Scoped(AliasName::new(Guest, "test"))), ) .expect("valid ip match"); @@ -893,7 +893,7 @@ mod tests { options = RuleOptions { proto: Some("tcp".to_string()), sport: Some("qwe".to_string()), - source: Some("qwe".to_string()), + source: Some("_qwe".to_string()), ..Default::default() }; -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel