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 237611FF173 for ; Mon, 25 Nov 2024 18:05:31 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DBA82194B9; Mon, 25 Nov 2024 18:05:23 +0100 (CET) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Mon, 25 Nov 2024 18:04:48 +0100 Message-Id: <20241125170449.238880-1-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.232 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 Subject: [pve-devel] [PATCH proxmox-ve-rs v2 1/2] fix #5927: firewall: alias: lowercase names of aliases 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" pve-firewall lowercases the names of aliases when reading from the configuration as well as when comparing source / destination entries with the entries in the parsed aliases. In order to stay backwards-compatible we also need to lowercase any parsed alias name. I decided to this in the constructor and switch all call sites to the new constructor, so there's only one place where we have to handle lowercasing the string. Signed-off-by: Stefan Hanreich --- Changes from v1 to v2: * fix typo and improve description in documentation of Alias * add bugzilla issue # to commit msg Also leaving the thoughts I posted under v1 here: Leo supplied a patch already [1] that changes the handling of names in the firewall. Might make sense to revisit this patch series in the future to unify how cases are handled in the firewall. Nevertheless, this patch acts as a quick fix for users running into issues with the firewall due to the difference in handling cases in pve-firewall and proxmox-firewall. [1] https://lists.proxmox.com/pipermail/pve-devel/2023-January/055596.html proxmox-ve-config/src/firewall/types/alias.rs | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/alias.rs b/proxmox-ve-config/src/firewall/types/alias.rs index 5dfaa41..553931e 100644 --- a/proxmox-ve-config/src/firewall/types/alias.rs +++ b/proxmox-ve-config/src/firewall/types/alias.rs @@ -53,10 +53,7 @@ impl FromStr for AliasName { fn from_str(s: &str) -> Result { match s.split_once('/') { - Some((prefix, name)) if !name.is_empty() => Ok(Self { - scope: prefix.parse()?, - name: name.to_string(), - }), + Some((prefix, name)) if !name.is_empty() => Ok(Self::new(prefix.parse()?, name)), _ => { bail!("Invalid Alias name!") } @@ -65,10 +62,18 @@ impl FromStr for AliasName { } impl AliasName { + /// Creates a new [`AliasName`]. + /// + /// It will convert any ASCII characters contained in the name into lowercase. This is for + /// maintaining backwards-compatiblity with pve-firewall, where all aliases are lowercased when + /// reading from the config. pub fn new(scope: AliasScope, name: impl Into) -> Self { + let mut lowercase_name = name.into(); + lowercase_name.make_ascii_lowercase(); + Self { scope, - name: name.into(), + name: lowercase_name, } } @@ -90,13 +95,21 @@ pub struct Alias { } impl Alias { + /// Creates a new [`Alias`]. + /// + /// It will convert any ASCII characters contained in the name into lowercase. This is for + /// maintaining backwards-compatiblity with pve-firewall, where all aliases are lowercased when + /// reading from the config. pub fn new( name: impl Into, address: impl Into, comment: impl Into>, ) -> Self { + let mut lowercase_name = name.into(); + lowercase_name.make_ascii_lowercase(); + Self { - name: name.into(), + name: lowercase_name, address: address.into(), comment: comment.into(), } @@ -135,11 +148,7 @@ impl FromStr for Alias { None => None, }; - Ok(Alias { - name: name.to_string(), - address, - comment, - }) + Ok(Alias::new(name, address, comment)) } } @@ -159,6 +168,16 @@ mod tests { for alias in ["-- 10.0.0.1/32", "0asd 10.0.0.1/32", "__test 10.0.0.0/32"] { alias.parse::().expect_err("invalid alias"); } + + let alias = "pRoxMox 10.0.0.0/32 # a comment" + .parse::() + .expect("valid alias"); + assert_eq!(alias.name(), "proxmox"); + assert_eq!( + alias.address(), + &Cidr::new_v4([10, 0, 0, 0], 32).expect("valid CIDR") + ); + assert_eq!(alias.comment(), Some("a comment")); } #[test] @@ -171,4 +190,12 @@ mod tests { name.parse::().expect_err("invalid alias name"); } } + + #[test] + fn test_parse_alias_case() { + for name in ["dc/PROxMoX", "guest/PROXMOX"] { + let alias_name = name.parse::().expect("valid alias name"); + assert_eq!(alias_name.name(), "proxmox"); + } + } } -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel