[pve-devel] [PATCH proxmox-firewall 1/1] vnet firewall: create chains in host table only if host fw is enabled

* [pve-devel] [PATCH proxmox-firewall 1/1] vnet firewall: create chains in host table only if host fw is enabled
@ 2025-09-05 12:13 Stefan Hanreich
  2025-10-04 12:58 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Stefan Hanreich @ 2025-09-05 12:13 UTC (permalink / raw)
  To: pve-devel

If the host firewall is not enabled, but the vnet firewall is enabled
for at least one vnet, then the firewall tries to create the chains
required for the vnet firewall in the cluster / host table, which is
unnecessary. This leads to an error in the generated nftables ruleset,
causing the firewall to not get applied.

In order to fix this, skip generating the bridge chains in the inet
table when the cluster/host firewall is disabled, since they're only
required for managing the traffic flowing from host <-> bridge ports.
If the host firewall is disabled, then we do not need to create rules
for traffic from host <-> bridge port in the first place.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/firewall.rs | 110 +++++++++++++++++--------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 8cac190..02f31d4 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -355,7 +355,16 @@ impl Firewall {
         }
 
         for (bridge_name, bridge_config) in enabled_bridges {
-            self.create_bridge_chain(&mut commands, bridge_name, bridge_config)?;
+            self.create_bridge_chain(&mut commands, &guest_table, bridge_name, bridge_config)?;
+
+            if self.config.host().is_enabled() {
+                self.create_bridge_chain(
+                    &mut commands,
+                    &cluster_host_table,
+                    bridge_name,
+                    bridge_config,
+                )?;
+            }
         }
 
         Ok(commands)
@@ -364,70 +373,69 @@ impl Firewall {
     fn create_bridge_chain(
         &self,
         commands: &mut Commands,
+        table: &TablePart,
         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());
+        log::info!("creating bridge chain {name} in table {}", table.table());
 
-            let chain = Self::bridge_chain(table.clone(), name);
+        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"),
-                )),
-            ]);
+        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,
-            };
+        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())));
-                }
+        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();
+        let default_policy = config.policy_forward();
 
-            self.create_log_rule(
-                commands,
-                config.log_level_forward(),
-                chain.clone(),
-                default_policy,
-                None,
-            )?;
+        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,
-            )));
+        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 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(),
-            );
+        let map_element = AddElement::map_from_expressions(
+            Self::bridge_vmap(table.clone()),
+            [(
+                key,
+                MapValue::from(Verdict::Jump {
+                    target: chain.name().to_string(),
+                }),
+            )]
+            .to_vec(),
+        );
 
-            commands.push(Add::element(map_element));
-        }
+        commands.push(Add::element(map_element));
 
         Ok(())
     }
-- 
2.47.2


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


^ permalink raw reply	[flat|nested] 2+ messages in thread