public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets
@ 2024-10-10 15:56 Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction Stefan Hanreich
                   ` (16 more replies)
  0 siblings, 17 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

## Introduction

This patch series introduces a new direction for firewall rules: forward.
Additionally this patch series introduces defining firewall rules on a vnet
level.

## Use Cases

For hosts:
* hosts utilizing NAT can define firewall rules for NATed traffic
* hosts utilizing EVPN zones can define rules for exit node traffic
* hosts acting as gateway can firewall the traffic that passes through them

For vnets:
* can create firewall rules globally without having to attach/update security
  groups to every newly created VM

This patch series is particularly useful when combined with my other current RFC
'autogenerate ipsets for sdn objects'. It enables users to quickly define rules
like:

on the host level:
* only SNAT HTTP traffic from hosts in this vnet to a specific host
* restricting traffic routed from hosts in one vnet to another vnet

on the vnet level:
* only allow DHCP/DNS traffic inside a bridge to the gateway

Not only does this streamline creating firewall rules, it also enables users to
create firewall rules that haven't been possible before and needed to rely on
external firewall appliances.

Since forwarded traffic goes *both* ways, you generally have to create two rules
in case of bi-directional traffic. It might make sense to simplify this in the
future by adding an additional option to the firewall config scheme that
specifies that rules in the other direction should also get automatically
generated.

## Usage

For creating forward rules on the cluster/host level, you simply create a new
rule with the new 'forward' direction. It uses the existing configuration files.

For creating them on a vnet level, there are new firewall configuration files
located under '/etc/pve/sdn/firewall/<vnet>.fw'. It utilizes the same
configuration format as the existing firewall configuration files. You can only
define rules with direction 'forward' on a vnet-level.


Changes from RFC to v2:
* Fixed several bugs
    * SDN Firewall folder does not automatically created (thanks @Gabriel)
    * Firewall flushes the bridge table if no guest firewall is active, even
      though VNet-level rules exist
* VNet-level firewall now matches on both input and output interface
* Introduced log option for VNet firewall
* Improved style of perl code (thanks @Thomas)
* promox-firewall now verifies the directions of rules
    * added some additional tests to verify this behavior
* added documentation

proxmox-ve-rs:

Stefan Hanreich (4):
  firewall: add forward direction
  firewall: add bridge firewall config parser
  config: firewall: add tests for interface and directions
  host: add struct representing bridge names

 proxmox-ve-config/Cargo.toml                 |  1 +
 proxmox-ve-config/src/firewall/bridge.rs     | 64 +++++++++++++++++++
 proxmox-ve-config/src/firewall/cluster.rs    | 11 ++++
 proxmox-ve-config/src/firewall/common.rs     | 11 ++++
 proxmox-ve-config/src/firewall/guest.rs      | 66 ++++++++++++++++++++
 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 ++-
 proxmox-ve-config/src/host/mod.rs            |  1 +
 proxmox-ve-config/src/host/types.rs          | 46 ++++++++++++++
 10 files changed, 220 insertions(+), 3 deletions(-)
 create mode 100644 proxmox-ve-config/src/firewall/bridge.rs
 create mode 100644 proxmox-ve-config/src/host/types.rs


proxmox-firewall:

Stefan Hanreich (5):
  nftables: derive additional traits for nftables types
  sdn: add support for loading vnet-level firewall config
  sdn: create forward firewall rules
  use std::mem::take over drain()
  cargo: make proxmox-ve-config a workspace dependency

 Cargo.toml                                    |   3 +
 proxmox-firewall/Cargo.toml                   |   2 +-
 .../resources/proxmox-firewall.nft            |  54 ++++++++
 proxmox-firewall/src/config.rs                |  88 ++++++++++++-
 proxmox-firewall/src/firewall.rs              | 122 +++++++++++++++++-
 proxmox-firewall/src/rule.rs                  |   7 +-
 proxmox-firewall/tests/integration_tests.rs   |  12 ++
 .../integration_tests__firewall.snap          |  86 ++++++++++++
 proxmox-nftables/Cargo.toml                   |   2 +-
 proxmox-nftables/src/expression.rs            |   8 ++
 proxmox-nftables/src/types.rs                 |  14 +-
 11 files changed, 383 insertions(+), 15 deletions(-)


pve-firewall:

Stefan Hanreich (2):
  sdn: add vnet firewall configuration
  api: add vnet endpoints

 src/PVE/API2/Firewall/Makefile |   1 +
 src/PVE/API2/Firewall/Rules.pm |  84 +++++++++++++++++
 src/PVE/API2/Firewall/Vnet.pm  | 168 +++++++++++++++++++++++++++++++++
 src/PVE/Firewall.pm            | 132 ++++++++++++++++++++++++--
 src/PVE/Firewall/Helpers.pm    |  12 +++
 5 files changed, 391 insertions(+), 6 deletions(-)
 create mode 100644 src/PVE/API2/Firewall/Vnet.pm


pve-manager:

Stefan Hanreich (4):
  firewall: add forward direction to rule panel
  firewall: add vnet to firewall options component
  firewall: make base_url dynamically configurable in options component
  sdn: add firewall panel

 www/manager6/Makefile                |  2 +
 www/manager6/dc/Config.js            |  9 ++++
 www/manager6/dc/SecurityGroups.js    |  1 +
 www/manager6/grid/FirewallOptions.js | 74 +++++++++++++++++++++-----
 www/manager6/grid/FirewallRules.js   | 32 ++++++++++--
 www/manager6/lxc/Config.js           |  1 +
 www/manager6/node/Config.js          |  1 +
 www/manager6/qemu/Config.js          |  1 +
 www/manager6/sdn/FirewallPanel.js    | 48 +++++++++++++++++
 www/manager6/sdn/FirewallVnetView.js | 77 ++++++++++++++++++++++++++++
 10 files changed, 228 insertions(+), 18 deletions(-)
 create mode 100644 www/manager6/sdn/FirewallPanel.js
 create mode 100644 www/manager6/sdn/FirewallVnetView.js


pve-network:

Stefan Hanreich (1):
  firewall: add endpoints for vnet-level firewall

 src/PVE/API2/Network/SDN/Vnets.pm | 6 ++++++
 1 file changed, 6 insertions(+)


pve-docs:

Stefan Hanreich (1):
  firewall: add documentation for forward direction

 Makefile                      |  1 +
 gen-pve-firewall-vnet-opts.pl | 12 ++++++++
 pve-firewall-vnet-opts.adoc   |  8 ++++++
 pve-firewall.adoc             | 53 ++++++++++++++++++++++++++++++++---
 4 files changed, 70 insertions(+), 4 deletions(-)
 create mode 100755 gen-pve-firewall-vnet-opts.pl
 create mode 100644 pve-firewall-vnet-opts.adoc


Summary over all repositories:
  41 files changed, 1298 insertions(+), 46 deletions(-)

-- 
Generated by git-murpp 0.6.0

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


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

* [pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 02/17] firewall: add bridge firewall config parser Stefan Hanreich
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

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


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

* [pve-devel] [PATCH proxmox-ve-rs v2 02/17] firewall: add bridge firewall config parser
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 03/17] config: firewall: add tests for interface and directions Stefan Hanreich
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

We introduce a new type of firewall config file that can be used for
defining rules on bridge-level, similar to the existing
cluster/host/vm configuration files.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/src/firewall/bridge.rs | 64 ++++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 proxmox-ve-config/src/firewall/bridge.rs

diff --git a/proxmox-ve-config/src/firewall/bridge.rs b/proxmox-ve-config/src/firewall/bridge.rs
new file mode 100644
index 0000000..4acb6fa
--- /dev/null
+++ b/proxmox-ve-config/src/firewall/bridge.rs
@@ -0,0 +1,64 @@
+use std::io;
+
+use anyhow::Error;
+use serde::Deserialize;
+
+use crate::firewall::parse::serde_option_bool;
+use crate::firewall::types::log::LogLevel;
+use crate::firewall::types::rule::{Direction, Verdict};
+
+use super::common::ParserConfig;
+use super::types::Rule;
+
+pub struct Config {
+    pub(crate) config: super::common::Config<Options>,
+}
+
+/// default return value for [`Config::enabled()`]
+pub const BRIDGE_ENABLED_DEFAULT: bool = false;
+/// default return value for [`Config::policy_forward()`]
+pub const BRIDGE_POLICY_FORWARD: 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: None,
+            allowed_directions: vec![Direction::Forward],
+        };
+
+        Ok(Self {
+            config: super::common::Config::parse(input, &parser_config)?,
+        })
+    }
+
+    pub fn enabled(&self) -> bool {
+        self.config.options.enable.unwrap_or(BRIDGE_ENABLED_DEFAULT)
+    }
+
+    pub fn rules(&self) -> impl Iterator<Item = &Rule> + '_ {
+        self.config.rules.iter()
+    }
+
+    pub fn log_level_forward(&self) -> LogLevel {
+        self.config.options.log_level_forward.unwrap_or_default()
+    }
+
+    pub fn policy_forward(&self) -> Verdict {
+        self.config
+            .options
+            .policy_forward
+            .unwrap_or(BRIDGE_POLICY_FORWARD)
+    }
+}
+
+#[derive(Debug, Default, Deserialize)]
+#[cfg_attr(test, derive(Eq, PartialEq))]
+pub struct Options {
+    #[serde(default, with = "serde_option_bool")]
+    enable: Option<bool>,
+
+    policy_forward: Option<Verdict>,
+
+    log_level_forward: Option<LogLevel>,
+}
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-ve-rs v2 03/17] config: firewall: add tests for interface and directions
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 02/17] firewall: add bridge firewall config parser Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 04/17] host: add struct representing bridge names Stefan Hanreich
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Add tests for validating the directions in the guest firewall
configuration. While I'm at it, I also added tests for validating
interface names, since this functionality did not get tested before.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/src/firewall/guest.rs | 53 +++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs
index 1e70a67..23eaa4e 100644
--- a/proxmox-ve-config/src/firewall/guest.rs
+++ b/proxmox-ve-config/src/firewall/guest.rs
@@ -247,4 +247,57 @@ policy_forward: DROP
             }
         );
     }
+
+    #[test]
+    fn test_parse_valid_interface_prefix() {
+        const CONFIG: &str = r#"
+[RULES]
+
+IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i tapeth0
+"#;
+
+        let config = CONFIG.as_bytes();
+        let network_config: Vec<u8> = Vec::new();
+        Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err();
+    }
+
+    #[test]
+    fn test_parse_invalid_interface_prefix() {
+        const CONFIG: &str = r#"
+[RULES]
+
+IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i eth0
+"#;
+
+        let config = CONFIG.as_bytes();
+        let network_config: Vec<u8> = Vec::new();
+        Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err();
+    }
+
+    #[test]
+    fn test_parse_valid_directions() {
+        const CONFIG: &str = r#"
+[RULES]
+
+IN ACCEPT -p udp -dport 33 -sport 22 -log warning
+OUT ACCEPT -p udp -dport 33 -sport 22 -log warning
+"#;
+
+        let config = CONFIG.as_bytes();
+        let network_config: Vec<u8> = Vec::new();
+        Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap();
+    }
+
+    #[test]
+    fn test_parse_invalid_direction() {
+        const CONFIG: &str = r#"
+[RULES]
+
+FORWARD ACCEPT -p udp -dport 33 -sport 22 -log warning
+"#;
+
+        let config = CONFIG.as_bytes();
+        let network_config: Vec<u8> = Vec::new();
+        Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err();
+    }
 }
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-ve-rs v2 04/17] host: add struct representing bridge names
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (2 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 03/17] config: firewall: add tests for interface and directions Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 05/17] nftables: derive additional traits for nftables types Stefan Hanreich
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/Cargo.toml        |  1 +
 proxmox-ve-config/src/host/mod.rs   |  1 +
 proxmox-ve-config/src/host/types.rs | 46 +++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 proxmox-ve-config/src/host/types.rs

diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml
index 79ba164..d07dff2 100644
--- a/proxmox-ve-config/Cargo.toml
+++ b/proxmox-ve-config/Cargo.toml
@@ -10,6 +10,7 @@ exclude.workspace = true
 log = "0.4"
 anyhow = "1"
 nix = "0.26"
+thiserror = "1.0.40"
 
 serde = { version = "1", features = [ "derive" ] }
 serde_json = "1"
diff --git a/proxmox-ve-config/src/host/mod.rs b/proxmox-ve-config/src/host/mod.rs
index b5614dd..b4ab6a6 100644
--- a/proxmox-ve-config/src/host/mod.rs
+++ b/proxmox-ve-config/src/host/mod.rs
@@ -1 +1,2 @@
+pub mod types;
 pub mod utils;
diff --git a/proxmox-ve-config/src/host/types.rs b/proxmox-ve-config/src/host/types.rs
new file mode 100644
index 0000000..7cbee01
--- /dev/null
+++ b/proxmox-ve-config/src/host/types.rs
@@ -0,0 +1,46 @@
+use std::{fmt::Display, str::FromStr};
+
+use thiserror::Error;
+
+#[derive(Error, Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
+pub enum BridgeNameError {
+    #[error("name is too long")]
+    TooLong,
+}
+
+#[derive(Error, Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
+pub struct BridgeName(String);
+
+impl BridgeName {
+    pub fn new(name: String) -> Result<Self, BridgeNameError> {
+        if name.len() > 15 {
+            return Err(BridgeNameError::TooLong);
+        }
+
+        Ok(Self(name))
+    }
+
+    pub fn name(&self) -> &str {
+        &self.0
+    }
+}
+
+impl FromStr for BridgeName {
+    type Err = BridgeNameError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Self::new(s.to_owned())
+    }
+}
+
+impl Display for BridgeName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+
+impl AsRef<str> for BridgeName {
+    fn as_ref(&self) -> &str {
+        &self.0
+    }
+}
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall v2 05/17] nftables: derive additional traits for nftables types
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (3 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 04/17] host: add struct representing bridge names Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 06/17] sdn: add support for loading vnet-level firewall config Stefan Hanreich
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-nftables/src/types.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs
index 3101436..d8f3b62 100644
--- a/proxmox-nftables/src/types.rs
+++ b/proxmox-nftables/src/types.rs
@@ -16,10 +16,10 @@ use proxmox_ve_config::firewall::types::ipset::IpsetName;
 #[cfg(feature = "config-ext")]
 use proxmox_ve_config::guest::types::Vmid;
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)]
 pub struct Handle(i32);
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)]
 #[serde(rename_all = "lowercase")]
 pub enum TableFamily {
     Ip,
@@ -187,7 +187,7 @@ impl From<LogRateLimitTimescale> for RateTimescale {
     }
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct TableName {
     family: TableFamily,
     name: String,
@@ -210,7 +210,7 @@ impl TableName {
     }
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct TablePart {
     family: TableFamily,
     table: String,
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall v2 06/17] sdn: add support for loading vnet-level firewall config
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (4 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 05/17] nftables: derive additional traits for nftables types Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules Stefan Hanreich
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/config.rs              | 88 ++++++++++++++++++++-
 proxmox-firewall/tests/integration_tests.rs | 12 +++
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index c27aac6..ac60e15 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -1,10 +1,11 @@
 use std::collections::BTreeMap;
 use std::default::Default;
-use std::fs::File;
+use std::fs::{self, DirEntry, File, ReadDir};
 use std::io::{self, BufReader};
 
-use anyhow::{format_err, Context, Error};
+use anyhow::{bail, format_err, Context, Error};
 
+use proxmox_ve_config::firewall::bridge::Config as BridgeConfig;
 use proxmox_ve_config::firewall::cluster::Config as ClusterConfig;
 use proxmox_ve_config::firewall::guest::Config as GuestConfig;
 use proxmox_ve_config::firewall::host::Config as HostConfig;
@@ -12,6 +13,7 @@ use proxmox_ve_config::firewall::types::alias::{Alias, AliasName, AliasScope};
 
 use proxmox_ve_config::guest::types::Vmid;
 use proxmox_ve_config::guest::{GuestEntry, GuestMap};
+use proxmox_ve_config::host::types::BridgeName;
 
 use proxmox_nftables::command::{CommandOutput, Commands, List, ListOutput};
 use proxmox_nftables::types::ListChain;
@@ -33,6 +35,11 @@ pub trait FirewallConfigLoader {
     fn guest_firewall_config(&self, vmid: &Vmid) -> Result<Option<Box<dyn io::BufRead>>, Error>;
     fn sdn_running_config(&self) -> Result<Option<Box<dyn io::BufRead>>, Error>;
     fn ipam(&self) -> Result<Option<Box<dyn io::BufRead>>, Error>;
+    fn bridge_list(&self) -> Result<Vec<BridgeName>, Error>;
+    fn bridge_firewall_config(
+        &self,
+        bridge_name: &BridgeName,
+    ) -> Result<Option<Box<dyn io::BufRead>>, Error>;
 }
 
 #[derive(Default)]
@@ -61,8 +68,31 @@ fn open_config_file(path: &str) -> Result<Option<File>, Error> {
     }
 }
 
+fn open_config_folder(path: &str) -> Result<Option<ReadDir>, Error> {
+    match fs::read_dir(path) {
+        Ok(paths) => Ok(Some(paths)),
+        Err(err) if err.kind() == io::ErrorKind::NotFound => {
+            log::info!("SDN config folder {path} does not exist");
+            Ok(None)
+        }
+        Err(err) => {
+            let context = format!("unable to open configuration folder at {BRIDGE_CONFIG_PATH}");
+            Err(anyhow::Error::new(err).context(context))
+        }
+    }
+}
+
+fn fw_name(dir_entry: DirEntry) -> Option<String> {
+    dir_entry
+        .file_name()
+        .to_str()?
+        .strip_suffix(".fw")
+        .map(str::to_string)
+}
+
 const CLUSTER_CONFIG_PATH: &str = "/etc/pve/firewall/cluster.fw";
 const HOST_CONFIG_PATH: &str = "/etc/pve/local/host.fw";
+const BRIDGE_CONFIG_PATH: &str = "/etc/pve/sdn/firewall";
 
 const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config";
 const SDN_IPAM_PATH: &str = "/etc/pve/priv/ipam.db";
@@ -154,6 +184,38 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
 
         Ok(None)
     }
+
+    fn bridge_list(&self) -> Result<Vec<BridgeName>, Error> {
+        let mut bridges = Vec::new();
+
+        if let Some(files) = open_config_folder(BRIDGE_CONFIG_PATH)? {
+            for file in files {
+                let bridge_name = fw_name(file?).map(BridgeName::new).transpose()?;
+
+                if let Some(bridge_name) = bridge_name {
+                    bridges.push(bridge_name);
+                }
+            }
+        }
+
+        Ok(bridges)
+    }
+
+    fn bridge_firewall_config(
+        &self,
+        bridge_name: &BridgeName,
+    ) -> Result<Option<Box<dyn io::BufRead>>, Error> {
+        log::info!("loading firewall config for bridge {bridge_name}");
+
+        let fd = open_config_file(&format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?;
+
+        if let Some(file) = fd {
+            let buf_reader = Box::new(BufReader::new(file)) as Box<dyn io::BufRead>;
+            return Ok(Some(buf_reader));
+        }
+
+        Ok(None)
+    }
 }
 
 pub trait NftConfigLoader {
@@ -184,6 +246,7 @@ pub struct FirewallConfig {
     cluster_config: ClusterConfig,
     host_config: HostConfig,
     guest_config: BTreeMap<Vmid, GuestConfig>,
+    bridge_config: BTreeMap<BridgeName, BridgeConfig>,
     nft_config: BTreeMap<String, ListChain>,
     sdn_config: Option<SdnConfig>,
     ipam_config: Option<Ipam>,
@@ -284,6 +347,22 @@ impl FirewallConfig {
         Ok(chains)
     }
 
+    pub fn parse_bridges(
+        firewall_loader: &dyn FirewallConfigLoader,
+    ) -> Result<BTreeMap<BridgeName, BridgeConfig>, Error> {
+        let mut bridge_config = BTreeMap::new();
+
+        for bridge_name in firewall_loader.bridge_list()? {
+            if let Some(config) = firewall_loader.bridge_firewall_config(&bridge_name)? {
+                bridge_config.insert(bridge_name, BridgeConfig::parse(config)?);
+            } else {
+                bail!("Could not read config for {bridge_name}")
+            }
+        }
+
+        Ok(bridge_config)
+    }
+
     pub fn new(
         firewall_loader: &dyn FirewallConfigLoader,
         nft_loader: &dyn NftConfigLoader,
@@ -292,6 +371,7 @@ impl FirewallConfig {
             cluster_config: Self::parse_cluster(firewall_loader)?,
             host_config: Self::parse_host(firewall_loader)?,
             guest_config: Self::parse_guests(firewall_loader)?,
+            bridge_config: Self::parse_bridges(firewall_loader)?,
             sdn_config: Self::parse_sdn(firewall_loader)?,
             ipam_config: Self::parse_ipam(firewall_loader)?,
             nft_config: Self::parse_nft(nft_loader)?,
@@ -310,6 +390,10 @@ impl FirewallConfig {
         &self.guest_config
     }
 
+    pub fn bridges(&self) -> &BTreeMap<BridgeName, BridgeConfig> {
+        &self.bridge_config
+    }
+
     pub fn nft_chains(&self) -> &BTreeMap<String, ListChain> {
         &self.nft_config
     }
diff --git a/proxmox-firewall/tests/integration_tests.rs b/proxmox-firewall/tests/integration_tests.rs
index 5de1a4e..61a8062 100644
--- a/proxmox-firewall/tests/integration_tests.rs
+++ b/proxmox-firewall/tests/integration_tests.rs
@@ -7,6 +7,7 @@ use proxmox_nftables::command::CommandOutput;
 use proxmox_sys::nodename;
 use proxmox_ve_config::guest::types::Vmid;
 use proxmox_ve_config::guest::{GuestEntry, GuestMap, GuestType};
+use proxmox_ve_config::host::types::BridgeName;
 
 struct MockFirewallConfigLoader {}
 
@@ -79,6 +80,17 @@ impl FirewallConfigLoader for MockFirewallConfigLoader {
     fn ipam(&self) -> Result<Option<Box<dyn std::io::BufRead>>, Error> {
         Ok(Some(Box::new(include_str!("input/ipam.db").as_bytes())))
     }
+
+    fn bridge_list(&self) -> Result<Vec<BridgeName>, Error> {
+        Ok(Vec::new())
+    }
+
+    fn bridge_firewall_config(
+        &self,
+        bridge_name: &BridgeName,
+    ) -> Result<Option<Box<dyn std::io::BufRead>>, Error> {
+        Ok(None)
+    }
 }
 
 struct MockNftConfigLoader {}
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (5 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 06/17] sdn: add support for loading vnet-level firewall config Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 08/17] use std::mem::take over drain() Stefan Hanreich
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

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


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

* [pve-devel] [PATCH proxmox-firewall v2 08/17] use std::mem::take over drain()
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (6 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 09/17] cargo: make proxmox-ve-config a workspace dependency Stefan Hanreich
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

This is more efficient than draining and collecting the Vec. It also
fixes the respective clippy lint.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/rule.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index 3b947f0..b20a9c5 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -764,7 +764,7 @@ impl ToNftRules for FwMacro {
     fn to_nft_rules(&self, rules: &mut Vec<NftRule>, env: &NftRuleEnv) -> Result<(), Error> {
         log::trace!("applying macro: {self:?}");
 
-        let initial_rules: Vec<NftRule> = rules.drain(..).collect();
+        let initial_rules: Vec<NftRule> = std::mem::take(rules);
 
         for protocol in &self.code {
             let mut new_rules = initial_rules.to_vec();
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall v2 09/17] cargo: make proxmox-ve-config a workspace dependency
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (7 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 08/17] use std::mem::take over drain() Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Since it is used by both libraries, and they need the same version.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 Cargo.toml                  | 3 +++
 proxmox-firewall/Cargo.toml | 2 +-
 proxmox-nftables/Cargo.toml | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index a819b2d..8fba806 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,3 +3,6 @@ members = [
     "proxmox-nftables",
     "proxmox-firewall",
 ]
+
+[workspace.dependencies]
+proxmox-ve-config = { version = "0.1.0" }
diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml
index b361ab1..c2adcac 100644
--- a/proxmox-firewall/Cargo.toml
+++ b/proxmox-firewall/Cargo.toml
@@ -21,7 +21,7 @@ serde_json = "1"
 signal-hook = "0.3"
 
 proxmox-nftables = { path = "../proxmox-nftables", features = ["config-ext"] }
-proxmox-ve-config = "0.1.0"
+proxmox-ve-config = { workspace = true }
 
 [dev-dependencies]
 insta = { version = "1.21", features = ["json"] }
diff --git a/proxmox-nftables/Cargo.toml b/proxmox-nftables/Cargo.toml
index cf06f93..4ff6f41 100644
--- a/proxmox-nftables/Cargo.toml
+++ b/proxmox-nftables/Cargo.toml
@@ -22,4 +22,4 @@ serde = { version = "1", features = [ "derive" ] }
 serde_json = "1"
 serde_plain = "1"
 
-proxmox-ve-config = { version = "0.1.0", optional = true }
+proxmox-ve-config = { workspace = true, optional = true }
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (8 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 09/17] cargo: make proxmox-ve-config a workspace dependency Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-11-06 14:32   ` Hannes Dürr
                     ` (2 more replies)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 11/17] api: add vnet endpoints Stefan Hanreich
                   ` (6 subsequent siblings)
  16 siblings, 3 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Firewall.pm         | 122 ++++++++++++++++++++++++++++++++++--
 src/PVE/Firewall/Helpers.pm |  12 ++++
 2 files changed, 128 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 9943f2e..e8096aa 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -29,6 +29,7 @@ use PVE::RS::Firewall::SDN;
 
 my $pvefw_conf_dir = "/etc/pve/firewall";
 my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
+my $vnetfw_conf_dir = "/etc/pve/sdn/firewall";
 
 # dynamically include PVE::QemuServer and PVE::LXC
 # to avoid dependency problems
@@ -1290,6 +1291,12 @@ our $cluster_option_properties = {
 	optional => 1,
 	enum => ['ACCEPT', 'REJECT', 'DROP'],
     },
+    policy_forward => {
+	description => "Forward policy.",
+	type => 'string',
+	optional => 1,
+	enum => ['ACCEPT', 'DROP'],
+    },
     log_ratelimit => {
 	description => "Log ratelimiting settings",
 	type => 'string', format => {
@@ -1329,6 +1336,8 @@ our $host_option_properties = {
 	description => "Log level for incoming traffic." }),
     log_level_out =>  get_standard_option('pve-fw-loglevel', {
 	description => "Log level for outgoing traffic." }),
+    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
+	description => "Log level for forwarded traffic." }),
     tcp_flags_log_level =>  get_standard_option('pve-fw-loglevel', {
 	description => "Log level for illegal tcp flags filter." }),
     smurf_log_level =>  get_standard_option('pve-fw-loglevel', {
@@ -1476,6 +1485,23 @@ our $vm_option_properties = {
 
 };
 
+our $vnet_option_properties = {
+    enable => {
+	description => "Enable/disable firewall rules.",
+	type => 'boolean',
+	default => 0,
+	optional => 1,
+    },
+    policy_forward => {
+	description => "Forward policy.",
+	type => 'string',
+	optional => 1,
+	enum => ['ACCEPT', 'DROP'],
+    },
+    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
+	description => "Log level for forwarded traffic." }),
+};
+
 
 my $addr_list_descr = "This can refer to a single IP address, an IP set ('+ipsetname') or an IP alias definition. You can also specify an address range like '20.34.101.207-201.3.9.99', or a list of IP addresses and networks (entries are separated by comma). Please do not mix IPv4 and IPv6 addresses inside such lists.";
 
@@ -1493,7 +1519,7 @@ my $rule_properties = {
 	description => "Rule type.",
 	type => 'string',
 	optional => 1,
-	enum => ['in', 'out', 'group'],
+	enum => ['in', 'out', 'forward', 'group'],
     },
     action => {
 	description => "Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name.",
@@ -1651,10 +1677,20 @@ my $rule_env_iface_lookup = {
     'ct' => 1,
     'vm' => 1,
     'group' => 0,
+    'vnet' => 0,
     'cluster' => 1,
     'host' => 1,
 };
 
+my $rule_env_direction_lookup = {
+    'ct' => ['in', 'out', 'group'],
+    'vm' => ['in', 'out', 'group'],
+    'group' => ['in', 'out', 'forward'],
+    'cluster' => ['in', 'out', 'forward', 'group'],
+    'host' => ['in', 'out', 'forward', 'group'],
+    'vnet' => ['forward', 'group'],
+};
+
 sub verify_rule {
     my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_;
 
@@ -1725,7 +1761,13 @@ sub verify_rule {
     &$add_error('action', "missing property") if !$action;
 
     if ($type) {
-	if ($type eq  'in' || $type eq 'out') {
+	my $valid_types = $rule_env_direction_lookup->{$rule_env}
+	    or die "unknown rule_env '$rule_env'\n";
+
+	&$add_error('type', "invalid rule type '$type' for rule_env '$rule_env'")
+	    if !grep (/^$type$/, @$valid_types);
+
+	if ($type eq  'in' || $type eq 'out' || $type eq 'forward') {
 	    &$add_error('action', "unknown action '$action'")
 		if $action && ($action !~ m/^(ACCEPT|DROP|REJECT)$/);
 	} elsif ($type eq 'group') {
@@ -2829,7 +2871,7 @@ sub parse_fw_rule {
     $rule->{type} = lc($1);
     $rule->{action} = $2;
 
-    if ($rule->{type} eq  'in' || $rule->{type} eq 'out') {
+    if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward') {
 	if ($rule->{action} =~ m/^(\S+)\((ACCEPT|DROP|REJECT)\)$/) {
 	    $rule->{macro} = $1;
 	    $rule->{action} = $2;
@@ -2943,7 +2985,7 @@ sub parse_hostfw_option {
     if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) {
 	$opt = lc($1);
 	$value = int($2);
-    } elsif ($line =~ m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
+    } elsif ($line =~ m/^(log_level_in|log_level_out|log_level_forward|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
 	$opt = lc($1);
 	$value = $2 ? lc($3) : '';
     } elsif ($line =~ m/^(nf_conntrack_helpers):\s*(((\S+)[,]?)+)\s*$/i) {
@@ -2974,7 +3016,7 @@ sub parse_clusterfw_option {
     } elsif ($line =~ m/^(ebtables):\s*(0|1)\s*$/i) {
 	$opt = lc($1);
 	$value = int($2);
-    } elsif ($line =~ m/^(policy_(in|out)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
+    } elsif ($line =~ m/^(policy_(in|out|forward)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
 	$opt = lc($1);
 	$value = uc($3);
     } elsif ($line =~ m/^(log_ratelimit):\s*(\S+)\s*$/) {
@@ -2987,6 +3029,24 @@ sub parse_clusterfw_option {
     return ($opt, $value);
 }
 
+sub parse_vnetfw_option {
+    my ($line) = @_;
+
+    my ($opt, $value);
+
+    if ($line =~ m/^(enable):\s*(\d+)\s*$/i) {
+	$opt = lc($1);
+	$value = int($2);
+    } elsif ($line =~ m/^(policy_forward):\s*(ACCEPT|DROP)\s*$/i) {
+	$opt = lc($1);
+	$value = uc($2);
+    } else {
+	die "can't parse option '$line'\n"
+    }
+
+    return ($opt, $value);
+}
+
 sub resolve_alias {
     my ($clusterfw_conf, $fw_conf, $cidr, $scope) = @_;
 
@@ -3153,6 +3213,8 @@ sub generic_fw_config_parser {
 		    ($opt, $value) = parse_clusterfw_option($line);
 		} elsif ($rule_env eq 'host') {
 		    ($opt, $value) = parse_hostfw_option($line);
+		} elsif ($rule_env eq 'vnet') {
+		    ($opt, $value) = parse_vnetfw_option($line);
 		} else {
 		    ($opt, $value) = parse_vmfw_option($line);
 		}
@@ -3292,6 +3354,10 @@ sub lock_vmfw_conf {
     return PVE::Firewall::Helpers::lock_vmfw_conf(@_);
 }
 
+sub lock_vnetfw_conf {
+    return PVE::Firewall::Helpers::lock_vnetfw_conf(@_);
+}
+
 sub load_vmfw_conf {
     my ($cluster_conf, $rule_env, $vmid, $dir) = @_;
 
@@ -3318,7 +3384,7 @@ my $format_rules = sub {
     my $raw = '';
 
     foreach my $rule (@$rules) {
-	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'group') {
+	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward' || $rule->{type} eq 'group') {
 	    $raw .= '|' if defined($rule->{enable}) && !$rule->{enable};
 	    $raw .= uc($rule->{type});
 	    if ($rule->{macro}) {
@@ -3794,6 +3860,50 @@ sub save_hostfw_conf {
     }
 }
 
+sub load_vnetfw_conf {
+    my ($cluster_conf, $rule_env, $vnet, $dir) = @_;
+
+    $rule_env = 'vnet' if !defined($rule_env);
+
+    my $filename = "$vnetfw_conf_dir/$vnet.fw";
+
+    my $empty_conf = {
+	rules => [],
+	options => {},
+    };
+
+    my $vnetfw_conf = generic_fw_config_parser($filename, $cluster_conf, $empty_conf, $rule_env);
+    $vnetfw_conf->{vnet} = $vnet;
+
+    return $vnetfw_conf;
+}
+
+sub save_vnetfw_conf {
+    my ($vnet, $conf) = @_;
+
+    my $filename = "$vnetfw_conf_dir/$vnet.fw";
+
+    my $raw = '';
+
+    my $options = $conf->{options};
+    $raw .= &$format_options($options) if $options && scalar(keys %$options);
+
+    my $rules = $conf->{rules};
+    if ($rules && scalar(@$rules)) {
+	$raw .= "[RULES]\n\n";
+	$raw .= &$format_rules($rules, 1);
+	$raw .= "\n";
+    }
+
+    mkdir($vnetfw_conf_dir, 0755) if !-d $vnetfw_conf_dir;
+
+    if ($raw) {
+	PVE::Tools::file_set_contents($filename, $raw);
+    } else {
+	unlink $filename;
+    }
+}
+
 sub compile {
     my ($cluster_conf, $hostfw_conf, $vmdata, $corosync_conf) = @_;
 
diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
index 7dcbca3..0b465ae 100644
--- a/src/PVE/Firewall/Helpers.pm
+++ b/src/PVE/Firewall/Helpers.pm
@@ -32,6 +32,18 @@ sub lock_vmfw_conf {
     return $res;
 }
 
+sub lock_vnetfw_conf {
+    my ($vnet, $timeout, $code, @param) = @_;
+
+    die "can't lock vnet firewall config for undefined vnet\n"
+	if !defined($vnet);
+
+    my $res = PVE::Cluster::cfs_lock_firewall("vnet-$vnet", $timeout, $code, @param);
+    die $@ if $@;
+
+    return $res;
+}
+
 sub remove_vmfw_conf {
     my ($vmid) = @_;
 
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-firewall v2 11/17] api: add vnet endpoints
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (9 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 12/17] firewall: add forward direction to rule panel Stefan Hanreich
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Firewall/Makefile |   1 +
 src/PVE/API2/Firewall/Rules.pm |  84 +++++++++++++++++
 src/PVE/API2/Firewall/Vnet.pm  | 168 +++++++++++++++++++++++++++++++++
 src/PVE/Firewall.pm            |  10 ++
 4 files changed, 263 insertions(+)
 create mode 100644 src/PVE/API2/Firewall/Vnet.pm

diff --git a/src/PVE/API2/Firewall/Makefile b/src/PVE/API2/Firewall/Makefile
index e916755..325c4d3 100644
--- a/src/PVE/API2/Firewall/Makefile
+++ b/src/PVE/API2/Firewall/Makefile
@@ -9,6 +9,7 @@ LIB_SOURCES=			\
 	Cluster.pm		\
 	Host.pm			\
 	VM.pm			\
+	Vnet.pm			\
 	Groups.pm
 
 all:
diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index f5cb002..e57b3de 100644
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -18,6 +18,25 @@ my $api_properties = {
     },
 };
 
+=head3 check_privileges_for_method($class, $method_name, $param)
+
+If the permission checks from the register_method() call are not sufficient,
+this function can be overriden for performing additional permission checks
+before API methods are executed. If the permission check fails, this function
+should die with an appropriate error message. The name of the method calling
+this function is provided by C<$method_name> and the parameters of the API
+method are provided by C<$param>
+
+Default implementation is a no-op to preserve backwards compatibility with
+existing subclasses, since this got added later on. It preserves existing
+behavior without having to change every subclass.
+
+=cut
+
+sub check_privileges_for_method {
+    my ($class, $method_name, $param) = @_;
+}
+
 sub lock_config {
     my ($class, $param, $code) = @_;
 
@@ -94,6 +113,8 @@ sub register_get_rules {
 	code => sub {
 	    my ($param) = @_;
 
+	    $class->check_privileges_for_method('get_rules', $param);
+
 	    my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
 	    my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules);
@@ -192,6 +213,8 @@ sub register_get_rule {
 	code => sub {
 	    my ($param) = @_;
 
+	    $class->check_privileges_for_method('get_rule', $param);
+
 	    my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
 	    my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules);
@@ -232,6 +255,8 @@ sub register_create_rule {
 	code => sub {
 	    my ($param) = @_;
 
+	    $class->check_privileges_for_method('create_rule', $param);
+
 	    $class->lock_config($param, sub {
 		my ($param) = @_;
 
@@ -293,6 +318,8 @@ sub register_update_rule {
 	code => sub {
 	    my ($param) = @_;
 
+	    $class->check_privileges_for_method('update_rule', $param);
+
 	    $class->lock_config($param, sub {
 		my ($param) = @_;
 
@@ -359,6 +386,8 @@ sub register_delete_rule {
 	code => sub {
 	    my ($param) = @_;
 
+	    $class->check_privileges_for_method('delete_rule', $param);
+
 	    $class->lock_config($param, sub {
 		my ($param) = @_;
 
@@ -634,4 +663,59 @@ sub save_rules {
 
 __PACKAGE__->register_handlers();
 
+package PVE::API2::Firewall::VnetRules;
+
+use strict;
+use warnings;
+use PVE::JSONSchema qw(get_standard_option);
+
+use base qw(PVE::API2::Firewall::RulesBase);
+
+__PACKAGE__->additional_parameters({
+    vnet => get_standard_option('pve-sdn-vnet-id'),
+});
+
+sub check_privileges_for_method {
+    my ($class, $method_name, $param) = @_;
+
+    if ($method_name eq 'get_rule' || $method_name eq 'get_rules') {
+	PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Audit', 'SDN.Allocate']);
+    } elsif ($method_name =~ '(update|create|delete)_rule') {
+	PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Allocate']);
+    } else {
+	die "unknown method: $method_name";
+    }
+}
+
+sub rule_env {
+    my ($class, $param) = @_;
+
+    return 'vnet';
+}
+
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_vnetfw_conf($param->{vnet}, 10, $code, $param);
+}
+
+sub load_config {
+    my ($class, $param) = @_;
+
+    my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 });
+    my $fw_conf = PVE::Firewall::load_vnetfw_conf($cluster_conf, 'vnet', $param->{vnet});
+    my $rules = $fw_conf->{rules};
+
+    return ($cluster_conf, $fw_conf, $rules);
+}
+
+sub save_rules {
+    my ($class, $param, $fw_conf, $rules) = @_;
+
+    $fw_conf->{rules} = $rules;
+    PVE::Firewall::save_vnetfw_conf($param->{vnet}, $fw_conf);
+}
+
+__PACKAGE__->register_handlers();
+
 1;
diff --git a/src/PVE/API2/Firewall/Vnet.pm b/src/PVE/API2/Firewall/Vnet.pm
new file mode 100644
index 0000000..cb49b67
--- /dev/null
+++ b/src/PVE/API2/Firewall/Vnet.pm
@@ -0,0 +1,168 @@
+package PVE::API2::Firewall::Vnet;
+
+use strict;
+use warnings;
+
+use Storable qw(dclone);
+
+use PVE::Exception qw(raise_param_exc);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RPCEnvironment;
+
+use PVE::Firewall;
+use PVE::API2::Firewall::Rules;
+
+
+use base qw(PVE::RESTHandler);
+
+sub check_vnet_access {
+    my ($vnetid, $privileges) = @_;
+
+    my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1)
+	or die "invalid vnet specified";
+
+    my $zoneid = $vnet->{zone};
+
+    my $rpcenv = PVE::RPCEnvironment::get();
+    my $authuser = $rpcenv->get_user();
+
+    $rpcenv->check_any($authuser, "/sdn/zones/$zoneid/$vnetid", $privileges);
+};
+
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Firewall::VnetRules",
+    path => 'rules',
+});
+
+__PACKAGE__->register_method({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "Directory index.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    vnet => get_standard_option('pve-sdn-vnet-id'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {},
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $result = [
+	    { name => 'rules' },
+	    { name => 'options' },
+	];
+
+	return $result;
+    }});
+
+my $option_properties = dclone($PVE::Firewall::vnet_option_properties);
+
+my sub add_option_properties {
+    my ($properties) = @_;
+
+    foreach my $k (keys %$option_properties) {
+	$properties->{$k} = $option_properties->{$k};
+    }
+
+    return $properties;
+};
+
+
+__PACKAGE__->register_method({
+    name => 'get_options',
+    path => 'options',
+    method => 'GET',
+    description => "Get vnet firewall options.",
+    permissions => {
+	description => "Needs SDN.Audit or SDN.Allocate permissions on '/sdn/zones/<zone>/<vnet>'",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    vnet => get_standard_option('pve-sdn-vnet-id'),
+	},
+    },
+    returns => {
+	type => "object",
+	properties => $option_properties,
+    },
+    code => sub {
+	my ($param) = @_;
+
+	check_vnet_access($param->{vnet}, ['SDN.Allocate', 'SDN.Audit']);
+
+	my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+	my $vnetfw_conf = PVE::Firewall::load_vnetfw_conf($cluster_conf, 'vnet', $param->{vnet});
+
+	return PVE::Firewall::copy_opject_with_digest($vnetfw_conf->{options});
+    }});
+
+__PACKAGE__->register_method({
+    name => 'set_options',
+    path => 'options',
+    method => 'PUT',
+    description => "Set Firewall options.",
+    protected => 1,
+    permissions => {
+	description => "Needs SDN.Allocate permissions on '/sdn/zones/<zone>/<vnet>'",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => add_option_properties({
+	    vnet => get_standard_option('pve-sdn-vnet-id'),
+	    delete => {
+		type => 'string', format => 'pve-configid-list',
+		description => "A list of settings you want to delete.",
+		optional => 1,
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	}),
+    },
+    returns => { type => "null" },
+    code => sub {
+	my ($param) = @_;
+
+	check_vnet_access($param->{vnet}, ['SDN.Allocate']);
+
+	PVE::Firewall::lock_vnetfw_conf($param->{vnet}, 10, sub {
+	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+	    my $vnetfw_conf = PVE::Firewall::load_vnetfw_conf($cluster_conf, 'vnet', $param->{vnet});
+
+	    my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($vnetfw_conf->{options});
+	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+
+	    if ($param->{delete}) {
+		for my $opt (PVE::Tools::split_list($param->{delete})) {
+		    raise_param_exc({ delete => "no such option '$opt'" })
+			if !$option_properties->{$opt};
+		    delete $vnetfw_conf->{options}->{$opt};
+		}
+	    }
+
+	    if (defined($param->{enable})) {
+		$param->{enable} = $param->{enable} ? 1 : 0;
+	    }
+
+	    for my $k (keys %$option_properties) {
+		next if !defined($param->{$k});
+		$vnetfw_conf->{options}->{$k} = $param->{$k};
+	    }
+
+	    PVE::Firewall::save_vnetfw_conf($param->{vnet}, $vnetfw_conf);
+	});
+
+	return undef;
+    }});
+
+1;
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index e8096aa..1a78caf 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1912,6 +1912,11 @@ sub rules_modify_permissions {
 	return {
 	    check => ['perm', '/vms/{vmid}', [ 'VM.Config.Network' ]],
 	}
+    } elsif ($rule_env eq 'vnet') {
+	return {
+	    description => "Needs SDN.Allocate permissions on '/sdn/zones/<zone>/<vnet>'",
+	    user => 'all',
+	}
     }
 
     return undef;
@@ -1932,6 +1937,11 @@ sub rules_audit_permissions {
 	return {
 	    check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
 	}
+    } elsif ($rule_env eq 'vnet') {
+	return {
+	    description => "Needs SDN.Audit or SDN.Allocate permissions on '/sdn/zones/<zone>/<vnet>'",
+	    user => 'all',
+	}
     }
 
     return undef;
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-manager v2 12/17] firewall: add forward direction to rule panel
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (10 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 11/17] api: add vnet endpoints Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 13/17] firewall: add vnet to firewall options component Stefan Hanreich
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Enables us to use the new forward direction as an option when creating
or editing firewall rules. By introducing firewall_type we can switch
between the available directions depending on which ruleset is being
edited.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/dc/Config.js          |  1 +
 www/manager6/dc/SecurityGroups.js  |  1 +
 www/manager6/grid/FirewallRules.js | 32 +++++++++++++++++++++++++-----
 www/manager6/lxc/Config.js         |  1 +
 www/manager6/node/Config.js        |  1 +
 www/manager6/qemu/Config.js        |  1 +
 6 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index ddbb58b12..720edefc6 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -241,6 +241,7 @@ Ext.define('PVE.dc.Config', {
 		list_refs_url: '/cluster/firewall/refs',
 		iconCls: 'fa fa-shield',
 		itemId: 'firewall',
+		firewall_type: 'dc',
 	    },
 	    {
 		xtype: 'pveFirewallOptions',
diff --git a/www/manager6/dc/SecurityGroups.js b/www/manager6/dc/SecurityGroups.js
index 9e26b84c9..e7aa8081c 100644
--- a/www/manager6/dc/SecurityGroups.js
+++ b/www/manager6/dc/SecurityGroups.js
@@ -214,6 +214,7 @@ Ext.define('PVE.SecurityGroups', {
 	    list_refs_url: '/cluster/firewall/refs',
 	    tbar_prefix: '<b>' + gettext('Rules') + ':</b>',
 	    border: false,
+	    firewall_type: 'group',
 	},
 	{
 	    xtype: 'pveSecurityGroupList',
diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index 11881bf79..5e7da2dda 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -147,6 +147,16 @@ let ICMPV6_TYPE_NAMES_STORE = Ext.create('Ext.data.Store', {
     ],
 });
 
+let DEFAULT_ALLOWED_DIRECTIONS = ['in', 'out'];
+
+let ALLOWED_DIRECTIONS = {
+    'dc': ['in', 'out', 'forward'],
+    'node': ['in', 'out', 'forward'],
+    'group': ['in', 'out', 'forward'],
+    'vm': ['in', 'out'],
+    'vnet': ['forward'],
+};
+
 Ext.define('PVE.FirewallRulePanel', {
     extend: 'Proxmox.panel.InputPanel',
 
@@ -154,6 +164,8 @@ Ext.define('PVE.FirewallRulePanel', {
 
     list_refs_url: undefined,
 
+    firewall_type: undefined,
+
     onGetValues: function(values) {
 	var me = this;
 
@@ -178,6 +190,8 @@ Ext.define('PVE.FirewallRulePanel', {
 	    throw "no list_refs_url specified";
 	}
 
+	let allowed_directions = ALLOWED_DIRECTIONS[me.firewall_type] ?? DEFAULT_ALLOWED_DIRECTIONS;
+
 	me.column1 = [
 	    {
 		// hack: we use this field to mark the form 'dirty' when the
@@ -190,8 +204,8 @@ Ext.define('PVE.FirewallRulePanel', {
 	    {
 		xtype: 'proxmoxKVComboBox',
 		name: 'type',
-		value: 'in',
-		comboItems: [['in', 'in'], ['out', 'out']],
+		value: allowed_directions[0],
+		comboItems: allowed_directions.map((dir) => [dir, dir]),
 		fieldLabel: gettext('Direction'),
 		allowBlank: false,
 	    },
@@ -387,6 +401,8 @@ Ext.define('PVE.FirewallRuleEdit', {
 
     allow_iface: false,
 
+    firewall_type: undefined,
+
     initComponent: function() {
 	var me = this;
 
@@ -412,6 +428,7 @@ Ext.define('PVE.FirewallRuleEdit', {
 	    list_refs_url: me.list_refs_url,
 	    allow_iface: me.allow_iface,
 	    rule_pos: me.rule_pos,
+	    firewall_type: me.firewall_type,
 	});
 
 	Ext.apply(me, {
@@ -555,6 +572,8 @@ Ext.define('PVE.FirewallRules', {
     allow_groups: true,
     allow_iface: false,
 
+    firewall_type: undefined,
+
     setBaseUrl: function(url) {
         var me = this;
 
@@ -661,7 +680,7 @@ Ext.define('PVE.FirewallRules', {
 	    var type = rec.data.type;
 
 	    var editor;
-	    if (type === 'in' || type === 'out') {
+	    if (type === 'in' || type === 'out' || type === 'forward') {
 		editor = 'PVE.FirewallRuleEdit';
 	    } else if (type === 'group') {
 		editor = 'PVE.FirewallGroupRuleEdit';
@@ -670,6 +689,7 @@ Ext.define('PVE.FirewallRules', {
 	    }
 
 	    var win = Ext.create(editor, {
+		firewall_type: me.firewall_type,
 		digest: rec.data.digest,
 		allow_iface: me.allow_iface,
 		base_url: me.base_url,
@@ -694,6 +714,7 @@ Ext.define('PVE.FirewallRules', {
 	    disabled: true,
 	    handler: function() {
 		var win = Ext.create('PVE.FirewallRuleEdit', {
+		    firewall_type: me.firewall_type,
 		    allow_iface: me.allow_iface,
 		    base_url: me.base_url,
 		    list_refs_url: me.list_refs_url,
@@ -709,11 +730,12 @@ Ext.define('PVE.FirewallRules', {
 		return;
 	    }
 	    let type = rec.data.type;
-	    if (!(type === 'in' || type === 'out')) {
+	    if (!(type === 'in' || type === 'out' || type === 'forward')) {
 		return;
 	    }
 
 	    let win = Ext.create('PVE.FirewallRuleEdit', {
+		firewall_type: me.firewall_type,
 		allow_iface: me.allow_iface,
 		base_url: me.base_url,
 		list_refs_url: me.list_refs_url,
@@ -726,7 +748,7 @@ Ext.define('PVE.FirewallRules', {
 	me.copyBtn = Ext.create('Proxmox.button.Button', {
 	    text: gettext('Copy'),
 	    selModel: sm,
-	    enableFn: ({ data }) => (data.type === 'in' || data.type === 'out') && me.canEdit,
+	    enableFn: ({ data }) => (data.type === 'in' || data.type === 'out' || data.type === 'forward') && me.canEdit,
 	    disabled: true,
 	    handler: run_copy_editor,
 	});
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index d0e40fc46..77aefd713 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -316,6 +316,7 @@ Ext.define('PVE.lxc.Config', {
 		    base_url: base_url + '/firewall/rules',
 		    list_refs_url: base_url + '/firewall/refs',
 		    itemId: 'firewall',
+		    firewall_type: 'vm',
 		},
 		{
 		    xtype: 'pveFirewallOptions',
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index d27592ce1..c242ba461 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -293,6 +293,7 @@ Ext.define('PVE.node.Config', {
 		    base_url: '/nodes/' + nodename + '/firewall/rules',
 		    list_refs_url: '/cluster/firewall/refs',
 		    itemId: 'firewall',
+		    firewall_type: 'node',
 		},
 		{
 		    xtype: 'pveFirewallOptions',
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index f28ee67bb..adceae8fb 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -351,6 +351,7 @@ Ext.define('PVE.qemu.Config', {
 		    base_url: base_url + '/firewall/rules',
 		    list_refs_url: base_url + '/firewall/refs',
 		    itemId: 'firewall',
+		    firewall_type: 'vm',
 		},
 		{
 		    xtype: 'pveFirewallOptions',
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-manager v2 13/17] firewall: add vnet to firewall options component
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (11 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 12/17] firewall: add forward direction to rule panel Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 14/17] firewall: make base_url dynamically configurable in " Stefan Hanreich
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Add the configuration options for vnet-level firewalls to the options
component. Additionally add the new policy_forward configuration
option to the datacenter-level firewall as well.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/grid/FirewallOptions.js | 38 +++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js
index 6aacb47be..fa482e0e4 100644
--- a/www/manager6/grid/FirewallOptions.js
+++ b/www/manager6/grid/FirewallOptions.js
@@ -2,7 +2,7 @@ Ext.define('PVE.FirewallOptions', {
     extend: 'Proxmox.grid.ObjectGrid',
     alias: ['widget.pveFirewallOptions'],
 
-    fwtype: undefined, // 'dc', 'node' or 'vm'
+    fwtype: undefined, // 'dc', 'node', 'vm' or 'vnet'
 
     base_url: undefined,
 
@@ -13,14 +13,14 @@ Ext.define('PVE.FirewallOptions', {
 	    throw "missing base_url configuration";
 	}
 
-	if (me.fwtype === 'dc' || me.fwtype === 'node' || me.fwtype === 'vm') {
-	    if (me.fwtype === 'node') {
-		me.cwidth1 = 250;
-	    }
-	} else {
+	if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) {
 	    throw "unknown firewall option type";
 	}
 
+	if (me.fwtype === 'node') {
+	    me.cwidth1 = 250;
+	}
+
 	let caps = Ext.state.Manager.get('GuiCap');
 	let canEdit = caps.vms['VM.Config.Network'] || caps.dc['Sys.Modify'] || caps.nodes['Sys.Modify'];
 
@@ -81,6 +81,7 @@ Ext.define('PVE.FirewallOptions', {
 			    'nf_conntrack_tcp_timeout_established', 7875, 250);
 	    add_log_row('log_level_in');
 	    add_log_row('log_level_out');
+	    add_log_row('log_level_forward');
 	    add_log_row('tcp_flags_log_level', 120);
 	    add_log_row('smurf_log_level');
 	    add_boolean_row('nftables', gettext('nftables (tech preview)'), 0);
@@ -114,6 +115,9 @@ Ext.define('PVE.FirewallOptions', {
 		    defaultValue: 'enable=1',
 		},
 	    };
+	} else if (me.fwtype === 'vnet') {
+	    add_boolean_row('enable', gettext('Firewall'), 0);
+	    add_log_row('log_level_forward');
 	}
 
 	if (me.fwtype === 'dc' || me.fwtype === 'vm') {
@@ -150,6 +154,28 @@ Ext.define('PVE.FirewallOptions', {
 	    };
 	}
 
+	if (me.fwtype === 'vnet' || me.fwtype === 'dc') {
+	    me.rows.policy_forward = {
+		header: gettext('Forward Policy'),
+		required: true,
+		defaultValue: 'ACCEPT',
+		editor: {
+		    xtype: 'proxmoxWindowEdit',
+		    subject: gettext('Forward Policy'),
+		    items: {
+			xtype: 'pveFirewallPolicySelector',
+			name: 'policy_forward',
+			value: 'ACCEPT',
+			fieldLabel: gettext('Forward Policy'),
+			comboItems: [
+			    ['ACCEPT', 'ACCEPT'],
+			    ['DROP', 'DROP'],
+			],
+		    },
+		},
+	    };
+	}
+
 	var edit_btn = new Ext.Button({
 	    text: gettext('Edit'),
 	    disabled: true,
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-manager v2 14/17] firewall: make base_url dynamically configurable in options component
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (12 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 13/17] firewall: add vnet to firewall options component Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 15/17] sdn: add firewall panel Stefan Hanreich
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

This adds the ability to dynamically configure and change the base_url
for the firewall options. This is needed for the SDN firewall dialog,
that updates the firewall components based on the selected vnet. This
avoids having to reinstantiate the component every time the user
selects a new vnet.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/grid/FirewallOptions.js | 38 ++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js
index fa482e0e4..d98cbe22c 100644
--- a/www/manager6/grid/FirewallOptions.js
+++ b/www/manager6/grid/FirewallOptions.js
@@ -9,10 +9,6 @@ Ext.define('PVE.FirewallOptions', {
     initComponent: function() {
 	var me = this;
 
-	if (!me.base_url) {
-	    throw "missing base_url configuration";
-	}
-
 	if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) {
 	    throw "unknown firewall option type";
 	}
@@ -197,23 +193,49 @@ Ext.define('PVE.FirewallOptions', {
 	};
 
 	Ext.apply(me, {
-	    url: "/api2/json" + me.base_url,
 	    tbar: [edit_btn],
-	    editorConfig: {
-		url: '/api2/extjs/' + me.base_url,
-	    },
 	    listeners: {
 		itemdblclick: () => { if (canEdit) { me.run_editor(); } },
 		selectionchange: set_button_status,
 	    },
 	});
 
+	if (me.base_url) {
+	    me.applyUrl(me.base_url);
+	} else {
+	    me.rstore = Ext.create('Proxmox.data.ObjectStore', {
+		interval: me.interval,
+		extraParams: me.extraParams,
+		rows: me.rows,
+	    });
+	}
+
 	me.callParent();
 
 	me.on('activate', me.rstore.startUpdate);
 	me.on('destroy', me.rstore.stopUpdate);
 	me.on('deactivate', me.rstore.stopUpdate);
     },
+    applyUrl: function(url) {
+	let me = this;
+
+	Ext.apply(me, {
+	    url: "/api2/json" + url,
+	    editorConfig: {
+		url: '/api2/extjs/' + url,
+	    },
+	});
+    },
+    setBaseUrl: function(url) {
+	let me = this;
+
+	me.base_url = url;
+
+	me.applyUrl(url);
+
+	me.rstore.getProxy().setConfig('url', `/api2/extjs/${url}`);
+	me.rstore.reload();
+    },
 });
 
 
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-manager v2 15/17] sdn: add firewall panel
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (13 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 14/17] firewall: make base_url dynamically configurable in " Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-network v2 16/17] firewall: add endpoints for vnet-level firewall Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction Stefan Hanreich
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Expose the ability to create vnet-level firewalls in the PVE UI

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/Makefile                |  2 +
 www/manager6/dc/Config.js            |  8 +++
 www/manager6/sdn/FirewallPanel.js    | 48 +++++++++++++++++
 www/manager6/sdn/FirewallVnetView.js | 77 ++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 www/manager6/sdn/FirewallPanel.js
 create mode 100644 www/manager6/sdn/FirewallVnetView.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 2c3a822bd..13a1c4177 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -279,6 +279,8 @@ JSSRC= 							\
 	sdn/SubnetView.js				\
 	sdn/ZoneContentView.js				\
 	sdn/ZoneContentPanel.js				\
+	sdn/FirewallPanel.js				\
+	sdn/FirewallVnetView.js				\
 	sdn/ZoneView.js					\
 	sdn/IpamEdit.js					\
 	sdn/OptionsPanel.js				\
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 720edefc6..d44554954 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -221,6 +221,14 @@ Ext.define('PVE.dc.Config', {
 		    hidden: true,
 		    iconCls: 'fa fa-map-signs',
 		    itemId: 'sdnmappings',
+		},
+		{
+		    xtype: 'pveSDNFirewall',
+		    groups: ['sdn'],
+		    title: gettext('Firewall'),
+		    hidden: true,
+		    iconCls: 'fa fa-shield',
+		    itemId: 'sdnfirewall',
 		});
 	    }
 
diff --git a/www/manager6/sdn/FirewallPanel.js b/www/manager6/sdn/FirewallPanel.js
new file mode 100644
index 000000000..f02ff5a35
--- /dev/null
+++ b/www/manager6/sdn/FirewallPanel.js
@@ -0,0 +1,48 @@
+
+Ext.define('PVE.sdn.FirewallPanel', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pveSDNFirewall',
+
+    title: 'VNet',
+
+    initComponent: function() {
+	let me = this;
+
+	let tabPanel = Ext.create('Ext.TabPanel', {
+	    fullscreen: true,
+	    region: 'center',
+	    border: false,
+	    split: true,
+	    disabled: true,
+	    items: [
+		{
+		    xtype: 'pveFirewallRules',
+		    title: gettext('Rules'),
+		    list_refs_url: '/cluster/firewall/refs',
+		    firewall_type: 'vnet',
+		},
+		{
+		    xtype: 'pveFirewallOptions',
+		    title: gettext('Options'),
+		    fwtype: 'vnet',
+		},
+	    ],
+	});
+
+	let vnetPanel = Ext.createWidget('pveSDNFirewallVnetView', {
+	    title: 'VNets',
+	    region: 'west',
+	    border: false,
+	    split: true,
+	    forceFit: true,
+	    tabPanel,
+	});
+
+	Ext.apply(me, {
+	    layout: 'border',
+	    items: [vnetPanel, tabPanel],
+	});
+
+	me.callParent();
+    },
+});
diff --git a/www/manager6/sdn/FirewallVnetView.js b/www/manager6/sdn/FirewallVnetView.js
new file mode 100644
index 000000000..861d4b5be
--- /dev/null
+++ b/www/manager6/sdn/FirewallVnetView.js
@@ -0,0 +1,77 @@
+Ext.define('PVE.sdn.FirewallVnetView', {
+    extend: 'Ext.grid.GridPanel',
+    alias: 'widget.pveSDNFirewallVnetView',
+
+    stateful: true,
+    stateId: 'grid-sdn-vnet-firewall',
+
+    tabPanel: undefined,
+
+    getRulesPanel: function() {
+	let me = this;
+	return me.tabPanel.items.getAt(0);
+    },
+
+    getOptionsPanel: function() {
+	let me = this;
+	return me.tabPanel.items.getAt(1);
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	let store = new Ext.data.Store({
+	    model: 'pve-sdn-vnet',
+	    proxy: {
+                type: 'proxmox',
+		url: "/api2/json/cluster/sdn/vnets",
+	    },
+	    sorters: {
+		property: ['zone', 'vnet'],
+		direction: 'ASC',
+	    },
+	});
+
+	let reload = () => store.load();
+
+	let sm = Ext.create('Ext.selection.RowModel', {});
+
+	Ext.apply(me, {
+	    store: store,
+	    reloadStore: reload,
+	    selModel: sm,
+	    viewConfig: {
+		trackOver: false,
+	    },
+	    columns: [
+		{
+		    header: 'ID',
+		    flex: 1,
+		    dataIndex: 'vnet',
+		},
+		{
+		    header: gettext('Zone'),
+		    flex: 1,
+		    dataIndex: 'zone',
+		},
+		{
+		    header: gettext('Alias'),
+		    flex: 1,
+		    dataIndex: 'alias',
+		},
+	    ],
+	    listeners: {
+		activate: reload,
+		show: reload,
+		select: function(_sm, rec) {
+		    me.tabPanel.setDisabled(false);
+
+		    me.getRulesPanel().setBaseUrl(`/cluster/sdn/vnets/${rec.id}/firewall/rules`);
+		    me.getOptionsPanel().setBaseUrl(`/cluster/sdn/vnets/${rec.id}/firewall/options`);
+		},
+	    },
+	});
+	store.load();
+	me.callParent();
+    },
+});
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-network v2 16/17] firewall: add endpoints for vnet-level firewall
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (14 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 15/17] sdn: add firewall panel Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction Stefan Hanreich
  16 siblings, 0 replies; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN/Vnets.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm
index 05915f6..e48b048 100644
--- a/src/PVE/API2/Network/SDN/Vnets.pm
+++ b/src/PVE/API2/Network/SDN/Vnets.pm
@@ -14,6 +14,7 @@ use PVE::Network::SDN::VnetPlugin;
 use PVE::Network::SDN::Subnets;
 use PVE::API2::Network::SDN::Subnets;
 use PVE::API2::Network::SDN::Ips;
+use PVE::API2::Firewall::Vnet;
 
 use Storable qw(dclone);
 use PVE::JSONSchema qw(get_standard_option);
@@ -24,6 +25,11 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Firewall::Vnet",
+    path => '{vnet}/firewall',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::Network::SDN::Subnets",
     path => '{vnet}/subnets',
-- 
2.39.5


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


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

* [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction
  2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
                   ` (15 preceding siblings ...)
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-network v2 16/17] firewall: add endpoints for vnet-level firewall Stefan Hanreich
@ 2024-10-10 15:56 ` Stefan Hanreich
  2024-11-07 15:57   ` Hannes Dürr
  16 siblings, 1 reply; 22+ messages in thread
From: Stefan Hanreich @ 2024-10-10 15:56 UTC (permalink / raw)
  To: pve-devel

Additionally add information about the SDN VNet firewall, which has
been introduced with this changes.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 Makefile                      |  1 +
 gen-pve-firewall-vnet-opts.pl | 12 ++++++++
 pve-firewall-vnet-opts.adoc   |  8 ++++++
 pve-firewall.adoc             | 53 ++++++++++++++++++++++++++++++++---
 4 files changed, 70 insertions(+), 4 deletions(-)
 create mode 100755 gen-pve-firewall-vnet-opts.pl
 create mode 100644 pve-firewall-vnet-opts.adoc

diff --git a/Makefile b/Makefile
index 801a2a3..f30d77a 100644
--- a/Makefile
+++ b/Makefile
@@ -62,6 +62,7 @@ GEN_SCRIPTS=					\
 	gen-pve-firewall-macros-adoc.pl		\
 	gen-pve-firewall-rules-opts.pl		\
 	gen-pve-firewall-vm-opts.pl		\
+	gen-pve-firewall-vnet-opts.pl		\
 	gen-output-format-opts.pl
 
 API_VIEWER_FILES=							\
diff --git a/gen-pve-firewall-vnet-opts.pl b/gen-pve-firewall-vnet-opts.pl
new file mode 100755
index 0000000..c9f4f13
--- /dev/null
+++ b/gen-pve-firewall-vnet-opts.pl
@@ -0,0 +1,12 @@
+#!/usr/bin/perl
+
+use lib '.';
+use strict;
+use warnings;
+
+use PVE::Firewall;
+use PVE::RESTHandler;
+
+my $prop = $PVE::Firewall::vnet_option_properties;
+
+print PVE::RESTHandler::dump_properties($prop);
diff --git a/pve-firewall-vnet-opts.adoc b/pve-firewall-vnet-opts.adoc
new file mode 100644
index 0000000..ed1e88f
--- /dev/null
+++ b/pve-firewall-vnet-opts.adoc
@@ -0,0 +1,8 @@
+`enable`: `<boolean>` ('default =' `0`)::
+
+Enable/disable firewall rules.
+
+`policy_forward`: `<ACCEPT | DROP>` ::
+
+Forward policy.
+
diff --git a/pve-firewall.adoc b/pve-firewall.adoc
index b428703..339a42f 100644
--- a/pve-firewall.adoc
+++ b/pve-firewall.adoc
@@ -52,14 +52,22 @@ The Proxmox VE firewall groups the network into the following logical zones:
 
 Host::
 
-Traffic from/to a cluster node
+Traffic from/to a cluster node or traffic forwarded by a cluster node
 
 VM::
 
 Traffic from/to a specific VM
 
-For each zone, you can define firewall rules for incoming and/or
-outgoing traffic.
+VNet::
+
+Traffic flowing through a SDN VNet
+
+For each zone, you can define firewall rules for incoming, outgoing or
+forwarded traffic.
+
+IMPORTANT: Creating rules for forwarded traffic or on a VNet-level is currently
+only possible when using the new
+xref:pve_firewall_nft[nftables-based proxmox-firewall].
 
 
 Configuration Files
@@ -202,10 +210,46 @@ can selectively enable the firewall for each interface. This is
 required in addition to the general firewall `enable` option.
 
 
+[[pve_firewall_vnet_configuration]]
+VNet Configuration
+~~~~~~~~~~~~~~~~~~
+VNet related configuration is read from:
+
+ /etc/pve/sdn/firewall/<vnet_name>.fw
+
+This can be used for setting firewall configuration globally on a VNet level,
+without having to set firewall rules for each VM inside the VNet separately. It
+can only contain rules for the `FORWARD` direction, since there is no notion of
+incoming or outgoing traffic. This affects all traffic travelling from one
+bridge port to another, including the host interface.
+
+WARNING: This feature is currently only available for the new
+xref:pve_firewall_nft[nftables-based proxmox-firewall]
+
+Since traffic passing the `FORWARD` chain is bi-directional, you need to create
+rules for both directions if you want traffic to pass both ways. For instance if
+HTTP traffic for a specific host should be allowed, you would need to create the
+following rules:
+
+----
+FORWARD ACCEPT -dest 10.0.0.1 -dport 80
+FORWARD ACCEPT -source 10.0.0.1 -sport 80
+----
+
+`[OPTIONS]`::
+
+This is used to set VNet related firewall options.
+
+include::pve-firewall-vnet-opts.adoc[]
+
+`[RULES]`::
+
+This section contains VNet specific firewall rules.
+
 Firewall Rules
 --------------
 
-Firewall rules consists of a direction (`IN` or `OUT`) and an
+Firewall rules consists of a direction (`IN`, `OUT` or `FORWARD`) and an
 action (`ACCEPT`, `DENY`, `REJECT`). You can also specify a macro
 name. Macros contain predefined sets of rules and options. Rules can be
 disabled by prefixing them with `|`.
@@ -639,6 +683,7 @@ Ports used by {pve}
 * live migration (VM memory and local-disk data): 60000-60050 (TCP)
 
 
+[[pve_firewall_nft]]
 nftables
 --------
 
-- 
2.39.5


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


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

* Re: [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
@ 2024-11-06 14:32   ` Hannes Dürr
  2024-11-06 15:46   ` Hannes Dürr
  2024-11-08 13:59   ` Wolfgang Bumiller
  2 siblings, 0 replies; 22+ messages in thread
From: Hannes Dürr @ 2024-11-06 14:32 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich


On 10/10/24 17:56, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   src/PVE/Firewall.pm         | 122 ++++++++++++++++++++++++++++++++++--
>   src/PVE/Firewall/Helpers.pm |  12 ++++
>   2 files changed, 128 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 9943f2e..e8096aa 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -29,6 +29,7 @@ use PVE::RS::Firewall::SDN;
>   
>   my $pvefw_conf_dir = "/etc/pve/firewall";
>   my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
> +my $vnetfw_conf_dir = "/etc/pve/sdn/firewall";
>   
>   # dynamically include PVE::QemuServer and PVE::LXC
>   # to avoid dependency problems
> @@ -1290,6 +1291,12 @@ our $cluster_option_properties = {
>   	optional => 1,
>   	enum => ['ACCEPT', 'REJECT', 'DROP'],
>       },
> +    policy_forward => {
> +	description => "Forward policy.",
> +	type => 'string',
> +	optional => 1,
> +	enum => ['ACCEPT', 'DROP'],
> +    },
>       log_ratelimit => {
>   	description => "Log ratelimiting settings",
>   	type => 'string', format => {
> @@ -1329,6 +1336,8 @@ our $host_option_properties = {
>   	description => "Log level for incoming traffic." }),
>       log_level_out =>  get_standard_option('pve-fw-loglevel', {
>   	description => "Log level for outgoing traffic." }),
> +    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
> +	description => "Log level for forwarded traffic." }),
>       tcp_flags_log_level =>  get_standard_option('pve-fw-loglevel', {
>   	description => "Log level for illegal tcp flags filter." }),
>       smurf_log_level =>  get_standard_option('pve-fw-loglevel', {
> @@ -1476,6 +1485,23 @@ our $vm_option_properties = {
>   
>   };
>   
> +our $vnet_option_properties = {
> +    enable => {
> +	description => "Enable/disable firewall rules.",
> +	type => 'boolean',
> +	default => 0,
> +	optional => 1,
> +    },
> +    policy_forward => {
> +	description => "Forward policy.",
> +	type => 'string',
> +	optional => 1,
> +	enum => ['ACCEPT', 'DROP'],
> +    },
> +    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
> +	description => "Log level for forwarded traffic." }),
> +};
> +
>   
>   my $addr_list_descr = "This can refer to a single IP address, an IP set ('+ipsetname') or an IP alias definition. You can also specify an address range like '20.34.101.207-201.3.9.99', or a list of IP addresses and networks (entries are separated by comma). Please do not mix IPv4 and IPv6 addresses inside such lists.";
>   
> @@ -1493,7 +1519,7 @@ my $rule_properties = {
>   	description => "Rule type.",
>   	type => 'string',
>   	optional => 1,
> -	enum => ['in', 'out', 'group'],
> +	enum => ['in', 'out', 'forward', 'group'],
>       },
>       action => {
>   	description => "Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name.",
> @@ -1651,10 +1677,20 @@ my $rule_env_iface_lookup = {
>       'ct' => 1,
>       'vm' => 1,
>       'group' => 0,
> +    'vnet' => 0,
>       'cluster' => 1,
>       'host' => 1,
>   };
>   
> +my $rule_env_direction_lookup = {
> +    'ct' => ['in', 'out', 'group'],
> +    'vm' => ['in', 'out', 'group'],
> +    'group' => ['in', 'out', 'forward'],
> +    'cluster' => ['in', 'out', 'forward', 'group'],
> +    'host' => ['in', 'out', 'forward', 'group'],
> +    'vnet' => ['forward', 'group'],
> +};
> +
>   sub verify_rule {
>       my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_;
>   
> @@ -1725,7 +1761,13 @@ sub verify_rule {
>       &$add_error('action', "missing property") if !$action;
nit: our perl style guide requires the following notation $add_error->(..)
>   
>       if ($type) {
> -	if ($type eq  'in' || $type eq 'out') {
> +	my $valid_types = $rule_env_direction_lookup->{$rule_env}
> +	    or die "unknown rule_env '$rule_env'\n";
> +
> +	&$add_error('type', "invalid rule type '$type' for rule_env '$rule_env'")
same
> +	    if !grep (/^$type$/, @$valid_types);
> +
> +	if ($type eq  'in' || $type eq 'out' || $type eq 'forward') {
>   	    &$add_error('action', "unknown action '$action'")
same
>   		if $action && ($action !~ m/^(ACCEPT|DROP|REJECT)$/);
>   	} elsif ($type eq 'group') {
> @@ -2829,7 +2871,7 @@ sub parse_fw_rule {
>       $rule->{type} = lc($1);
>       $rule->{action} = $2;
>   
> -    if ($rule->{type} eq  'in' || $rule->{type} eq 'out') {
> +    if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward') {
>   	if ($rule->{action} =~ m/^(\S+)\((ACCEPT|DROP|REJECT)\)$/) {
>   	    $rule->{macro} = $1;
>   	    $rule->{action} = $2;
> @@ -2943,7 +2985,7 @@ sub parse_hostfw_option {
>       if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) {
>   	$opt = lc($1);
>   	$value = int($2);
> -    } elsif ($line =~ m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
> +    } elsif ($line =~ m/^(log_level_in|log_level_out|log_level_forward|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
>   	$opt = lc($1);
>   	$value = $2 ? lc($3) : '';
>       } elsif ($line =~ m/^(nf_conntrack_helpers):\s*(((\S+)[,]?)+)\s*$/i) {
> @@ -2974,7 +3016,7 @@ sub parse_clusterfw_option {
>       } elsif ($line =~ m/^(ebtables):\s*(0|1)\s*$/i) {
>   	$opt = lc($1);
>   	$value = int($2);
> -    } elsif ($line =~ m/^(policy_(in|out)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
> +    } elsif ($line =~ m/^(policy_(in|out|forward)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
>   	$opt = lc($1);
>   	$value = uc($3);
>       } elsif ($line =~ m/^(log_ratelimit):\s*(\S+)\s*$/) {
> @@ -2987,6 +3029,24 @@ sub parse_clusterfw_option {
>       return ($opt, $value);
>   }
>   
> +sub parse_vnetfw_option {
> +    my ($line) = @_;
> +
> +    my ($opt, $value);
> +
> +    if ($line =~ m/^(enable):\s*(\d+)\s*$/i) {
> +	$opt = lc($1);
> +	$value = int($2);
> +    } elsif ($line =~ m/^(policy_forward):\s*(ACCEPT|DROP)\s*$/i) {
> +	$opt = lc($1);
> +	$value = uc($2);
> +    } else {
> +	die "can't parse option '$line'\n"
> +    }
> +
> +    return ($opt, $value);
> +}
> +
>   sub resolve_alias {
>       my ($clusterfw_conf, $fw_conf, $cidr, $scope) = @_;
>   
> @@ -3153,6 +3213,8 @@ sub generic_fw_config_parser {
>   		    ($opt, $value) = parse_clusterfw_option($line);
>   		} elsif ($rule_env eq 'host') {
>   		    ($opt, $value) = parse_hostfw_option($line);
> +		} elsif ($rule_env eq 'vnet') {
> +		    ($opt, $value) = parse_vnetfw_option($line);
>   		} else {
>   		    ($opt, $value) = parse_vmfw_option($line);
>   		}
> @@ -3292,6 +3354,10 @@ sub lock_vmfw_conf {
>       return PVE::Firewall::Helpers::lock_vmfw_conf(@_);
>   }
>   
> +sub lock_vnetfw_conf {
> +    return PVE::Firewall::Helpers::lock_vnetfw_conf(@_);
> +}
> +
>   sub load_vmfw_conf {
>       my ($cluster_conf, $rule_env, $vmid, $dir) = @_;
>   
> @@ -3318,7 +3384,7 @@ my $format_rules = sub {
>       my $raw = '';
>   
>       foreach my $rule (@$rules) {
> -	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'group') {
> +	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward' || $rule->{type} eq 'group') {
>   	    $raw .= '|' if defined($rule->{enable}) && !$rule->{enable};
>   	    $raw .= uc($rule->{type});
>   	    if ($rule->{macro}) {
> @@ -3794,6 +3860,50 @@ sub save_hostfw_conf {
>       }
>   }
>   
> +sub load_vnetfw_conf {
> +    my ($cluster_conf, $rule_env, $vnet, $dir) = @_;
> +
> +    $rule_env = 'vnet' if !defined($rule_env);
> +
> +    my $filename = "$vnetfw_conf_dir/$vnet.fw";
> +
> +    my $empty_conf = {
> +	rules => [],
> +	options => {},
> +    };
> +
> +    my $vnetfw_conf = generic_fw_config_parser($filename, $cluster_conf, $empty_conf, $rule_env);
> +    $vnetfw_conf->{vnet} = $vnet;
> +
> +    return $vnetfw_conf;
> +}
> +
> +sub save_vnetfw_conf {
> +    my ($vnet, $conf) = @_;
> +
> +    my $filename = "$vnetfw_conf_dir/$vnet.fw";
> +
> +    my $raw = '';
> +
> +    my $options = $conf->{options};
> +    $raw .= &$format_options($options) if $options && scalar(keys %$options);
same
> +
> +    my $rules = $conf->{rules};
> +    if ($rules && scalar(@$rules)) {
> +	$raw .= "[RULES]\n\n";
> +	$raw .= &$format_rules($rules, 1);
same
> +	$raw .= "\n";
> +    }
> +
> +    mkdir($vnetfw_conf_dir, 0755) if !-d $vnetfw_conf_dir;
> +
> +    if ($raw) {
> +	PVE::Tools::file_set_contents($filename, $raw);
> +    } else {
> +	unlink $filename;
> +    }
> +}
> +
>   sub compile {
>       my ($cluster_conf, $hostfw_conf, $vmdata, $corosync_conf) = @_;
>   
> diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
> index 7dcbca3..0b465ae 100644
> --- a/src/PVE/Firewall/Helpers.pm
> +++ b/src/PVE/Firewall/Helpers.pm
> @@ -32,6 +32,18 @@ sub lock_vmfw_conf {
>       return $res;
>   }
>   
> +sub lock_vnetfw_conf {
> +    my ($vnet, $timeout, $code, @param) = @_;
> +
> +    die "can't lock vnet firewall config for undefined vnet\n"
> +	if !defined($vnet);
> +
> +    my $res = PVE::Cluster::cfs_lock_firewall("vnet-$vnet", $timeout, $code, @param);
> +    die $@ if $@;
> +
> +    return $res;
> +}
> +
>   sub remove_vmfw_conf {
>       my ($vmid) = @_;
>   


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


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

* Re: [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
  2024-11-06 14:32   ` Hannes Dürr
@ 2024-11-06 15:46   ` Hannes Dürr
  2024-11-08 13:59   ` Wolfgang Bumiller
  2 siblings, 0 replies; 22+ messages in thread
From: Hannes Dürr @ 2024-11-06 15:46 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

As we talked off-list, it is still possible to create Rules on the 
forward chain which Reject traffic on cluster, host and bridge level 
which should not be possible


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


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

* Re: [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction Stefan Hanreich
@ 2024-11-07 15:57   ` Hannes Dürr
  0 siblings, 0 replies; 22+ messages in thread
From: Hannes Dürr @ 2024-11-07 15:57 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich


On 10/10/24 17:56, Stefan Hanreich wrote:
> Additionally add information about the SDN VNet firewall, which has
> been introduced with this changes.
>
> Signed-off-by: Stefan Hanreich<s.hanreich@proxmox.com>
> ---
>   Makefile                      |  1 +
>   gen-pve-firewall-vnet-opts.pl | 12 ++++++++
>   pve-firewall-vnet-opts.adoc   |  8 ++++++
>   pve-firewall.adoc             | 53 ++++++++++++++++++++++++++++++++---
>   4 files changed, 70 insertions(+), 4 deletions(-)
>   create mode 100755 gen-pve-firewall-vnet-opts.pl
>   create mode 100644 pve-firewall-vnet-opts.adoc
>
> diff --git a/Makefile b/Makefile
> index 801a2a3..f30d77a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -62,6 +62,7 @@ GEN_SCRIPTS=					\
>   	gen-pve-firewall-macros-adoc.pl		\
>   	gen-pve-firewall-rules-opts.pl		\
>   	gen-pve-firewall-vm-opts.pl		\
> +	gen-pve-firewall-vnet-opts.pl		\
>   	gen-output-format-opts.pl
>   
>   API_VIEWER_FILES=							\
> diff --git a/gen-pve-firewall-vnet-opts.pl b/gen-pve-firewall-vnet-opts.pl
> new file mode 100755
> index 0000000..c9f4f13
> --- /dev/null
> +++ b/gen-pve-firewall-vnet-opts.pl
> @@ -0,0 +1,12 @@
> +#!/usr/bin/perl
> +
> +use lib '.';
> +use strict;
> +use warnings;
> +
> +use PVE::Firewall;
> +use PVE::RESTHandler;
> +
> +my $prop = $PVE::Firewall::vnet_option_properties;
> +
> +print PVE::RESTHandler::dump_properties($prop);
> diff --git a/pve-firewall-vnet-opts.adoc b/pve-firewall-vnet-opts.adoc
> new file mode 100644
> index 0000000..ed1e88f
> --- /dev/null
> +++ b/pve-firewall-vnet-opts.adoc
> @@ -0,0 +1,8 @@
> +`enable`: `<boolean>` ('default =' `0`)::
> +
> +Enable/disable firewall rules.
> +
> +`policy_forward`: `<ACCEPT | DROP>` ::
> +
> +Forward policy.
> +
> diff --git a/pve-firewall.adoc b/pve-firewall.adoc
> index b428703..339a42f 100644
> --- a/pve-firewall.adoc
> +++ b/pve-firewall.adoc
> @@ -52,14 +52,22 @@ The Proxmox VE firewall groups the network into the following logical zones:
>   
>   Host::
>   
> -Traffic from/to a cluster node
> +Traffic from/to a cluster node or traffic forwarded by a cluster node
>   
>   VM::
>   
>   Traffic from/to a specific VM
>   
> -For each zone, you can define firewall rules for incoming and/or
> -outgoing traffic.
> +VNet::
> +
> +Traffic flowing through a SDN VNet
> +
> +For each zone, you can define firewall rules for incoming, outgoing or
> +forwarded traffic.

This is not really true, I can not create rules on the forward chain of 
VMs, can I?

I think the "Zones" section could benefit from some rewording because 
IMO the Zone representation is not really fitting and also in the rest 
of the article we are talking about 'Levels' and not 'Zones'.
I'd propose something like this:

Firewall rules can be created on 4 levels, Cluster, Node, Vnet, VM. 
However, the Rules only act on the 3 levels Node, Vnet and VM.
The reason for this is the distributed architecture: if a firewall rule 
is created at cluster level, it gets rolled out to all hosts and acts at 
host level.

At host level the rules can act on and manipulate traffic from/into the 
host. With the new proxmox-firewall based on nftables it is additionally 
possible to create rules that act on and manipulate traffic passing 
trough the host (forwarded).

The Vnet level is only available with the new proxmox-firewall. At Vnet 
level the rules can act on and manipulate traffic passing through the 
Vnet (forwarded).

At VM level the rules can act on and manipulate traffic from/into a VM.

> +
> +IMPORTANT: Creating rules for forwarded traffic or on a VNet-level is currently
> +only possible when using the new
> +xref:pve_firewall_nft[nftables-based proxmox-firewall].
>   
>   
>   Configuration Files
> @@ -202,10 +210,46 @@ can selectively enable the firewall for each interface. This is
>   required in addition to the general firewall `enable` option.
>   
>   
> +[[pve_firewall_vnet_configuration]]
> +VNet Configuration
> +~~~~~~~~~~~~~~~~~~
> +VNet related configuration is read from:
> +
> + /etc/pve/sdn/firewall/<vnet_name>.fw
> +
> +This can be used for setting firewall configuration globally on a VNet level,
> +without having to set firewall rules for each VM inside the VNet separately. It
> +can only contain rules for the `FORWARD` direction, since there is no notion of
> +incoming or outgoing traffic. This affects all traffic travelling from one
> +bridge port to another, including the host interface.
> +
> +WARNING: This feature is currently only available for the new
> +xref:pve_firewall_nft[nftables-based proxmox-firewall]
> +
> +Since traffic passing the `FORWARD` chain is bi-directional, you need to create
> +rules for both directions if you want traffic to pass both ways. For instance if
> +HTTP traffic for a specific host should be allowed, you would need to create the
> +following rules:
> +
> +----
> +FORWARD ACCEPT -dest 10.0.0.1 -dport 80
> +FORWARD ACCEPT -source 10.0.0.1 -sport 80
> +----
> +
> +`[OPTIONS]`::
> +
> +This is used to set VNet related firewall options.
> +
> +include::pve-firewall-vnet-opts.adoc[]
> +
> +`[RULES]`::
> +
> +This section contains VNet specific firewall rules.
> +
>   Firewall Rules
>   --------------
>   
> -Firewall rules consists of a direction (`IN` or `OUT`) and an
> +Firewall rules consists of a direction (`IN`, `OUT` or `FORWARD`) and an
>   action (`ACCEPT`, `DENY`, `REJECT`). You can also specify a macro
>   name. Macros contain predefined sets of rules and options. Rules can be
>   disabled by prefixing them with `|`.
> @@ -639,6 +683,7 @@ Ports used by {pve}
>   * live migration (VM memory and local-disk data): 60000-60050 (TCP)
Here I'd also add that it is dependent on the Level the Rule is applied to.
>   
>   
> +[[pve_firewall_nft]]
>   nftables
>   --------
>   


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


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

* Re: [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration
  2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
  2024-11-06 14:32   ` Hannes Dürr
  2024-11-06 15:46   ` Hannes Dürr
@ 2024-11-08 13:59   ` Wolfgang Bumiller
  2 siblings, 0 replies; 22+ messages in thread
From: Wolfgang Bumiller @ 2024-11-08 13:59 UTC (permalink / raw)
  To: Stefan Hanreich; +Cc: pve-devel

On Thu, Oct 10, 2024 at 05:56:43PM GMT, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/Firewall.pm         | 122 ++++++++++++++++++++++++++++++++++--
>  src/PVE/Firewall/Helpers.pm |  12 ++++
>  2 files changed, 128 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 9943f2e..e8096aa 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -29,6 +29,7 @@ use PVE::RS::Firewall::SDN;
>  
>  my $pvefw_conf_dir = "/etc/pve/firewall";
>  my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
> +my $vnetfw_conf_dir = "/etc/pve/sdn/firewall";
>  
>  # dynamically include PVE::QemuServer and PVE::LXC
>  # to avoid dependency problems
> @@ -1290,6 +1291,12 @@ our $cluster_option_properties = {
>  	optional => 1,
>  	enum => ['ACCEPT', 'REJECT', 'DROP'],
>      },
> +    policy_forward => {
> +	description => "Forward policy.",
> +	type => 'string',
> +	optional => 1,
> +	enum => ['ACCEPT', 'DROP'],
> +    },
>      log_ratelimit => {
>  	description => "Log ratelimiting settings",
>  	type => 'string', format => {
> @@ -1329,6 +1336,8 @@ our $host_option_properties = {
>  	description => "Log level for incoming traffic." }),
>      log_level_out =>  get_standard_option('pve-fw-loglevel', {
>  	description => "Log level for outgoing traffic." }),
> +    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
> +	description => "Log level for forwarded traffic." }),
>      tcp_flags_log_level =>  get_standard_option('pve-fw-loglevel', {
>  	description => "Log level for illegal tcp flags filter." }),
>      smurf_log_level =>  get_standard_option('pve-fw-loglevel', {
> @@ -1476,6 +1485,23 @@ our $vm_option_properties = {
>  
>  };
>  
> +our $vnet_option_properties = {
> +    enable => {
> +	description => "Enable/disable firewall rules.",
> +	type => 'boolean',
> +	default => 0,
> +	optional => 1,
> +    },
> +    policy_forward => {
> +	description => "Forward policy.",
> +	type => 'string',
> +	optional => 1,
> +	enum => ['ACCEPT', 'DROP'],
> +    },
> +    log_level_forward =>  get_standard_option('pve-fw-loglevel', {
> +	description => "Log level for forwarded traffic." }),
> +};
> +
>  
>  my $addr_list_descr = "This can refer to a single IP address, an IP set ('+ipsetname') or an IP alias definition. You can also specify an address range like '20.34.101.207-201.3.9.99', or a list of IP addresses and networks (entries are separated by comma). Please do not mix IPv4 and IPv6 addresses inside such lists.";
>  
> @@ -1493,7 +1519,7 @@ my $rule_properties = {
>  	description => "Rule type.",
>  	type => 'string',
>  	optional => 1,
> -	enum => ['in', 'out', 'group'],
> +	enum => ['in', 'out', 'forward', 'group'],
>      },
>      action => {
>  	description => "Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name.",
> @@ -1651,10 +1677,20 @@ my $rule_env_iface_lookup = {
>      'ct' => 1,
>      'vm' => 1,
>      'group' => 0,
> +    'vnet' => 0,
>      'cluster' => 1,
>      'host' => 1,
>  };
>  
> +my $rule_env_direction_lookup = {
> +    'ct' => ['in', 'out', 'group'],
> +    'vm' => ['in', 'out', 'group'],
> +    'group' => ['in', 'out', 'forward'],
> +    'cluster' => ['in', 'out', 'forward', 'group'],
> +    'host' => ['in', 'out', 'forward', 'group'],
> +    'vnet' => ['forward', 'group'],
> +};
> +
>  sub verify_rule {
>      my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_;
>  
> @@ -1725,7 +1761,13 @@ sub verify_rule {
>      &$add_error('action', "missing property") if !$action;
>  
>      if ($type) {
> -	if ($type eq  'in' || $type eq 'out') {
> +	my $valid_types = $rule_env_direction_lookup->{$rule_env}
> +	    or die "unknown rule_env '$rule_env'\n";
> +
> +	&$add_error('type', "invalid rule type '$type' for rule_env '$rule_env'")
> +	    if !grep (/^$type$/, @$valid_types);

^ Please use `$_ eq $type` instead of turning the thing you're supposed
to be verifying into a regex.

> +
> +	if ($type eq  'in' || $type eq 'out' || $type eq 'forward') {
>  	    &$add_error('action', "unknown action '$action'")
>  		if $action && ($action !~ m/^(ACCEPT|DROP|REJECT)$/);
>  	} elsif ($type eq 'group') {
> @@ -2829,7 +2871,7 @@ sub parse_fw_rule {
>      $rule->{type} = lc($1);
>      $rule->{action} = $2;
>  
> -    if ($rule->{type} eq  'in' || $rule->{type} eq 'out') {
> +    if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward') {
>  	if ($rule->{action} =~ m/^(\S+)\((ACCEPT|DROP|REJECT)\)$/) {
>  	    $rule->{macro} = $1;
>  	    $rule->{action} = $2;
> @@ -2943,7 +2985,7 @@ sub parse_hostfw_option {
>      if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) {
>  	$opt = lc($1);
>  	$value = int($2);
> -    } elsif ($line =~ m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
> +    } elsif ($line =~ m/^(log_level_in|log_level_out|log_level_forward|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {

This is getting quite long. I know we already have such massive lines,
but I wonder if we should at least do something like
    (log_level_(?:in|out|forward)|...)
(tbh I wouldn't even condense the `(tcp_flags|smurf)_log_level`, but
somehow `log_level_(in|out|forward)` IMO looks fine...

No strong feelings though, if you think this doesn't help the readability anyway.

>  	$opt = lc($1);
>  	$value = $2 ? lc($3) : '';
>      } elsif ($line =~ m/^(nf_conntrack_helpers):\s*(((\S+)[,]?)+)\s*$/i) {
> @@ -2974,7 +3016,7 @@ sub parse_clusterfw_option {
>      } elsif ($line =~ m/^(ebtables):\s*(0|1)\s*$/i) {
>  	$opt = lc($1);
>  	$value = int($2);
> -    } elsif ($line =~ m/^(policy_(in|out)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
> +    } elsif ($line =~ m/^(policy_(in|out|forward)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {

^ I mean we do it here :)

>  	$opt = lc($1);
>  	$value = uc($3);
>      } elsif ($line =~ m/^(log_ratelimit):\s*(\S+)\s*$/) {
> @@ -2987,6 +3029,24 @@ sub parse_clusterfw_option {
>      return ($opt, $value);
>  }
>  
> +sub parse_vnetfw_option {
> +    my ($line) = @_;
> +
> +    my ($opt, $value);
> +
> +    if ($line =~ m/^(enable):\s*(\d+)\s*$/i) {
> +	$opt = lc($1);
> +	$value = int($2);
> +    } elsif ($line =~ m/^(policy_forward):\s*(ACCEPT|DROP)\s*$/i) {
> +	$opt = lc($1);
> +	$value = uc($2);
> +    } else {
> +	die "can't parse option '$line'\n"
> +    }
> +
> +    return ($opt, $value);
> +}
> +
>  sub resolve_alias {
>      my ($clusterfw_conf, $fw_conf, $cidr, $scope) = @_;
>  
> @@ -3153,6 +3213,8 @@ sub generic_fw_config_parser {
>  		    ($opt, $value) = parse_clusterfw_option($line);
>  		} elsif ($rule_env eq 'host') {
>  		    ($opt, $value) = parse_hostfw_option($line);
> +		} elsif ($rule_env eq 'vnet') {
> +		    ($opt, $value) = parse_vnetfw_option($line);
>  		} else {
>  		    ($opt, $value) = parse_vmfw_option($line);
>  		}
> @@ -3292,6 +3354,10 @@ sub lock_vmfw_conf {
>      return PVE::Firewall::Helpers::lock_vmfw_conf(@_);
>  }
>  
> +sub lock_vnetfw_conf {
> +    return PVE::Firewall::Helpers::lock_vnetfw_conf(@_);
> +}
> +
>  sub load_vmfw_conf {
>      my ($cluster_conf, $rule_env, $vmid, $dir) = @_;
>  
> @@ -3318,7 +3384,7 @@ my $format_rules = sub {
>      my $raw = '';
>  
>      foreach my $rule (@$rules) {
> -	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'group') {
> +	if ($rule->{type} eq  'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward' || $rule->{type} eq 'group') {

could drop the double-space after the first `eq` ;-)

and maybe it's worth switching to

    grep { $rule->{type} eq $_ } qw(in out forward group)

>  	    $raw .= '|' if defined($rule->{enable}) && !$rule->{enable};
>  	    $raw .= uc($rule->{type});
>  	    if ($rule->{macro}) {
> @@ -3794,6 +3860,50 @@ sub save_hostfw_conf {
>      }
>  }
>  
> +sub load_vnetfw_conf {
> +    my ($cluster_conf, $rule_env, $vnet, $dir) = @_;
> +
> +    $rule_env = 'vnet' if !defined($rule_env);
> +
> +    my $filename = "$vnetfw_conf_dir/$vnet.fw";
> +
> +    my $empty_conf = {
> +	rules => [],
> +	options => {},
> +    };
> +
> +    my $vnetfw_conf = generic_fw_config_parser($filename, $cluster_conf, $empty_conf, $rule_env);
> +    $vnetfw_conf->{vnet} = $vnet;
> +
> +    return $vnetfw_conf;
> +}
> +
> +sub save_vnetfw_conf {
> +    my ($vnet, $conf) = @_;
> +
> +    my $filename = "$vnetfw_conf_dir/$vnet.fw";
> +
> +    my $raw = '';
> +
> +    my $options = $conf->{options};
> +    $raw .= &$format_options($options) if $options && scalar(keys %$options);
> +
> +    my $rules = $conf->{rules};
> +    if ($rules && scalar(@$rules)) {
> +	$raw .= "[RULES]\n\n";
> +	$raw .= &$format_rules($rules, 1);
> +	$raw .= "\n";
> +    }
> +
> +    mkdir($vnetfw_conf_dir, 0755) if !-d $vnetfw_conf_dir;
> +
> +    if ($raw) {
> +	PVE::Tools::file_set_contents($filename, $raw);
> +    } else {
> +	unlink $filename;
> +    }
> +}
> +
>  sub compile {
>      my ($cluster_conf, $hostfw_conf, $vmdata, $corosync_conf) = @_;
>  
> diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
> index 7dcbca3..0b465ae 100644
> --- a/src/PVE/Firewall/Helpers.pm
> +++ b/src/PVE/Firewall/Helpers.pm
> @@ -32,6 +32,18 @@ sub lock_vmfw_conf {
>      return $res;
>  }
>  
> +sub lock_vnetfw_conf {
> +    my ($vnet, $timeout, $code, @param) = @_;
> +
> +    die "can't lock vnet firewall config for undefined vnet\n"
> +	if !defined($vnet);
> +
> +    my $res = PVE::Cluster::cfs_lock_firewall("vnet-$vnet", $timeout, $code, @param);
> +    die $@ if $@;
> +
> +    return $res;
> +}
> +
>  sub remove_vmfw_conf {
>      my ($vmid) = @_;
>  
> -- 
> 2.39.5


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


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

end of thread, other threads:[~2024-11-08 14:00 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-10 15:56 [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 02/17] firewall: add bridge firewall config parser Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 03/17] config: firewall: add tests for interface and directions Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-ve-rs v2 04/17] host: add struct representing bridge names Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 05/17] nftables: derive additional traits for nftables types Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 06/17] sdn: add support for loading vnet-level firewall config Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 08/17] use std::mem::take over drain() Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH proxmox-firewall v2 09/17] cargo: make proxmox-ve-config a workspace dependency Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration Stefan Hanreich
2024-11-06 14:32   ` Hannes Dürr
2024-11-06 15:46   ` Hannes Dürr
2024-11-08 13:59   ` Wolfgang Bumiller
2024-10-10 15:56 ` [pve-devel] [PATCH pve-firewall v2 11/17] api: add vnet endpoints Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 12/17] firewall: add forward direction to rule panel Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 13/17] firewall: add vnet to firewall options component Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 14/17] firewall: make base_url dynamically configurable in " Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-manager v2 15/17] sdn: add firewall panel Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-network v2 16/17] firewall: add endpoints for vnet-level firewall Stefan Hanreich
2024-10-10 15:56 ` [pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction Stefan Hanreich
2024-11-07 15:57   ` Hannes Dürr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal