firewall: add forward direction

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>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>

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),
             }
         );
 

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(())
     }

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),
             }
         );
     }

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),

proxmox-ve-config/src/firewall/mod.rs

@@ -1,3 +1,4 @@
+pub mod bridge;
 pub mod cluster;
 pub mod common;
 pub mod ct_helper;

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"),
         }
     }
 }