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 534D31FF16B for ; Fri, 12 Sep 2025 18:11:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 17856423; Fri, 12 Sep 2025 18:11:27 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Fri, 12 Sep 2025 18:11:17 +0200 Message-ID: <20250912161121.229819-3-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. [guest.rs, cluster.rs, common.rs, rule.rs, ipset.rs] Subject: [pve-devel] [PATCH proxmox-ve-rs 2/2] config: firewall: add support for legacy ipset 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. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/cluster.rs | 4 + proxmox-ve-config/src/firewall/common.rs | 4 + proxmox-ve-config/src/firewall/guest.rs | 4 + proxmox-ve-config/src/firewall/types/ipset.rs | 97 ++++++++++++++++++- proxmox-ve-config/src/firewall/types/rule.rs | 7 +- .../src/firewall/types/rule_match.rs | 4 +- 6 files changed, 114 insertions(+), 6 deletions(-) diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs index a477a53..d82e66f 100644 --- a/proxmox-ve-config/src/firewall/cluster.rs +++ b/proxmox-ve-config/src/firewall/cluster.rs @@ -53,6 +53,10 @@ impl Config { &self.config.ipsets } + pub fn ipset(&self, name: &str) -> Option<&Ipset> { + self.config.ipsets.get(name) + } + pub fn alias(&self, name: &str) -> Option<&Alias> { self.config.alias(name) } diff --git a/proxmox-ve-config/src/firewall/common.rs b/proxmox-ve-config/src/firewall/common.rs index 3999168..d77fe72 100644 --- a/proxmox-ve-config/src/firewall/common.rs +++ b/proxmox-ve-config/src/firewall/common.rs @@ -189,6 +189,10 @@ where &self.ipsets } + pub fn ipset(&self, name: &str) -> Option<&Ipset> { + self.ipsets.get(name) + } + pub fn alias(&self, name: &str) -> Option<&Alias> { self.aliases.get(name) } diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index 349fdb4..65ef29d 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -108,6 +108,10 @@ impl Config { self.vmid } + pub fn ipset(&self, name: &str) -> Option<&Ipset> { + self.config.ipset(name) + } + pub fn alias(&self, name: &str) -> Option<&Alias> { self.config.alias(name) } diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 2df6943..3f93bb2 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -10,6 +10,7 @@ use crate::guest::vm::NetworkConfig; use super::alias::RuleAliasName; +/// The scope of an ipset. #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum IpsetScope { Datacenter, @@ -42,8 +43,47 @@ impl Display for IpsetScope { } } -#[derive(Debug, Clone)] -#[cfg_attr(test, derive(Eq, PartialEq))] +/// An ipset name in the firewall rules without any scope identifier. +/// +/// Legacy ipset names start with a +, followed by an alphabetic character and only contain +/// alphanumeric characters or hyphens and underscores. +#[derive(Debug, Clone, Eq, PartialEq)] +#[repr(transparent)] +pub struct LegacyIpsetName(String); + +impl AsRef for LegacyIpsetName { + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl FromStr for LegacyIpsetName { + type Err = Error; + + fn from_str(s: &str) -> Result { + if let Some(name) = s.strip_prefix("+") { + if let Some((name, "")) = match_name(name) { + return Ok(Self(name.to_string())); + } + + bail!("not a valid ipset name: {s}"); + } + + bail!("ipset name does not start with '+': {s}"); + } +} + +impl Display for LegacyIpsetName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// The new format of ipset names in firewall rules (with scope). +/// +/// It starts with a plus sign, then a scope [`IpsetScope`], followed by a slash, and then has the +/// same format as [`LegacyIpsetName`]. +#[derive(Debug, Clone, Eq, PartialEq)] pub struct IpsetName { pub scope: IpsetScope, pub name: String, @@ -90,6 +130,39 @@ impl Display for IpsetName { } } +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum RuleIpsetName { + Scoped(IpsetName), + Legacy(LegacyIpsetName), +} + +proxmox_serde::forward_deserialize_to_from_str!(RuleIpsetName); + +impl FromStr for RuleIpsetName { + 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!("is neither a valid scoped nor legacy ipset name: {s}"); + } +} + +impl Display for RuleIpsetName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Scoped(ipset_name) => ipset_name.fmt(f), + Self::Legacy(ipset_name) => ipset_name.fmt(f), + } + } +} + #[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpsetAddress { @@ -304,6 +377,26 @@ mod tests { } } + #[test] + fn test_parse_legacy_ipset_name() { + for test_case in [ + ("+proxmox_123", "proxmox_123"), + ("+proxmox_---123", "proxmox_---123"), + ] { + let ipset_name = test_case + .0 + .parse::() + .expect("valid ipset name"); + + assert_eq!(ipset_name, LegacyIpsetName(test_case.1.to_string(),)) + } + + for name in ["guest/proxmox_123", "+-qwe", "+1qwe"] { + name.parse::() + .expect_err("invalid ipset name"); + } + } + #[test] fn test_parse_ipset_address() { let mut ipset_address = "10.0.0.1" diff --git a/proxmox-ve-config/src/firewall/types/rule.rs b/proxmox-ve-config/src/firewall/types/rule.rs index 049f270..3b2b8ca 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}, - ipset::{IpsetName, IpsetScope}, alias::{AliasName, AliasScope, RuleAliasName}, + ipset::{IpsetName, IpsetScope, RuleIpsetName}, log::LogLevel, rule_match::{Icmp, IcmpCode, IpAddrMatch, IpMatch, Ports, Protocol, Udp}, }; @@ -385,7 +385,10 @@ mod tests { AliasScope::Datacenter, "test" ))), - IpAddrMatch::Set(IpsetName::new(IpsetScope::Datacenter, "test"),), + IpAddrMatch::Set(RuleIpsetName::Scoped(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 2be84c5..9b474d2 100644 --- a/proxmox-ve-config/src/firewall/types/rule_match.rs +++ b/proxmox-ve-config/src/firewall/types/rule_match.rs @@ -13,7 +13,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::RuleAliasName; -use crate::firewall::types::ipset::IpsetName; +use crate::firewall::types::ipset::RuleIpsetName; use crate::firewall::types::log::LogLevel; use crate::firewall::types::port::PortList; use crate::firewall::types::rule::{Direction, Verdict}; @@ -253,7 +253,7 @@ impl IpMatch { #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpAddrMatch { Ip(IpList), - Set(IpsetName), + Set(RuleIpsetName), Alias(RuleAliasName), } -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel