From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 67CE7EB83 for ; Wed, 19 Jul 2023 18:31:47 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 49314ACAF for ; Wed, 19 Jul 2023 18:31:47 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 19 Jul 2023 18:31:45 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1BF93417FB for ; Wed, 19 Jul 2023 18:31:45 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Wed, 19 Jul 2023 18:31:30 +0200 Message-ID: <20230719163130.773813-2-c.heiss@proxmox.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.056 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [utils.rs, foo-bar.com, 123foo.com, main.rs, 123.com, foo123.com, proxmox.com, a-b.com] Subject: [pve-devel] [PATCH installer v2] tui: fix FQDN validation 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: , X-List-Received-Date: Wed, 19 Jul 2023 16:31:47 -0000 Add checks to ensure that: * It is actually has a hostname, not just a domain name * Properly check if the hostname is purely numeric, which was broken/different to how the GUI installer does it The custom error type also allows for easier future adaptions, as the changes can be entirely contained to the `Fqdn` type. Signed-off-by: Christoph Heiss --- v1: https://lists.proxmox.com/pipermail/pve-devel/2023-June/057845.html Changes v1 -> v2: * Fix & add some more unit tests * Add custom error type `FqdnParseError` * Move hostname checks directly into `Fqdn::from()` proxmox-tui-installer/src/main.rs | 5 +-- proxmox-tui-installer/src/utils.rs | 68 ++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 42f278b..5dd8ef8 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -587,7 +587,7 @@ fn network_dialog(siv: &mut Cursive) -> InstallerView { .get_value::(1) .ok_or("failed to retrieve host FQDN")? .parse::() - .map_err(|_| "failed to parse hostname".to_owned())?; + .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?; let address = view .get_value::(2) @@ -609,9 +609,6 @@ fn network_dialog(siv: &mut Cursive) -> InstallerView { Err("host and gateway IP address version must not differ".to_owned()) } else if address.addr().is_ipv4() != dns_server.is_ipv4() { Err("host and DNS IP address version must not differ".to_owned()) - } else if fqdn.to_string().chars().all(|c| c.is_ascii_digit()) { - // Not supported/allowed on Debian - Err("hostname cannot be purely numeric".to_owned()) } else if fqdn.to_string().ends_with(".invalid") { Err("hostname does not look valid".to_owned()) } else { diff --git a/proxmox-tui-installer/src/utils.rs b/proxmox-tui-installer/src/utils.rs index 3245fac..516f9c6 100644 --- a/proxmox-tui-installer/src/utils.rs +++ b/proxmox-tui-installer/src/utils.rs @@ -110,20 +110,51 @@ fn mask_limit(addr: &IpAddr) -> usize { } } -#[derive(Clone, Debug)] +/// Possible errors that might occur when parsing FQDNs. +#[derive(Debug, Eq, PartialEq)] +pub enum FqdnParseError { + MissingHostname, + NumericHostname, + InvalidPart(String), +} + +impl fmt::Display for FqdnParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use FqdnParseError::*; + match self { + MissingHostname => write!(f, "missing hostname part"), + NumericHostname => write!(f, "hostname cannot be purely numeric"), + InvalidPart(part) => write!( + f, + "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'", + ), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] pub struct Fqdn { parts: Vec, } impl Fqdn { - pub fn from(fqdn: &str) -> Result { + pub fn from(fqdn: &str) -> Result { let parts = fqdn .split('.') .map(ToOwned::to_owned) .collect::>(); - if !parts.iter().all(&Self::validate_single) { - Err(()) + for part in &parts { + if !Self::validate_single(part) { + return Err(FqdnParseError::InvalidPart(part.clone())); + } + } + + if parts.len() < 2 { + Err(FqdnParseError::MissingHostname) + } else if parts[0].chars().all(|c| c.is_ascii_digit()) { + // Not allowed/supported on Debian systems. + Err(FqdnParseError::NumericHostname) } else { Ok(Self { parts }) } @@ -143,20 +174,24 @@ impl Fqdn { parts.join(".") } + /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part. fn has_host(&self) -> bool { self.parts.len() > 1 } fn validate_single(s: &String) -> bool { !s.is_empty() + // First character must be alphanumeric && s.chars() .next() .map(|c| c.is_ascii_alphanumeric()) .unwrap_or_default() + // .. last character as well, && s.chars() .last() .map(|c| c.is_ascii_alphanumeric()) .unwrap_or_default() + // and anything between must be alphanumeric or - && s.chars() .skip(1) .take(s.len().saturating_sub(2)) @@ -165,7 +200,7 @@ impl Fqdn { } impl FromStr for Fqdn { - type Err = (); + type Err = FqdnParseError; fn from_str(value: &str) -> Result { Self::from(value) @@ -194,17 +229,22 @@ mod tests { use super::*; #[test] - fn fqdn_validate_single() { + fn fqdn_construct() { + use FqdnParseError::*; assert!(Fqdn::from("foo.example.com").is_ok()); - assert!(Fqdn::from("1.example.com").is_ok()); assert!(Fqdn::from("foo-bar.com").is_ok()); assert!(Fqdn::from("a-b.com").is_ok()); - assert!(Fqdn::from("foo").is_err()); - assert!(Fqdn::from("-foo.com").is_err()); - assert!(Fqdn::from("foo-.com").is_err()); - assert!(Fqdn::from("foo.com-").is_err()); - assert!(Fqdn::from("-o-.com").is_err()); + assert_eq!(Fqdn::from("foo"), Err(MissingHostname)); + + assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned()))); + assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned()))); + assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned()))); + assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned()))); + + assert_eq!(Fqdn::from("123.com"), Err(NumericHostname)); + assert!(Fqdn::from("foo123.com").is_ok()); + assert!(Fqdn::from("123foo.com").is_ok()); } #[test] @@ -212,6 +252,10 @@ mod tests { let fqdn = Fqdn::from("pve.example.com").unwrap(); assert_eq!(fqdn.host().unwrap(), "pve"); assert_eq!(fqdn.domain(), "example.com"); + assert_eq!( + fqdn.parts, + &["pve".to_owned(), "example".to_owned(), "com".to_owned()] + ); } #[test] -- 2.41.0