* [pve-devel] [PATCH proxmox-ve-rs 01/15] cargo: bump dependencies
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 02/15] firewall: add forward direction Stefan Hanreich
` (16 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
proxmox-ve-config/Cargo.toml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml
index 5f11bf9..3fc09eb 100644
--- a/proxmox-ve-config/Cargo.toml
+++ b/proxmox-ve-config/Cargo.toml
@@ -10,12 +10,13 @@ exclude.workspace = true
log = "0.4"
anyhow = "1"
nix = "0.26"
+thiserror = "1.0.40"
serde = { version = "1", features = [ "derive" ] }
serde_json = "1"
serde_plain = "1"
serde_with = "3"
-proxmox-schema = "3.1.1"
-proxmox-sys = "0.5.8"
+proxmox-schema = "3.1.2"
+proxmox-sys = "0.6.0"
proxmox-sortable-macro = "0.1.3"
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-ve-rs 02/15] firewall: add forward direction
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 01/15] cargo: bump dependencies Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 03/15] firewall: add bridge firewall config parser Stefan Hanreich
` (15 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
proxmox-ve-config/src/firewall/cluster.rs | 10 ++++++++++
proxmox-ve-config/src/firewall/guest.rs | 15 +++++++++++++++
proxmox-ve-config/src/firewall/host.rs | 4 ++++
proxmox-ve-config/src/firewall/mod.rs | 1 +
proxmox-ve-config/src/firewall/types/rule.rs | 10 ++++++++--
5 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs
index 223124b..b7bebae 100644
--- a/proxmox-ve-config/src/firewall/cluster.rs
+++ b/proxmox-ve-config/src/firewall/cluster.rs
@@ -25,6 +25,8 @@ 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> {
@@ -86,6 +88,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 +128,7 @@ pub struct Options {
policy_in: Option<Verdict>,
policy_out: Option<Verdict>,
+ policy_forward: Option<Verdict>,
}
#[cfg(test)]
@@ -148,6 +156,7 @@ log_ratelimit: 1,rate=10/second,burst=20
ebtables: 0
policy_in: REJECT
policy_out: REJECT
+policy_forward: DROP
[ALIASES]
@@ -191,6 +200,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/guest.rs b/proxmox-ve-config/src/firewall/guest.rs
index c7e282f..b097f56 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))]
@@ -52,6 +54,7 @@ pub struct Options {
log_level_in: Option<LogLevel>,
log_level_out: Option<LogLevel>,
+ log_level_forward: Option<LogLevel>,
#[serde(default, with = "serde_option_bool")]
macfilter: Option<bool>,
@@ -61,6 +64,8 @@ pub struct Options {
#[serde(rename = "policy_out")]
policy_out: Option<Verdict>,
+
+ policy_forward: Option<Verdict>,
}
#[derive(Debug)]
@@ -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(),
+ Direction::Forward => self.config.options.log_level_forward.unwrap_or_default(),
}
}
@@ -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),
}
}
@@ -206,11 +217,13 @@ dhcp: 1
ipfilter: 0
log_level_in: emerg
log_level_out: crit
+log_level_forward: warn
macfilter: 0
ndp:1
radv:1
policy_in: REJECT
policy_out: REJECT
+policy_forward: DROP
"#;
let config = CONFIG.as_bytes();
@@ -228,9 +241,11 @@ policy_out: REJECT
radv: Some(true),
log_level_in: Some(LogLevel::Emergency),
log_level_out: Some(LogLevel::Critical),
+ log_level_forward: Some(LogLevel::Warning),
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..56ed46d 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>,
@@ -262,6 +263,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 +286,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 +319,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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-ve-rs 03/15] firewall: add bridge firewall config parser
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 01/15] cargo: bump dependencies Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 02/15] firewall: add forward direction Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 04/15] host: add struct representing bridge names Stefan Hanreich
` (14 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 | 59 ++++++++++++++++++++++++
1 file changed, 59 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..13103e8
--- /dev/null
+++ b/proxmox-ve-config/src/firewall/bridge.rs
@@ -0,0 +1,59 @@
+use std::io;
+
+use anyhow::Error;
+use serde::Deserialize;
+
+use crate::firewall::parse::serde_option_bool;
+use crate::firewall::types::rule::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,
+ };
+
+ Ok(Self {
+ config: super::common::Config::parse(input, &parser_config)?,
+ })
+ }
+
+ pub fn enabled(&self) -> bool {
+ self.config
+ .options
+ .enabled
+ .unwrap_or(BRIDGE_ENABLED_DEFAULT)
+ }
+
+ pub fn rules(&self) -> impl Iterator<Item = &Rule> + '_ {
+ self.config.rules.iter()
+ }
+
+ 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")]
+ enabled: Option<bool>,
+
+ policy_forward: Option<Verdict>,
+}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-ve-rs 04/15] host: add struct representing bridge names
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (2 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 03/15] firewall: add bridge firewall config parser Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 05/15] sdn: add support for loading vnet-level firewall config Stefan Hanreich
` (13 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
proxmox-ve-config/src/host/mod.rs | 1 +
proxmox-ve-config/src/host/types.rs | 46 +++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 proxmox-ve-config/src/host/types.rs
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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-firewall 05/15] sdn: add support for loading vnet-level firewall config
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (3 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-ve-rs 04/15] host: add struct representing bridge names Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 06/15] sdn: create forward firewall rules Stefan Hanreich
` (12 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-firewall 06/15] sdn: create forward firewall rules
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (4 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 05/15] sdn: add support for loading vnet-level firewall config Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 07/15] use std::mem::take over drain() Stefan Hanreich
` (11 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 | 97 ++++++++++++++++++-
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, 251 insertions(+), 5 deletions(-)
diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index f42255c..ea0b4e9 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 : 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 vmap @bridge-map
+ }
}
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 347f3af..546a0ff 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())));
}
@@ -283,6 +307,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 +333,68 @@ impl Firewall {
self.create_guest_rules(&mut commands, *vmid, config, Direction::Out)?;
}
+ for (bridge_name, bridge_config) in self.config.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())));
+ }
+ }
+
+ commands.push(Add::rule(AddRule::from_statement(
+ chain.clone(),
+ config.policy_forward(),
+ )));
+
+ let map_element = AddElement::map_from_expressions(
+ Self::bridge_vmap(table),
+ [(
+ name.into(),
+ 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 +871,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 a969f65..81f0773 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 a83e958..de8b4f9 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-firewall 07/15] use std::mem::take over drain()
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (5 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 06/15] sdn: create forward firewall rules Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 08/15] cargo: bump dependencies Stefan Hanreich
` (10 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH proxmox-firewall 08/15] cargo: bump dependencies
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (6 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 07/15] use std::mem::take over drain() Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-firewall 09/15] sdn: add vnet firewall configuration Stefan Hanreich
` (9 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 UTC (permalink / raw)
To: pve-devel
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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-firewall 09/15] sdn: add vnet firewall configuration
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (7 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH proxmox-firewall 08/15] cargo: bump dependencies Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints Stefan Hanreich
` (8 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Firewall.pm | 114 ++++++++++++++++++++++++++++++++++--
src/PVE/Firewall/Helpers.pm | 12 ++++
2 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 95325a0..d2b3dbb 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 => {
@@ -1476,6 +1483,21 @@ 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'],
+ },
+};
+
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 +1515,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 +1673,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) = @_;
@@ -1723,7 +1755,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};
+ die "unknown rule_env '$rule_env'\n" if !defined($valid_types);
+
+ &$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') {
@@ -2825,7 +2863,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;
@@ -2970,7 +3008,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*$/) {
@@ -2983,6 +3021,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|REJECT)\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) = @_;
@@ -3149,6 +3205,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);
}
@@ -3288,6 +3346,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) = @_;
@@ -3314,7 +3376,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}) {
@@ -3789,6 +3851,48 @@ 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";
+ }
+
+ 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (8 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-firewall 09/15] sdn: add vnet firewall configuration Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-26 6:37 ` Thomas Lamprecht
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 11/15] firewall: add forward direction to rule panel Stefan Hanreich
` (7 subsequent siblings)
17 siblings, 1 reply; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 | 71 ++++++++++++++
src/PVE/API2/Firewall/Vnet.pm | 166 +++++++++++++++++++++++++++++++++
src/PVE/Firewall.pm | 10 ++
4 files changed, 248 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 ebb51af..906c6d7 100644
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -18,6 +18,10 @@ my $api_properties = {
},
};
+sub before_method {
+ my ($class, $method_name, $param) = @_;
+}
+
sub lock_config {
my ($class, $param, $code) = @_;
@@ -93,6 +97,7 @@ sub register_get_rules {
},
code => sub {
my ($param) = @_;
+ $class->before_method('get_rules', $param);
my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
@@ -191,6 +196,7 @@ sub register_get_rule {
},
code => sub {
my ($param) = @_;
+ $class->before_method('get_rule', $param);
my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
@@ -231,6 +237,7 @@ sub register_create_rule {
returns => { type => "null" },
code => sub {
my ($param) = @_;
+ $class->before_method('create_rule', $param);
$class->lock_config($param, sub {
my ($param) = @_;
@@ -292,6 +299,7 @@ sub register_update_rule {
returns => { type => "null" },
code => sub {
my ($param) = @_;
+ $class->before_method('update_rule', $param);
$class->lock_config($param, sub {
my ($param) = @_;
@@ -358,6 +366,7 @@ sub register_delete_rule {
returns => { type => "null" },
code => sub {
my ($param) = @_;
+ $class->before_method('delete_rule', $param);
$class->lock_config($param, sub {
my ($param) = @_;
@@ -636,4 +645,66 @@ 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 before_method {
+ my ($class, $method_name, $param) = @_;
+
+ my $privs;
+
+ if ($method_name eq 'get_rule' || $method_name eq 'get_rules') {
+ $privs = ['SDN.Audit', 'SDN.Allocate'];
+ } elsif ($method_name eq 'update_rule'
+ || $method_name eq 'create_rule'
+ || $method_name eq 'delete_rule'
+ ) {
+ $privs = ['SDN.Allocate'];
+ } else {
+ die "unknown method: $method_name";
+ }
+
+ PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, $privs);
+}
+
+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, 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..bdfa163
--- /dev/null
+++ b/src/PVE/API2/Firewall/Vnet.pm
@@ -0,0 +1,166 @@
+package PVE::API2::Firewall::Vnet;
+
+use strict;
+use warnings;
+
+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);
+ die "invalid vnet specified" if !$vnet;
+
+ 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 = $PVE::Firewall::vnet_option_properties;
+
+my $add_option_properties = sub {
+ 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}) {
+ foreach 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;
+ }
+
+ foreach 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 d2b3dbb..4117d59 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1906,6 +1906,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;
@@ -1926,6 +1931,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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints
2024-09-11 9:31 ` [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints Stefan Hanreich
@ 2024-09-26 6:37 ` Thomas Lamprecht
2024-10-04 15:36 ` Stefan Hanreich
0 siblings, 1 reply; 21+ messages in thread
From: Thomas Lamprecht @ 2024-09-26 6:37 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Hanreich
Am 11/09/2024 um 11:31 schrieb Stefan Hanreich:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> src/PVE/API2/Firewall/Makefile | 1 +
> src/PVE/API2/Firewall/Rules.pm | 71 ++++++++++++++
> src/PVE/API2/Firewall/Vnet.pm | 166 +++++++++++++++++++++++++++++++++
> src/PVE/Firewall.pm | 10 ++
> 4 files changed, 248 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 ebb51af..906c6d7 100644
> --- a/src/PVE/API2/Firewall/Rules.pm
> +++ b/src/PVE/API2/Firewall/Rules.pm
> @@ -18,6 +18,10 @@ my $api_properties = {
> },
> };
>
> +sub before_method {
> + my ($class, $method_name, $param) = @_;
a short comment here stating explicitly that/why this is a no-op implementation
by design might be good.
> +}
> +
> sub lock_config {
> my ($class, $param, $code) = @_;
>
> @@ -93,6 +97,7 @@ sub register_get_rules {
> },
> code => sub {
> my ($param) = @_;
please keep the empty line after above's method param extraction one.
> + $class->before_method('get_rules', $param);
>
> my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
>
> @@ -191,6 +196,7 @@ sub register_get_rule {
> },
> code => sub {
> my ($param) = @_;
same as above
> + $class->before_method('get_rule', $param);
>
> my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
>
> @@ -231,6 +237,7 @@ sub register_create_rule {
> returns => { type => "null" },
> code => sub {
> my ($param) = @_;
> + $class->before_method('create_rule', $param);
same as above
>
> $class->lock_config($param, sub {
> my ($param) = @_;
> @@ -292,6 +299,7 @@ sub register_update_rule {
> returns => { type => "null" },
> code => sub {
> my ($param) = @_;
same as above
> + $class->before_method('update_rule', $param);
>
> $class->lock_config($param, sub {
> my ($param) = @_;
> @@ -358,6 +366,7 @@ sub register_delete_rule {
> returns => { type => "null" },
> code => sub {
> my ($param) = @_;
same as above
> + $class->before_method('delete_rule', $param);
>
> $class->lock_config($param, sub {
> my ($param) = @_;
> @@ -636,4 +645,66 @@ 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 before_method {
I'd prefer a _hook suffix for such a method for slightly added clarity.
And FWIW, if all you do now is check privileges, and there's nothing you already
know that's gonna get added here soon, you could just name it after what it does
and avoid being all to generic, i.e. something like
sub assert_privs_for_method
> + my ($class, $method_name, $param) = @_;
> +
> + my $privs;
this is a "one of those privs", not "all of those privs" thing FWICT, maybe encode
that also in the name to slightly reduce potential for introducing errors (as in:
unlikely, but too cheap to not do it), like e.g.:
my $requires_one_of_privs:
Alternatively, avoid the variable and call check_vnet_access directly, but no hard
feelings.
> +
> + if ($method_name eq 'get_rule' || $method_name eq 'get_rules') {
> + $privs = ['SDN.Audit', 'SDN.Allocate'];
> + } elsif ($method_name eq 'update_rule'
> + || $method_name eq 'create_rule'
> + || $method_name eq 'delete_rule'
It's certainly not always better but IMO a regex match would improve readability
slightly, e.g.:
} elsif ($method_name =~ /^(create|delete|update)_rule$/) {
> + ) {
> + $privs = ['SDN.Allocate'];
> + } else {
> + die "unknown method: $method_name";
> + }
> +
> + PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, $privs);
> +}
> +
> +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, 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;
> +sub check_vnet_access {
> + my ($vnetid, $privileges) = @_;
> +
> + my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
> + die "invalid vnet specified" if !$vnet;
nit: could be combined
my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1) or die "invalid vnet '$vnetid'\n";
for sake of completeness: what's not ok is a conditional definition like e.g:
my $foo = 'bar' if baz();
As that keeps the value of the last call if the if evaluates to false.
But, as long as the definition itself is not conditional it's fine.
> +
> + my $zoneid = $vnet->{zone};
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + $rpcenv->check_any($authuser, "/sdn/zones/$zoneid/$vnetid", $privileges);
> +};
> + }});
> +
> +my $option_properties = $PVE::Firewall::vnet_option_properties;
might need a clone to avoid modifying the original reference I think
> +
> +my $add_option_properties = sub {
> + my ($properties) = @_;
> +
> + foreach my $k (keys %$option_properties) {
> + $properties->{$k} = $option_properties->{$k};
> + }
> +
> + return $properties;
> +};
> +
> +
> +__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>'",
Hmm, I'm wondering might it make sense to add a SDN.Firewall privilege to separate
allowing VNet allocation and allowing to configure a VNet's firewall?
While adding privs is a bit tricky, this one might be dooable later one too though.
But whatever gets chosen should then be also addressed in a commit message with some
background reasoning (if it's already then I might have overlooked, I did not checked
every all too closely yet).
> + user => 'all',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => &$add_option_properties({
nit: probably stems from copying this from existing code, but please prefer modern
$code_ref->() calling style, i.e.:
properties => $add_option_properties->({
alternatively the $add_option_properties could be replaced with a locally scoped sub:
my sub 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}) {
> + foreach my $opt (PVE::Tools::split_list($param->{delete})) {
nit: we prefer for over foreach for new code:
https://pve.proxmox.com/wiki/Perl_Style_Guide#Perl_syntax_choices
> + 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;
> + }
> +
> + foreach my $k (keys %$option_properties) {
same nit w.r.t. foreach as above
> + next if !defined($param->{$k});
> + $vnetfw_conf->{options}->{$k} = $param->{$k};
> + }
> +
> + PVE::Firewall::save_vnetfw_conf($param->{vnet}, $vnetfw_conf);
> + });
> +
> + return undef;
> + }});
> +
> +1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints
2024-09-26 6:37 ` Thomas Lamprecht
@ 2024-10-04 15:36 ` Stefan Hanreich
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-10-04 15:36 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 9/26/24 08:37, Thomas Lamprecht wrote:
> I'd prefer a _hook suffix for such a method for slightly added clarity.
>
> And FWIW, if all you do now is check privileges, and there's nothing you already
> know that's gonna get added here soon, you could just name it after what it does
> and avoid being all to generic, i.e. something like
>
> sub assert_privs_for_method
Thats's probably the better approach for naming this function, since I
don't have any future additions in mind.
>> +my $option_properties = $PVE::Firewall::vnet_option_properties;
>
> might need a clone to avoid modifying the original reference I think
Yes, I think we never write to it - but accidents happen and better safe
than sorry. Might make sense to also do this in the other API modules as
well in a separate patch then.
> Hmm, I'm wondering might it make sense to add a SDN.Firewall privilege to separate
> allowing VNet allocation and allowing to configure a VNet's firewall?
> While adding privs is a bit tricky, this one might be dooable later one too though.
>
> But whatever gets chosen should then be also addressed in a commit message with some
> background reasoning (if it's already then I might have overlooked, I did not checked
> every all too closely yet).
Initial reasoning was that there's not really a privilege like that for
DC / Host / Guests, where it is always tied to the networking permissions.
Maybe it would make sense to create a completely separate Firewall
permission that is then also used for DC / Host / Guests? Of course this
could only happen in the next major release.
Thanks for the review!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pve-devel] [PATCH pve-manager 11/15] firewall: add forward direction to rule panel
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (9 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 12/15] firewall: add vnet to firewall options component Stefan Hanreich
` (6 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 ddbb58b1..720edefc 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 9e26b84c..e7aa8081 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 11881bf7..5e7da2dd 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 d0e40fc4..77aefd71 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 d27592ce..c242ba46 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 f28ee67b..adceae8f 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-manager 12/15] firewall: add vnet to firewall options component
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (10 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 11/15] firewall: add forward direction to rule panel Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 13/15] firewall: make base_url dynamically configurable in " Stefan Hanreich
` (5 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 | 36 +++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js
index 6aacb47b..ffafc9d3 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'];
@@ -114,6 +114,8 @@ Ext.define('PVE.FirewallOptions', {
defaultValue: 'enable=1',
},
};
+ } else if (me.fwtype === 'vnet') {
+ add_boolean_row('enable', gettext('Firewall'), 0);
}
if (me.fwtype === 'dc' || me.fwtype === 'vm') {
@@ -150,6 +152,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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-manager 13/15] firewall: make base_url dynamically configurable in options component
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (11 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 12/15] firewall: add vnet to firewall options component Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 14/15] sdn: add firewall panel Stefan Hanreich
` (4 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 ffafc9d3..7e25f72e 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";
}
@@ -195,23 +191,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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-manager 14/15] sdn: add firewall panel
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (12 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 13/15] firewall: make base_url dynamically configurable in " Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 9:31 ` [pve-devel] [PATCH pve-network 15/15] firewall: add endpoints for vnet-level firewall Stefan Hanreich
` (3 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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 2c3a822b..13a1c417 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 720edefc..d4455495 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 00000000..f02ff5a3
--- /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 00000000..861d4b5b
--- /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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* [pve-devel] [PATCH pve-network 15/15] firewall: add endpoints for vnet-level firewall
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (13 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-manager 14/15] sdn: add firewall panel Stefan Hanreich
@ 2024-09-11 9:31 ` Stefan Hanreich
2024-09-11 12:31 ` [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (2 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 9:31 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.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (14 preceding siblings ...)
2024-09-11 9:31 ` [pve-devel] [PATCH pve-network 15/15] firewall: add endpoints for vnet-level firewall Stefan Hanreich
@ 2024-09-11 12:31 ` Stefan Hanreich
2024-09-11 15:22 ` Gabriel Goller
2024-10-10 16:00 ` Stefan Hanreich
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-09-11 12:31 UTC (permalink / raw)
To: pve-devel
Some notes on the dependencies of this series, since I forgot to include
it in my cover letter.
This series is based on my initial SDN integration patch series [1] -
with one additional patch (I haven't sent a v2 for that yet):
diff --git a/proxmox-ve-config/src/sdn/config.rs
b/proxmox-ve-config/src/sdn/config.rs
index 8a7316d..328c58c 100644
--- a/proxmox-ve-config/src/sdn/config.rs
+++ b/proxmox-ve-config/src/sdn/config.rs
@@ -134,7 +134,7 @@ impl Display for DhcpType {
pub struct ZoneRunningConfig {
#[serde(rename = "type")]
ty: ZoneType,
- dhcp: DhcpType,
+ dhcp: Option<DhcpType>,
}
proxmox-firewall depends on proxmox-ve-rs
pve-firewall depends on the proxmox-perl-rs from the SDN integration
pve-network depends on pve-firewall
pve-manager depends on pve-{network, manager}
For your convenience I have the state of the series available on my
staff repos:
staff/s.hanreich/proxmox-ve-rs (firewall-forward)
staff/s.hanreich/proxmox-firewall (firewall-forward)
staff/s.hanreich/proxmox-perl-rs (sdn-integration-fw)
[1] https://lists.proxmox.com/pipermail/pve-devel/2024-July/064682.html
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (15 preceding siblings ...)
2024-09-11 12:31 ` [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
@ 2024-09-11 15:22 ` Gabriel Goller
2024-10-10 16:00 ` Stefan Hanreich
17 siblings, 0 replies; 21+ messages in thread
From: Gabriel Goller @ 2024-09-11 15:22 UTC (permalink / raw)
To: Proxmox VE development discussion
Spent this afternoon testing this series.
Problems I found:
- when creating a rule in a vnet for the first time, I get an error
that a firewall conf file cannot be opened (it's because the
/etc/pve/sdn/firewall folder does not exist and we don't create
folders recursively when opening the file).
- When creating a "forward" rule on a vnet and guest-firewall is
enabled, there are a lot of nftables errors on the syslog.
Everything else works perfectly fine, also using the auto-generated
ipsets in "forward" rules.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
2024-09-11 9:31 [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges Stefan Hanreich
` (16 preceding siblings ...)
2024-09-11 15:22 ` Gabriel Goller
@ 2024-10-10 16:00 ` Stefan Hanreich
17 siblings, 0 replies; 21+ messages in thread
From: Stefan Hanreich @ 2024-10-10 16:00 UTC (permalink / raw)
To: pve-devel
v2 here:
https://lore.proxmox.com/pve-devel/20241010155650.255698-1-s.hanreich@proxmox.com/T/
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread