[pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules

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

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 .../resources/proxmox-firewall.nft            |  54 ++++++++
 proxmox-firewall/src/firewall.rs              | 122 +++++++++++++++++-
 proxmox-firewall/src/rule.rs                  |   5 +-
 .../integration_tests__firewall.snap          |  86 ++++++++++++
 proxmox-nftables/src/expression.rs            |   8 ++
 proxmox-nftables/src/types.rs                 |   6 +
 6 files changed, 275 insertions(+), 6 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index f42255c..af9454d 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -20,8 +20,12 @@ add chain inet proxmox-firewall allow-icmp
 add chain inet proxmox-firewall log-drop-smurfs
 add chain inet proxmox-firewall default-in
 add chain inet proxmox-firewall default-out
+add chain inet proxmox-firewall before-bridge
+add chain inet proxmox-firewall host-bridge-input {type filter hook input priority filter - 1; policy accept;}
+add chain inet proxmox-firewall host-bridge-output {type filter hook output priority filter + 1; policy accept;}
 add chain inet proxmox-firewall input {type filter hook input priority filter; policy drop;}
 add chain inet proxmox-firewall output {type filter hook output priority filter; policy accept;}
+add chain inet proxmox-firewall forward {type filter hook forward priority filter; policy accept;}
 
 add chain bridge proxmox-firewall-guests allow-dhcp-in
 add chain bridge proxmox-firewall-guests allow-dhcp-out
@@ -39,6 +43,8 @@ add chain bridge proxmox-firewall-guests pre-vm-out
 add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting priority 0; policy accept;}
 add chain bridge proxmox-firewall-guests pre-vm-in
 add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting priority 0; policy accept;}
+add chain bridge proxmox-firewall-guests before-bridge
+add chain bridge proxmox-firewall-guests forward {type filter hook forward priority 0; policy accept;}
 
 flush chain inet proxmox-firewall do-reject
 flush chain inet proxmox-firewall accept-management
@@ -55,8 +61,12 @@ flush chain inet proxmox-firewall allow-icmp
 flush chain inet proxmox-firewall log-drop-smurfs
 flush chain inet proxmox-firewall default-in
 flush chain inet proxmox-firewall default-out
+flush chain inet proxmox-firewall before-bridge
+flush chain inet proxmox-firewall host-bridge-input
+flush chain inet proxmox-firewall host-bridge-output
 flush chain inet proxmox-firewall input
 flush chain inet proxmox-firewall output
+flush chain inet proxmox-firewall forward
 
 flush chain bridge proxmox-firewall-guests allow-dhcp-in
 flush chain bridge proxmox-firewall-guests allow-dhcp-out
@@ -74,6 +84,8 @@ flush chain bridge proxmox-firewall-guests pre-vm-out
 flush chain bridge proxmox-firewall-guests vm-out
 flush chain bridge proxmox-firewall-guests pre-vm-in
 flush chain bridge proxmox-firewall-guests vm-in
+flush chain bridge proxmox-firewall-guests before-bridge
+flush chain bridge proxmox-firewall-guests forward
 
 table inet proxmox-firewall {
     chain do-reject {
@@ -223,6 +235,25 @@ table inet proxmox-firewall {
     chain option-in {}
     chain option-out {}
 
+    map bridge-map {
+        type ifname : verdict
+    }
+
+    chain before-bridge {
+        meta protocol arp accept
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+    }
+
+    chain host-bridge-input {
+        type filter hook input priority filter - 1; policy accept;
+        meta iifname vmap @bridge-map
+    }
+
+    chain host-bridge-output {
+        type filter hook output priority filter + 1; policy accept;
+        meta oifname vmap @bridge-map
+    }
+
     chain input {
         type filter hook input priority filter; policy accept;
         jump default-in
@@ -240,12 +271,21 @@ table inet proxmox-firewall {
         jump cluster-out
     }
 
+    chain forward {
+        type filter hook forward priority filter; policy accept;
+        jump host-forward
+        jump cluster-forward
+    }
+
     chain cluster-in {}
     chain cluster-out {}
 
     chain host-in {}
     chain host-out {}
 
+    chain cluster-forward {}
+    chain host-forward {}
+
     chain ct-in {}
 }
 
@@ -335,4 +375,18 @@ table bridge proxmox-firewall-guests {
         jump allow-icmp
         oifname vmap @vm-map-in
     }
+
+    map bridge-map {
+        type ifname . ifname : verdict
+    }
+
+    chain before-bridge {
+        meta protocol arp accept
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+    }
+
+    chain forward {
+        type filter hook forward priority 0; policy accept;
+        meta ibrname . meta obrname vmap @bridge-map
+    }
 }
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 347f3af..bb54023 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 use std::fs;
 
-use anyhow::Error;
+use anyhow::{bail, Error};
 
 use proxmox_nftables::command::{Add, Commands, Delete, Flush};
 use proxmox_nftables::expression::{Meta, Payload};
@@ -13,6 +13,9 @@ use proxmox_nftables::types::{
 };
 use proxmox_nftables::{Expression, Statement};
 
+use proxmox_ve_config::host::types::BridgeName;
+
+use proxmox_ve_config::firewall::bridge::Config as BridgeConfig;
 use proxmox_ve_config::firewall::ct_helper::get_cthelper;
 use proxmox_ve_config::firewall::guest::Config as GuestConfig;
 use proxmox_ve_config::firewall::host::Config as HostConfig;
@@ -112,6 +115,14 @@ impl Firewall {
         ChainPart::new(Self::host_table(), "log-smurfs")
     }
 
+    fn bridge_vmap(table: TablePart) -> SetName {
+        SetName::new(table, "bridge-map")
+    }
+
+    fn bridge_chain(table: TablePart, bridge_name: &BridgeName) -> ChainPart {
+        ChainPart::new(table, format!("bridge-{bridge_name}"))
+    }
+
     fn default_log_limit(&self) -> Option<LogRateLimit> {
         self.config.cluster().log_ratelimit()
     }
@@ -120,14 +131,18 @@ impl Firewall {
         commands.append(&mut vec![
             Flush::chain(Self::cluster_chain(Direction::In)),
             Flush::chain(Self::cluster_chain(Direction::Out)),
+            Flush::chain(Self::cluster_chain(Direction::Forward)),
             Add::chain(Self::host_chain(Direction::In)),
             Flush::chain(Self::host_chain(Direction::In)),
             Flush::chain(Self::host_option_chain(Direction::In)),
             Add::chain(Self::host_chain(Direction::Out)),
             Flush::chain(Self::host_chain(Direction::Out)),
             Flush::chain(Self::host_option_chain(Direction::Out)),
+            Flush::chain(Self::host_chain(Direction::Forward)),
             Flush::map(Self::guest_vmap(Direction::In)),
             Flush::map(Self::guest_vmap(Direction::Out)),
+            Flush::map(Self::bridge_vmap(Self::guest_table())),
+            Flush::map(Self::bridge_vmap(Self::host_table())),
             Flush::chain(Self::host_conntrack_chain()),
             Flush::chain(Self::synflood_limit_chain()),
             Flush::chain(Self::log_invalid_tcp_chain()),
@@ -144,8 +159,8 @@ impl Firewall {
         }
         */
 
-        // we need to remove guest chains before group chains
-        for prefix in ["guest-", "group-"] {
+        // we need to remove guest & bridge chains before group chains
+        for prefix in ["guest-", "bridge-", "group-"] {
             for (name, chain) in self.config.nft_chains() {
                 if name.starts_with(prefix) {
                     commands.push(Delete::chain(chain.clone()))
@@ -246,10 +261,18 @@ impl Firewall {
                     name,
                     Direction::Out,
                 )?;
+                self.create_group_chain(
+                    &mut commands,
+                    &cluster_host_table,
+                    group,
+                    name,
+                    Direction::Forward,
+                )?;
             }
 
             self.create_cluster_rules(&mut commands, Direction::In)?;
             self.create_cluster_rules(&mut commands, Direction::Out)?;
+            self.create_cluster_rules(&mut commands, Direction::Forward)?;
 
             log::debug!("Generating host firewall config");
 
@@ -259,6 +282,7 @@ impl Firewall {
 
             self.create_host_rules(&mut commands, Direction::In)?;
             self.create_host_rules(&mut commands, Direction::Out)?;
+            self.create_host_rules(&mut commands, Direction::Forward)?;
         } else {
             commands.push(Delete::table(TableName::from(Self::cluster_table())));
         }
@@ -270,7 +294,14 @@ impl Firewall {
             .filter(|(_, config)| config.is_enabled())
             .collect();
 
-        if !enabled_guests.is_empty() {
+        let enabled_bridges: BTreeMap<&BridgeName, &BridgeConfig> = self
+            .config
+            .bridges()
+            .iter()
+            .filter(|(_, config)| config.enabled())
+            .collect();
+
+        if !(enabled_guests.is_empty() && enabled_bridges.is_empty()) {
             log::info!("creating guest configuration");
 
             self.create_ipsets(
@@ -283,6 +314,13 @@ impl Firewall {
             for (name, group) in self.config.cluster().groups() {
                 self.create_group_chain(&mut commands, &guest_table, group, name, Direction::In)?;
                 self.create_group_chain(&mut commands, &guest_table, group, name, Direction::Out)?;
+                self.create_group_chain(
+                    &mut commands,
+                    &guest_table,
+                    group,
+                    name,
+                    Direction::Forward,
+                )?;
             }
         } else {
             commands.push(Delete::table(TableName::from(Self::guest_table())));
@@ -302,9 +340,84 @@ impl Firewall {
             self.create_guest_rules(&mut commands, *vmid, config, Direction::Out)?;
         }
 
+        for (bridge_name, bridge_config) in enabled_bridges {
+            self.create_bridge_chain(&mut commands, bridge_name, bridge_config)?;
+        }
+
         Ok(commands)
     }
 
+    fn create_bridge_chain(
+        &self,
+        commands: &mut Commands,
+        name: &BridgeName,
+        config: &BridgeConfig,
+    ) -> Result<(), Error> {
+        for table in [Self::host_table(), Self::guest_table()] {
+            log::info!("creating bridge chain {name} in table {}", table.table());
+
+            let chain = Self::bridge_chain(table.clone(), name);
+
+            commands.append(&mut vec![
+                Add::chain(chain.clone()),
+                Flush::chain(chain.clone()),
+                Add::rule(AddRule::from_statement(
+                    chain.clone(),
+                    Statement::jump("before-bridge"),
+                )),
+            ]);
+
+            let env = NftRuleEnv {
+                chain: chain.clone(),
+                direction: Direction::Forward,
+                firewall_config: &self.config,
+                vmid: None,
+            };
+
+            for config_rule in config.rules() {
+                for rule in NftRule::from_config_rule(config_rule, &env)? {
+                    commands.push(Add::rule(rule.into_add_rule(chain.clone())));
+                }
+            }
+
+            let default_policy = config.policy_forward();
+
+            self.create_log_rule(
+                commands,
+                config.log_level_forward(),
+                chain.clone(),
+                default_policy,
+                None,
+            )?;
+
+            commands.push(Add::rule(AddRule::from_statement(
+                chain.clone(),
+                default_policy,
+            )));
+
+            let key = if table == Self::host_table() {
+                name.into()
+            } else {
+                Expression::concat([name.into(), name.into()])
+            };
+
+            let map_element = AddElement::map_from_expressions(
+                Self::bridge_vmap(table),
+                [(
+                    key,
+                    MapValue::from(Verdict::Jump {
+                        target: chain.name().to_string(),
+                    }),
+                )]
+                .to_vec(),
+            );
+
+            commands.push(Add::element(map_element));
+        }
+
+        Ok(())
+    }
+
     fn handle_host_options(&self, commands: &mut Commands) -> Result<(), Error> {
         log::info!("setting host options");
 
@@ -781,6 +894,7 @@ impl Firewall {
         let pre_chain = match direction {
             Direction::In => "pre-vm-in",
             Direction::Out => "pre-vm-out",
+            Direction::Forward => bail!("cannot create guest_chain in direction forward"),
         };
 
         commands.append(&mut vec![
diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index 02f964e..3b947f0 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -1,6 +1,6 @@
 use std::ops::{Deref, DerefMut};
 
-use anyhow::{format_err, Error};
+use anyhow::{bail, format_err, Error};
 use proxmox_nftables::{
     expression::{Ct, IpFamily, Meta, Payload, Prefix},
     statement::{Log, LogLevel, Match, Operator},
@@ -179,6 +179,7 @@ fn handle_iface(rules: &mut [NftRule], env: &NftRuleEnv, name: &str) -> Result<(
         (Some(_), Direction::Out) => "iifname",
         (None, Direction::In) => "iifname",
         (None, Direction::Out) => "oifname",
+        (_, Direction::Forward) => bail!("cannot define interfaces for forward direction"),
     };
 
     let iface_name = env.iface_name(name);
@@ -693,8 +694,8 @@ impl ToNftRules for Ipfilter<'_> {
                     rules.push(base_rule);
                 }
             }
+            Direction::Forward => bail!("cannot generate IP filter for direction forward"),
         }
-
         Ok(())
     }
 }
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index e1b599c..06b6beb 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -22,6 +22,15 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "flush": {
+        "chain": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "cluster-forward"
+        }
+      }
+    },
     {
       "add": {
         "chain": {
@@ -76,6 +85,15 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "flush": {
+        "chain": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "host-forward"
+        }
+      }
+    },
     {
       "flush": {
         "map": {
@@ -94,6 +112,24 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "flush": {
+        "map": {
+          "family": "bridge",
+          "table": "proxmox-firewall-guests",
+          "name": "bridge-map"
+        }
+      }
+    },
+    {
+      "flush": {
+        "map": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "bridge-map"
+        }
+      }
+    },
     {
       "flush": {
         "chain": {
@@ -1784,6 +1820,24 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "add": {
+        "chain": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "group-network1-forward"
+        }
+      }
+    },
+    {
+      "flush": {
+        "chain": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "group-network1-forward"
+        }
+      }
+    },
     {
       "add": {
         "rule": {
@@ -1874,6 +1928,20 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "cluster-forward",
+          "expr": [
+            {
+              "accept": null
+            }
+          ]
+        }
+      }
+    },
     {
       "add": {
         "ct helper": {
@@ -3813,6 +3881,24 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "add": {
+        "chain": {
+          "family": "bridge",
+          "table": "proxmox-firewall-guests",
+          "name": "group-network1-forward"
+        }
+      }
+    },
+    {
+      "flush": {
+        "chain": {
+          "family": "bridge",
+          "table": "proxmox-firewall-guests",
+          "name": "group-network1-forward"
+        }
+      }
+    },
     {
       "add": {
         "chain": {
diff --git a/proxmox-nftables/src/expression.rs b/proxmox-nftables/src/expression.rs
index 71a90eb..aefb4c4 100644
--- a/proxmox-nftables/src/expression.rs
+++ b/proxmox-nftables/src/expression.rs
@@ -1,5 +1,6 @@
 use crate::types::{ElemConfig, Verdict};
 use proxmox_ve_config::firewall::types::address::IpRange;
+use proxmox_ve_config::host::types::BridgeName;
 use serde::{Deserialize, Serialize};
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
@@ -259,6 +260,13 @@ impl From<&PortList> for Expression {
     }
 }
 
+#[cfg(feature = "config-ext")]
+impl From<&BridgeName> for Expression {
+    fn from(value: &BridgeName) -> Self {
+        Expression::String(value.name().to_string())
+    }
+}
+
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct Meta {
     key: String,
diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs
index d8f3b62..320c757 100644
--- a/proxmox-nftables/src/types.rs
+++ b/proxmox-nftables/src/types.rs
@@ -742,6 +742,12 @@ impl AddElement {
     }
 }
 
+impl From<AddMapElement> for AddElement {
+    fn from(value: AddMapElement) -> Self {
+        AddElement::Map(value)
+    }
+}
+
 impl From<AddSetElement> for AddElement {
     fn from(value: AddSetElement) -> Self {
         AddElement::Set(value)
-- 
2.39.5


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