From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 5595791168 for ; Wed, 3 Apr 2024 12:47:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 37C8F1606F for ; Wed, 3 Apr 2024 12:47:05 +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 ; Wed, 3 Apr 2024 12:47:01 +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 B09FC44D2D for ; Wed, 3 Apr 2024 12:47:01 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Wed, 03 Apr 2024 12:46:56 +0200 Message-Id: Cc: "Wolfgang Bumiller" To: "Proxmox VE development discussion" From: "Max Carrara" 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-10-s.hanreich@proxmox.com> In-Reply-To: <20240402171629.536804-10-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 09/37] config: firewall: add types for rules X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Apr 2024 10:47:35 -0000 On Tue Apr 2, 2024 at 7:16 PM CEST, Stefan Hanreich wrote: > Additionally we implement FromStr for all rule types and parts, which > can be used for parsing firewall config rules. Initial rule parsing > works by parsing the different options into a HashMap and only then > de-serializing a struct from the parsed options. > > This intermediate step makes rule parsing a lot easier, since we can > reuse the deserialization logic from serde. Also, we can split the > parsing/deserialization logic from the validation logic. > > Co-authored-by: Wolfgang Bumiller > Signed-off-by: Stefan Hanreich > --- > proxmox-ve-config/src/firewall/parse.rs | 185 ++++ > proxmox-ve-config/src/firewall/types/mod.rs | 3 + > proxmox-ve-config/src/firewall/types/rule.rs | 412 ++++++++ > .../src/firewall/types/rule_match.rs | 953 ++++++++++++++++++ > 4 files changed, 1553 insertions(+) > create mode 100644 proxmox-ve-config/src/firewall/types/rule.rs > create mode 100644 proxmox-ve-config/src/firewall/types/rule_match.rs > > diff --git a/proxmox-ve-config/src/firewall/parse.rs b/proxmox-ve-config/= src/firewall/parse.rs > index 669623b..227e045 100644 > --- a/proxmox-ve-config/src/firewall/parse.rs > +++ b/proxmox-ve-config/src/firewall/parse.rs > @@ -1,3 +1,5 @@ > +use std::fmt; > + > use anyhow::{bail, format_err, Error}; > =20 > /// Parses out a "name" which can be alphanumeric and include dashes. > @@ -78,3 +80,186 @@ pub fn parse_bool(value: &str) -> Result= { > }, > ) > } > + > +/// `&str` deserializer which also accepts an `Option`. > +/// > +/// Serde's `StringDeserializer` does not. > +#[derive(Clone, Copy, Debug)] > +pub struct SomeStrDeserializer<'a, E>(serde::de::value::StrDeserializer<= 'a, E>); > + > +impl<'de, 'a, E> serde::de::Deserializer<'de> for SomeStrDeserializer<'a= , E> > +where > + E: serde::de::Error, > +{ > + type Error =3D E; > + > + fn deserialize_any(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_any(visitor) > + } > + > + fn deserialize_option(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + visitor.visit_some(self.0) > + } > + > + fn deserialize_str(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_str(visitor) > + } > + > + fn deserialize_string(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_string(visitor) > + } > + > + fn deserialize_enum( > + self, > + _name: &str, > + _variants: &'static [&'static str], > + visitor: V, > + ) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + visitor.visit_enum(self.0) > + } > + > + serde::forward_to_deserialize_any! { > + bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char > + bytes byte_buf unit unit_struct newtype_struct seq tuple > + tuple_struct map struct identifier ignored_any > + } > +} > + > +/// `&str` wrapper which implements `IntoDeserializer` via `SomeStrDeser= ializer`. > +#[derive(Clone, Debug)] > +pub struct SomeStr<'a>(pub &'a str); > + > +impl<'a> From<&'a str> for SomeStr<'a> { > + fn from(s: &'a str) -> Self { > + Self(s) > + } > +} > + > +impl<'de, 'a, E> serde::de::IntoDeserializer<'de, E> for SomeStr<'a> > +where > + E: serde::de::Error, > +{ > + type Deserializer =3D SomeStrDeserializer<'a, E>; > + > + fn into_deserializer(self) -> Self::Deserializer { > + SomeStrDeserializer(self.0.into_deserializer()) > + } > +} > + > +/// `String` deserializer which also accepts an `Option`. > +/// > +/// Serde's `StringDeserializer` does not. > +#[derive(Clone, Debug)] > +pub struct SomeStringDeserializer(serde::de::value::StringDeserialize= r); > + > +impl<'de, E> serde::de::Deserializer<'de> for SomeStringDeserializer > +where > + E: serde::de::Error, > +{ > + type Error =3D E; > + > + fn deserialize_any(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_any(visitor) > + } > + > + fn deserialize_option(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + visitor.visit_some(self.0) > + } > + > + fn deserialize_str(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_str(visitor) > + } > + > + fn deserialize_string(self, visitor: V) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + self.0.deserialize_string(visitor) > + } > + > + fn deserialize_enum( > + self, > + _name: &str, > + _variants: &'static [&'static str], > + visitor: V, > + ) -> Result > + where > + V: serde::de::Visitor<'de>, > + { > + visitor.visit_enum(self.0) > + } > + > + serde::forward_to_deserialize_any! { > + bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char > + bytes byte_buf unit unit_struct newtype_struct seq tuple > + tuple_struct map struct identifier ignored_any > + } > +} > + > +/// `&str` wrapper which implements `IntoDeserializer` via `SomeStringDe= serializer`. > +#[derive(Clone, Debug)] > +pub struct SomeString(pub String); > + > +impl From<&str> for SomeString { > + fn from(s: &str) -> Self { > + Self::from(s.to_string()) > + } > +} > + > +impl From for SomeString { > + fn from(s: String) -> Self { > + Self(s) > + } > +} > + > +impl<'de, E> serde::de::IntoDeserializer<'de, E> for SomeString > +where > + E: serde::de::Error, > +{ > + type Deserializer =3D SomeStringDeserializer; > + > + fn into_deserializer(self) -> Self::Deserializer { > + SomeStringDeserializer(self.0.into_deserializer()) > + } > +} > + > +#[derive(Debug)] > +pub struct SerdeStringError(String); > + > +impl std::error::Error for SerdeStringError {} > + > +impl fmt::Display for SerdeStringError { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + f.write_str(&self.0) > + } > +} > + > +impl serde::de::Error for SerdeStringError { > + fn custom(msg: T) -> Self { > + Self(msg.to_string()) > + } > +} > diff --git a/proxmox-ve-config/src/firewall/types/mod.rs b/proxmox-ve-con= fig/src/firewall/types/mod.rs > index 5833787..b4a6b12 100644 > --- a/proxmox-ve-config/src/firewall/types/mod.rs > +++ b/proxmox-ve-config/src/firewall/types/mod.rs > @@ -3,7 +3,10 @@ pub mod alias; > pub mod ipset; > pub mod log; > pub mod port; > +pub mod rule; > +pub mod rule_match; > =20 > pub use address::Cidr; > pub use alias::Alias; > pub use ipset::Ipset; > +pub use rule::Rule; > diff --git a/proxmox-ve-config/src/firewall/types/rule.rs b/proxmox-ve-co= nfig/src/firewall/types/rule.rs > new file mode 100644 > index 0000000..20deb3a > --- /dev/null > +++ b/proxmox-ve-config/src/firewall/types/rule.rs > @@ -0,0 +1,412 @@ > +use core::fmt::Display; > +use std::fmt; > +use std::str::FromStr; > + > +use anyhow::{bail, ensure, format_err, Error}; > + > +use crate::firewall::parse::match_name; > +use crate::firewall::types::rule_match::RuleMatch; > +use crate::firewall::types::rule_match::RuleOptions; > + > +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] > +pub enum Direction { > + #[default] > + In, > + Out, > +} > + > +impl std::str::FromStr for Direction { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + for (name, dir) in [("IN", Direction::In), ("OUT", Direction::Ou= t)] { > + if s.eq_ignore_ascii_case(name) { > + return Ok(dir); > + } > + } > + > + bail!("invalid direction: {s:?}, expect 'IN' or 'OUT'"); > + } > +} > + > +serde_plain::derive_deserialize_from_fromstr!(Direction, "valid packet d= irection"); > + > +impl fmt::Display for Direction { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + Direction::In =3D> f.write_str("in"), > + Direction::Out =3D> f.write_str("out"), > + } > + } > +} > + > +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] > +pub enum Verdict { > + Accept, > + Reject, > + #[default] > + Drop, > +} > + > +impl std::str::FromStr for Verdict { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + for (name, verdict) in [ > + ("ACCEPT", Verdict::Accept), > + ("REJECT", Verdict::Reject), > + ("DROP", Verdict::Drop), > + ] { > + if s.eq_ignore_ascii_case(name) { > + return Ok(verdict); > + } > + } > + bail!("invalid verdict {s:?}, expected one of 'ACCEPT', 'REJECT'= or 'DROP'"); > + } > +} > + > +impl Display for Verdict { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + let string =3D match self { > + Verdict::Accept =3D> "ACCEPT", > + Verdict::Drop =3D> "DROP", > + Verdict::Reject =3D> "REJECT", > + }; > + > + write!(f, "{string}") > + } > +} > + > +serde_plain::derive_deserialize_from_fromstr!(Verdict, "valid verdict"); > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Rule { > + pub(crate) disabled: bool, > + pub(crate) kind: Kind, > + pub(crate) comment: Option, > +} > + > +impl std::ops::Deref for Rule { > + type Target =3D Kind; > + > + fn deref(&self) -> &Self::Target { > + &self.kind > + } > +} > + > +impl std::ops::DerefMut for Rule { > + fn deref_mut(&mut self) -> &mut Self::Target { > + &mut self.kind > + } > +} > + > +impl FromStr for Rule { > + type Err =3D Error; > + > + fn from_str(input: &str) -> Result { > + if input.contains(['\n', '\r']) { > + bail!("rule must not contain any newlines!"); > + } > + > + let (line, comment) =3D match input.rsplit_once('#') { > + Some((line, comment)) if !comment.is_empty() =3D> (line.trim= (), Some(comment.trim())), > + _ =3D> (input.trim(), None), > + }; > + > + let (disabled, line) =3D match line.strip_prefix('|') { > + Some(line) =3D> (true, line.trim_start()), > + None =3D> (false, line), > + }; > + > + // todo: case insensitive? > + let kind =3D if line.starts_with("GROUP") { > + Kind::from(line.parse::()?) > + } else { > + Kind::from(line.parse::()?) > + }; > + > + Ok(Self { > + disabled, > + comment: comment.map(str::to_string), > + kind, > + }) > + } > +} > + > +impl Rule { > + pub fn iface(&self) -> Option<&str> { > + match &self.kind { > + Kind::Group(group) =3D> group.iface(), > + Kind::Match(rule) =3D> rule.iface(), > + } > + } > + > + pub fn disabled(&self) -> bool { > + self.disabled > + } > + > + pub fn kind(&self) -> &Kind { > + &self.kind > + } > + > + pub fn comment(&self) -> Option<&str> { > + self.comment.as_deref() > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum Kind { > + Group(RuleGroup), > + Match(RuleMatch), > +} > + > +impl Kind { > + pub fn is_group(&self) -> bool { > + matches!(self, Kind::Group(_)) > + } > + > + pub fn is_match(&self) -> bool { > + matches!(self, Kind::Match(_)) > + } > +} > + > +impl From for Kind { > + fn from(value: RuleGroup) -> Self { > + Kind::Group(value) > + } > +} > + > +impl From for Kind { > + fn from(value: RuleMatch) -> Self { > + Kind::Match(value) > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct RuleGroup { > + pub(crate) group: String, > + pub(crate) iface: Option, > +} > + > +impl RuleGroup { > + pub(crate) fn from_options(group: String, options: RuleOptions) -> R= esult { > + ensure!( > + options.proto.is_none() > + && options.dport.is_none() > + && options.sport.is_none() > + && options.dest.is_none() > + && options.source.is_none() > + && options.log.is_none() > + && options.icmp_type.is_none(), > + "only interface parameter is permitted for group rules" > + ); > + > + Ok(Self { > + group, > + iface: options.iface, > + }) > + } > + > + pub fn group(&self) -> &str { > + &self.group > + } > + > + pub fn iface(&self) -> Option<&str> { > + self.iface.as_deref() > + } > +} > + > +impl FromStr for RuleGroup { > + type Err =3D Error; > + > + fn from_str(input: &str) -> Result { > + let (keyword, rest) =3D match_name(input) > + .ok_or_else(|| format_err!("expected a leading keyword in ru= le group"))?; > + > + if !keyword.eq_ignore_ascii_case("group") { > + bail!("Expected keyword GROUP") > + } > + > + let (name, rest) =3D > + match_name(rest.trim()).ok_or_else(|| format_err!("expected = a name for rule group"))?; > + > + let options =3D rest.trim_start().parse()?; > + > + Self::from_options(name.to_string(), options) > + } > +} > + > +#[cfg(test)] > +mod tests { > + use crate::firewall::types::{ > + address::{IpEntry, IpList}, > + alias::{AliasName, AliasScope}, > + ipset::{IpsetName, IpsetScope}, > + log::LogLevel, > + rule_match::{Icmp, IcmpCode, IpAddrMatch, IpMatch, Ports, Protoc= ol, Udp}, > + Cidr, > + }; > + > + use super::*; > + > + #[test] > + fn test_parse_rule() { > + let mut rule: Rule =3D "|GROUP tgr -i eth0 # acomm".parse().expe= ct("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: true, > + comment: Some("acomm".to_string()), > + kind: Kind::Group(RuleGroup { > + group: "tgr".to_string(), > + iface: Some("eth0".to_string()), > + }), > + }, > + ); > + > + rule =3D "IN ACCEPT -p udp -dport 33 -sport 22 -log warning" > + .parse() > + .expect("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::In, > + verdict: Verdict::Accept, > + proto: Some(Udp::new(Ports::from_u16(22, 33)).into()= ), > + log: Some(LogLevel::Warning), > + ..Default::default() > + }), > + } > + ); > + > + rule =3D "IN ACCEPT --proto udp -i eth0".parse().expect("valid r= ule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::In, > + verdict: Verdict::Accept, > + proto: Some(Udp::new(Ports::new(None, None)).into())= , > + iface: Some("eth0".to_string()), > + ..Default::default() > + }), > + } > + ); > + > + rule =3D " OUT DROP \ > + -source 10.0.0.0/24 -dest 20.0.0.0-20.255.255.255,192.168.0.0/= 16 \ > + -p icmp -log nolog -icmp-type port-unreachable " > + .parse() > + .expect("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::Out, > + verdict: Verdict::Drop, > + ip: IpMatch::new( > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0= , 0, 0], 24).unwrap())), > + IpAddrMatch::Ip( > + IpList::new(vec![ > + IpEntry::Range([20, 0, 0, 0].into(), [20= , 255, 255, 255].into()), > + IpEntry::Cidr(Cidr::new_v4([192, 168, 0,= 0], 16).unwrap()), > + ]) > + .unwrap() > + ), > + ) > + .ok(), > + proto: Some(Protocol::Icmp(Icmp::new_code(IcmpCode::= Named( > + "port-unreachable" > + )))), > + log: Some(LogLevel::Nolog), > + ..Default::default() > + }), > + } > + ); > + > + rule =3D "IN BGP(ACCEPT) --log crit --iface eth0" > + .parse() > + .expect("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::In, > + verdict: Verdict::Accept, > + log: Some(LogLevel::Critical), > + fw_macro: Some("BGP".to_string()), > + iface: Some("eth0".to_string()), > + ..Default::default() > + }), > + } > + ); > + > + rule =3D "IN ACCEPT --source dc/test --dest +dc/test" > + .parse() > + .expect("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::In, > + verdict: Verdict::Accept, > + ip: Some( > + IpMatch::new( > + IpAddrMatch::Alias(AliasName::new(AliasScope= ::Datacenter, "test")), > + IpAddrMatch::Set(IpsetName::new(IpsetScope::= Datacenter, "test"),), > + ) > + .unwrap() > + ), > + ..Default::default() > + }), > + } > + ); > + > + rule =3D "IN REJECT".parse().expect("valid rule"); > + > + assert_eq!( > + rule, > + Rule { > + disabled: false, > + comment: None, > + kind: Kind::Match(RuleMatch { > + dir: Direction::In, > + verdict: Verdict::Reject, > + ..Default::default() > + }), > + } > + ); > + > + "IN DROP ---log crit" > + .parse::() > + .expect_err("too many dashes in option"); > + > + "IN DROP --log --iface eth0" > + .parse::() > + .expect_err("no value for option"); > + > + "IN DROP --log crit --iface" > + .parse::() > + .expect_err("no value for option"); > + } > +} > diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox= -ve-config/src/firewall/types/rule_match.rs > new file mode 100644 > index 0000000..ae5345c > --- /dev/null > +++ b/proxmox-ve-config/src/firewall/types/rule_match.rs > @@ -0,0 +1,953 @@ > +use std::collections::HashMap; > +use std::fmt; > +use std::str::FromStr; > + > +use serde::Deserialize; > + > +use anyhow::{bail, format_err, Error}; > +use serde::de::IntoDeserializer; > + > +use crate::firewall::parse::{match_name, match_non_whitespace, SomeStr}; > +use crate::firewall::types::address::{Family, IpList}; > +use crate::firewall::types::alias::AliasName; > +use crate::firewall::types::ipset::IpsetName; > +use crate::firewall::types::log::LogLevel; > +use crate::firewall::types::port::PortList; > +use crate::firewall::types::rule::{Direction, Verdict}; > + > +#[derive(Clone, Debug, Default, Deserialize)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +#[serde(deny_unknown_fields, rename_all =3D "kebab-case")] > +pub(crate) struct RuleOptions { > + #[serde(alias =3D "p")] > + pub(crate) proto: Option, > + > + pub(crate) dport: Option, > + pub(crate) sport: Option, > + > + pub(crate) dest: Option, > + pub(crate) source: Option, > + > + #[serde(alias =3D "i")] > + pub(crate) iface: Option, > + > + pub(crate) log: Option, > + pub(crate) icmp_type: Option, > +} > + > +impl FromStr for RuleOptions { > + type Err =3D Error; > + > + fn from_str(mut line: &str) -> Result { > + let mut options =3D HashMap::new(); > + > + loop { > + line =3D line.trim_start(); > + > + if line.is_empty() { > + break; > + } > + > + line =3D line > + .strip_prefix('-') > + .ok_or_else(|| format_err!("expected an option starting = with '-'"))?; > + > + // second dash is optional > + line =3D line.strip_prefix('-').unwrap_or(line); > + > + let param; > + (param, line) =3D match_name(line) > + .ok_or_else(|| format_err!("expected a parameter name af= ter '-'"))?; > + > + let value; > + (value, line) =3D match_non_whitespace(line.trim_start()) > + .ok_or_else(|| format_err!("expected a value for {param:= ?}"))?; > + > + if options.insert(param, SomeStr(value)).is_some() { > + bail!("duplicate option in rule: {param}") > + } > + } > + > + Ok(RuleOptions::deserialize(IntoDeserializer::< > + '_, > + crate::firewall::parse::SerdeStringError, > + >::into_deserializer( > + options > + ))?) > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct RuleMatch { > + pub(crate) dir: Direction, > + pub(crate) verdict: Verdict, > + pub(crate) fw_macro: Option, > + > + pub(crate) iface: Option, > + pub(crate) log: Option, > + pub(crate) ip: Option, > + pub(crate) proto: Option, > +} > + > +impl RuleMatch { > + pub(crate) fn from_options( > + dir: Direction, > + verdict: Verdict, > + fw_macro: impl Into>, > + options: RuleOptions, > + ) -> Result { > + if options.dport.is_some() && options.icmp_type.is_some() { > + bail!("dport and icmp-type are mutually exclusive"); > + } > + > + let ip =3D IpMatch::from_options(&options)?; > + let proto =3D Protocol::from_options(&options)?; > + > + // todo: check protocol & IP Version compatibility > + > + Ok(Self { > + dir, > + verdict, > + fw_macro: fw_macro.into(), > + iface: options.iface, > + log: options.log, > + ip, > + proto, > + }) > + } > + > + pub fn direction(&self) -> Direction { > + self.dir > + } > + > + pub fn iface(&self) -> Option<&str> { > + self.iface.as_deref() > + } > + > + pub fn verdict(&self) -> Verdict { > + self.verdict > + } > + > + pub fn fw_macro(&self) -> Option<&str> { > + self.fw_macro.as_deref() > + } > + > + pub fn log(&self) -> Option { > + self.log > + } > + > + pub fn ip(&self) -> Option<&IpMatch> { > + self.ip.as_ref() > + } > + > + pub fn proto(&self) -> Option<&Protocol> { > + self.proto.as_ref() > + } > +} > + > +/// Returns `(Macro name, Verdict, RestOfTheLine)`. > +fn parse_action(line: &str) -> Result<(Option<&str>, Verdict, &str), Err= or> { Hmm, since this is only used below, IMO it's fine that this returns a tuple like that on `Ok` - but should functions like that be used in multiple places, it might be beneficial to use a type alias or even a tuple struct for readability's sake. > + let (verdict, line) =3D > + match_name(line).ok_or_else(|| format_err!("expected a verdict o= r macro name"))?; > + > + Ok(if let Some(line) =3D line.strip_prefix('(') { > + // () > + > + let macro_name =3D verdict; > + let (verdict, line) =3D match_name(line).ok_or_else(|| format_er= r!("expected a verdict"))?; > + let line =3D line > + .strip_prefix(')') > + .ok_or_else(|| format_err!("expected closing ')' after verdi= ct"))?; > + > + let verdict: Verdict =3D verdict.parse()?; > + > + (Some(macro_name), verdict, line.trim_start()) > + } else { > + (None, verdict.parse()?, line.trim_start()) > + }) > +} > + > +impl FromStr for RuleMatch { > + type Err =3D Error; > + > + fn from_str(line: &str) -> Result { > + let (dir, rest) =3D match_name(line).ok_or_else(|| format_err!("= expected a direction"))?; > + > + let direction: Direction =3D dir.parse()?; > + > + let (fw_macro, verdict, rest) =3D parse_action(rest.trim_start()= )?; > + > + let options: RuleOptions =3D rest.trim_start().parse()?; > + > + Self::from_options(direction, verdict, fw_macro.map(str::to_stri= ng), options) > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct IpMatch { > + pub(crate) src: Option, > + pub(crate) dst: Option, > +} > + > +impl IpMatch { > + pub fn new( > + src: impl Into>, > + dst: impl Into>, > + ) -> Result { > + let source =3D src.into(); > + let dest =3D dst.into(); > + > + if source.is_none() && dest.is_none() { > + bail!("either src or dst must be set") > + } > + > + if let (Some(src), Some(dst)) =3D (&source, &dest) { > + if src.family() !=3D dst.family() { > + bail!("src and dst family must be equal") > + } > + } > + > + let ip_match =3D Self { > + src: source, > + dst: dest, > + }; > + > + Ok(ip_match) > + } > + > + fn from_options(options: &RuleOptions) -> Result, Error= > { > + let src =3D options > + .source > + .as_ref() > + .map(|elem| elem.parse::()) > + .transpose()?; > + > + let dst =3D options > + .dest > + .as_ref() > + .map(|elem| elem.parse::()) > + .transpose()?; > + > + Ok(IpMatch::new(src, dst).ok()) > + } > + > + pub fn src(&self) -> Option<&IpAddrMatch> { > + self.src.as_ref() > + } > + > + pub fn dst(&self) -> Option<&IpAddrMatch> { > + self.dst.as_ref() > + } > +} > + > +#[derive(Clone, Debug, Deserialize)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum IpAddrMatch { > + Ip(IpList), > + Set(IpsetName), > + Alias(AliasName), > +} > + > +impl IpAddrMatch { > + pub fn family(&self) -> Option { > + if let IpAddrMatch::Ip(list) =3D self { > + return Some(list.family()); > + } > + > + None > + } > +} > + > +impl FromStr for IpAddrMatch { > + type Err =3D Error; > + > + fn from_str(value: &str) -> Result { > + if value.is_empty() { > + bail!("empty IP specification"); > + } > + > + if let Ok(ip_list) =3D value.parse() { > + return Ok(IpAddrMatch::Ip(ip_list)); > + } > + > + if let Ok(ipset) =3D value.parse() { > + return Ok(IpAddrMatch::Set(ipset)); > + } > + > + if let Ok(name) =3D value.parse() { > + return Ok(IpAddrMatch::Alias(name)); > + } > + > + bail!("invalid IP specification: {value}") > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum Protocol { > + Dccp(Ports), > + Sctp(Sctp), > + Tcp(Tcp), > + Udp(Udp), > + UdpLite(Ports), > + Icmp(Icmp), > + Icmpv6(Icmpv6), > + Named(String), > + Numeric(u8), > +} > + > +impl Protocol { > + pub(crate) fn from_options(options: &RuleOptions) -> Result, Error> { > + let proto =3D match options.proto.as_deref() { > + Some(p) =3D> p, > + None =3D> return Ok(None), > + }; > + > + Ok(Some(match proto { > + "dccp" | "33" =3D> Protocol::Dccp(Ports::from_options(option= s)?), > + "sctp" | "132" =3D> Protocol::Sctp(Sctp::from_options(option= s)?), > + "tcp" | "6" =3D> Protocol::Tcp(Tcp::from_options(options)?), > + "udp" | "17" =3D> Protocol::Udp(Udp::from_options(options)?)= , > + "udplite" | "136" =3D> Protocol::UdpLite(Ports::from_options= (options)?), > + "icmp" | "1" =3D> Protocol::Icmp(Icmp::from_options(options)= ?), > + "ipv6-icmp" | "icmpv6" | "58" =3D> Protocol::Icmpv6(Icmpv6::= from_options(options)?), > + other =3D> match other.parse::() { > + Ok(num) =3D> Protocol::Numeric(num), > + Err(_) =3D> Protocol::Named(other.to_string()), > + }, > + })) > + } > + > + pub fn family(&self) -> Option { > + match self { > + Self::Icmp(_) =3D> Some(Family::V4), > + Self::Icmpv6(_) =3D> Some(Family::V6), > + _ =3D> None, > + } > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Udp { > + ports: Ports, > +} > + > +impl Udp { > + fn from_options(options: &RuleOptions) -> Result { > + Ok(Self { > + ports: Ports::from_options(options)?, > + }) > + } > + > + pub fn new(ports: Ports) -> Self { > + Self { ports } > + } > + > + pub fn ports(&self) -> &Ports { > + &self.ports > + } > +} > + > +impl From for Protocol { > + fn from(value: Udp) -> Self { > + Protocol::Udp(value) > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Ports { > + sport: Option, > + dport: Option, > +} > + > +impl Ports { > + pub fn new(sport: impl Into>, dport: impl Into>) -> Self { > + Self { > + sport: sport.into(), > + dport: dport.into(), > + } > + } > + > + fn from_options(options: &RuleOptions) -> Result { > + Ok(Self { > + sport: options.sport.as_deref().map(|s| s.parse()).transpose= ()?, > + dport: options.dport.as_deref().map(|s| s.parse()).transpose= ()?, > + }) > + } > + > + pub fn from_u16(sport: impl Into>, dport: impl Into>) -> Self { > + Self::new( > + sport.into().map(PortList::from), > + dport.into().map(PortList::from), > + ) > + } > + > + pub fn sport(&self) -> Option<&PortList> { > + self.sport.as_ref() > + } > + > + pub fn dport(&self) -> Option<&PortList> { > + self.dport.as_ref() > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Tcp { > + ports: Ports, > +} > + > +impl Tcp { > + pub fn new(ports: Ports) -> Self { > + Self { ports } > + } > + > + fn from_options(options: &RuleOptions) -> Result { > + Ok(Self { > + ports: Ports::from_options(options)?, > + }) > + } > + > + pub fn ports(&self) -> &Ports { > + &self.ports > + } > +} > + > +impl From for Protocol { > + fn from(value: Tcp) -> Self { > + Protocol::Tcp(value) > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Sctp { > + ports: Ports, > +} > + > +impl Sctp { > + fn from_options(options: &RuleOptions) -> Result { > + Ok(Self { > + ports: Ports::from_options(options)?, > + }) > + } > + > + pub fn ports(&self) -> &Ports { > + &self.ports > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Icmp { > + ty: Option, > + code: Option, > +} > + > +impl Icmp { > + pub fn new_ty(ty: IcmpType) -> Self { > + Self { > + ty: Some(ty), > + ..Default::default() > + } > + } > + > + pub fn new_code(code: IcmpCode) -> Self { > + Self { > + code: Some(code), > + ..Default::default() > + } > + } > + > + fn from_options(options: &RuleOptions) -> Result { > + if let Some(ty) =3D &options.icmp_type { > + return ty.parse(); > + } > + > + Ok(Self::default()) > + } > + > + pub fn ty(&self) -> Option<&IcmpType> { > + self.ty.as_ref() > + } > + > + pub fn code(&self) -> Option<&IcmpCode> { > + self.code.as_ref() > + } > +} > + > +impl FromStr for Icmp { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + let mut this =3D Self::default(); > + > + if let Ok(ty) =3D s.parse() { > + this.ty =3D Some(ty); > + return Ok(this); > + } > + > + if let Ok(code) =3D s.parse() { > + this.code =3D Some(code); > + return Ok(this); > + } > + > + bail!("supplied string is neither a valid icmp type nor code"); > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum IcmpType { > + Numeric(u8), > + Named(&'static str), > +} > + > +// MUST BE SORTED! Should maaaybe note that it must be sorted for binary search, not just for any reason. :P > +const ICMP_TYPES: &[(&str, u8)] =3D &[ > + ("address-mask-reply", 18), > + ("address-mask-request", 17), > + ("destination-unreachable", 3), > + ("echo-reply", 0), > + ("echo-request", 8), > + ("info-reply", 16), > + ("info-request", 15), > + ("parameter-problem", 12), > + ("redirect", 5), > + ("router-advertisement", 9), > + ("router-solicitation", 10), > + ("source-quench", 4), > + ("time-exceeded", 11), > + ("timestamp-reply", 14), > + ("timestamp-request", 13), > +]; > + > +impl std::str::FromStr for IcmpType { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + if let Ok(ty) =3D s.trim().parse::() { > + return Ok(Self::Numeric(ty)); > + } > + > + if let Ok(index) =3D ICMP_TYPES.binary_search_by(|v| v.0.cmp(s))= { > + return Ok(Self::Named(ICMP_TYPES[index].0)); > + } > + > + bail!("{s:?} is not a valid icmp type"); > + } > +} > + > +impl fmt::Display for IcmpType { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + IcmpType::Numeric(ty) =3D> write!(f, "{ty}"), > + IcmpType::Named(ty) =3D> write!(f, "{ty}"), > + } > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum IcmpCode { > + Numeric(u8), > + Named(&'static str), > +} > + > +// MUST BE SORTED! Same here. > +const ICMP_CODES: &[(&str, u8)] =3D &[ > + ("admin-prohibited", 13), > + ("host-prohibited", 10), > + ("host-unreachable", 1), > + ("net-prohibited", 9), > + ("net-unreachable", 0), > + ("port-unreachable", 3), > + ("prot-unreachable", 2), > +]; > + > +impl std::str::FromStr for IcmpCode { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + if let Ok(code) =3D s.trim().parse::() { > + return Ok(Self::Numeric(code)); > + } > + > + if let Ok(index) =3D ICMP_CODES.binary_search_by(|v| v.0.cmp(s))= { > + return Ok(Self::Named(ICMP_CODES[index].0)); > + } > + > + bail!("{s:?} is not a valid icmp code"); > + } > +} > + > +impl fmt::Display for IcmpCode { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + IcmpCode::Numeric(code) =3D> write!(f, "{code}"), > + IcmpCode::Named(code) =3D> write!(f, "{code}"), > + } > + } > +} > + > +#[derive(Clone, Debug, Default)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub struct Icmpv6 { > + pub ty: Option, > + pub code: Option, > +} > + > +impl Icmpv6 { > + pub fn new_ty(ty: Icmpv6Type) -> Self { > + Self { > + ty: Some(ty), > + ..Default::default() > + } > + } > + > + pub fn new_code(code: Icmpv6Code) -> Self { > + Self { > + code: Some(code), > + ..Default::default() > + } > + } > + > + fn from_options(options: &RuleOptions) -> Result { > + if let Some(ty) =3D &options.icmp_type { > + return ty.parse(); > + } > + > + Ok(Self::default()) > + } > + > + pub fn ty(&self) -> Option<&Icmpv6Type> { > + self.ty.as_ref() > + } > + > + pub fn code(&self) -> Option<&Icmpv6Code> { > + self.code.as_ref() > + } > +} > + > +impl FromStr for Icmpv6 { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + let mut this =3D Self::default(); > + > + if let Ok(ty) =3D s.parse() { > + this.ty =3D Some(ty); > + return Ok(this); > + } > + > + if let Ok(code) =3D s.parse() { > + this.code =3D Some(code); > + return Ok(this); > + } > + > + bail!("supplied string is neither a valid icmpv6 type nor code")= ; > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum Icmpv6Type { > + Numeric(u8), > + Named(&'static str), > +} > + > +// MUST BE SORTED! And here too. > +const ICMPV6_TYPES: &[(&str, u8)] =3D &[ > + ("destination-unreachable", 1), > + ("echo-reply", 129), > + ("echo-request", 128), > + ("ind-neighbor-advert", 142), > + ("ind-neighbor-solicit", 141), > + ("mld-listener-done", 132), > + ("mld-listener-query", 130), > + ("mld-listener-reduction", 132), > + ("mld-listener-report", 131), > + ("mld2-listener-report", 143), > + ("nd-neighbor-advert", 136), > + ("nd-neighbor-solicit", 135), > + ("nd-redirect", 137), > + ("nd-router-advert", 134), > + ("nd-router-solicit", 133), > + ("packet-too-big", 2), > + ("parameter-problem", 4), > + ("router-renumbering", 138), > + ("time-exceeded", 3), > +]; > + > +impl std::str::FromStr for Icmpv6Type { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + if let Ok(ty) =3D s.trim().parse::() { > + return Ok(Self::Numeric(ty)); > + } > + > + if let Ok(index) =3D ICMPV6_TYPES.binary_search_by(|v| v.0.cmp(s= )) { > + return Ok(Self::Named(ICMPV6_TYPES[index].0)); > + } > + > + bail!("{s:?} is not a valid icmpv6 type"); > + } > +} > + > +impl fmt::Display for Icmpv6Type { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + Icmpv6Type::Numeric(ty) =3D> write!(f, "{ty}"), > + Icmpv6Type::Named(ty) =3D> write!(f, "{ty}"), > + } > + } > +} > + > +#[derive(Clone, Debug)] > +#[cfg_attr(test, derive(Eq, PartialEq))] > +pub enum Icmpv6Code { > + Numeric(u8), > + Named(&'static str), > +} > + > +// MUST BE SORTED! As well as here. > +const ICMPV6_CODES: &[(&str, u8)] =3D &[ > + ("addr-unreachable", 3), > + ("admin-prohibited", 1), > + ("no-route", 0), > + ("policy-fail", 5), > + ("port-unreachable", 4), > + ("reject-route", 6), > +]; > + > +impl std::str::FromStr for Icmpv6Code { > + type Err =3D Error; > + > + fn from_str(s: &str) -> Result { > + if let Ok(code) =3D s.trim().parse::() { > + return Ok(Self::Numeric(code)); > + } > + > + if let Ok(index) =3D ICMPV6_CODES.binary_search_by(|v| v.0.cmp(s= )) { > + return Ok(Self::Named(ICMPV6_CODES[index].0)); > + } > + > + bail!("{s:?} is not a valid icmpv6 code"); > + } > +} > + > +impl fmt::Display for Icmpv6Code { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + Icmpv6Code::Numeric(code) =3D> write!(f, "{code}"), > + Icmpv6Code::Named(code) =3D> write!(f, "{code}"), > + } > + } > +} > + > +#[cfg(test)] > +mod tests { > + use crate::firewall::types::Cidr; > + > + use super::*; > + > + #[test] > + fn test_parse_action() { > + assert_eq!(parse_action("REJECT").unwrap(), (None, Verdict::Reje= ct, "")); > + > + assert_eq!( > + parse_action("SSH(ACCEPT) qweasd").unwrap(), > + (Some("SSH"), Verdict::Accept, "qweasd") > + ); > + } > + > + #[test] > + fn test_parse_ip_addr_match() { > + for input in [ > + "10.0.0.0/8", > + "10.0.0.0/8,192.168.0.0-192.168.255.255,172.16.0.1", > + "dc/test", > + "+guest/proxmox", > + ] { > + input.parse::().expect("valid ip match"); > + } > + > + for input in [ > + "10.0.0.0/", > + "10.0.0.0/8,192.168.256.0-192.168.255.255,172.16.0.1", > + "dcc/test", > + "+guest/", > + "", > + ] { > + input.parse::().expect_err("invalid ip match"); > + } > + } > + > + #[test] > + fn test_parse_options() { > + let mut options: RuleOptions =3D > + "-p udp --sport 123 --dport 234 -source 127.0.0.1 --dest 127= .0.0.1 -i ens1 --log crit" > + .parse() > + .expect("valid option string"); > + > + assert_eq!( > + options, > + RuleOptions { > + proto: Some("udp".to_string()), > + sport: Some("123".to_string()), > + dport: Some("234".to_string()), > + source: Some("127.0.0.1".to_string()), > + dest: Some("127.0.0.1".to_string()), > + iface: Some("ens1".to_string()), > + log: Some(LogLevel::Critical), > + icmp_type: None, > + } > + ); > + > + options =3D "".parse().expect("valid option string"); > + > + assert_eq!(options, RuleOptions::default(),); > + } > + > + #[test] > + fn test_construct_ip_match() { > + IpMatch::new( > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0, 0, 0], 8).= unwrap())), > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0, 0, 0], 8).= unwrap())), > + ) > + .expect("valid ip match"); > + > + IpMatch::new( > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0, 0, 0], 8).= unwrap())), > + IpAddrMatch::Ip(IpList::from(Cidr::new_v6([0x0000; 8], 8).un= wrap())), > + ) > + .expect_err("cannot mix ip families"); > + > + IpMatch::new(None, None).expect_err("at least one ip must be set= "); > + } > + > + #[test] > + fn test_from_options() { > + let mut options =3D RuleOptions { > + proto: Some("tcp".to_string()), > + sport: Some("123".to_string()), > + dport: Some("234".to_string()), > + source: Some("192.168.0.1".to_string()), > + dest: Some("10.0.0.1".to_string()), > + iface: Some("eth123".to_string()), > + log: Some(LogLevel::Error), > + ..Default::default() > + }; > + > + assert_eq!( > + Protocol::from_options(&options).unwrap().unwrap(), > + Protocol::Tcp(Tcp::new(Ports::from_u16(123, 234))), > + ); > + > + assert_eq!( > + IpMatch::from_options(&options).unwrap().unwrap(), > + IpMatch::new( > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([192, 168, 0, = 1], 32).unwrap()),), > + IpAddrMatch::Ip(IpList::from(Cidr::new_v4([10, 0, 0, 1],= 32).unwrap()),) > + ) > + .unwrap(), > + ); > + > + options =3D RuleOptions::default(); > + > + assert_eq!(Protocol::from_options(&options).unwrap(), None,); > + > + assert_eq!(IpMatch::from_options(&options).unwrap(), None,); > + > + options =3D RuleOptions { > + proto: Some("tcp".to_string()), > + sport: Some("qwe".to_string()), > + source: Some("qwe".to_string()), > + ..Default::default() > + }; > + > + Protocol::from_options(&options).expect_err("invalid source port= "); > + > + IpMatch::from_options(&options).expect_err("invalid source addre= ss"); > + > + options =3D RuleOptions { > + icmp_type: Some("port-unreachable".to_string()), > + dport: Some("123".to_string()), > + ..Default::default() > + }; > + > + RuleMatch::from_options(Direction::In, Verdict::Drop, None, opti= ons) > + .expect_err("cannot mix dport and icmp-type"); > + } > + > + #[test] > + fn test_parse_icmp() { > + let mut icmp: Icmp =3D "info-request".parse().expect("valid icmp= type"); > + > + assert_eq!( > + icmp, > + Icmp { > + ty: Some(IcmpType::Named("info-request")), > + code: None > + } > + ); > + > + icmp =3D "12".parse().expect("valid icmp type"); > + > + assert_eq!( > + icmp, > + Icmp { > + ty: Some(IcmpType::Numeric(12)), > + code: None > + } > + ); > + > + icmp =3D "port-unreachable".parse().expect("valid icmp code"); > + > + assert_eq!( > + icmp, > + Icmp { > + ty: None, > + code: Some(IcmpCode::Named("port-unreachable")) > + } > + ); > + } > + > + #[test] > + fn test_parse_icmp6() { > + let mut icmp: Icmpv6 =3D "echo-reply".parse().expect("valid icmp= v6 type"); > + > + assert_eq!( > + icmp, > + Icmpv6 { > + ty: Some(Icmpv6Type::Named("echo-reply")), > + code: None > + } > + ); > + > + icmp =3D "12".parse().expect("valid icmpv6 type"); > + > + assert_eq!( > + icmp, > + Icmpv6 { > + ty: Some(Icmpv6Type::Numeric(12)), > + code: None > + } > + ); > + > + icmp =3D "admin-prohibited".parse().expect("valid icmpv6 code"); > + > + assert_eq!( > + icmp, > + Icmpv6 { > + ty: None, > + code: Some(Icmpv6Code::Named("admin-prohibited")) > + } > + ); > + } > +}