all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal