[pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction

[Stefan Hanreich <s.hanreich@proxmox.com>]

This direction will be used for specifying rules on bridge-level
firewalls as well as rules on the cluster / host level that are for
forwarded network packets.

Since with the introduction of this direction not every type of
firewall configuration can contain all types of directions, we
additionally add validation logic to the parser, so rules with an
invalid direction get rejected by the parser.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/src/firewall/cluster.rs    | 11 +++++++++++
 proxmox-ve-config/src/firewall/common.rs     | 11 +++++++++++
 proxmox-ve-config/src/firewall/guest.rs      | 13 +++++++++++++
 proxmox-ve-config/src/firewall/host.rs       | 12 +++++++++++-
 proxmox-ve-config/src/firewall/mod.rs        |  1 +
 proxmox-ve-config/src/firewall/types/rule.rs | 10 ++++++++--
 6 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs
index 223124b..ce3dd53 100644
--- a/proxmox-ve-config/src/firewall/cluster.rs
+++ b/proxmox-ve-config/src/firewall/cluster.rs
@@ -25,12 +25,15 @@ pub const CLUSTER_EBTABLES_DEFAULT: bool = false;
 pub const CLUSTER_POLICY_IN_DEFAULT: Verdict = Verdict::Drop;
 /// default setting for [`Config::default_policy()`]
 pub const CLUSTER_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept;
+/// default setting for [`Config::default_policy()`]
+pub const CLUSTER_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept;
 
 impl Config {
     pub fn parse<R: io::BufRead>(input: R) -> Result<Self, Error> {
         let parser_config = ParserConfig {
             guest_iface_names: false,
             ipset_scope: Some(IpsetScope::Datacenter),
+            allowed_directions: vec![Direction::In, Direction::Out, Direction::Forward],
         };
 
         Ok(Self {
@@ -86,6 +89,11 @@ impl Config {
                 .options
                 .policy_out
                 .unwrap_or(CLUSTER_POLICY_OUT_DEFAULT),
+            Direction::Forward => self
+                .config
+                .options
+                .policy_forward
+                .unwrap_or(CLUSTER_POLICY_FORWARD_DEFAULT),
         }
     }
 
@@ -121,6 +129,7 @@ pub struct Options {
 
     policy_in: Option<Verdict>,
     policy_out: Option<Verdict>,
+    policy_forward: Option<Verdict>,
 }
 
 #[cfg(test)]
@@ -148,6 +157,7 @@ log_ratelimit: 1,rate=10/second,burst=20
 ebtables: 0
 policy_in: REJECT
 policy_out: REJECT
+policy_forward: DROP
 
 [ALIASES]
 
@@ -191,6 +201,7 @@ IN BGP(REJECT) -log crit -source 1.2.3.4
                 )),
                 policy_in: Some(Verdict::Reject),
                 policy_out: Some(Verdict::Reject),
+                policy_forward: Some(Verdict::Drop),
             }
         );
 
diff --git a/proxmox-ve-config/src/firewall/common.rs b/proxmox-ve-config/src/firewall/common.rs
index a08f19c..3999168 100644
--- a/proxmox-ve-config/src/firewall/common.rs
+++ b/proxmox-ve-config/src/firewall/common.rs
@@ -6,6 +6,7 @@ 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::rule::{Direction, Kind};
 use crate::firewall::types::{Alias, Group, Ipset, Rule};
 
 #[derive(Debug, Default)]
@@ -34,6 +35,7 @@ pub struct ParserConfig {
     /// Network interfaces must be of the form `netX`.
     pub guest_iface_names: bool,
     pub ipset_scope: Option<IpsetScope>,
+    pub allowed_directions: Vec<Direction>,
 }
 
 impl<O> Config<O>
@@ -150,6 +152,15 @@ where
             }
         }
 
+        if let Kind::Match(rule) = rule.kind() {
+            if !parser_cfg.allowed_directions.contains(&rule.dir) {
+                bail!(
+                    "found not allowed direction in firewall config: {0}",
+                    rule.dir
+                );
+            }
+        }
+
         self.rules.push(rule);
         Ok(())
     }
diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs
index c7e282f..1e70a67 100644
--- a/proxmox-ve-config/src/firewall/guest.rs
+++ b/proxmox-ve-config/src/firewall/guest.rs
@@ -31,6 +31,8 @@ pub const GUEST_IPFILTER_DEFAULT: bool = false;
 pub const GUEST_POLICY_IN_DEFAULT: Verdict = Verdict::Drop;
 /// default return value for [`Config::default_policy()`]
 pub const GUEST_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept;
+/// default return value for [`Config::default_policy()`]
+pub const GUEST_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept;
 
 #[derive(Debug, Default, Deserialize)]
 #[cfg_attr(test, derive(Eq, PartialEq))]
@@ -61,6 +63,8 @@ pub struct Options {
 
     #[serde(rename = "policy_out")]
     policy_out: Option<Verdict>,
+
+    policy_forward: Option<Verdict>,
 }
 
 #[derive(Debug)]
@@ -84,6 +88,7 @@ impl Config {
         let parser_cfg = super::common::ParserConfig {
             guest_iface_names: true,
             ipset_scope: Some(IpsetScope::Guest),
+            allowed_directions: vec![Direction::In, Direction::Out],
         };
 
         let config = super::common::Config::parse(firewall_input, &parser_cfg)?;
@@ -131,6 +136,7 @@ impl Config {
         match dir {
             Direction::In => self.config.options.log_level_in.unwrap_or_default(),
             Direction::Out => self.config.options.log_level_out.unwrap_or_default(),
+            _ => LogLevel::Nolog,
         }
     }
 
@@ -179,6 +185,11 @@ impl Config {
                 .options
                 .policy_out
                 .unwrap_or(GUEST_POLICY_OUT_DEFAULT),
+            Direction::Forward => self
+                .config
+                .options
+                .policy_forward
+                .unwrap_or(GUEST_POLICY_FORWARD_DEFAULT),
         }
     }
 
@@ -211,6 +222,7 @@ ndp:1
 radv:1
 policy_in: REJECT
 policy_out: REJECT
+policy_forward: DROP
 "#;
 
         let config = CONFIG.as_bytes();
@@ -231,6 +243,7 @@ policy_out: REJECT
                 macfilter: Some(false),
                 policy_in: Some(Verdict::Reject),
                 policy_out: Some(Verdict::Reject),
+                policy_forward: Some(Verdict::Drop),
             }
         );
     }
diff --git a/proxmox-ve-config/src/firewall/host.rs b/proxmox-ve-config/src/firewall/host.rs
index 3de6fad..394896c 100644
--- a/proxmox-ve-config/src/firewall/host.rs
+++ b/proxmox-ve-config/src/firewall/host.rs
@@ -44,6 +44,7 @@ pub struct Options {
 
     log_level_in: Option<LogLevel>,
     log_level_out: Option<LogLevel>,
+    log_level_forward: Option<LogLevel>,
 
     #[serde(default, with = "parse::serde_option_bool")]
     log_nf_conntrack: Option<bool>,
@@ -94,7 +95,13 @@ impl Config {
     }
 
     pub fn parse<R: io::BufRead>(input: R) -> Result<Self, Error> {
-        let config = super::common::Config::parse(input, &Default::default())?;
+        let parser_cfg = super::common::ParserConfig {
+            guest_iface_names: false,
+            ipset_scope: None,
+            allowed_directions: vec![Direction::In, Direction::Out, Direction::Forward],
+        };
+
+        let config = super::common::Config::parse(input, &parser_cfg)?;
 
         if !config.groups.is_empty() {
             bail!("host firewall config cannot declare groups");
@@ -262,6 +269,7 @@ impl Config {
         match dir {
             Direction::In => self.config.options.log_level_in.unwrap_or_default(),
             Direction::Out => self.config.options.log_level_out.unwrap_or_default(),
+            Direction::Forward => self.config.options.log_level_forward.unwrap_or_default(),
         }
     }
 }
@@ -284,6 +292,7 @@ enable: 1
 nftables: 1
 log_level_in: debug
 log_level_out: emerg
+log_level_forward: warn
 log_nf_conntrack: 0
 ndp: 1
 nf_conntrack_allow_invalid: yes
@@ -316,6 +325,7 @@ IN ACCEPT -p udp -dport 33 -sport 22 -log warning
                 nftables: Some(true),
                 log_level_in: Some(LogLevel::Debug),
                 log_level_out: Some(LogLevel::Emergency),
+                log_level_forward: Some(LogLevel::Warning),
                 log_nf_conntrack: Some(false),
                 ndp: Some(true),
                 nf_conntrack_allow_invalid: Some(true),
diff --git a/proxmox-ve-config/src/firewall/mod.rs b/proxmox-ve-config/src/firewall/mod.rs
index 2cf57e2..6ee3c31 100644
--- a/proxmox-ve-config/src/firewall/mod.rs
+++ b/proxmox-ve-config/src/firewall/mod.rs
@@ -1,3 +1,4 @@
+pub mod bridge;
 pub mod cluster;
 pub mod common;
 pub mod ct_helper;
diff --git a/proxmox-ve-config/src/firewall/types/rule.rs b/proxmox-ve-config/src/firewall/types/rule.rs
index 5374bb0..2c8f49c 100644
--- a/proxmox-ve-config/src/firewall/types/rule.rs
+++ b/proxmox-ve-config/src/firewall/types/rule.rs
@@ -13,19 +13,24 @@ pub enum Direction {
     #[default]
     In,
     Out,
+    Forward,
 }
 
 impl std::str::FromStr for Direction {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Error> {
-        for (name, dir) in [("IN", Direction::In), ("OUT", Direction::Out)] {
+        for (name, dir) in [
+            ("IN", Direction::In),
+            ("OUT", Direction::Out),
+            ("FORWARD", Direction::Forward),
+        ] {
             if s.eq_ignore_ascii_case(name) {
                 return Ok(dir);
             }
         }
 
-        bail!("invalid direction: {s:?}, expect 'IN' or 'OUT'");
+        bail!("invalid direction: {s:?}, expect 'IN', 'OUT' or 'FORWARD'");
     }
 }
 
@@ -36,6 +41,7 @@ impl fmt::Display for Direction {
         match self {
             Direction::In => f.write_str("in"),
             Direction::Out => f.write_str("out"),
+            Direction::Forward => f.write_str("forward"),
         }
     }
 }
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel