From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <m.carrara@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id E420B91136
 for <pve-devel@lists.proxmox.com>; Wed,  3 Apr 2024 12:47:09 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id CD4AB16140
 for <pve-devel@lists.proxmox.com>; Wed,  3 Apr 2024 12:47:09 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Wed,  3 Apr 2024 12:47:08 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 70E6B44D2A
 for <pve-devel@lists.proxmox.com>; Wed,  3 Apr 2024 12:47:08 +0200 (CEST)
Content-Type: text/plain; charset=UTF-8
Date: Wed, 03 Apr 2024 12:47:05 +0200
Message-Id: <D0AFESIVEQ0B.SYXRTSNUE4BB@proxmox.com>
Cc: "Wolfgang Bumiller" <w.bumiller@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
From: "Max Carrara" <m.carrara@proxmox.com>
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0
X-Mailer: aerc 0.17.0-72-g6a84f1331f1c
References: <20240402171629.536804-1-s.hanreich@proxmox.com>
 <20240402171629.536804-12-s.hanreich@proxmox.com>
In-Reply-To: <20240402171629.536804-12-s.hanreich@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.028 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH proxmox-firewall 11/37] config: firewall:
 add generic parser for firewall configs
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 03 Apr 2024 10:47:09 -0000

On Tue Apr 2, 2024 at 7:16 PM CEST, Stefan Hanreich wrote:
> Since the basic format of cluster, host and guest firewall
> configurations is the same, we create a generic parser that can handle
> the common config format. The main difference is in the available
> options, which can be passed via a generic parameter.
>
> Co-authored-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  proxmox-ve-config/src/firewall/common.rs | 182 +++++++++++++++++++++
>  proxmox-ve-config/src/firewall/mod.rs    |   1 +
>  proxmox-ve-config/src/firewall/parse.rs  | 200 +++++++++++++++++++++++
>  3 files changed, 383 insertions(+)
>  create mode 100644 proxmox-ve-config/src/firewall/common.rs
>
> diff --git a/proxmox-ve-config/src/firewall/common.rs b/proxmox-ve-config=
/src/firewall/common.rs
> new file mode 100644
> index 0000000..887339b
> --- /dev/null
> +++ b/proxmox-ve-config/src/firewall/common.rs
> @@ -0,0 +1,182 @@
> +use std::collections::HashMap;
> +use std::io;
> +
> +use anyhow::{bail, format_err, Error};
> +use serde::de::IntoDeserializer;
> +
> +use crate::firewall::parse::{parse_named_section_tail, split_key_value, =
SomeString};
> +use crate::firewall::types::ipset::{IpsetName, IpsetScope};
> +use crate::firewall::types::{Alias, Group, Ipset, Rule};
> +
> +#[derive(Debug, Default)]
> +pub struct Config<O>
> +where
> +    O: Default + std::fmt::Debug + serde::de::DeserializeOwned,
> +{
> +    pub(crate) options: O,
> +    pub(crate) rules: Vec<Rule>,
> +    pub(crate) aliases: HashMap<String, Alias>,
> +    pub(crate) ipsets: HashMap<String, Ipset>,
> +    pub(crate) groups: HashMap<String, Group>,
> +}
> +
> +enum Sec {
> +    None,
> +    Options,
> +    Aliases,
> +    Rules,
> +    Ipset(String, Ipset),
> +    Group(String, Group),
> +}
> +
> +#[derive(Default)]
> +pub struct ParserConfig {
> +    /// Network interfaces must be of the form `netX`.
> +    pub guest_iface_names: bool,
> +    pub ipset_scope: Option<IpsetScope>,
> +}
> +
> +impl<O> Config<O>
> +where
> +    O: Default + std::fmt::Debug + serde::de::DeserializeOwned,
> +{
> +    pub fn new() -> Self {
> +        Self::default()
> +    }
> +
> +    pub fn parse<R: io::BufRead>(input: R, parser_cfg: &ParserConfig) ->=
 Result<Self, Error> {
> +        let mut section =3D Sec::None;
> +
> +        let mut this =3D Self::new();
> +        let mut options =3D HashMap::new();
> +
> +        for line in input.lines() {
> +            let line =3D line?;
> +            let line =3D line.trim();
> +
> +            if line.is_empty() || line.starts_with('#') {
> +                continue;
> +            }
> +
> +            if line.eq_ignore_ascii_case("[OPTIONS]") {
> +                this.set_section(&mut section, Sec::Options)?;
> +            } else if line.eq_ignore_ascii_case("[ALIASES]") {
> +                this.set_section(&mut section, Sec::Aliases)?;
> +            } else if line.eq_ignore_ascii_case("[RULES]") {
> +                this.set_section(&mut section, Sec::Rules)?;
> +            } else if let Some(line) =3D line.strip_prefix("[IPSET") {
> +                let (name, comment) =3D parse_named_section_tail("ipset"=
, line)?;
> +
> +                let scope =3D parser_cfg.ipset_scope.ok_or_else(|| {
> +                    format_err!("IPSET in config, but no scope set in pa=
rser config")
> +                })?;
> +
> +                let ipset_name =3D IpsetName::new(scope, name.to_string(=
));
> +                let mut ipset =3D Ipset::new(ipset_name);
> +                ipset.comment =3D comment.map(str::to_owned);
> +
> +                this.set_section(&mut section, Sec::Ipset(name.to_string=
(), ipset))?;
> +            } else if let Some(line) =3D line.strip_prefix("[group") {
> +                let (name, comment) =3D parse_named_section_tail("group"=
, line)?;
> +                let mut group =3D Group::new();
> +
> +                group.set_comment(comment.map(str::to_owned));
> +
> +                this.set_section(&mut section, Sec::Group(name.to_owned(=
), group))?;
> +            } else if line.starts_with('[') {
> +                bail!("invalid section {line:?}");
> +            } else {
> +                match &mut section {
> +                    Sec::None =3D> bail!("config line with no section: {=
line:?}"),
> +                    Sec::Options =3D> Self::parse_option(line, &mut opti=
ons)?,
> +                    Sec::Aliases =3D> this.parse_alias(line)?,
> +                    Sec::Rules =3D> this.parse_rule(line, parser_cfg)?,
> +                    Sec::Ipset(_name, ipset) =3D> ipset.parse_entry(line=
)?,
> +                    Sec::Group(_name, group) =3D> group.parse_entry(line=
)?,
> +                }
> +            }
> +        }
> +        this.set_section(&mut section, Sec::None)?;
> +
> +        this.options =3D O::deserialize(IntoDeserializer::<
> +            '_,
> +            crate::firewall::parse::SerdeStringError,
> +        >::into_deserializer(options))?;
> +
> +        Ok(this)
> +    }
> +
> +    fn parse_option(line: &str, options: &mut HashMap<String, SomeString=
>) -> Result<(), Error> {
> +        let (key, value) =3D split_key_value(line)
> +            .ok_or_else(|| format_err!("expected colon separated key and=
 value, found {line:?}"))?;
> +
> +        if options.insert(key.to_string(), value.into()).is_some() {
> +            bail!("duplicate option {key:?}");
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn parse_alias(&mut self, line: &str) -> Result<(), Error> {
> +        let alias: Alias =3D line.parse()?;
> +
> +        if self
> +            .aliases
> +            .insert(alias.name().to_string(), alias)
> +            .is_some()
> +        {
> +            bail!("duplicate alias: {line}");
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn parse_rule(&mut self, line: &str, parser_cfg: &ParserConfig) -> R=
esult<(), Error> {
> +        let rule: Rule =3D line.parse()?;
> +
> +        if parser_cfg.guest_iface_names {
> +            if let Some(iface) =3D rule.iface() {
> +                let _ =3D iface
> +                    .strip_prefix("net")
> +                    .ok_or_else(|| {
> +                        format_err!("interface name must be of the form =
\"net<number>\"")
> +                    })?
> +                    .parse::<u16>()
> +                    .map_err(|_| {
> +                        format_err!("interface name must be of the form =
\"net<number>\"")
> +                    })?;
> +            }
> +        }
> +
> +        self.rules.push(rule);
> +        Ok(())
> +    }
> +
> +    fn set_section(&mut self, sec: &mut Sec, to: Sec) -> Result<(), Erro=
r> {
> +        let prev =3D std::mem::replace(sec, to);
> +
> +        match prev {
> +            Sec::Ipset(name, ipset) =3D> {
> +                if self.ipsets.insert(name.clone(), ipset).is_some() {
> +                    bail!("duplicate ipset: {name:?}");
> +                }
> +            }
> +            Sec::Group(name, group) =3D> {
> +                if self.groups.insert(name.clone(), group).is_some() {
> +                    bail!("duplicate group: {name:?}");
> +                }
> +            }
> +            _ =3D> (),
> +        }
> +
> +        Ok(())
> +    }
> +
> +    pub fn ipsets(&self) -> &HashMap<String, Ipset> {
> +        &self.ipsets
> +    }
> +
> +    pub fn alias(&self, name: &str) -> Option<&Alias> {
> +        self.aliases.get(name)
> +    }
> +}
> diff --git a/proxmox-ve-config/src/firewall/mod.rs b/proxmox-ve-config/sr=
c/firewall/mod.rs
> index 2e0f31e..591ee52 100644
> --- a/proxmox-ve-config/src/firewall/mod.rs
> +++ b/proxmox-ve-config/src/firewall/mod.rs
> @@ -1,3 +1,4 @@
> +pub mod common;
>  pub mod ports;
>  pub mod types;
> =20
> diff --git a/proxmox-ve-config/src/firewall/parse.rs b/proxmox-ve-config/=
src/firewall/parse.rs
> index 227e045..9cc2b8a 100644
> --- a/proxmox-ve-config/src/firewall/parse.rs
> +++ b/proxmox-ve-config/src/firewall/parse.rs
> @@ -61,6 +61,16 @@ pub fn match_digits(line: &str) -> Option<(&str, &str)=
> {
> =20
>      None
>  }
> +
> +/// Separate a `key: value` line, trimming whitespace.
> +///
> +/// Returns `None` if the `key` would be empty.
> +pub fn split_key_value(line: &str) -> Option<(&str, &str)> {
> +    line.split_once(':')
> +        .map(|(key, value)| (key.trim(), value.trim()))
> +}
> +
> +/// Parse a boolean.
>  pub fn parse_bool(value: &str) -> Result<bool, Error> {
>      Ok(
>          if value =3D=3D "0"
> @@ -81,6 +91,196 @@ pub fn parse_bool(value: &str) -> Result<bool, Error>=
 {
>      )
>  }
> =20
> +/// Parse the *remainder* of a section line, that is `<whitespace>NAME] =
#optional comment`.
> +/// The `kind` parameter is used for error messages and should be the se=
ction type.
> +///
> +/// Return the name and the optional comment.
> +pub fn parse_named_section_tail<'a>(
> +    kind: &'static str,
> +    line: &'a str,
> +) -> Result<(&'a str, Option<&'a str>), Error> {
> +    if line.is_empty() || !line.as_bytes()[0].is_ascii_whitespace() {
> +        bail!("incomplete {kind} section");
> +    }
> +
> +    let line =3D line.trim_start();
> +    let (name, line) =3D match_name(line)
> +        .ok_or_else(|| format_err!("expected a name for the {kind} at {l=
ine:?}"))?;
> +
> +    let line =3D line
> +        .strip_prefix(']')
> +        .ok_or_else(|| format_err!("expected closing ']' in {kind} secti=
on header"))?
> +        .trim_start();
> +
> +    Ok(match line.strip_prefix('#') {
> +        Some(comment) =3D> (name, Some(comment.trim())),
> +        None if !line.is_empty() =3D> bail!("trailing characters after {=
kind} section: {line:?}"),
> +        None =3D> (name, None),
> +    })
> +}
> +
> +// parses a number from a string OR number
> +pub mod serde_option_number {

Since this is `pub`, I think a more complete docstring here would be
better instead of a comment. Though I haven't generated the docs for all
of this (yet) I have to admit, so I'm not sure if this actually shows
up.

> +    use std::fmt;
> +
> +    use serde::de::{Deserializer, Error, Visitor};
> +
> +    pub fn deserialize<'de, D: Deserializer<'de>>(
> +        deserializer: D,
> +    ) -> Result<Option<i64>, D::Error> {
> +        struct V;
> +
> +        impl<'de> Visitor<'de> for V {
> +            type Value =3D Option<i64>;
> +
> +            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +                f.write_str("a numerical value")
> +            }
> +
> +            fn visit_str<E: Error>(self, v: &str) -> Result<Self::Value,=
 E> {
> +                v.parse().map_err(E::custom).map(Some)
> +            }
> +
> +            fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
> +                Ok(None)
> +            }
> +
> +            fn visit_some<D>(self, deserializer: D) -> Result<Self::Valu=
e, D::Error>
> +            where
> +                D: Deserializer<'de>,
> +            {
> +                deserializer.deserialize_any(self)
> +            }
> +        }
> +
> +        deserializer.deserialize_any(V)
> +    }
> +}
> +
> +// parses a bool from a string OR bool
> +pub mod serde_option_bool {

^ Same as above.

> +    use std::fmt;
> +
> +    use serde::de::{Deserializer, Error, Visitor};
> +
> +    pub fn deserialize<'de, D: Deserializer<'de>>(
> +        deserializer: D,
> +    ) -> Result<Option<bool>, D::Error> {
> +        struct V;
> +
> +        impl<'de> Visitor<'de> for V {
> +            type Value =3D Option<bool>;
> +
> +            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +                f.write_str("a boolean-like value")
> +            }
> +
> +            fn visit_bool<E: Error>(self, v: bool) -> Result<Self::Value=
, E> {
> +                Ok(Some(v))
> +            }
> +
> +            fn visit_str<E: Error>(self, v: &str) -> Result<Self::Value,=
 E> {
> +                super::parse_bool(v).map_err(E::custom).map(Some)
> +            }
> +
> +            fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
> +                Ok(None)
> +            }
> +
> +            fn visit_some<D>(self, deserializer: D) -> Result<Self::Valu=
e, D::Error>
> +            where
> +                D: Deserializer<'de>,
> +            {
> +                deserializer.deserialize_any(self)
> +            }
> +        }
> +
> +        deserializer.deserialize_any(V)
> +    }
> +}
> +
> +// parses a comma_separated list of strings
> +pub mod serde_option_conntrack_helpers {

^ Same as above here as well.

> +    use std::fmt;
> +
> +    use serde::de::{Deserializer, Error, Visitor};
> +
> +    pub fn deserialize<'de, D: Deserializer<'de>>(
> +        deserializer: D,
> +    ) -> Result<Option<Vec<String>>, D::Error> {
> +        struct V;
> +
> +        impl<'de> Visitor<'de> for V {
> +            type Value =3D Option<Vec<String>>;
> +
> +            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +                f.write_str("A list of conntrack helpers")
> +            }
> +
> +            fn visit_str<E: Error>(self, v: &str) -> Result<Self::Value,=
 E> {
> +                if v.is_empty() {
> +                    return Ok(None);
> +                }
> +
> +                Ok(Some(v.split(',').map(String::from).collect()))
> +            }
> +
> +            fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
> +                Ok(None)
> +            }
> +
> +            fn visit_some<D>(self, deserializer: D) -> Result<Self::Valu=
e, D::Error>
> +            where
> +                D: Deserializer<'de>,
> +            {
> +                deserializer.deserialize_any(self)
> +            }
> +        }
> +
> +        deserializer.deserialize_any(V)
> +    }
> +}
> +
> +// parses a log_ratelimit string: '[enable=3D]<1|0> [,burst=3D<integer>]=
 [,rate=3D<rate>]'
> +pub mod serde_option_log_ratelimit {

^ And here.

> +    use std::fmt;
> +
> +    use serde::de::{Deserializer, Error, Visitor};
> +
> +    use crate::firewall::types::log::LogRateLimit;
> +
> +    pub fn deserialize<'de, D: Deserializer<'de>>(
> +        deserializer: D,
> +    ) -> Result<Option<LogRateLimit>, D::Error> {
> +        struct V;
> +
> +        impl<'de> Visitor<'de> for V {
> +            type Value =3D Option<LogRateLimit>;
> +
> +            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +                f.write_str("a boolean-like value")
> +            }
> +
> +            fn visit_str<E: Error>(self, v: &str) -> Result<Self::Value,=
 E> {
> +                v.parse().map_err(E::custom).map(Some)
> +            }
> +
> +            fn visit_none<E: Error>(self) -> Result<Self::Value, E> {
> +                Ok(None)
> +            }
> +
> +            fn visit_some<D>(self, deserializer: D) -> Result<Self::Valu=
e, D::Error>
> +            where
> +                D: Deserializer<'de>,
> +            {
> +                deserializer.deserialize_any(self)
> +            }
> +        }
> +
> +        deserializer.deserialize_any(V)
> +    }
> +}
> +
>  /// `&str` deserializer which also accepts an `Option`.
>  ///
>  /// Serde's `StringDeserializer` does not.