Add checks for valid subnet mask (greater than /0 and at most /32 for IPv4). In addition, check if the address entered by the user is valid within the given subnet, i.e. not a network address or broadcast address. /31 is considered an exception in accordance with RFC3021 [0], considering any of the 2 available addresses to be valid host addresses.
[0] https://datatracker.ietf.org/doc/html/rfc3021 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. proxmox-auto-installer/tests/parse-answer.rs | 3 + .../parse_answer/ipv4_and_subnet_31.json | 19 +++++ .../parse_answer/ipv4_and_subnet_31.toml | 18 +++++ .../ipv4_and_subnet_addr_is_network.json | 3 + .../ipv4_and_subnet_addr_is_network.toml | 18 +++++ .../ipv4_and_subnet_mask_33.json | 3 + .../ipv4_and_subnet_mask_33.toml | 18 +++++ proxmox-installer-common/src/utils.rs | 81 ++++++++++++++----- 8 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.toml create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.json create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.toml diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs index f560483..6f627a0 100644 --- a/proxmox-auto-installer/tests/parse-answer.rs +++ b/proxmox-auto-installer/tests/parse-answer.rs @@ -127,6 +127,7 @@ mod tests { fqdn_from_dhcp_no_dhcp_domain_with_default_domain, full_fqdn_from_dhcp_with_default_domain, hashed_root_password, + ipv4_and_subnet_31, minimal, nic_matching, specific_nic, @@ -146,6 +147,8 @@ mod tests { duplicate_disk, fqdn_from_dhcp_no_default_domain, fqdn_hostname_only, + ipv4_and_subnet_addr_is_network, + ipv4_and_subnet_mask_33, lvm_maxroot_greater_than_maximum, lvm_swapsize_greater_than_hdsize, no_fqdn_from_dhcp, diff --git a/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.json b/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.json new file mode 100644 index 0000000..2a475fe --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.json @@ -0,0 +1,19 @@ +{ + "autoreboot": 1, + "cidr": "10.10.10.10/31", + "country": "at", + "dns": "10.10.10.1", + "domain": "testinstall", + "filesys": "ext4", + "gateway": "10.10.10.1", + "hdsize": 223.57088470458984, + "existing_storage_auto_rename": 1, + "hostname": "pveauto", + "keymap": "de", + "mailto": "mail@no.invalid", + "mngmt_nic": "enp129s0f1np1", + "root_password": { "plain": "12345678" }, + "target_hd": "/dev/sda", + "timezone": "Europe/Vienna", + "first_boot": { "enabled": 0 } +} diff --git a/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.toml b/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.toml new file mode 100644 index 0000000..0ca6ca5 --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer/ipv4_and_subnet_31.toml @@ -0,0 +1,18 @@ +[global] +keyboard = "de" +country = "at" +fqdn = "pveauto.testinstall" +mailto = "mail@no.invalid" +timezone = "Europe/Vienna" +root_password = "12345678" + +[network] +source = "from-answer" +cidr = "10.10.10.10/31" +dns = "10.10.10.1" +gateway = "10.10.10.1" +filter.ID_NET_NAME = "enp129s0f1np1" + +[disk-setup] +filesystem = "ext4" +disk_list = ["sda"] diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json new file mode 100644 index 0000000..b28699d --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json @@ -0,0 +1,3 @@ +{ + "parse-error": "error parsing answer.toml: Invalid CIDR: address is a network address" +} diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml new file mode 100644 index 0000000..7a9141b --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml @@ -0,0 +1,18 @@ +[global] +keyboard = "de" +country = "at" +fqdn = "pveauto.testinstall" +mailto = "mail@no.invalid" +timezone = "Europe/Vienna" +root_password = "12345678" + +[network] +source = "from-answer" +cidr = "10.10.10.32/27" +dns = "10.10.10.1" +gateway = "10.10.10.1" +filter.ID_NET_NAME = "enp129s0f1np1" + +[disk-setup] +filesystem = "ext4" +disk_list = ["sda"] diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.json new file mode 100644 index 0000000..6b2888b --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.json @@ -0,0 +1,3 @@ +{ + "parse-error": "error parsing answer.toml: Invalid CIDR: mask cannot be greater than 32" +} diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.toml new file mode 100644 index 0000000..16a560f --- /dev/null +++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_mask_33.toml @@ -0,0 +1,18 @@ +[global] +keyboard = "de" +country = "at" +fqdn = "pveauto.testinstall" +mailto = "mail@no.invalid" +timezone = "Europe/Vienna" +root_password = "12345678" + +[network] +source = "from-answer" +cidr = "10.10.10.10/33" +dns = "10.10.10.1" +gateway = "10.10.10.1" +filter.ID_NET_NAME = "enp129s0f1np1" + +[disk-setup] +filesystem = "ext4" +disk_list = ["sda"] diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs index 1fe6a74..fea98db 100644 --- a/proxmox-installer-common/src/utils.rs +++ b/proxmox-installer-common/src/utils.rs @@ -1,7 +1,7 @@ use std::{ + error::Error, fmt, net::{AddrParseError, IpAddr}, - num::ParseIntError, str::FromStr, }; @@ -15,20 +15,26 @@ pub enum CidrAddressParseError { /// The IP address part could not be parsed. InvalidAddr(AddrParseError), /// The mask could not be parsed. - InvalidMask(Option<ParseIntError>), + InvalidMask(Box<dyn Error>), + /// The IP address is a network address. + IsNetworkAddr, + /// The IP address is a broadcast address. + IsBroadcastAddr, } impl fmt::Display for CidrAddressParseError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let msg = match &self { + write!(f, "Invalid CIDR: ")?; + + match self { CidrAddressParseError::NoDelimiter => { - String::from("No delimiter for separating address and mask was found") + write!(f, "no delimiter for separating address and mask was found") } - CidrAddressParseError::InvalidAddr(addr_parse_error) => format!("{addr_parse_error}"), - CidrAddressParseError::InvalidMask(parse_int_error) => format!("{:?}", parse_int_error), - }; - - write!(f, "Invalid CIDR: {msg}") + CidrAddressParseError::InvalidAddr(err) => write!(f, "{err}"), + CidrAddressParseError::InvalidMask(err) => write!(f, "{err}"), + CidrAddressParseError::IsNetworkAddr => write!(f, "address is a network address"), + CidrAddressParseError::IsBroadcastAddr => write!(f, "address is a broadcast address"), + } } } @@ -61,11 +67,10 @@ impl CidrAddress { pub fn new<T: Into<IpAddr>>(addr: T, mask: usize) -> Result<Self, CidrAddressParseError> { let addr = addr.into(); - if mask > mask_limit(&addr) { - Err(CidrAddressParseError::InvalidMask(None)) - } else { - Ok(Self { addr, mask }) - } + check_mask_limit(&addr, mask)?; + check_addr_valid_in_subnet(&addr, mask)?; + + Ok(Self { addr, mask }) } /// Returns only the IP address part of the address. @@ -101,13 +106,12 @@ impl FromStr for CidrAddress { let mask = mask .parse() - .map_err(|err| CidrAddressParseError::InvalidMask(Some(err)))?; + .map_err(|err| CidrAddressParseError::InvalidMask(Box::new(err)))?; - if mask > mask_limit(&addr) { - Err(CidrAddressParseError::InvalidMask(None)) - } else { - Ok(Self { addr, mask }) - } + check_mask_limit(&addr, mask)?; + check_addr_valid_in_subnet(&addr, mask)?; + + Ok(Self { addr, mask }) } } @@ -133,6 +137,43 @@ fn mask_limit(addr: &IpAddr) -> usize { if addr.is_ipv4() { 32 } else { 128 } } +fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> { + return if mask > mask_limit(&addr) { + Err(CidrAddressParseError::InvalidMask( + "mask cannot be greater than 32".into(), + )) + } else { + Ok(()) + }; +} + +fn check_addr_valid_in_subnet(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> { + match &addr { + IpAddr::V4(addr_v4) => { + if mask >= 31 { + return Ok(()); + } + + 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
