* [pve-devel] [PATCH installer 1/1] remove network and broadcast address checks
@ 2025-07-16 10:30 Michael Köppl
2025-07-16 10:45 ` Michael Köppl
0 siblings, 1 reply; 2+ messages in thread
From: Michael Köppl @ 2025-07-16 10:30 UTC (permalink / raw)
To: pve-devel
To avoid artificially limiting what users can do, remove the checks
that ascertain if a supplied address is a network or broadcast address.
RFC1812 [0] does not explicitly forbid this, nor does RFC3021.
[0] https://www.rfc-editor.org/rfc/rfc1812
[1] https://www.rfc-editor.org/rfc/rfc3021
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
proxmox-auto-installer/tests/parse-answer.rs | 1 -
.../ipv4_and_subnet_addr_is_network.json | 3 --
.../ipv4_and_subnet_addr_is_network.toml | 18 -----------
proxmox-installer-common/src/utils.rs | 32 -------------------
4 files changed, 54 deletions(-)
delete mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json
delete mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index a354f21..594575a 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -148,7 +148,6 @@ 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_swapsize_greater_than_hdsize,
no_fqdn_from_dhcp,
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
deleted file mode 100644
index b28699d..0000000
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json
+++ /dev/null
@@ -1,3 +0,0 @@
-{
- "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
deleted file mode 100644
index 7a9141b..0000000
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml
+++ /dev/null
@@ -1,18 +0,0 @@
-[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-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
index 929cc6b..378d259 100644
--- a/proxmox-installer-common/src/utils.rs
+++ b/proxmox-installer-common/src/utils.rs
@@ -16,8 +16,6 @@ pub enum CidrAddressParseError {
InvalidAddr(AddrParseError),
/// The mask could not be parsed.
InvalidMask(Box<dyn Error>),
- /// The IP address is a network address.
- IsNetworkAddr,
/// The IP address is a broadcast address.
IsBroadcastAddr,
}
@@ -32,7 +30,6 @@ impl fmt::Display for CidrAddressParseError {
}
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"),
}
}
@@ -68,7 +65,6 @@ impl CidrAddress {
let addr = addr.into();
check_mask_limit(&addr, mask)?;
- check_addr_valid_in_subnet(&addr, mask)?;
Ok(Self { addr, mask })
}
@@ -109,7 +105,6 @@ impl FromStr for CidrAddress {
.map_err(|err| CidrAddressParseError::InvalidMask(Box::new(err)))?;
check_mask_limit(&addr, mask)?;
- check_addr_valid_in_subnet(&addr, mask)?;
Ok(Self { addr, mask })
}
@@ -148,33 +143,6 @@ fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseEr
};
}
-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.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pve-devel] [PATCH installer 1/1] remove network and broadcast address checks
2025-07-16 10:30 [pve-devel] [PATCH installer 1/1] remove network and broadcast address checks Michael Köppl
@ 2025-07-16 10:45 ` Michael Köppl
0 siblings, 0 replies; 2+ messages in thread
From: Michael Köppl @ 2025-07-16 10:45 UTC (permalink / raw)
To: pve-devel
Missed some of the original changes, sorry about that.
Superseded-by: https://lore.proxmox.com/pve-devel/20250716104351.46784-1-m.koeppl@proxmox.com
On 7/16/25 12:30, Michael Köppl wrote:
> To avoid artificially limiting what users can do, remove the checks
> that ascertain if a supplied address is a network or broadcast address.
> RFC1812 [0] does not explicitly forbid this, nor does RFC3021.
>
> [0] https://www.rfc-editor.org/rfc/rfc1812
> [1] https://www.rfc-editor.org/rfc/rfc3021
>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> proxmox-auto-installer/tests/parse-answer.rs | 1 -
> .../ipv4_and_subnet_addr_is_network.json | 3 --
> .../ipv4_and_subnet_addr_is_network.toml | 18 -----------
> proxmox-installer-common/src/utils.rs | 32 -------------------
> 4 files changed, 54 deletions(-)
> delete mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json
> delete mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml
>
> diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
> index a354f21..594575a 100644
> --- a/proxmox-auto-installer/tests/parse-answer.rs
> +++ b/proxmox-auto-installer/tests/parse-answer.rs
> @@ -148,7 +148,6 @@ 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_swapsize_greater_than_hdsize,
> no_fqdn_from_dhcp,
> 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
> deleted file mode 100644
> index b28699d..0000000
> --- a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.json
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -{
> - "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
> deleted file mode 100644
> index 7a9141b..0000000
> --- a/proxmox-auto-installer/tests/resources/parse_answer_fail/ipv4_and_subnet_addr_is_network.toml
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -[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-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
> index 929cc6b..378d259 100644
> --- a/proxmox-installer-common/src/utils.rs
> +++ b/proxmox-installer-common/src/utils.rs
> @@ -16,8 +16,6 @@ pub enum CidrAddressParseError {
> InvalidAddr(AddrParseError),
> /// The mask could not be parsed.
> InvalidMask(Box<dyn Error>),
> - /// The IP address is a network address.
> - IsNetworkAddr,
> /// The IP address is a broadcast address.
> IsBroadcastAddr,
> }
> @@ -32,7 +30,6 @@ impl fmt::Display for CidrAddressParseError {
> }
> 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"),
> }
> }
> @@ -68,7 +65,6 @@ impl CidrAddress {
> let addr = addr.into();
>
> check_mask_limit(&addr, mask)?;
> - check_addr_valid_in_subnet(&addr, mask)?;
>
> Ok(Self { addr, mask })
> }
> @@ -109,7 +105,6 @@ impl FromStr for CidrAddress {
> .map_err(|err| CidrAddressParseError::InvalidMask(Box::new(err)))?;
>
> check_mask_limit(&addr, mask)?;
> - check_addr_valid_in_subnet(&addr, mask)?;
>
> Ok(Self { addr, mask })
> }
> @@ -148,33 +143,6 @@ fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseEr
> };
> }
>
> -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 {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-07-16 10:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-16 10:30 [pve-devel] [PATCH installer 1/1] remove network and broadcast address checks Michael Köppl
2025-07-16 10:45 ` Michael Köppl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.