[pve-devel] [PATCH proxmox-ve-rs v2 05/25] firewall: add ip range types
Wolfgang Bumiller
w.bumiller at proxmox.com
Wed Nov 6 14:13:03 CET 2024
On Thu, Oct 10, 2024 at 05:56:17PM GMT, Stefan Hanreich wrote:
> Currently we are using tuples to represent IP ranges which is
> suboptimal. Validation logic and invariant checking needs to happen at
> every site using the IP range rather than having a unified struct for
> enforcing those invariants.
>
> Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
> ---
> .../src/firewall/types/address.rs | 230 +++++++++++++++++-
> 1 file changed, 228 insertions(+), 2 deletions(-)
>
> diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs
> index e48ac1b..42ec1a1 100644
> --- a/proxmox-ve-config/src/firewall/types/address.rs
> +++ b/proxmox-ve-config/src/firewall/types/address.rs
> @@ -1,9 +1,9 @@
> -use std::fmt;
> +use std::fmt::{self, Display};
> use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
> use std::ops::Deref;
>
> use anyhow::{bail, format_err, Error};
> -use serde_with::DeserializeFromStr;
> +use serde_with::{DeserializeFromStr, SerializeDisplay};
>
> #[derive(Clone, Copy, Debug, Eq, PartialEq)]
> pub enum Family {
> @@ -239,6 +239,202 @@ impl<T: Into<Ipv6Addr>> From<T> for Ipv6Cidr {
> }
> }
>
> +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
> +pub enum IpRangeError {
> + MismatchedFamilies,
> + StartGreaterThanEnd,
> + InvalidFormat,
> +}
> +
> +impl std::error::Error for IpRangeError {}
> +
> +impl Display for IpRangeError {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + f.write_str(match self {
> + IpRangeError::MismatchedFamilies => "mismatched ip address families",
> + IpRangeError::StartGreaterThanEnd => "start is greater than end",
> + IpRangeError::InvalidFormat => "invalid ip range format",
> + })
> + }
> +}
> +
> +/// Represents a range of IPv4 or IPv6 addresses
> +///
> +/// For more information see [`AddressRange`]
> +#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)]
> +pub enum IpRange {
> + V4(AddressRange<Ipv4Addr>),
> + V6(AddressRange<Ipv6Addr>),
> +}
> +
> +impl IpRange {
> + /// returns the family of the IpRange
> + pub fn family(&self) -> Family {
> + match self {
> + IpRange::V4(_) => Family::V4,
> + IpRange::V6(_) => Family::V6,
> + }
> + }
> +
> + /// creates a new [`IpRange`] from two [`IpAddr`]
> + ///
> + /// # Errors
> + ///
> + /// This function will return an error if start and end IP address are not from the same family.
> + pub fn new(start: impl Into<IpAddr>, end: impl Into<IpAddr>) -> Result<Self, IpRangeError> {
> + match (start.into(), end.into()) {
> + (IpAddr::V4(start), IpAddr::V4(end)) => Self::new_v4(start, end),
> + (IpAddr::V6(start), IpAddr::V6(end)) => Self::new_v6(start, end),
> + _ => Err(IpRangeError::MismatchedFamilies),
> + }
> + }
> +
> + /// construct a new Ipv4 Range
> + pub fn new_v4(
> + start: impl Into<Ipv4Addr>,
> + end: impl Into<Ipv4Addr>,
> + ) -> Result<Self, IpRangeError> {
> + Ok(IpRange::V4(AddressRange::new_v4(start, end)?))
> + }
> +
> + pub fn new_v6(
> + start: impl Into<Ipv6Addr>,
> + end: impl Into<Ipv6Addr>,
> + ) -> Result<Self, IpRangeError> {
> + Ok(IpRange::V6(AddressRange::new_v6(start, end)?))
> + }
> +}
> +
> +impl std::str::FromStr for IpRange {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Ok(range) = s.parse() {
> + return Ok(IpRange::V4(range));
> + }
> +
> + if let Ok(range) = s.parse() {
> + return Ok(IpRange::V6(range));
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl fmt::Display for IpRange {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + match self {
> + IpRange::V4(range) => range.fmt(f),
> + IpRange::V6(range) => range.fmt(f),
> + }
> + }
> +}
> +
> +/// Represents a range of IP addresses from start to end
> +///
> +/// This type is for encapsulation purposes for the [`IpRange`] enum and should be instantiated via
> +/// that enum.
> +///
> +/// # Invariants
> +///
> +/// * start and end have the same IP address family
> +/// * start is lesser than or equal to end
lesser -> less
Also:
This range *includes* the `end`. In rust `std` we have `std::ops::Range`
while *this* works like `std::ops::RangeInclusive`.
This might be fine, given it's a vastly different context, however...
> +///
> +/// # Textual representation
> +///
> +/// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255`
> +#[derive(Clone, Copy, Debug, PartialEq, Eq)]
> +pub struct AddressRange<T> {
> + start: T,
> + end: T,
...I think we should name this `last`, so it's less confusing and...
> +}
> +
> +impl AddressRange<Ipv4Addr> {
> + pub(crate) fn new_v4(
> + start: impl Into<Ipv4Addr>,
> + end: impl Into<Ipv4Addr>,
> + ) -> Result<AddressRange<Ipv4Addr>, IpRangeError> {
> + let (start, end) = (start.into(), end.into());
> +
> + if start > end {
> + return Err(IpRangeError::StartGreaterThanEnd);
> + }
> +
> + Ok(Self { start, end })
> + }
> +}
> +
> +impl AddressRange<Ipv6Addr> {
> + pub(crate) fn new_v6(
> + start: impl Into<Ipv6Addr>,
> + end: impl Into<Ipv6Addr>,
> + ) -> Result<AddressRange<Ipv6Addr>, IpRangeError> {
> + let (start, end) = (start.into(), end.into());
> +
> + if start > end {
> + return Err(IpRangeError::StartGreaterThanEnd);
> + }
> +
> + Ok(Self { start, end })
> + }
> +}
> +
> +impl<T> AddressRange<T> {
> + pub fn start(&self) -> &T {
> + &self.start
> + }
> +
> + pub fn end(&self) -> &T {
... similarly these getters should be named `last`. Mainly because with
the ranges being inclusive, this represents the "*last* usable address",
while "end" is also used in `std::ops::Range` to mean "fist *unusable*
number".
> + &self.end
> + }
> +}
> +
> +impl std::str::FromStr for AddressRange<Ipv4Addr> {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Some((start, end)) = s.split_once('-') {
> + let start_address = start
> + .parse::<Ipv4Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + let end_address = end
> + .parse::<Ipv4Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + return Self::new_v4(start_address, end_address);
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl std::str::FromStr for AddressRange<Ipv6Addr> {
> + type Err = IpRangeError;
> +
> + fn from_str(s: &str) -> Result<Self, Self::Err> {
> + if let Some((start, end)) = s.split_once('-') {
> + let start_address = start
> + .parse::<Ipv6Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + let end_address = end
> + .parse::<Ipv6Addr>()
> + .map_err(|_| IpRangeError::InvalidFormat)?;
> +
> + return Self::new_v6(start_address, end_address);
> + }
> +
> + Err(IpRangeError::InvalidFormat)
> + }
> +}
> +
> +impl<T: fmt::Display> fmt::Display for AddressRange<T> {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + write!(f, "{}-{}", self.start, self.end)
> + }
> +}
> +
> #[derive(Clone, Debug)]
> #[cfg_attr(test, derive(Eq, PartialEq))]
> pub enum IpEntry {
> @@ -612,4 +808,34 @@ mod tests {
> ])
> .expect_err("cannot mix ip families in ip list");
> }
> +
> + #[test]
> + fn test_ip_range() {
> + IpRange::new([10, 0, 0, 2], [10, 0, 0, 1]).unwrap_err();
> +
> + IpRange::new(
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0x1000],
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0],
> + )
> + .unwrap_err();
> +
> + let v4_range = IpRange::new([10, 0, 0, 0], [10, 0, 0, 100]).unwrap();
> + assert_eq!(v4_range.family(), Family::V4);
> +
> + let v6_range = IpRange::new(
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0],
> + [0x2001, 0x0db8, 0, 0, 0, 0, 0, 0x1000],
> + )
> + .unwrap();
> + assert_eq!(v6_range.family(), Family::V6);
> +
> + "10.0.0.1-10.0.0.100".parse::<IpRange>().unwrap();
> + "2001:db8::1-2001:db8::f".parse::<IpRange>().unwrap();
> +
> + "10.0.0.1-2001:db8::1000".parse::<IpRange>().unwrap_err();
> + "2001:db8::1-192.168.0.2".parse::<IpRange>().unwrap_err();
> +
> + "10.0.0.1-10.0.0.0".parse::<IpRange>().unwrap_err();
> + "2001:db8::1-2001:db8::0".parse::<IpRange>().unwrap_err();
> + }
> }
> --
> 2.39.5
More information about the pve-devel
mailing list