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 96AC11FF141 for ; Tue, 19 May 2026 21:14:49 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B10581076E; Tue, 19 May 2026 21:14:45 +0200 (CEST) From: Gabriel Goller To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox] network-types: stricter HostnameOrIpAddr parsing Date: Tue, 19 May 2026 21:14:04 +0200 Message-ID: <20260519191406.293932-1-g.goller@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779218036514 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.272 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 ENA_SUBJ_ODD_CASE 2.6 Subject has odd case 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 WEIRD_PORT 0.001 Uses non-standard port number for HTTP Message-ID-Hash: KOMFX2FEYA2SD6NG2QYJHXKM3OYWZJ4S X-Message-ID-Hash: KOMFX2FEYA2SD6NG2QYJHXKM3OYWZJ4S X-MailFrom: g.goller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Previously the HostnameOrIpAddr struct had a String as hostname, so everything that wasn't an ip was automatically a hostname. This was weird because it didn't fail on e.g. a prefix, which resultet in the "2001:1::/64:3000" wireguard config entries. In order to catch these common errors and make everything a bit tighter, validate hostnames correctly (only alphanumeric and hyphens, according to the rfc). Signed-off-by: Gabriel Goller --- proxmox-network-types/src/endpoint.rs | 132 ++++++++++++++++++++------ 1 file changed, 103 insertions(+), 29 deletions(-) diff --git a/proxmox-network-types/src/endpoint.rs b/proxmox-network-types/src/endpoint.rs index 609de58a6e7a..61332b53d767 100644 --- a/proxmox-network-types/src/endpoint.rs +++ b/proxmox-network-types/src/endpoint.rs @@ -8,23 +8,74 @@ use std::{ }; #[cfg(feature = "api-types")] -use proxmox_schema::StringSchema; -#[cfg(feature = "api-types")] -use proxmox_schema::{ApiType, UpdaterType}; +use proxmox_schema::{ + api_types::{DNS_NAME_OR_IP_FORMAT, HOST_PORT_FORMAT}, + ApiType, StringSchema, UpdaterType, +}; use serde::{Deserialize, Serialize}; use serde_with::{DeserializeFromStr, SerializeDisplay}; /// Represents either a resolvable hostname or an IPv4/IPv6 address. /// IPv6 address are correctly bracketed on [`Display`], and parsing /// automatically tries parsing it as an IP address first, falling back to a -/// plain hostname in the other case. -#[derive(Clone, Debug, PartialEq, Hash, Deserialize, Serialize)] +/// validated plain hostname in the other case. CIDR notation is rejected. +#[derive(Clone, Debug, PartialEq, Hash, Serialize)] #[serde(untagged)] pub enum HostnameOrIpAddr { Hostname(String), IpAddr(IpAddr), } +#[derive(thiserror::Error, Debug)] +pub enum HostnameOrIpAddrParseError { + #[error("invalid hostname or IP address: '{0}'")] + Invalid(String), +} + +impl FromStr for HostnameOrIpAddr { + type Err = HostnameOrIpAddrParseError; + + fn from_str(s: &str) -> Result { + if let Ok(ip_addr) = IpAddr::from_str(s) { + return Ok(Self::IpAddr(ip_addr)); + } + + if is_valid_hostname(s) { + return Ok(Self::Hostname(s.to_owned())); + } + + Err(HostnameOrIpAddrParseError::Invalid(s.to_owned())) + } +} + +impl<'de> Deserialize<'de> for HostnameOrIpAddr { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + s.parse().map_err(serde::de::Error::custom) + } +} + +fn is_valid_hostname(s: &str) -> bool { + !s.is_empty() + && s.len() <= 253 + && s.split('.').all(|label| { + !label.is_empty() + && label.len() <= 63 + && label + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphanumeric()) + && label + .chars() + .last() + .is_some_and(|c| c.is_ascii_alphanumeric()) + && label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') + }) +} + impl Display for HostnameOrIpAddr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -37,20 +88,11 @@ impl Display for HostnameOrIpAddr { } } -impl> From for HostnameOrIpAddr { - fn from(value: S) -> Self { - let s = value.into(); - if let Ok(ip_addr) = IpAddr::from_str(&s) { - Self::IpAddr(ip_addr) - } else { - Self::Hostname(s) - } - } -} - #[cfg(feature = "api-types")] impl ApiType for HostnameOrIpAddr { - const API_SCHEMA: proxmox_schema::Schema = StringSchema::new("hostname or ip address").schema(); + const API_SCHEMA: proxmox_schema::Schema = StringSchema::new("DNS name or IP address.") + .format(&DNS_NAME_OR_IP_FORMAT) + .schema(); } #[cfg(feature = "api-types")] @@ -67,12 +109,11 @@ pub struct ServiceEndpoint { } impl ServiceEndpoint { - pub fn new>(host: S, port: u16) -> Self { - let s = host.into(); - Self { - host: s.into(), + pub fn new>(host: S, port: u16) -> Result { + Ok(Self { + host: host.as_ref().parse()?, port, - } + }) } } @@ -90,6 +131,8 @@ pub enum ParseError { MissingHost, #[error("invalid port: {0}")] InvalidPort(String), + #[error("invalid host: {0}")] + InvalidHost(#[from] HostnameOrIpAddrParseError), } impl FromStr for ServiceEndpoint { @@ -107,7 +150,7 @@ impl FromStr for ServiceEndpoint { host = host.trim_matches(['[', ']']); Ok(ServiceEndpoint { - host: host.into(), + host: host.parse()?, port: port .parse() .map_err(|err: std::num::ParseIntError| Self::Err::InvalidPort(err.to_string()))?, @@ -122,7 +165,10 @@ impl UpdaterType for ServiceEndpoint { #[cfg(feature = "api-types")] impl ApiType for ServiceEndpoint { - const API_SCHEMA: proxmox_schema::Schema = StringSchema::new("service endpoint").schema(); + const API_SCHEMA: proxmox_schema::Schema = + StringSchema::new("service endpoint (DNS name or IP address with port).") + .format(&HOST_PORT_FORMAT) + .schema(); } #[cfg(test)] @@ -133,20 +179,44 @@ mod tests { #[test] fn display_works() { - let s = ServiceEndpoint::new("127.0.0.1", 123); + let s = ServiceEndpoint::new("127.0.0.1", 123).unwrap(); assert_eq!(s.to_string(), "127.0.0.1:123"); - let s = ServiceEndpoint::new("fc00:f00d::4321", 123); + let s = ServiceEndpoint::new("fc00:f00d::4321", 123).unwrap(); assert_eq!(s.to_string(), "[fc00:f00d::4321]:123"); - let s = ServiceEndpoint::new("::", 123); + let s = ServiceEndpoint::new("::", 123).unwrap(); assert_eq!(s.to_string(), "[::]:123"); - let s = ServiceEndpoint::new("fc00::", 123); + let s = ServiceEndpoint::new("fc00::", 123).unwrap(); assert_eq!(s.to_string(), "[fc00::]:123"); - let s = ServiceEndpoint::new("example.com", 123); + let s = ServiceEndpoint::new("example.com", 123).unwrap(); assert_eq!(s.to_string(), "example.com:123"); + + assert!(ServiceEndpoint::new("fc00::/64", 123).is_err()); + assert!(ServiceEndpoint::new("192.0.2.0/24", 123).is_err()); + assert!(ServiceEndpoint::new("foo/bar", 123).is_err()); + } + + #[test] + fn hostname_or_ip_fromstr_works() { + assert_eq!( + "127.0.0.1".parse::().unwrap(), + HostnameOrIpAddr::IpAddr([127, 0, 0, 1].into()), + ); + assert_eq!( + "example.com".parse::().unwrap(), + HostnameOrIpAddr::Hostname("example.com".to_owned()), + ); + assert_eq!( + "foo".parse::().unwrap(), + HostnameOrIpAddr::Hostname("foo".to_owned()), + ); + + assert!("fc00::/64".parse::().is_err()); + assert!("192.0.2.0/24".parse::().is_err()); + assert!("foo/bar".parse::().is_err()); } #[test] @@ -176,5 +246,9 @@ mod tests { port: 123 } ); + + assert!("fc00::/64:123".parse::().is_err()); + assert!("[fc00::/64]:123".parse::().is_err()); + assert!("192.0.2.0/24:123".parse::().is_err()); } } -- 2.47.3