Implement check if the address entered by the user is valid within the given subnet, i.e. not a network address or broadcast address.
Partially closes [0]. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5757 Signed-off-by: Michael Köppl <m.koe...@proxmox.com> --- Some input / discussion would be much appreciated here, since this might again be considered too restrictive. Multiple questions came up during in-person discussion: * Is check for broadcast address desired or is it considered a valid configuration for PVE? * Is IPv6 check necessary and if so, is allowing to set the address to a broadcast address a valid setting for IPv6? proxmox-installer-common/src/utils.rs | 47 +++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs index 49f1c9f..3d79749 100644 --- a/proxmox-installer-common/src/utils.rs +++ b/proxmox-installer-common/src/utils.rs @@ -16,6 +16,10 @@ pub enum CidrAddressParseError { InvalidAddr(AddrParseError), /// The mask could not be parsed. InvalidMask(Option<ParseIntError>), + /// The IP address is a network address. + IsNetworkAddr, + /// The IP address is a broadcast address. + IsBroadcastAddr, } impl fmt::Display for CidrAddressParseError { @@ -26,6 +30,10 @@ impl fmt::Display for CidrAddressParseError { } CidrAddressParseError::InvalidAddr(addr_parse_error) => format!("{addr_parse_error}"), CidrAddressParseError::InvalidMask(parse_int_error) => format!("{:?}", parse_int_error), + CidrAddressParseError::IsNetworkAddr => String::from("Address is a network address"), + CidrAddressParseError::IsBroadcastAddr => { + String::from("Address is a broadcast address") + } }; write!(f, "Invalid CIDR: {msg}") @@ -62,10 +70,12 @@ impl CidrAddress { let addr = addr.into(); if mask > mask_limit(&addr) { - Err(CidrAddressParseError::InvalidMask(None)) - } else { - Ok(Self { addr, mask }) + return Err(CidrAddressParseError::InvalidMask(None)); } + + check_addr_valid_in_subnet(&addr, mask)?; + + Ok(Self { addr, mask }) } /// Returns only the IP address part of the address. @@ -104,10 +114,12 @@ impl FromStr for CidrAddress { .map_err(|err| CidrAddressParseError::InvalidMask(Some(err)))?; if mask > mask_limit(&addr) { - Err(CidrAddressParseError::InvalidMask(None)) - } else { - Ok(Self { addr, mask }) + return Err(CidrAddressParseError::InvalidMask(None)); } + + check_addr_valid_in_subnet(&addr, mask)?; + + Ok(Self { addr, mask }) } } @@ -134,6 +146,29 @@ fn mask_limit(addr: &IpAddr) -> usize { if addr.is_ipv4() { 32 } else { 128 } } +fn check_addr_valid_in_subnet(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> { + match &addr { + IpAddr::V4(addr_v4) => { + let num_host_bits = 32 - mask; + let host_part_mask = (1u32 << num_host_bits) - 1; + + let ip_bits = u32::from_be_bytes(addr_v4.octets()); + + let network_addr = ip_bits & !host_part_mask; + let broadcast_addr = network_addr | host_part_mask; + + if ip_bits == network_addr { + Err(CidrAddressParseError::IsNetworkAddr) + } else if ip_bits == broadcast_addr { + Err(CidrAddressParseError::IsBroadcastAddr) + } else { + Ok(()) + } + } + IpAddr::V6(_) => Ok(()), + } +} + /// Possible errors that might occur when parsing FQDNs. #[derive(Debug, Eq, PartialEq)] pub enum FqdnParseError { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
