[pve-devel] [PATCH proxmox-firewall v2 07/17] sdn: create forward firewall rules
Signed-off-by: Stefan Hanreich --- .../resources/proxmox-firewall.nft| 54 proxmox-firewall/src/firewall.rs | 122 +- proxmox-firewall/src/rule.rs | 5 +- .../integration_tests__firewall.snap | 86 proxmox-nftables/src/expression.rs| 8 ++ proxmox-nftables/src/types.rs | 6 + 6 files changed, 275 insertions(+), 6 deletions(-) diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft index f42255c..af9454d 100644 --- a/proxmox-firewall/resources/proxmox-firewall.nft +++ b/proxmox-firewall/resources/proxmox-firewall.nft @@ -20,8 +20,12 @@ add chain inet proxmox-firewall allow-icmp add chain inet proxmox-firewall log-drop-smurfs add chain inet proxmox-firewall default-in add chain inet proxmox-firewall default-out +add chain inet proxmox-firewall before-bridge +add chain inet proxmox-firewall host-bridge-input {type filter hook input priority filter - 1; policy accept;} +add chain inet proxmox-firewall host-bridge-output {type filter hook output priority filter + 1; policy accept;} add chain inet proxmox-firewall input {type filter hook input priority filter; policy drop;} add chain inet proxmox-firewall output {type filter hook output priority filter; policy accept;} +add chain inet proxmox-firewall forward {type filter hook forward priority filter; policy accept;} add chain bridge proxmox-firewall-guests allow-dhcp-in add chain bridge proxmox-firewall-guests allow-dhcp-out @@ -39,6 +43,8 @@ add chain bridge proxmox-firewall-guests pre-vm-out add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting priority 0; policy accept;} add chain bridge proxmox-firewall-guests pre-vm-in add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting priority 0; policy accept;} +add chain bridge proxmox-firewall-guests before-bridge +add chain bridge proxmox-firewall-guests forward {type filter hook forward priority 0; policy accept;} flush chain inet proxmox-firewall do-reject flush chain inet proxmox-firewall accept-management @@ -55,8 +61,12 @@ flush chain inet proxmox-firewall allow-icmp flush chain inet proxmox-firewall log-drop-smurfs flush chain inet proxmox-firewall default-in flush chain inet proxmox-firewall default-out +flush chain inet proxmox-firewall before-bridge +flush chain inet proxmox-firewall host-bridge-input +flush chain inet proxmox-firewall host-bridge-output flush chain inet proxmox-firewall input flush chain inet proxmox-firewall output +flush chain inet proxmox-firewall forward flush chain bridge proxmox-firewall-guests allow-dhcp-in flush chain bridge proxmox-firewall-guests allow-dhcp-out @@ -74,6 +84,8 @@ flush chain bridge proxmox-firewall-guests pre-vm-out flush chain bridge proxmox-firewall-guests vm-out flush chain bridge proxmox-firewall-guests pre-vm-in flush chain bridge proxmox-firewall-guests vm-in +flush chain bridge proxmox-firewall-guests before-bridge +flush chain bridge proxmox-firewall-guests forward table inet proxmox-firewall { chain do-reject { @@ -223,6 +235,25 @@ table inet proxmox-firewall { chain option-in {} chain option-out {} +map bridge-map { +type ifname : verdict +} + +chain before-bridge { +meta protocol arp accept +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +} + +chain host-bridge-input { +type filter hook input priority filter - 1; policy accept; +meta iifname vmap @bridge-map +} + +chain host-bridge-output { +type filter hook output priority filter + 1; policy accept; +meta oifname vmap @bridge-map +} + chain input { type filter hook input priority filter; policy accept; jump default-in @@ -240,12 +271,21 @@ table inet proxmox-firewall { jump cluster-out } +chain forward { +type filter hook forward priority filter; policy accept; +jump host-forward +jump cluster-forward +} + chain cluster-in {} chain cluster-out {} chain host-in {} chain host-out {} +chain cluster-forward {} +chain host-forward {} + chain ct-in {} } @@ -335,4 +375,18 @@ table bridge proxmox-firewall-guests { jump allow-icmp oifname vmap @vm-map-in } + +map bridge-map { +type ifname . ifname : verdict +} + +chain before-bridge { +meta protocol arp accept +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +} + +chain forward { +type filter hook forward priority 0; policy accept; +meta ibrname . meta obrname vmap @bridge-map +} } diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 347f3af..bb54023 100644 --- a/proxmox
[pve-devel] [PATCH proxmox-ve-rs v2 04/17] host: add struct representing bridge names
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/Cargo.toml| 1 + proxmox-ve-config/src/host/mod.rs | 1 + proxmox-ve-config/src/host/types.rs | 46 + 3 files changed, 48 insertions(+) create mode 100644 proxmox-ve-config/src/host/types.rs diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index 79ba164..d07dff2 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -10,6 +10,7 @@ exclude.workspace = true log = "0.4" anyhow = "1" nix = "0.26" +thiserror = "1.0.40" serde = { version = "1", features = [ "derive" ] } serde_json = "1" diff --git a/proxmox-ve-config/src/host/mod.rs b/proxmox-ve-config/src/host/mod.rs index b5614dd..b4ab6a6 100644 --- a/proxmox-ve-config/src/host/mod.rs +++ b/proxmox-ve-config/src/host/mod.rs @@ -1 +1,2 @@ +pub mod types; pub mod utils; diff --git a/proxmox-ve-config/src/host/types.rs b/proxmox-ve-config/src/host/types.rs new file mode 100644 index 000..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 { +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::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 for BridgeName { +fn as_ref(&self) -> &str { +&self.0 +} +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v2 21/25] add support for loading sdn firewall configuration
This also includes support for parsing rules referencing IPSets in the new SDN scope and generating those IPSets in the firewall. Loading SDN configuration is optional, since loading it requires root privileges which we do not have in all call sites. Adding the flag allows us to selectively load the SDN configuration only where required and at the same time allows us to only elevate privileges in the API where absolutely needed. Signed-off-by: Stefan Hanreich --- src/PVE/Firewall.pm | 59 +++-- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 09544ba..9943f2e 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -25,6 +25,7 @@ use PVE::Tools qw($IPV4RE $IPV6RE); use PVE::Tools qw(run_command lock_file dir_glob_foreach); use PVE::Firewall::Helpers; +use PVE::RS::Firewall::SDN; my $pvefw_conf_dir = "/etc/pve/firewall"; my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw"; @@ -1689,9 +1690,11 @@ sub verify_rule { if (my $value = $rule->{$name}) { if ($value =~ m/^\+/) { - if ($value =~ m@^\+(guest/|dc/)?(${ipset_name_pattern})$@) { + if ($value =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) { &$add_error($name, "no such ipset '$2'") - if !($cluster_conf->{ipset}->{$2} || ($fw_conf && $fw_conf->{ipset}->{$2})); + if !($cluster_conf->{ipset}->{$2} + || ($fw_conf && $fw_conf->{ipset}->{$2}) + || ($cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$2})); } else { &$add_error($name, "invalid ipset name '$value'"); @@ -2108,13 +2111,15 @@ sub ipt_gen_src_or_dst_match { my $match; if ($adr =~ m/^\+/) { - if ($adr =~ m@^\+(guest/|dc/)?(${ipset_name_pattern})$@) { + if ($adr =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) { my $scope = $1 // ""; my $name = $2; my $ipset_chain; - if ($scope ne 'dc/' && $fw_conf && $fw_conf->{ipset}->{$name}) { + if ((!$scope || $scope eq 'guest/') && $fw_conf && $fw_conf->{ipset}->{$name}) { $ipset_chain = compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion); - } elsif ($scope ne 'guest/' && $cluster_conf && $cluster_conf->{ipset}->{$name}) { + } elsif ((!$scope || $scope eq 'guest/') && $cluster_conf && $cluster_conf->{ipset}->{$name}) { + $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion); + } elsif ((!$scope || $scope eq 'sdn/') && $cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$name}) { $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion); } else { die "no such ipset '$name'\n"; @@ -3644,7 +3649,12 @@ sub lock_clusterfw_conf { } sub load_clusterfw_conf { -my ($filename) = @_; +my ($filename, $options) = @_; + +my $sdn_conf = {}; +if ($options->{load_sdn_config}) { + $sdn_conf = load_sdn_conf(); +} $filename = $clusterfw_conf_filename if !defined($filename); my $empty_conf = { @@ -3655,6 +3665,7 @@ sub load_clusterfw_conf { group_comments => {}, ipset => {} , ipset_comments => {}, + sdn => $sdn_conf, }; my $cluster_conf = generic_fw_config_parser($filename, $empty_conf, $empty_conf, 'cluster'); @@ -3663,6 +3674,39 @@ sub load_clusterfw_conf { return $cluster_conf; } +sub load_sdn_conf { +my $rpcenv = PVE::RPCEnvironment::get(); +my $authuser = $rpcenv->get_user(); + +my $guests = PVE::Cluster::get_vmlist(); +my $allowed_vms = []; +foreach my $vmid (sort keys %{$guests->{ids}}) { + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1); + push @$allowed_vms, $vmid; +} + +my $vnets = PVE::Network::SDN::Vnets::config(1); +my $privs = [ 'SDN.Audit', 'SDN.Allocate' ]; +my $allowed_vnets = []; +foreach my $vnet (sort keys %{$vnets->{ids}}) { + my $zone = $vnets->{ids}->{$vnet}->{zone}; + next if !$rpcenv->check_any($authuser, "/sdn/zones/$zone/$vnet", $privs, 1); + push @$allowed_vnets, $vnet; +} + +my $sdn_config = { + ipset => {} , + ipset_comments => {}, +}; + +eval { + $sdn_config = PVE::RS::Firewall::SDN::config($allowed_vnets, $allowed_vms); +}; +warn $@ if $@; + +ret
[pve-devel] [PATCH pve-manager v2 13/17] firewall: add vnet to firewall options component
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 --- www/manager6/grid/FirewallOptions.js | 38 +++- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js index 6aacb47be..fa482e0e4 100644 --- a/www/manager6/grid/FirewallOptions.js +++ b/www/manager6/grid/FirewallOptions.js @@ -2,7 +2,7 @@ Ext.define('PVE.FirewallOptions', { extend: 'Proxmox.grid.ObjectGrid', alias: ['widget.pveFirewallOptions'], -fwtype: undefined, // 'dc', 'node' or 'vm' +fwtype: undefined, // 'dc', 'node', 'vm' or 'vnet' base_url: undefined, @@ -13,14 +13,14 @@ Ext.define('PVE.FirewallOptions', { throw "missing base_url configuration"; } - if (me.fwtype === 'dc' || me.fwtype === 'node' || me.fwtype === 'vm') { - if (me.fwtype === 'node') { - me.cwidth1 = 250; - } - } else { + if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) { throw "unknown firewall option type"; } + if (me.fwtype === 'node') { + me.cwidth1 = 250; + } + let caps = Ext.state.Manager.get('GuiCap'); let canEdit = caps.vms['VM.Config.Network'] || caps.dc['Sys.Modify'] || caps.nodes['Sys.Modify']; @@ -81,6 +81,7 @@ Ext.define('PVE.FirewallOptions', { 'nf_conntrack_tcp_timeout_established', 7875, 250); add_log_row('log_level_in'); add_log_row('log_level_out'); + add_log_row('log_level_forward'); add_log_row('tcp_flags_log_level', 120); add_log_row('smurf_log_level'); add_boolean_row('nftables', gettext('nftables (tech preview)'), 0); @@ -114,6 +115,9 @@ Ext.define('PVE.FirewallOptions', { defaultValue: 'enable=1', }, }; + } else if (me.fwtype === 'vnet') { + add_boolean_row('enable', gettext('Firewall'), 0); + add_log_row('log_level_forward'); } if (me.fwtype === 'dc' || me.fwtype === 'vm') { @@ -150,6 +154,28 @@ Ext.define('PVE.FirewallOptions', { }; } + if (me.fwtype === 'vnet' || me.fwtype === 'dc') { + me.rows.policy_forward = { + header: gettext('Forward Policy'), + required: true, + defaultValue: 'ACCEPT', + editor: { + xtype: 'proxmoxWindowEdit', + subject: gettext('Forward Policy'), + items: { + xtype: 'pveFirewallPolicySelector', + name: 'policy_forward', + value: 'ACCEPT', + fieldLabel: gettext('Forward Policy'), + comboItems: [ + ['ACCEPT', 'ACCEPT'], + ['DROP', 'DROP'], + ], + }, + }, + }; + } + var edit_btn = new Ext.Button({ text: gettext('Edit'), disabled: true, -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v2 11/17] api: add vnet endpoints
Signed-off-by: Stefan Hanreich --- src/PVE/API2/Firewall/Makefile | 1 + src/PVE/API2/Firewall/Rules.pm | 84 + src/PVE/API2/Firewall/Vnet.pm | 168 + src/PVE/Firewall.pm| 10 ++ 4 files changed, 263 insertions(+) create mode 100644 src/PVE/API2/Firewall/Vnet.pm diff --git a/src/PVE/API2/Firewall/Makefile b/src/PVE/API2/Firewall/Makefile index e916755..325c4d3 100644 --- a/src/PVE/API2/Firewall/Makefile +++ b/src/PVE/API2/Firewall/Makefile @@ -9,6 +9,7 @@ LIB_SOURCES=\ Cluster.pm \ Host.pm \ VM.pm \ + Vnet.pm \ Groups.pm all: diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm index f5cb002..e57b3de 100644 --- a/src/PVE/API2/Firewall/Rules.pm +++ b/src/PVE/API2/Firewall/Rules.pm @@ -18,6 +18,25 @@ my $api_properties = { }, }; +=head3 check_privileges_for_method($class, $method_name, $param) + +If the permission checks from the register_method() call are not sufficient, +this function can be overriden for performing additional permission checks +before API methods are executed. If the permission check fails, this function +should die with an appropriate error message. The name of the method calling +this function is provided by C<$method_name> and the parameters of the API +method are provided by C<$param> + +Default implementation is a no-op to preserve backwards compatibility with +existing subclasses, since this got added later on. It preserves existing +behavior without having to change every subclass. + +=cut + +sub check_privileges_for_method { +my ($class, $method_name, $param) = @_; +} + sub lock_config { my ($class, $param, $code) = @_; @@ -94,6 +113,8 @@ sub register_get_rules { code => sub { my ($param) = @_; + $class->check_privileges_for_method('get_rules', $param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -192,6 +213,8 @@ sub register_get_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('get_rule', $param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -232,6 +255,8 @@ sub register_create_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('create_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -293,6 +318,8 @@ sub register_update_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('update_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -359,6 +386,8 @@ sub register_delete_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('delete_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -634,4 +663,59 @@ sub save_rules { __PACKAGE__->register_handlers(); +package PVE::API2::Firewall::VnetRules; + +use strict; +use warnings; +use PVE::JSONSchema qw(get_standard_option); + +use base qw(PVE::API2::Firewall::RulesBase); + +__PACKAGE__->additional_parameters({ +vnet => get_standard_option('pve-sdn-vnet-id'), +}); + +sub check_privileges_for_method { +my ($class, $method_name, $param) = @_; + +if ($method_name eq 'get_rule' || $method_name eq 'get_rules') { + PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Audit', 'SDN.Allocate']); +} elsif ($method_name =~ '(update|create|delete)_rule') { + PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Allocate']); +} else { + die "unknown method: $method_name"; +} +} + +sub rule_env { +my ($class, $param) = @_; + +return 'vnet'; +} + +sub lock_config { +my ($class, $param, $code) = @_; + +PVE::Firewall::lock_vnetfw_conf($param->{vnet}, 10, $code, $param); +} + +sub load_config { +my ($class, $param) = @_; + +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); +my $fw_conf = PVE::Firewall::load_vnetfw_conf($cluster_conf, 'vnet', $param->{vnet}); +my $rules = $fw_conf->{rules}; + +return ($cluster_conf, $fw_conf, $rules); +} + +sub save_rules { +my ($class, $param, $fw_conf, $rules) = @_; + +$fw_conf->{rules} = $rules; +PVE::Firewall::save_vnetfw_conf($param->{vnet},
[pve-devel] [PATCH proxmox-ve-rs v2 16/25] sdn: config: add method for generating ipsets
We generate the following ipsets for every vnet in the running sdn configuration: * {vnet}-all: contains all subnets of the vnet * {vnet}-no-gateway: contains all subnets of the vnet except for all gateways * {vnet}-gateway: contains all gateways in the vnet * {vnet}-dhcp: contains all dhcp ranges configured in the vnet All of them are in the new SDN scope, so the fully qualified name would look something like this: `+sdn/{vnet-all}`. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/sdn/config.rs | 72 + 1 file changed, 72 insertions(+) diff --git a/proxmox-ve-config/src/sdn/config.rs b/proxmox-ve-config/src/sdn/config.rs index b71084b..f6fc8c2 100644 --- a/proxmox-ve-config/src/sdn/config.rs +++ b/proxmox-ve-config/src/sdn/config.rs @@ -529,6 +529,78 @@ impl SdnConfig { self.zones() .flat_map(|zone| zone.vnets().map(move |vnet| (zone, vnet))) } + +/// Generates multiple [`Ipset`] for all SDN VNets. +/// +/// # Arguments +/// * `filter` - A [`Allowlist`] of VNet names for which IPsets should get returned +/// +/// It generates the following [`Ipset`] for all VNets in the config: +/// * all: Contains all CIDRs of all subnets in the VNet +/// * gateway: Contains all gateways of all subnets in the VNet (if any gateway exists) +/// * no-gateway: Matches all CIDRs of all subnets, except for the gateways (if any gateway +/// exists) +/// * dhcp: Contains all DHCP ranges of all subnets in the VNet (if any dhcp range exists) +pub fn ipsets<'a>( +&'a self, +filter: impl Into>>, +) -> impl Iterator + '_ { +let filter = filter.into(); + +self.zones +.values() +.flat_map(|zone| zone.vnets()) +.filter(move |vnet| { +filter +.map(|list| list.is_allowed(&vnet.name)) +.unwrap_or(true) +}) +.flat_map(|vnet| { +let mut ipset_all = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-all", vnet.name), +)); +ipset_all.comment = Some(format!("All subnets of VNet {}", vnet.name)); + +let mut ipset_gateway = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-gateway", vnet.name), +)); +ipset_gateway.comment = Some(format!("All gateways of VNet {}", vnet.name)); + +let mut ipset_all_wo_gateway = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-no-gateway", vnet.name), +)); +ipset_all_wo_gateway.comment = Some(format!( +"All subnets of VNet {}, excluding gateways", +vnet.name +)); + +let mut ipset_dhcp = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-dhcp", vnet.name), +)); +ipset_dhcp.comment = Some(format!("DHCP ranges of VNet {}", vnet.name)); + +for subnet in vnet.subnets.values() { +ipset_all.push((*subnet.cidr()).into()); + +ipset_all_wo_gateway.push((*subnet.cidr()).into()); + +if let Some(gateway) = subnet.gateway { +let gateway_nomatch = IpsetEntry::new(gateway, true, None); +ipset_all_wo_gateway.push(gateway_nomatch); + +ipset_gateway.push(gateway.into()); +} + + ipset_dhcp.extend(subnet.dhcp_range.iter().cloned().map(IpsetEntry::from)); +} + +[ipset_all, ipset_gateway, ipset_all_wo_gateway, ipset_dhcp] +}) +} } impl TryFrom for SdnConfig { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v2 19/25] config: tests: add support for loading sdn and ipam config
Also add example SDN configuration files that get automatically loaded, which can be used for future tests. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/config.rs| 69 +++ .../tests/input/.running-config.json | 45 proxmox-firewall/tests/input/ipam.db | 32 + proxmox-firewall/tests/integration_tests.rs | 10 +++ proxmox-nftables/src/types.rs | 2 +- 5 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 proxmox-firewall/tests/input/.running-config.json create mode 100644 proxmox-firewall/tests/input/ipam.db diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs index 5bd2512..c27aac6 100644 --- a/proxmox-firewall/src/config.rs +++ b/proxmox-firewall/src/config.rs @@ -16,6 +16,10 @@ use proxmox_ve_config::guest::{GuestEntry, GuestMap}; use proxmox_nftables::command::{CommandOutput, Commands, List, ListOutput}; use proxmox_nftables::types::ListChain; use proxmox_nftables::NftClient; +use proxmox_ve_config::sdn::{ +config::{RunningConfig, SdnConfig}, +ipam::{Ipam, IpamJson}, +}; pub trait FirewallConfigLoader { fn cluster(&self) -> Result>, Error>; @@ -27,6 +31,8 @@ pub trait FirewallConfigLoader { guest: &GuestEntry, ) -> Result>, Error>; fn guest_firewall_config(&self, vmid: &Vmid) -> Result>, Error>; +fn sdn_running_config(&self) -> Result>, Error>; +fn ipam(&self) -> Result>, Error>; } #[derive(Default)] @@ -58,6 +64,9 @@ fn open_config_file(path: &str) -> Result, Error> { const CLUSTER_CONFIG_PATH: &str = "/etc/pve/firewall/cluster.fw"; const HOST_CONFIG_PATH: &str = "/etc/pve/local/host.fw"; +const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config"; +const SDN_IPAM_PATH: &str = "/etc/pve/priv/ipam.db"; + impl FirewallConfigLoader for PveFirewallConfigLoader { fn cluster(&self) -> Result>, Error> { log::info!("loading cluster config"); @@ -119,6 +128,32 @@ impl FirewallConfigLoader for PveFirewallConfigLoader { Ok(None) } + +fn sdn_running_config(&self) -> Result>, Error> { +log::info!("loading SDN running-config"); + +let fd = open_config_file(SDN_RUNNING_CONFIG_PATH)?; + +if let Some(file) = fd { +let buf_reader = Box::new(BufReader::new(file)) as Box; +return Ok(Some(buf_reader)); +} + +Ok(None) +} + +fn ipam(&self) -> Result>, Error> { +log::info!("loading IPAM config"); + +let fd = open_config_file(SDN_IPAM_PATH)?; + +if let Some(file) = fd { +let buf_reader = Box::new(BufReader::new(file)) as Box; +return Ok(Some(buf_reader)); +} + +Ok(None) +} } pub trait NftConfigLoader { @@ -150,6 +185,8 @@ pub struct FirewallConfig { host_config: HostConfig, guest_config: BTreeMap, nft_config: BTreeMap, +sdn_config: Option, +ipam_config: Option, } impl FirewallConfig { @@ -207,6 +244,28 @@ impl FirewallConfig { Ok(guests) } +pub fn parse_sdn( +firewall_loader: &dyn FirewallConfigLoader, +) -> Result, Error> { +Ok(match firewall_loader.sdn_running_config()? { +Some(data) => { +let running_config: RunningConfig = serde_json::from_reader(data)?; +Some(SdnConfig::try_from(running_config)?) +} +_ => None, +}) +} + +pub fn parse_ipam(firewall_loader: &dyn FirewallConfigLoader) -> Result, Error> { +Ok(match firewall_loader.ipam()? { +Some(data) => { +let raw_ipam: IpamJson = serde_json::from_reader(data)?; +Some(Ipam::try_from(raw_ipam)?) +} +_ => None, +}) +} + pub fn parse_nft( nft_loader: &dyn NftConfigLoader, ) -> Result, Error> { @@ -233,6 +292,8 @@ impl FirewallConfig { cluster_config: Self::parse_cluster(firewall_loader)?, host_config: Self::parse_host(firewall_loader)?, guest_config: Self::parse_guests(firewall_loader)?, +sdn_config: Self::parse_sdn(firewall_loader)?, +ipam_config: Self::parse_ipam(firewall_loader)?, nft_config: Self::parse_nft(nft_loader)?, }) } @@ -253,6 +314,14 @@ impl FirewallConfig { &self.nft_config } +pub fn sdn(&self) -> Option<&SdnConfig> { +self.sdn_config.as_ref() +} + +pub fn ipam(&self) -> Option<&Ipam> { +self.ipam_config.as_ref() +} + pub fn is_enabled(&self) -> bool { self.cluster().is_enabled() &am
[pve-devel] [PATCH pve-docs v2 17/17] firewall: add documentation for forward direction
Additionally add information about the SDN VNet firewall, which has been introduced with this changes. Signed-off-by: Stefan Hanreich --- Makefile | 1 + gen-pve-firewall-vnet-opts.pl | 12 pve-firewall-vnet-opts.adoc | 8 ++ pve-firewall.adoc | 53 --- 4 files changed, 70 insertions(+), 4 deletions(-) create mode 100755 gen-pve-firewall-vnet-opts.pl create mode 100644 pve-firewall-vnet-opts.adoc diff --git a/Makefile b/Makefile index 801a2a3..f30d77a 100644 --- a/Makefile +++ b/Makefile @@ -62,6 +62,7 @@ GEN_SCRIPTS= \ gen-pve-firewall-macros-adoc.pl \ gen-pve-firewall-rules-opts.pl \ gen-pve-firewall-vm-opts.pl \ + gen-pve-firewall-vnet-opts.pl \ gen-output-format-opts.pl API_VIEWER_FILES= \ diff --git a/gen-pve-firewall-vnet-opts.pl b/gen-pve-firewall-vnet-opts.pl new file mode 100755 index 000..c9f4f13 --- /dev/null +++ b/gen-pve-firewall-vnet-opts.pl @@ -0,0 +1,12 @@ +#!/usr/bin/perl + +use lib '.'; +use strict; +use warnings; + +use PVE::Firewall; +use PVE::RESTHandler; + +my $prop = $PVE::Firewall::vnet_option_properties; + +print PVE::RESTHandler::dump_properties($prop); diff --git a/pve-firewall-vnet-opts.adoc b/pve-firewall-vnet-opts.adoc new file mode 100644 index 000..ed1e88f --- /dev/null +++ b/pve-firewall-vnet-opts.adoc @@ -0,0 +1,8 @@ +`enable`: `` ('default =' `0`):: + +Enable/disable firewall rules. + +`policy_forward`: `` :: + +Forward policy. + diff --git a/pve-firewall.adoc b/pve-firewall.adoc index b428703..339a42f 100644 --- a/pve-firewall.adoc +++ b/pve-firewall.adoc @@ -52,14 +52,22 @@ The Proxmox VE firewall groups the network into the following logical zones: Host:: -Traffic from/to a cluster node +Traffic from/to a cluster node or traffic forwarded by a cluster node VM:: Traffic from/to a specific VM -For each zone, you can define firewall rules for incoming and/or -outgoing traffic. +VNet:: + +Traffic flowing through a SDN VNet + +For each zone, you can define firewall rules for incoming, outgoing or +forwarded traffic. + +IMPORTANT: Creating rules for forwarded traffic or on a VNet-level is currently +only possible when using the new +xref:pve_firewall_nft[nftables-based proxmox-firewall]. Configuration Files @@ -202,10 +210,46 @@ can selectively enable the firewall for each interface. This is required in addition to the general firewall `enable` option. +[[pve_firewall_vnet_configuration]] +VNet Configuration +~~ +VNet related configuration is read from: + + /etc/pve/sdn/firewall/.fw + +This can be used for setting firewall configuration globally on a VNet level, +without having to set firewall rules for each VM inside the VNet separately. It +can only contain rules for the `FORWARD` direction, since there is no notion of +incoming or outgoing traffic. This affects all traffic travelling from one +bridge port to another, including the host interface. + +WARNING: This feature is currently only available for the new +xref:pve_firewall_nft[nftables-based proxmox-firewall] + +Since traffic passing the `FORWARD` chain is bi-directional, you need to create +rules for both directions if you want traffic to pass both ways. For instance if +HTTP traffic for a specific host should be allowed, you would need to create the +following rules: + + +FORWARD ACCEPT -dest 10.0.0.1 -dport 80 +FORWARD ACCEPT -source 10.0.0.1 -sport 80 + + +`[OPTIONS]`:: + +This is used to set VNet related firewall options. + +include::pve-firewall-vnet-opts.adoc[] + +`[RULES]`:: + +This section contains VNet specific firewall rules. + Firewall Rules -- -Firewall rules consists of a direction (`IN` or `OUT`) and an +Firewall rules consists of a direction (`IN`, `OUT` or `FORWARD`) and an action (`ACCEPT`, `DENY`, `REJECT`). You can also specify a macro name. Macros contain predefined sets of rules and options. Rules can be disabled by prefixing them with `|`. @@ -639,6 +683,7 @@ Ports used by {pve} * live migration (VM memory and local-disk data): 6-60050 (TCP) +[[pve_firewall_nft]] nftables -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v2 05/17] nftables: derive additional traits for nftables types
Signed-off-by: Stefan Hanreich --- proxmox-nftables/src/types.rs | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs index 3101436..d8f3b62 100644 --- a/proxmox-nftables/src/types.rs +++ b/proxmox-nftables/src/types.rs @@ -16,10 +16,10 @@ use proxmox_ve_config::firewall::types::ipset::IpsetName; #[cfg(feature = "config-ext")] use proxmox_ve_config::guest::types::Vmid; -#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] pub struct Handle(i32); -#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum TableFamily { Ip, @@ -187,7 +187,7 @@ impl From for RateTimescale { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] pub struct TableName { family: TableFamily, name: String, @@ -210,7 +210,7 @@ impl TableName { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] pub struct TablePart { family: TableFamily, table: String, -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-docs v2 25/25] sdn: add documentation for firewall integration
Signed-off-by: Stefan Hanreich --- pvesdn.adoc | 92 + 1 file changed, 92 insertions(+) diff --git a/pvesdn.adoc b/pvesdn.adoc index 39de80f..c187365 100644 --- a/pvesdn.adoc +++ b/pvesdn.adoc @@ -702,6 +702,98 @@ For more information please consult the documentation of xref:pvesdn_ipam_plugin_pveipam[the PVE IPAM plugin]. Changing DHCP leases is currently not supported for the other IPAM plugins. +Firewall Integration + + +SDN integrates with the Proxmox VE firewall by automatically generating IPSets +which can then be referenced in the source / destination fields of firewall +rules. This happens automatically for VNets and IPAM entries. + +VNets and Subnets +~ + +The firewall automatically generates the following IPSets in the SDN scope for +every VNet: + +`vnet-all`:: + Contains the CIDRs of all subnets in a VNet +`vnet-gateway`:: + Contains the IPs of the gateways of all subnets in a VNet +`vnet-no-gateway`:: + Contains the CIDRs of all subnets in a VNet, but excludes the gateways +`vnet-dhcp`:: + Contains all DHCP ranges configured in the subnets in a VNet + +When making changes to your configuration, the IPSets update automatically, so +you do not have to update your firewall rules when changing the configuration of +your Subnets. + +Simple Zone Example +^^^ + +Assuming the configuration below for a VNet and its contained subnets: + + +# /etc/pve/sdn/vnets.cfg + +vnet: vnet0 +zone simple + +# /etc/pve/sdn/subnets.cfg + +subnet: simple-192.0.2.0-24 +vnet vnet0 +dhcp-range start-address=192.0.2.100,end-address=192.0.2.199 +gateway 192.0.2.1 + +subnet: simple-2001:db8::-64 +vnet vnet0 +dhcp-range start-address=2001:db8::1000,end-address=2001:db8::1999 +gateway 2001:db8::1 + + +In this example we configured an IPv4 subnet in the VNet `vnet0`, with +'192.0.2.0/24' as its IP Range, '192.0.2.1' as the gateway and the DHCP range is +'192.0.2.100' - '192.0.2.199'. + +Additionally we configured an IPv6 subnet with '2001:db8::/64' as the IP range, +'2001:db8::1' as the gateway and a DHCP range of '2001:db8::1000' - +'2001:db8::1999'. + +The respective auto-generated IPsets for vnet0 would then contain the following +elements: + +`vnet0-all`:: +* '192.0.2.0/24' +* '2001:db8::/64' +`vnet0-gateway`:: +* '192.0.2.1' +* '2001:db8::1' +`vnet0-no-gateway`:: +* '192.0.2.0/24' +* '2001:db8::/64' +* '!192.0.2.1' +* '!2001:db8::1' +`vnet0-dhcp`:: +* '192.0.2.100 - 192.0.2.199' +* '2001:db8::1000 - 2001:db8::1999' + +IPAM + + +If you are using the built-in PVE IPAM, then the firewall automatically +generates an IPset for every guest that has entries in the IPAM. The respective +IPset for a guest with ID 100 would be `guest-ipam-100`. It contains all IP +addresses from all IPAM entries. So if guest 100 is member of multiple VNets, +then the IPset would contain the IPs from *all* VNets. + +When entries get added / updated / deleted, then the respective IPSets will be +updated accordingly. + +WARNING: When removing all entries for a guest and there are firewall rules +still referencing the auto-generated IPSet then the firewall will fail to update +the ruleset, since it references a non-existing IPSet. + [[pvesdn_setup_examples]] Examples -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
v2 here: https://lore.proxmox.com/pve-devel/20241010155650.255698-1-s.hanre...@proxmox.com/T/ ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC firewall/proxmox{-ve-rs, -firewall, -perl-rs} 00/21] autogenerate ipsets for sdn objects
v2 here: https://lore.proxmox.com/pve-devel/20241010155637.255451-1-s.hanre...@proxmox.com/T/ ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v2 20/25] ipsets: autogenerate ipsets for vnets and ipam
They act like virtual ipsets, similar to ipfilter-net, that can be used for defining firewall rules for sdn objects dynamically. The changes in proxmox-ve-config also introduced a dedicated struct for representing ip ranges, so we update the existing code, so that it uses that struct as well. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/firewall.rs | 22 +- proxmox-firewall/src/object.rs| 41 +- .../integration_tests__firewall.snap | 1288 + proxmox-nftables/src/expression.rs| 17 +- 4 files changed, 1354 insertions(+), 14 deletions(-) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 941aa20..347f3af 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -197,6 +197,27 @@ impl Firewall { self.reset_firewall(&mut commands); let cluster_host_table = Self::cluster_table(); +let guest_table = Self::guest_table(); + +if let Some(sdn_config) = self.config.sdn() { +let ipsets = sdn_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} + +if let Some(ipam_config) = self.config.ipam() { +let ipsets = ipam_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} if self.config.host().is_enabled() { log::info!("creating cluster / host configuration"); @@ -242,7 +263,6 @@ impl Firewall { commands.push(Delete::table(TableName::from(Self::cluster_table(; } -let guest_table = Self::guest_table(); let enabled_guests: BTreeMap<&Vmid, &GuestConfig> = self .config .guests() diff --git a/proxmox-firewall/src/object.rs b/proxmox-firewall/src/object.rs index 32c4ddb..cf7e773 100644 --- a/proxmox-firewall/src/object.rs +++ b/proxmox-firewall/src/object.rs @@ -72,20 +72,37 @@ impl ToNftObjects for Ipset { let mut nomatch_elements = Vec::new(); for element in self.iter() { -let cidr = match &element.address { -IpsetAddress::Cidr(cidr) => cidr, -IpsetAddress::Alias(alias) => env -.alias(alias) -.ok_or(format_err!("could not find alias {alias} in environment"))? -.address(), +let expression = match &element.address { +IpsetAddress::Range(range) => { +if family != range.family() { +continue; +} + +Expression::from(range) +} +IpsetAddress::Cidr(cidr) => { +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} +IpsetAddress::Alias(alias) => { +let cidr = env +.alias(alias) +.ok_or_else(|| { +format_err!("could not find alias {alias} in environment") +})? +.address(); + +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} }; -if family != cidr.family() { -continue; -} - -let expression = Expression::from(Prefix::from(cidr)); - if element.nomatch { nomatch_elements.push(expression); } else { diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 40d4405..e1b599c 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -202,6 +202,1294 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, +{ + "add": { +"set": { + "family&qu
[pve-devel] [PATCH pve-manager v2 15/17] sdn: add firewall panel
Expose the ability to create vnet-level firewalls in the PVE UI Signed-off-by: Stefan Hanreich --- www/manager6/Makefile| 2 + www/manager6/dc/Config.js| 8 +++ www/manager6/sdn/FirewallPanel.js| 48 + www/manager6/sdn/FirewallVnetView.js | 77 4 files changed, 135 insertions(+) create mode 100644 www/manager6/sdn/FirewallPanel.js create mode 100644 www/manager6/sdn/FirewallVnetView.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index 2c3a822bd..13a1c4177 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -279,6 +279,8 @@ JSSRC= \ sdn/SubnetView.js \ sdn/ZoneContentView.js \ sdn/ZoneContentPanel.js \ + sdn/FirewallPanel.js\ + sdn/FirewallVnetView.js \ sdn/ZoneView.js \ sdn/IpamEdit.js \ sdn/OptionsPanel.js \ diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 720edefc6..d44554954 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -221,6 +221,14 @@ Ext.define('PVE.dc.Config', { hidden: true, iconCls: 'fa fa-map-signs', itemId: 'sdnmappings', + }, + { + xtype: 'pveSDNFirewall', + groups: ['sdn'], + title: gettext('Firewall'), + hidden: true, + iconCls: 'fa fa-shield', + itemId: 'sdnfirewall', }); } diff --git a/www/manager6/sdn/FirewallPanel.js b/www/manager6/sdn/FirewallPanel.js new file mode 100644 index 0..f02ff5a35 --- /dev/null +++ b/www/manager6/sdn/FirewallPanel.js @@ -0,0 +1,48 @@ + +Ext.define('PVE.sdn.FirewallPanel', { +extend: 'Ext.panel.Panel', +alias: 'widget.pveSDNFirewall', + +title: 'VNet', + +initComponent: function() { + let me = this; + + let tabPanel = Ext.create('Ext.TabPanel', { + fullscreen: true, + region: 'center', + border: false, + split: true, + disabled: true, + items: [ + { + xtype: 'pveFirewallRules', + title: gettext('Rules'), + list_refs_url: '/cluster/firewall/refs', + firewall_type: 'vnet', + }, + { + xtype: 'pveFirewallOptions', + title: gettext('Options'), + fwtype: 'vnet', + }, + ], + }); + + let vnetPanel = Ext.createWidget('pveSDNFirewallVnetView', { + title: 'VNets', + region: 'west', + border: false, + split: true, + forceFit: true, + tabPanel, + }); + + Ext.apply(me, { + layout: 'border', + items: [vnetPanel, tabPanel], + }); + + me.callParent(); +}, +}); diff --git a/www/manager6/sdn/FirewallVnetView.js b/www/manager6/sdn/FirewallVnetView.js new file mode 100644 index 0..861d4b5be --- /dev/null +++ b/www/manager6/sdn/FirewallVnetView.js @@ -0,0 +1,77 @@ +Ext.define('PVE.sdn.FirewallVnetView', { +extend: 'Ext.grid.GridPanel', +alias: 'widget.pveSDNFirewallVnetView', + +stateful: true, +stateId: 'grid-sdn-vnet-firewall', + +tabPanel: undefined, + +getRulesPanel: function() { + let me = this; + return me.tabPanel.items.getAt(0); +}, + +getOptionsPanel: function() { + let me = this; + return me.tabPanel.items.getAt(1); +}, + +initComponent: function() { + let me = this; + + let store = new Ext.data.Store({ + model: 'pve-sdn-vnet', + proxy: { +type: 'proxmox', + url: "/api2/json/cluster/sdn/vnets", + }, + sorters: { + property: ['zone', 'vnet'], + direction: 'ASC', + }, + }); + + let reload = () => store.load(); + + let sm = Ext.create('Ext.selection.RowModel', {}); + + Ext.apply(me, { + store: store, + reloadStore: reload, + selModel: sm, + viewConfig: { + trackOver: false, + }, + columns: [ + {
[pve-devel] [PATCH pve-manager v2 12/17] firewall: add forward direction to rule panel
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 --- www/manager6/dc/Config.js | 1 + www/manager6/dc/SecurityGroups.js | 1 + www/manager6/grid/FirewallRules.js | 32 +- www/manager6/lxc/Config.js | 1 + www/manager6/node/Config.js| 1 + www/manager6/qemu/Config.js| 1 + 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index ddbb58b12..720edefc6 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -241,6 +241,7 @@ Ext.define('PVE.dc.Config', { list_refs_url: '/cluster/firewall/refs', iconCls: 'fa fa-shield', itemId: 'firewall', + firewall_type: 'dc', }, { xtype: 'pveFirewallOptions', diff --git a/www/manager6/dc/SecurityGroups.js b/www/manager6/dc/SecurityGroups.js index 9e26b84c9..e7aa8081c 100644 --- a/www/manager6/dc/SecurityGroups.js +++ b/www/manager6/dc/SecurityGroups.js @@ -214,6 +214,7 @@ Ext.define('PVE.SecurityGroups', { list_refs_url: '/cluster/firewall/refs', tbar_prefix: '' + gettext('Rules') + ':', border: false, + firewall_type: 'group', }, { xtype: 'pveSecurityGroupList', diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js index 11881bf79..5e7da2dda 100644 --- a/www/manager6/grid/FirewallRules.js +++ b/www/manager6/grid/FirewallRules.js @@ -147,6 +147,16 @@ let ICMPV6_TYPE_NAMES_STORE = Ext.create('Ext.data.Store', { ], }); +let DEFAULT_ALLOWED_DIRECTIONS = ['in', 'out']; + +let ALLOWED_DIRECTIONS = { +'dc': ['in', 'out', 'forward'], +'node': ['in', 'out', 'forward'], +'group': ['in', 'out', 'forward'], +'vm': ['in', 'out'], +'vnet': ['forward'], +}; + Ext.define('PVE.FirewallRulePanel', { extend: 'Proxmox.panel.InputPanel', @@ -154,6 +164,8 @@ Ext.define('PVE.FirewallRulePanel', { list_refs_url: undefined, +firewall_type: undefined, + onGetValues: function(values) { var me = this; @@ -178,6 +190,8 @@ Ext.define('PVE.FirewallRulePanel', { throw "no list_refs_url specified"; } + let allowed_directions = ALLOWED_DIRECTIONS[me.firewall_type] ?? DEFAULT_ALLOWED_DIRECTIONS; + me.column1 = [ { // hack: we use this field to mark the form 'dirty' when the @@ -190,8 +204,8 @@ Ext.define('PVE.FirewallRulePanel', { { xtype: 'proxmoxKVComboBox', name: 'type', - value: 'in', - comboItems: [['in', 'in'], ['out', 'out']], + value: allowed_directions[0], + comboItems: allowed_directions.map((dir) => [dir, dir]), fieldLabel: gettext('Direction'), allowBlank: false, }, @@ -387,6 +401,8 @@ Ext.define('PVE.FirewallRuleEdit', { allow_iface: false, +firewall_type: undefined, + initComponent: function() { var me = this; @@ -412,6 +428,7 @@ Ext.define('PVE.FirewallRuleEdit', { list_refs_url: me.list_refs_url, allow_iface: me.allow_iface, rule_pos: me.rule_pos, + firewall_type: me.firewall_type, }); Ext.apply(me, { @@ -555,6 +572,8 @@ Ext.define('PVE.FirewallRules', { allow_groups: true, allow_iface: false, +firewall_type: undefined, + setBaseUrl: function(url) { var me = this; @@ -661,7 +680,7 @@ Ext.define('PVE.FirewallRules', { var type = rec.data.type; var editor; - if (type === 'in' || type === 'out') { + if (type === 'in' || type === 'out' || type === 'forward') { editor = 'PVE.FirewallRuleEdit'; } else if (type === 'group') { editor = 'PVE.FirewallGroupRuleEdit'; @@ -670,6 +689,7 @@ Ext.define('PVE.FirewallRules', { } var win = Ext.create(editor, { + firewall_type: me.firewall_type, digest: rec.data.digest,
[pve-devel] [PATCH pve-manager v2 14/17] firewall: make base_url dynamically configurable in options component
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 --- www/manager6/grid/FirewallOptions.js | 38 ++-- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js index fa482e0e4..d98cbe22c 100644 --- a/www/manager6/grid/FirewallOptions.js +++ b/www/manager6/grid/FirewallOptions.js @@ -9,10 +9,6 @@ Ext.define('PVE.FirewallOptions', { initComponent: function() { var me = this; - if (!me.base_url) { - throw "missing base_url configuration"; - } - if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) { throw "unknown firewall option type"; } @@ -197,23 +193,49 @@ Ext.define('PVE.FirewallOptions', { }; Ext.apply(me, { - url: "/api2/json" + me.base_url, tbar: [edit_btn], - editorConfig: { - url: '/api2/extjs/' + me.base_url, - }, listeners: { itemdblclick: () => { if (canEdit) { me.run_editor(); } }, selectionchange: set_button_status, }, }); + if (me.base_url) { + me.applyUrl(me.base_url); + } else { + me.rstore = Ext.create('Proxmox.data.ObjectStore', { + interval: me.interval, + extraParams: me.extraParams, + rows: me.rows, + }); + } + me.callParent(); me.on('activate', me.rstore.startUpdate); me.on('destroy', me.rstore.stopUpdate); me.on('deactivate', me.rstore.stopUpdate); }, +applyUrl: function(url) { + let me = this; + + Ext.apply(me, { + url: "/api2/json" + url, + editorConfig: { + url: '/api2/extjs/' + url, + }, + }); +}, +setBaseUrl: function(url) { + let me = this; + + me.base_url = url; + + me.applyUrl(url); + + me.rstore.getProxy().setConfig('url', `/api2/extjs/${url}`); + me.rstore.reload(); +}, }); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 08/25] iprange: add methods for converting an ip range to cidrs
This is mainly used in proxmox-perl-rs, so the generated ipsets can be used in pve-firewall where only CIDRs are supported. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 818 ++ 1 file changed, 818 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 019884c..4baafa7 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -303,6 +303,17 @@ impl IpRange { ) -> Result { Ok(IpRange::V6(AddressRange::new_v6(start, end)?)) } + +/// converts an IpRange into the minimal amount of CIDRs +/// +/// see the concrete implementations of [`AddressRange`] or [`AddressRange`] +/// respectively +pub fn to_cidrs(&self) -> Vec { +match self { +IpRange::V4(range) => range.to_cidrs().into_iter().map(Cidr::from).collect(), +IpRange::V6(range) => range.to_cidrs().into_iter().map(Cidr::from).collect(), +} +} } impl std::str::FromStr for IpRange { @@ -362,6 +373,71 @@ impl AddressRange { Ok(Self { start, end }) } + +/// Returns the minimum amount of CIDRs that exactly represent the range +/// +/// The idea behind this algorithm is as follows: +/// +/// Start iterating with current = start of the IP range +/// +/// Find two netmasks +/// * The largest CIDR that the current IP can be the first of +/// * The largest CIDR that *only* contains IPs from current - end +/// +/// Add the smaller of the two CIDRs to our result and current to the first IP that is in +/// the range but not in the CIDR we just added. Proceed until we reached the end of the IP +/// range. +/// +pub fn to_cidrs(&self) -> Vec { +let mut cidrs = Vec::new(); + +let mut current = u32::from_be_bytes(self.start.octets()); +let end = u32::from_be_bytes(self.end.octets()); + +if current == end { +// valid Ipv4 since netmask is 32 +cidrs.push(Ipv4Cidr::new(current, 32).unwrap()); +return cidrs; +} + +// special case this, since this is the only possibility of overflow +// when calculating delta_min_mask - makes everything a lot easier +if current == u32::MIN && end == u32::MAX { +// valid Ipv4 since it is `0.0.0.0/0` +cidrs.push(Ipv4Cidr::new(current, 0).unwrap()); +return cidrs; +} + +while current <= end { +// netmask of largest CIDR that current IP can be the first of +// cast is safe, because trailing zeroes can at most be 32 +let current_max_mask = IPV4_LENGTH - (current.trailing_zeros() as u8); + +// netmask of largest CIDR that *only* contains IPs of the remaining range +// is at most 32 due to unwrap_or returning 32 and ilog2 being at most 31 +let delta_min_mask = ((end - current) + 1) // safe due to special case above +.checked_ilog2() // should never occur due to special case, but for good measure +.map(|mask| IPV4_LENGTH - mask as u8) +.unwrap_or(IPV4_LENGTH); + +// at most 32, due to current/delta being at most 32 +let netmask = u8::max(current_max_mask, delta_min_mask); + +// netmask is at most 32, therefore safe to unwrap +cidrs.push(Ipv4Cidr::new(current, netmask).unwrap()); + +let delta = 2u32.saturating_pow((IPV4_LENGTH - netmask).into()); + +if let Some(result) = current.checked_add(delta) { +current = result +} else { +// we reached the end of IP address space +break; +} +} + +cidrs +} } impl AddressRange { @@ -377,6 +453,61 @@ impl AddressRange { Ok(Self { start, end }) } + +/// Returns the minimum amount of CIDRs that exactly represent the range +/// +/// This function works analogous to the IPv4 version, please refer to the respective +/// documentation of [`AddressRange`] +pub fn to_cidrs(&self) -> Vec { +let mut cidrs = Vec::new(); + +let mut current = u128::from_be_bytes(self.start.octets()); +let end = u128::from_be_bytes(self.end.octets()); + +if current == end { +// valid Ipv6 since netmask is 128 +cidrs.push(Ipv6Cidr::new(current, 128).unwrap()); +return cidrs; +} + +// special case this, since this is the only possibility of overflow +// when calculating delta_min_mask - makes everything a lot easier +if current == u128::MIN && end == u128::MAX { +// valid Ipv6 since it is `::/0` +cidrs.push(Ipv6Cidr::new(current, 0).unwrap()); +
[pve-devel] [PATCH proxmox-firewall v2 06/17] sdn: add support for loading vnet-level firewall config
Signed-off-by: Stefan Hanreich --- 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>, Error>; fn sdn_running_config(&self) -> Result>, Error>; fn ipam(&self) -> Result>, Error>; +fn bridge_list(&self) -> Result, Error>; +fn bridge_firewall_config( +&self, +bridge_name: &BridgeName, +) -> Result>, Error>; } #[derive(Default)] @@ -61,8 +68,31 @@ fn open_config_file(path: &str) -> Result, Error> { } } +fn open_config_folder(path: &str) -> Result, 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 { +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, 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>, 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; +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, +bridge_config: BTreeMap, nft_config: BTreeMap, sdn_config: Option, ipam_config: Option, @@ -284,6 +347,22 @@ impl FirewallConfig { Ok(chains) } +pub fn parse_bridges( +firewall_loader: &dyn FirewallConfigLoader, +) -> Result, 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) +} +
[pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets
## Introduction This patch series introduces a new direction for firewall rules: forward. Additionally this patch series introduces defining firewall rules on a vnet level. ## Use Cases For hosts: * hosts utilizing NAT can define firewall rules for NATed traffic * hosts utilizing EVPN zones can define rules for exit node traffic * hosts acting as gateway can firewall the traffic that passes through them For vnets: * can create firewall rules globally without having to attach/update security groups to every newly created VM This patch series is particularly useful when combined with my other current RFC 'autogenerate ipsets for sdn objects'. It enables users to quickly define rules like: on the host level: * only SNAT HTTP traffic from hosts in this vnet to a specific host * restricting traffic routed from hosts in one vnet to another vnet on the vnet level: * only allow DHCP/DNS traffic inside a bridge to the gateway Not only does this streamline creating firewall rules, it also enables users to create firewall rules that haven't been possible before and needed to rely on external firewall appliances. Since forwarded traffic goes *both* ways, you generally have to create two rules in case of bi-directional traffic. It might make sense to simplify this in the future by adding an additional option to the firewall config scheme that specifies that rules in the other direction should also get automatically generated. ## Usage For creating forward rules on the cluster/host level, you simply create a new rule with the new 'forward' direction. It uses the existing configuration files. For creating them on a vnet level, there are new firewall configuration files located under '/etc/pve/sdn/firewall/.fw'. It utilizes the same configuration format as the existing firewall configuration files. You can only define rules with direction 'forward' on a vnet-level. Changes from RFC to v2: * Fixed several bugs * SDN Firewall folder does not automatically created (thanks @Gabriel) * Firewall flushes the bridge table if no guest firewall is active, even though VNet-level rules exist * VNet-level firewall now matches on both input and output interface * Introduced log option for VNet firewall * Improved style of perl code (thanks @Thomas) * promox-firewall now verifies the directions of rules * added some additional tests to verify this behavior * added documentation proxmox-ve-rs: Stefan Hanreich (4): firewall: add forward direction firewall: add bridge firewall config parser config: firewall: add tests for interface and directions host: add struct representing bridge names proxmox-ve-config/Cargo.toml | 1 + proxmox-ve-config/src/firewall/bridge.rs | 64 +++ proxmox-ve-config/src/firewall/cluster.rs| 11 proxmox-ve-config/src/firewall/common.rs | 11 proxmox-ve-config/src/firewall/guest.rs | 66 proxmox-ve-config/src/firewall/host.rs | 12 +++- proxmox-ve-config/src/firewall/mod.rs| 1 + proxmox-ve-config/src/firewall/types/rule.rs | 10 ++- proxmox-ve-config/src/host/mod.rs| 1 + proxmox-ve-config/src/host/types.rs | 46 ++ 10 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 proxmox-ve-config/src/firewall/bridge.rs create mode 100644 proxmox-ve-config/src/host/types.rs proxmox-firewall: Stefan Hanreich (5): nftables: derive additional traits for nftables types sdn: add support for loading vnet-level firewall config sdn: create forward firewall rules use std::mem::take over drain() cargo: make proxmox-ve-config a workspace dependency Cargo.toml| 3 + proxmox-firewall/Cargo.toml | 2 +- .../resources/proxmox-firewall.nft| 54 proxmox-firewall/src/config.rs| 88 - proxmox-firewall/src/firewall.rs | 122 +- proxmox-firewall/src/rule.rs | 7 +- proxmox-firewall/tests/integration_tests.rs | 12 ++ .../integration_tests__firewall.snap | 86 proxmox-nftables/Cargo.toml | 2 +- proxmox-nftables/src/expression.rs| 8 ++ proxmox-nftables/src/types.rs | 14 +- 11 files changed, 383 insertions(+), 15 deletions(-) pve-firewall: Stefan Hanreich (2): sdn: add vnet firewall configuration api: add vnet endpoints src/PVE/API2/Firewall/Makefile | 1 + src/PVE/API2/Firewall/Rules.pm | 84 + src/PVE/API2/Firewall/Vnet.pm | 168 + src/PVE/Firewall.pm| 132 -- src/PVE/Firewall/Helpers.pm| 12 +++ 5 files changed, 391 insertions(+), 6 deletions(-) create mode 100644 src/PVE/API2/Firewall/Vnet.pm pve-manager: Stefan Hanreich (4): firewall: add forward di
[pve-devel] [PATCH proxmox-firewall v2 08/17] use std::mem::take over drain()
This is more efficient than draining and collecting the Vec. It also fixes the respective clippy lint. Signed-off-by: Stefan Hanreich --- 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, env: &NftRuleEnv) -> Result<(), Error> { log::trace!("applying macro: {self:?}"); -let initial_rules: Vec = rules.drain(..).collect(); +let initial_rules: Vec = std::mem::take(rules); for protocol in &self.code { let mut new_rules = initial_rules.to_vec(); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 13/25] sdn: add ipam module
This module includes structs for representing the JSON schema from the PVE ipam. Those can be used to parse the current IPAM state. We also include a general Ipam struct, and provide a method for converting the PVE IPAM to the general struct. The idea behind this is that we have multiple IPAM plugins in PVE and will likely add support for importing them in the future. With the split, we can have our dedicated structs for representing the different data formats from the different IPAM plugins and then convert them into a common representation that can then be used internally, decoupling the concrete plugin from the code using the IPAM configuration. Enforcing the invariants the way we currently do adds a bit of runtime complexity when building the object, but we get the upside of never being able to construct an invalid struct. For the amount of entries the ipam usually has, this should be fine. Should it turn out to be not performant enough we could always add a HashSet for looking up values and speeding up the validation. For now, I wanted to avoid the additional complexity. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 8 + proxmox-ve-config/src/guest/vm.rs | 4 + proxmox-ve-config/src/sdn/ipam.rs | 330 ++ proxmox-ve-config/src/sdn/mod.rs | 2 + 4 files changed, 344 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/ipam.rs diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 6978a8f..a7bb6ad 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -61,6 +61,14 @@ impl Cidr { pub fn is_ipv6(&self) -> bool { matches!(self, Cidr::Ipv6(_)) } + +pub fn contains_address(&self, ip: &IpAddr) -> bool { +match (self, ip) { +(Cidr::Ipv4(cidr), IpAddr::V4(ip)) => cidr.contains_address(ip), +(Cidr::Ipv6(cidr), IpAddr::V6(ip)) => cidr.contains_address(ip), +_ => false, +} +} } impl fmt::Display for Cidr { diff --git a/proxmox-ve-config/src/guest/vm.rs b/proxmox-ve-config/src/guest/vm.rs index ed6c66a..3476b93 100644 --- a/proxmox-ve-config/src/guest/vm.rs +++ b/proxmox-ve-config/src/guest/vm.rs @@ -18,6 +18,10 @@ static LOCAL_PART: [u8; 8] = [0xFE, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; static EUI64_MIDDLE_PART: [u8; 2] = [0xFF, 0xFE]; impl MacAddress { +pub fn new(address: [u8; 6]) -> Self { +Self(address) +} + /// generates a link local IPv6-address according to RFC 4291 (Appendix A) pub fn eui64_link_local_address(&self) -> Ipv6Addr { let head = &self.0[..3]; diff --git a/proxmox-ve-config/src/sdn/ipam.rs b/proxmox-ve-config/src/sdn/ipam.rs new file mode 100644 index 000..682bbe7 --- /dev/null +++ b/proxmox-ve-config/src/sdn/ipam.rs @@ -0,0 +1,330 @@ +use std::{ +collections::{BTreeMap, HashMap}, +error::Error, +fmt::Display, +net::IpAddr, +}; + +use serde::Deserialize; + +use crate::{ +firewall::types::Cidr, +guest::{types::Vmid, vm::MacAddress}, +sdn::{SdnNameError, SubnetName, ZoneName}, +}; + +/// struct for deserializing a gateway entry in PVE IPAM +/// +/// They are automatically generated by the PVE SDN module when creating a new subnet. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataGateway { +#[serde(rename = "gateway")] +_gateway: u8, +} + +/// struct for deserializing a guest entry in PVE IPAM +/// +/// They are automatically created when adding a guest to a VNet that has a Subnet with DHCP +/// configured. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataVm { +vmid: Vmid, +hostname: Option, +mac: MacAddress, +} + +/// struct for deserializing a custom entry in PVE IPAM +/// +/// Custom entries are created manually by the user via the Web UI / API. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataCustom { +mac: MacAddress, +} + +/// Enum representing the different kinds of entries that can be located in PVE IPAM +/// +/// For more information about the members see the documentation of the respective structs in the +/// enum. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[serde(untagged)] +pub enum IpamJsonData { +Vm(IpamJsonDataVm), +Gateway(IpamJsonDataGateway), +Custom(IpamJsonDataCustom), +} + +/// struct for deserializing IPs from the PVE IPAM +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Default)] +pub struct IpJson { +ips: BTreeMap, +} + +/// struct for deserializing subnets from the PVE IPAM +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Default)] +pub struct Subnet
[pve-devel] [PATCH pve-firewall v2 22/25] api: load sdn ipsets
Since the SDN configuration reads the IPAM config file, which resides in /etc/pve/priv we need to add the protected flag to several endpoints. Signed-off-by: Stefan Hanreich --- src/PVE/API2/Firewall/Cluster.pm | 8 ++-- src/PVE/API2/Firewall/Rules.pm | 12 +++- src/PVE/API2/Firewall/VM.pm | 3 ++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm index 48ad90d..d5e8268 100644 --- a/src/PVE/API2/Firewall/Cluster.pm +++ b/src/PVE/API2/Firewall/Cluster.pm @@ -214,6 +214,7 @@ __PACKAGE__->register_method({ permissions => { check => ['perm', '/', [ 'Sys.Audit' ]], }, +protected => 1, parameters => { additionalProperties => 0, properties => { @@ -253,9 +254,12 @@ __PACKAGE__->register_method({ code => sub { my ($param) = @_; - my $conf = PVE::Firewall::load_clusterfw_conf(); + my $conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); + + my $cluster_refs = PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc"); + my $sdn_refs = PVE::Firewall::Helpers::collect_refs($conf->{sdn}, $param->{type}, "sdn"); - return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc"); + return [@$sdn_refs, @$cluster_refs]; }}); 1; diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm index 9fcfb20..f5cb002 100644 --- a/src/PVE/API2/Firewall/Rules.pm +++ b/src/PVE/API2/Firewall/Rules.pm @@ -72,6 +72,7 @@ sub register_get_rules { path => '', method => 'GET', description => "List rules.", + protected => 1, permissions => PVE::Firewall::rules_audit_permissions($rule_env), parameters => { additionalProperties => 0, @@ -120,6 +121,7 @@ sub register_get_rule { path => '{pos}', method => 'GET', description => "Get single rule data.", + protected => 1, permissions => PVE::Firewall::rules_audit_permissions($rule_env), parameters => { additionalProperties => 0, @@ -412,7 +414,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $fw_conf = PVE::Firewall::load_clusterfw_conf(); +my $fw_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $rules = $fw_conf->{groups}->{$param->{group}}; die "no such security group '$param->{group}'\n" if !defined($rules); @@ -488,7 +490,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $fw_conf = PVE::Firewall::load_clusterfw_conf(); +my $fw_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $rules = $fw_conf->{rules}; return (undef, $fw_conf, $rules); @@ -528,7 +530,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf); my $rules = $fw_conf->{rules}; @@ -572,7 +574,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'vm', $param->{vmid}); my $rules = $fw_conf->{rules}; @@ -616,7 +618,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'ct', $param->{vmid}); my $rules = $fw_conf->{rules}; diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm index 4222103..4df725d 100644 --- a/src/PVE/API2/Firewall/VM.pm +++ b/src/PVE/API2/Firewall/VM.pm @@ -234,6 +234,7 @@ sub register_handlers { path => 'refs', method => 'GET', description => "Lists possible IPSet/Alias reference which are allowed in source/dest properties.", + protected => 1, permissions => { check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]], }, @@ -278,7 +279,7 @@ sub register_handlers { code => sub { my ($param) = @_; - my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); + my $cluster_conf
[pve-devel] [PATCH pve-manager v2 24/25] firewall: add sdn scope to IPRefSelector
Signed-off-by: Stefan Hanreich --- www/manager6/form/IPRefSelector.js | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/www/manager6/form/IPRefSelector.js b/www/manager6/form/IPRefSelector.js index d41cde5f5..16078e428 100644 --- a/www/manager6/form/IPRefSelector.js +++ b/www/manager6/form/IPRefSelector.js @@ -67,6 +67,12 @@ Ext.define('PVE.form.IPRefSelector', { }); } + let scopes = { + 'dc': gettext("Datacenter"), + 'guest': gettext("Guest"), + 'sdn': gettext("SDN"), + }; + columns.push( { header: gettext('Name'), @@ -80,7 +86,7 @@ Ext.define('PVE.form.IPRefSelector', { hideable: false, width: 140, renderer: function(value) { - return value === 'dc' ? gettext("Datacenter") : gettext("Guest"); + return scopes[value] ?? "unknown scope"; }, }, { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 09/25] ipset: address: add helper methods
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/address.rs | 10 ++ proxmox-ve-config/src/firewall/types/ipset.rs | 14 ++ 2 files changed, 24 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 4baafa7..9f4ad02 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -11,6 +11,16 @@ pub enum Family { V6, } +impl Family { +pub fn is_ipv4(&self) -> bool { +*self == Self::V4 +} + +pub fn is_ipv6(&self) -> bool { +*self == Self::V6 +} +} + impl fmt::Display for Family { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index e8131b5..7511490 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -132,6 +132,20 @@ pub struct IpsetEntry { pub comment: Option, } +impl IpsetEntry { +pub fn new( +address: impl Into, +nomatch: bool, +comment: impl Into>, +) -> IpsetEntry { +IpsetEntry { +nomatch, +address: address.into(), +comment: comment.into(), +} +} +} + impl> From for IpsetEntry { fn from(value: T) -> Self { Self { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 04/25] firewall: add sdn scope for ipsets
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/ipset.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index c1af642..6fbdca8 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -14,6 +14,7 @@ use crate::guest::vm::NetworkConfig; pub enum IpsetScope { Datacenter, Guest, +Sdn, } impl FromStr for IpsetScope { @@ -23,6 +24,7 @@ impl FromStr for IpsetScope { Ok(match s { "+dc" => IpsetScope::Datacenter, "+guest" => IpsetScope::Guest, +"+sdn" => IpsetScope::Sdn, _ => bail!("invalid scope for ipset: {s}"), }) } @@ -33,6 +35,7 @@ impl Display for IpsetScope { let prefix = match self { Self::Datacenter => "dc", Self::Guest => "guest", +Self::Sdn => "sdn", }; f.write_str(prefix) -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 02/25] bump serde_with to 3
From: Fabian Grünbichler Signed-off-by: Fabian Grünbichler Signed-off-by: Stefan Hanreich --- proxmox-ve-config/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index ab8a7a0..5f11bf9 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -14,7 +14,7 @@ nix = "0.26" serde = { version = "1", features = [ "derive" ] } serde_json = "1" serde_plain = "1" -serde_with = "2.3.3" +serde_with = "3" proxmox-schema = "3.1.1" proxmox-sys = "0.5.8" -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 03/25] bump dependencies
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index 5f11bf9..79ba164 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -16,6 +16,6 @@ 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.4" proxmox-sortable-macro = "0.1.3" -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v2 10/17] sdn: add vnet firewall configuration
Signed-off-by: Stefan Hanreich --- src/PVE/Firewall.pm | 122 ++-- src/PVE/Firewall/Helpers.pm | 12 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 9943f2e..e8096aa 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -29,6 +29,7 @@ use PVE::RS::Firewall::SDN; my $pvefw_conf_dir = "/etc/pve/firewall"; my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw"; +my $vnetfw_conf_dir = "/etc/pve/sdn/firewall"; # dynamically include PVE::QemuServer and PVE::LXC # to avoid dependency problems @@ -1290,6 +1291,12 @@ our $cluster_option_properties = { optional => 1, enum => ['ACCEPT', 'REJECT', 'DROP'], }, +policy_forward => { + description => "Forward policy.", + type => 'string', + optional => 1, + enum => ['ACCEPT', 'DROP'], +}, log_ratelimit => { description => "Log ratelimiting settings", type => 'string', format => { @@ -1329,6 +1336,8 @@ our $host_option_properties = { description => "Log level for incoming traffic." }), log_level_out => get_standard_option('pve-fw-loglevel', { description => "Log level for outgoing traffic." }), +log_level_forward => get_standard_option('pve-fw-loglevel', { + description => "Log level for forwarded traffic." }), tcp_flags_log_level => get_standard_option('pve-fw-loglevel', { description => "Log level for illegal tcp flags filter." }), smurf_log_level => get_standard_option('pve-fw-loglevel', { @@ -1476,6 +1485,23 @@ our $vm_option_properties = { }; +our $vnet_option_properties = { +enable => { + description => "Enable/disable firewall rules.", + type => 'boolean', + default => 0, + optional => 1, +}, +policy_forward => { + description => "Forward policy.", + type => 'string', + optional => 1, + enum => ['ACCEPT', 'DROP'], +}, +log_level_forward => get_standard_option('pve-fw-loglevel', { + description => "Log level for forwarded traffic." }), +}; + my $addr_list_descr = "This can refer to a single IP address, an IP set ('+ipsetname') or an IP alias definition. You can also specify an address range like '20.34.101.207-201.3.9.99', or a list of IP addresses and networks (entries are separated by comma). Please do not mix IPv4 and IPv6 addresses inside such lists."; @@ -1493,7 +1519,7 @@ my $rule_properties = { description => "Rule type.", type => 'string', optional => 1, - enum => ['in', 'out', 'group'], + enum => ['in', 'out', 'forward', 'group'], }, action => { description => "Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name.", @@ -1651,10 +1677,20 @@ my $rule_env_iface_lookup = { 'ct' => 1, 'vm' => 1, 'group' => 0, +'vnet' => 0, 'cluster' => 1, 'host' => 1, }; +my $rule_env_direction_lookup = { +'ct' => ['in', 'out', 'group'], +'vm' => ['in', 'out', 'group'], +'group' => ['in', 'out', 'forward'], +'cluster' => ['in', 'out', 'forward', 'group'], +'host' => ['in', 'out', 'forward', 'group'], +'vnet' => ['forward', 'group'], +}; + sub verify_rule { my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_; @@ -1725,7 +1761,13 @@ sub verify_rule { &$add_error('action', "missing property") if !$action; if ($type) { - if ($type eq 'in' || $type eq 'out') { + my $valid_types = $rule_env_direction_lookup->{$rule_env} + or die "unknown rule_env '$rule_env'\n"; + + &$add_error('type', "invalid rule type '$type' for rule_env '$rule_env'") + if !grep (/^$type$/, @$valid_types); + + if ($type eq 'in' || $type eq 'out' || $type eq 'forward') { &$add_error('action', "unknown action '$action'&
[pve-devel] [PATCH pve-network v2 16/17] firewall: add endpoints for vnet-level firewall
Signed-off-by: Stefan Hanreich --- src/PVE/API2/Network/SDN/Vnets.pm | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm index 05915f6..e48b048 100644 --- a/src/PVE/API2/Network/SDN/Vnets.pm +++ b/src/PVE/API2/Network/SDN/Vnets.pm @@ -14,6 +14,7 @@ use PVE::Network::SDN::VnetPlugin; use PVE::Network::SDN::Subnets; use PVE::API2::Network::SDN::Subnets; use PVE::API2::Network::SDN::Ips; +use PVE::API2::Firewall::Vnet; use Storable qw(dclone); use PVE::JSONSchema qw(get_standard_option); @@ -24,6 +25,11 @@ use PVE::RESTHandler; use base qw(PVE::RESTHandler); +__PACKAGE__->register_method ({ +subclass => "PVE::API2::Firewall::Vnet", +path => '{vnet}/firewall', +}); + __PACKAGE__->register_method ({ subclass => "PVE::API2::Network::SDN::Subnets", path => '{vnet}/subnets', -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 01/17] firewall: add forward direction
This direction will be used for specifying rules on bridge-level firewalls as well as rules on the cluster / host level that are for forwarded network packets. Since with the introduction of this direction not every type of firewall configuration can contain all types of directions, we additionally add validation logic to the parser, so rules with an invalid direction get rejected by the parser. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/cluster.rs| 11 +++ proxmox-ve-config/src/firewall/common.rs | 11 +++ proxmox-ve-config/src/firewall/guest.rs | 13 + proxmox-ve-config/src/firewall/host.rs | 12 +++- proxmox-ve-config/src/firewall/mod.rs| 1 + proxmox-ve-config/src/firewall/types/rule.rs | 10 -- 6 files changed, 55 insertions(+), 3 deletions(-) diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs index 223124b..ce3dd53 100644 --- a/proxmox-ve-config/src/firewall/cluster.rs +++ b/proxmox-ve-config/src/firewall/cluster.rs @@ -25,12 +25,15 @@ pub const CLUSTER_EBTABLES_DEFAULT: bool = false; pub const CLUSTER_POLICY_IN_DEFAULT: Verdict = Verdict::Drop; /// default setting for [`Config::default_policy()`] pub const CLUSTER_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept; +/// default setting for [`Config::default_policy()`] +pub const CLUSTER_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept; impl Config { pub fn parse(input: R) -> Result { let parser_config = ParserConfig { guest_iface_names: false, ipset_scope: Some(IpsetScope::Datacenter), +allowed_directions: vec![Direction::In, Direction::Out, Direction::Forward], }; Ok(Self { @@ -86,6 +89,11 @@ impl Config { .options .policy_out .unwrap_or(CLUSTER_POLICY_OUT_DEFAULT), +Direction::Forward => self +.config +.options +.policy_forward +.unwrap_or(CLUSTER_POLICY_FORWARD_DEFAULT), } } @@ -121,6 +129,7 @@ pub struct Options { policy_in: Option, policy_out: Option, +policy_forward: Option, } #[cfg(test)] @@ -148,6 +157,7 @@ log_ratelimit: 1,rate=10/second,burst=20 ebtables: 0 policy_in: REJECT policy_out: REJECT +policy_forward: DROP [ALIASES] @@ -191,6 +201,7 @@ IN BGP(REJECT) -log crit -source 1.2.3.4 )), policy_in: Some(Verdict::Reject), policy_out: Some(Verdict::Reject), +policy_forward: Some(Verdict::Drop), } ); diff --git a/proxmox-ve-config/src/firewall/common.rs b/proxmox-ve-config/src/firewall/common.rs index a08f19c..3999168 100644 --- a/proxmox-ve-config/src/firewall/common.rs +++ b/proxmox-ve-config/src/firewall/common.rs @@ -6,6 +6,7 @@ use serde::de::IntoDeserializer; use crate::firewall::parse::{parse_named_section_tail, split_key_value, SomeString}; use crate::firewall::types::ipset::{IpsetName, IpsetScope}; +use crate::firewall::types::rule::{Direction, Kind}; use crate::firewall::types::{Alias, Group, Ipset, Rule}; #[derive(Debug, Default)] @@ -34,6 +35,7 @@ pub struct ParserConfig { /// Network interfaces must be of the form `netX`. pub guest_iface_names: bool, pub ipset_scope: Option, +pub allowed_directions: Vec, } impl Config @@ -150,6 +152,15 @@ where } } +if let Kind::Match(rule) = rule.kind() { +if !parser_cfg.allowed_directions.contains(&rule.dir) { +bail!( +"found not allowed direction in firewall config: {0}", +rule.dir +); +} +} + self.rules.push(rule); Ok(()) } diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index c7e282f..1e70a67 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -31,6 +31,8 @@ pub const GUEST_IPFILTER_DEFAULT: bool = false; pub const GUEST_POLICY_IN_DEFAULT: Verdict = Verdict::Drop; /// default return value for [`Config::default_policy()`] pub const GUEST_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept; +/// default return value for [`Config::default_policy()`] +pub const GUEST_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept; #[derive(Debug, Default, Deserialize)] #[cfg_attr(test, derive(Eq, PartialEq))] @@ -61,6 +63,8 @@ pub struct Options { #[serde(rename = "policy_out")] policy_out: Option, + +policy_forward: Option, } #[derive(Debug)] @@ -84,6 +88,7 @@ impl Config { let parser_cfg = super::common::ParserConfig { guest_iface_names: true, ipset_scope: Some(IpsetScope::Guest), +allowed_directions: vec![Direction::In, Direction::Out],
[pve-devel] [PATCH proxmox-ve-rs v2 15/25] sdn: add config module
Similar to how the IPAM module works, we separate the internal representation from the concrete schema of the configuration file. We provide structs for parsing the running SDN configuration and a struct that is used internally for representing an SDN configuration, as well as a method for converting the running configuration to the internal representation. This is necessary because there are two possible sources for the SDN configuration: The running configuration as well as the SectionConfig that contains possible changes from the UI, that have not yet been applied. Simlarly to the IPAM, enforcing the invariants the way we currently do adds some runtime complexity when building the object, but we get the upside of never being able to construct an invalid struct. For the amount of entries the sdn config usually has, this should be fine. Should it turn out to be not performant enough we could always add a HashSet for looking up values and speeding up the validation. For now, I wanted to avoid the additional complexity. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/sdn/config.rs | 570 proxmox-ve-config/src/sdn/mod.rs| 1 + 2 files changed, 571 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/config.rs diff --git a/proxmox-ve-config/src/sdn/config.rs b/proxmox-ve-config/src/sdn/config.rs new file mode 100644 index 000..b71084b --- /dev/null +++ b/proxmox-ve-config/src/sdn/config.rs @@ -0,0 +1,570 @@ +use std::{ +collections::{BTreeMap, HashMap}, +error::Error, +fmt::Display, +net::IpAddr, +str::FromStr, +}; + +use proxmox_schema::{property_string::PropertyString, ApiType, ObjectSchema, StringSchema}; + +use serde::Deserialize; +use serde_with::{DeserializeFromStr, SerializeDisplay}; + +use crate::{ +common::Allowlist, +firewall::types::{ +address::{IpRange, IpRangeError}, +ipset::{IpsetEntry, IpsetName, IpsetScope}, +Cidr, Ipset, +}, +sdn::{SdnNameError, SubnetName, VnetName, ZoneName}, +}; + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum SdnConfigError { +InvalidZoneType, +InvalidDhcpType, +ZoneNotFound, +VnetNotFound, +MismatchedCidrGateway, +MismatchedSubnetZone, +NameError(SdnNameError), +InvalidDhcpRange(IpRangeError), +DuplicateVnetName, +} + +impl Error for SdnConfigError { +fn source(&self) -> Option<&(dyn Error + 'static)> { +match self { +SdnConfigError::NameError(e) => Some(e), +SdnConfigError::InvalidDhcpRange(e) => Some(e), +_ => None, +} +} +} + +impl Display for SdnConfigError { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +match self { +SdnConfigError::NameError(err) => write!(f, "invalid name: {err}"), +SdnConfigError::InvalidDhcpRange(err) => write!(f, "invalid dhcp range: {err}"), +SdnConfigError::ZoneNotFound => write!(f, "zone not found"), +SdnConfigError::VnetNotFound => write!(f, "vnet not found"), +SdnConfigError::MismatchedCidrGateway => { +write!(f, "mismatched ip address family for gateway and CIDR") +} +SdnConfigError::InvalidZoneType => write!(f, "invalid zone type"), +SdnConfigError::InvalidDhcpType => write!(f, "invalid dhcp type"), +SdnConfigError::DuplicateVnetName => write!(f, "vnet name occurs in multiple zones"), +SdnConfigError::MismatchedSubnetZone => { +write!(f, "subnet zone does not match actual zone") +} +} +} +} + +#[derive( +Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] +pub enum ZoneType { +Simple, +Vlan, +Qinq, +Vxlan, +Evpn, +} + +impl FromStr for ZoneType { +type Err = SdnConfigError; + +fn from_str(s: &str) -> Result { +match s { +"simple" => Ok(ZoneType::Simple), +"vlan" => Ok(ZoneType::Vlan), +"qinq" => Ok(ZoneType::Qinq), +"vxlan" => Ok(ZoneType::Vxlan), +"evpn" => Ok(ZoneType::Evpn), +_ => Err(SdnConfigError::InvalidZoneType), +} +} +} + +impl Display for ZoneType { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +f.write_str(match self { +ZoneType::Simple => "simple", +ZoneType::Vlan => "vlan", +ZoneType::Qinq => "qinq", +ZoneType::Vxlan => "vxlan", +ZoneType::Evpn => "evpn", +}) +
[pve-devel] [PATCH proxmox-firewall v2 09/17] cargo: make proxmox-ve-config a workspace dependency
Since it is used by both libraries, and they need the same version. Signed-off-by: Stefan Hanreich --- Cargo.toml | 3 +++ proxmox-firewall/Cargo.toml | 2 +- proxmox-nftables/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a819b2d..8fba806 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,3 +3,6 @@ members = [ "proxmox-nftables", "proxmox-firewall", ] + +[workspace.dependencies] +proxmox-ve-config = { version = "0.1.0" } diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml index b361ab1..c2adcac 100644 --- a/proxmox-firewall/Cargo.toml +++ b/proxmox-firewall/Cargo.toml @@ -21,7 +21,7 @@ serde_json = "1" signal-hook = "0.3" proxmox-nftables = { path = "../proxmox-nftables", features = ["config-ext"] } -proxmox-ve-config = "0.1.0" +proxmox-ve-config = { workspace = true } [dev-dependencies] insta = { version = "1.21", features = ["json"] } diff --git a/proxmox-nftables/Cargo.toml b/proxmox-nftables/Cargo.toml index cf06f93..4ff6f41 100644 --- a/proxmox-nftables/Cargo.toml +++ b/proxmox-nftables/Cargo.toml @@ -22,4 +22,4 @@ serde = { version = "1", features = [ "derive" ] } serde_json = "1" serde_plain = "1" -proxmox-ve-config = { version = "0.1.0", optional = true } +proxmox-ve-config = { workspace = true, optional = true } -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 02/17] firewall: add bridge firewall config parser
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 --- proxmox-ve-config/src/firewall/bridge.rs | 64 1 file changed, 64 insertions(+) create mode 100644 proxmox-ve-config/src/firewall/bridge.rs diff --git a/proxmox-ve-config/src/firewall/bridge.rs b/proxmox-ve-config/src/firewall/bridge.rs new file mode 100644 index 000..4acb6fa --- /dev/null +++ b/proxmox-ve-config/src/firewall/bridge.rs @@ -0,0 +1,64 @@ +use std::io; + +use anyhow::Error; +use serde::Deserialize; + +use crate::firewall::parse::serde_option_bool; +use crate::firewall::types::log::LogLevel; +use crate::firewall::types::rule::{Direction, Verdict}; + +use super::common::ParserConfig; +use super::types::Rule; + +pub struct Config { +pub(crate) config: super::common::Config, +} + +/// 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(input: R) -> Result { +let parser_config = ParserConfig { +guest_iface_names: false, +ipset_scope: None, +allowed_directions: vec![Direction::Forward], +}; + +Ok(Self { +config: super::common::Config::parse(input, &parser_config)?, +}) +} + +pub fn enabled(&self) -> bool { +self.config.options.enable.unwrap_or(BRIDGE_ENABLED_DEFAULT) +} + +pub fn rules(&self) -> impl Iterator + '_ { +self.config.rules.iter() +} + +pub fn log_level_forward(&self) -> LogLevel { +self.config.options.log_level_forward.unwrap_or_default() +} + +pub fn policy_forward(&self) -> Verdict { +self.config +.options +.policy_forward +.unwrap_or(BRIDGE_POLICY_FORWARD) +} +} + +#[derive(Debug, Default, Deserialize)] +#[cfg_attr(test, derive(Eq, PartialEq))] +pub struct Options { +#[serde(default, with = "serde_option_bool")] +enable: Option, + +policy_forward: Option, + +log_level_forward: Option, +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 03/17] config: firewall: add tests for interface and directions
Add tests for validating the directions in the guest firewall configuration. While I'm at it, I also added tests for validating interface names, since this functionality did not get tested before. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/guest.rs | 53 + 1 file changed, 53 insertions(+) diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index 1e70a67..23eaa4e 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -247,4 +247,57 @@ policy_forward: DROP } ); } + +#[test] +fn test_parse_valid_interface_prefix() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i tapeth0 +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} + +#[test] +fn test_parse_invalid_interface_prefix() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i eth0 +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} + +#[test] +fn test_parse_valid_directions() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning +OUT ACCEPT -p udp -dport 33 -sport 22 -log warning +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap(); +} + +#[test] +fn test_parse_invalid_direction() { +const CONFIG: &str = r#" +[RULES] + +FORWARD ACCEPT -p udp -dport 33 -sport 22 -log warning +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} } -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs v2 23/25] add PVE::RS::Firewall::SDN module
Used for obtaining the IPSets that get autogenerated by the nftables firewall. The returned configuration has the same format as the pve-firewall uses internally, making it compatible with the existing pve-firewall code. Signed-off-by: Stefan Hanreich --- pve-rs/Cargo.toml | 1 + pve-rs/Makefile| 1 + pve-rs/src/firewall/mod.rs | 1 + pve-rs/src/firewall/sdn.rs | 130 + pve-rs/src/lib.rs | 1 + 5 files changed, 134 insertions(+) create mode 100644 pve-rs/src/firewall/mod.rs create mode 100644 pve-rs/src/firewall/sdn.rs diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml index 72d548d..cab9a83 100644 --- a/pve-rs/Cargo.toml +++ b/pve-rs/Cargo.toml @@ -45,3 +45,4 @@ proxmox-subscription = "0.4" proxmox-sys = "0.6" proxmox-tfa = { version = "5", features = ["api"] } proxmox-time = "2" +proxmox-ve-config = { version = "0.1.0" } diff --git a/pve-rs/Makefile b/pve-rs/Makefile index c6b4e08..d01da69 100644 --- a/pve-rs/Makefile +++ b/pve-rs/Makefile @@ -28,6 +28,7 @@ PERLMOD_GENPACKAGE := /usr/lib/perlmod/genpackage.pl \ PERLMOD_PACKAGES := \ PVE::RS::APT::Repositories \ + PVE::RS::Firewall::SDN \ PVE::RS::OpenId \ PVE::RS::ResourceScheduling::Static \ PVE::RS::TFA diff --git a/pve-rs/src/firewall/mod.rs b/pve-rs/src/firewall/mod.rs new file mode 100644 index 000..8bd18a8 --- /dev/null +++ b/pve-rs/src/firewall/mod.rs @@ -0,0 +1 @@ +pub mod sdn; diff --git a/pve-rs/src/firewall/sdn.rs b/pve-rs/src/firewall/sdn.rs new file mode 100644 index 000..5049f74 --- /dev/null +++ b/pve-rs/src/firewall/sdn.rs @@ -0,0 +1,130 @@ +#[perlmod::package(name = "PVE::RS::Firewall::SDN", lib = "pve_rs")] +mod export { +use std::collections::HashMap; +use std::{fs, io}; + +use anyhow::{bail, Context, Error}; +use serde::Serialize; + +use proxmox_ve_config::{ +common::Allowlist, +firewall::types::ipset::{IpsetAddress, IpsetEntry}, +firewall::types::Ipset, +guest::types::Vmid, +sdn::{ +config::{RunningConfig, SdnConfig}, +ipam::{Ipam, IpamJson}, +VnetName, +}, +}; + +#[derive(Clone, Debug, Default, Serialize)] +pub struct LegacyIpsetEntry { +nomatch: bool, +cidr: String, +comment: Option, +} + +impl LegacyIpsetEntry { +pub fn from_ipset_entry(entry: &IpsetEntry) -> Vec { +let mut entries = Vec::new(); + +match &entry.address { +IpsetAddress::Alias(name) => { +entries.push(Self { +nomatch: entry.nomatch, +cidr: name.to_string(), +comment: entry.comment.clone(), +}); +} +IpsetAddress::Cidr(cidr) => { +entries.push(Self { +nomatch: entry.nomatch, +cidr: cidr.to_string(), +comment: entry.comment.clone(), +}); +} +IpsetAddress::Range(range) => { +entries.extend(range.to_cidrs().into_iter().map(|cidr| Self { +nomatch: entry.nomatch, +cidr: cidr.to_string(), +comment: entry.comment.clone(), +})) +} +}; + +entries +} +} + +#[derive(Clone, Debug, Default, Serialize)] +pub struct SdnFirewallConfig { +ipset: HashMap>, +ipset_comments: HashMap, +} + +impl SdnFirewallConfig { +pub fn new() -> Self { +Default::default() +} + +pub fn extend_ipsets(&mut self, ipsets: impl IntoIterator) { +for ipset in ipsets { +let entries = ipset +.iter() +.flat_map(LegacyIpsetEntry::from_ipset_entry) +.collect(); + +self.ipset.insert(ipset.name().name().to_string(), entries); + +if let Some(comment) = &ipset.comment { +self.ipset_comments +.insert(ipset.name().name().to_string(), comment.to_string()); +} +} +} +} + +const SDN_RUNNING_CONFIG: &str = "/etc/pve/sdn/.running-config"; +const SDN_IPAM: &str = "/etc/pve/priv/ipam.db"; + +#[export] +pub fn config( +vnet_filter: Option>, +vm_filter: Option>, +) -> Result { +let mut refs = SdnFirewallConfig::new(); + +match fs::read_to_string(SDN_RUNNING_CONFIG) { +Ok(data) => { +let running_config: RunningConfig = serde_json::from_str(&a
[pve-devel] [PATCH proxmox-ve-rs v2 17/25] tests: add sdn config tests
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/tests/sdn/main.rs | 144 ++ .../tests/sdn/resources/running-config.json | 54 +++ 2 files changed, 198 insertions(+) create mode 100644 proxmox-ve-config/tests/sdn/main.rs create mode 100644 proxmox-ve-config/tests/sdn/resources/running-config.json diff --git a/proxmox-ve-config/tests/sdn/main.rs b/proxmox-ve-config/tests/sdn/main.rs new file mode 100644 index 000..2ac0cb3 --- /dev/null +++ b/proxmox-ve-config/tests/sdn/main.rs @@ -0,0 +1,144 @@ +use std::{ +net::{IpAddr, Ipv4Addr, Ipv6Addr}, +str::FromStr, +}; + +use proxmox_ve_config::{ +firewall::types::{address::IpRange, Cidr}, +sdn::{ +config::{ +RunningConfig, SdnConfig, SdnConfigError, SubnetConfig, VnetConfig, ZoneConfig, +ZoneType, +}, +SubnetName, VnetName, ZoneName, +}, +}; + +#[test] +fn parse_running_config() { +let running_config: RunningConfig = + serde_json::from_str(include_str!("resources/running-config.json")).unwrap(); + +let parsed_config = SdnConfig::try_from(running_config).unwrap(); + +let sdn_config = SdnConfig::from_zones([ZoneConfig::from_vnets( +ZoneName::from_str("zone0").unwrap(), +ZoneType::Simple, +[ +VnetConfig::from_subnets( +VnetName::from_str("vnet0").unwrap(), +[ +SubnetConfig::new( +SubnetName::from_str("zone0-fd80::-64").unwrap(), +Some(Ipv6Addr::new(0xFD80, 0, 0, 0, 0, 0, 0, 0x1).into()), +true, +[IpRange::new_v6( +[0xFD80, 0, 0, 0, 0, 0, 0, 0x1000], +[0xFD80, 0, 0, 0, 0, 0, 0, 0x], +) +.unwrap()], +) +.unwrap(), +SubnetConfig::new( +SubnetName::from_str("zone0-10.101.0.0-16").unwrap(), +Some(Ipv4Addr::new(10, 101, 1, 1).into()), +true, +[ +IpRange::new_v4([10, 101, 98, 100], [10, 101, 98, 200]).unwrap(), +IpRange::new_v4([10, 101, 99, 100], [10, 101, 99, 200]).unwrap(), +], +) +.unwrap(), +], +) +.unwrap(), +VnetConfig::from_subnets( +VnetName::from_str("vnet1").unwrap(), +[SubnetConfig::new( +SubnetName::from_str("zone0-10.102.0.0-16").unwrap(), +None, +false, +[], +) +.unwrap()], +) +.unwrap(), +], +) +.unwrap()]) +.unwrap(); + +assert_eq!(sdn_config, parsed_config); +} + +#[test] +fn sdn_config() { +let mut sdn_config = SdnConfig::new(); + +let zone0_name = ZoneName::new("zone0".to_string()).unwrap(); +let zone1_name = ZoneName::new("zone1".to_string()).unwrap(); + +let vnet0_name = VnetName::new("vnet0".to_string()).unwrap(); +let vnet1_name = VnetName::new("vnet1".to_string()).unwrap(); + +let zone0 = ZoneConfig::new(zone0_name.clone(), ZoneType::Qinq); +sdn_config.add_zone(zone0).unwrap(); + +let vnet0 = VnetConfig::new(vnet0_name.clone()); +assert_eq!( +sdn_config.add_vnet(&zone1_name, vnet0.clone()), +Err(SdnConfigError::ZoneNotFound) +); + +sdn_config.add_vnet(&zone0_name, vnet0.clone()).unwrap(); + +let subnet = SubnetConfig::new( +SubnetName::new(zone0_name.clone(), Cidr::new_v4([10, 0, 0, 0], 16).unwrap()), +IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)), +true, +[], +) +.unwrap(); + +assert_eq!( +sdn_config.add_subnet(&zone0_name, &vnet1_name, subnet.clone()), +Err(SdnConfigError::VnetNotFound), +); + +sdn_config +.add_subnet(&zone0_name, &vnet0_name, subnet) +.unwrap(); + +let zone1 = ZoneConfig::from_vnets( +zone1_name.clone(), +ZoneType::Evpn, +[VnetConfig::from_subnets( +vnet1_name.clone(), +[SubnetConfig::new( +SubnetName::new( +zone0_name.clone(), +Cidr::new_v4([192, 168, 0, 0], 24).unwrap(), +), +None, +false, +[], +) +.unwrap()], +) +.unwrap()], +) +.unwrap(); + +assert_eq!( +sdn_config.add_zones([zone1]), +Err(SdnConfigError::MismatchedSubnetZone), +); + +let zone1 = ZoneConfig::new(zone1_name.clone(
[pve-devel] [PATCH proxmox-ve-rs v2 18/25] tests: add ipam tests
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/tests/sdn/main.rs | 45 +++ proxmox-ve-config/tests/sdn/resources/ipam.db | 26 +++ 2 files changed, 71 insertions(+) create mode 100644 proxmox-ve-config/tests/sdn/resources/ipam.db diff --git a/proxmox-ve-config/tests/sdn/main.rs b/proxmox-ve-config/tests/sdn/main.rs index 2ac0cb3..1815bec 100644 --- a/proxmox-ve-config/tests/sdn/main.rs +++ b/proxmox-ve-config/tests/sdn/main.rs @@ -5,11 +5,13 @@ use std::{ use proxmox_ve_config::{ firewall::types::{address::IpRange, Cidr}, +guest::vm::MacAddress, sdn::{ config::{ RunningConfig, SdnConfig, SdnConfigError, SubnetConfig, VnetConfig, ZoneConfig, ZoneType, }, +ipam::{Ipam, IpamDataVm, IpamEntry, IpamJson}, SubnetName, VnetName, ZoneName, }, }; @@ -142,3 +144,46 @@ fn sdn_config() { Err(SdnConfigError::DuplicateVnetName), ) } + +#[test] +fn parse_ipam() { +let ipam_json: IpamJson = serde_json::from_str(include_str!("resources/ipam.db")).unwrap(); +let ipam = Ipam::try_from(ipam_json).unwrap(); + +let zone_name = ZoneName::new("zone0".to_string()).unwrap(); + +assert_eq!( +Ipam::from_entries([ +IpamEntry::new( +SubnetName::new( +zone_name.clone(), +Cidr::new_v6([0xFD80, 0, 0, 0, 0, 0, 0, 0], 64).unwrap() +), +IpamDataVm::new( +Ipv6Addr::new(0xFD80, 0, 0, 0, 0, 0, 0, 0x1000), +1000, +MacAddress::new([0xBC, 0x24, 0x11, 0, 0, 0x01]), +"test0".to_string() +) +.into() +) +.unwrap(), +IpamEntry::new( +SubnetName::new( +zone_name.clone(), +Cidr::new_v4([10, 101, 0, 0], 16).unwrap() +), +IpamDataVm::new( +Ipv4Addr::new(10, 101, 99, 101), +1000, +MacAddress::new([0xBC, 0x24, 0x11, 0, 0, 0x01]), +"test0".to_string() +) +.into() +) +.unwrap(), +]) +.unwrap(), +ipam +) +} diff --git a/proxmox-ve-config/tests/sdn/resources/ipam.db b/proxmox-ve-config/tests/sdn/resources/ipam.db new file mode 100644 index 000..a3e6c87 --- /dev/null +++ b/proxmox-ve-config/tests/sdn/resources/ipam.db @@ -0,0 +1,26 @@ +{ + "zones": { +"zone0": { + "subnets": { +"fd80::/64": { + "ips": { +"fd80::1000": { + "vmid": "1000", + "mac": "BC:24:11:00:00:01", + "hostname": "test0" +} + } +}, +"10.101.0.0/16": { + "ips": { +"10.101.99.101": { + "mac": "BC:24:11:00:00:01", + "vmid": "1000", + "hostname": "test0" +} + } +} + } +} + } +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 12/25] sdn: add name types
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/lib.rs | 1 + proxmox-ve-config/src/sdn/mod.rs | 240 +++ 2 files changed, 241 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/mod.rs diff --git a/proxmox-ve-config/src/lib.rs b/proxmox-ve-config/src/lib.rs index 1b6feae..d17136c 100644 --- a/proxmox-ve-config/src/lib.rs +++ b/proxmox-ve-config/src/lib.rs @@ -2,3 +2,4 @@ pub mod common; pub mod firewall; pub mod guest; pub mod host; +pub mod sdn; diff --git a/proxmox-ve-config/src/sdn/mod.rs b/proxmox-ve-config/src/sdn/mod.rs new file mode 100644 index 000..4e7c525 --- /dev/null +++ b/proxmox-ve-config/src/sdn/mod.rs @@ -0,0 +1,240 @@ +use std::{error::Error, fmt::Display, str::FromStr}; + +use serde_with::DeserializeFromStr; + +use crate::firewall::types::Cidr; + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum SdnNameError { +Empty, +TooLong, +InvalidSymbols, +InvalidSubnetCidr, +InvalidSubnetFormat, +} + +impl Error for SdnNameError {} + +impl Display for SdnNameError { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +f.write_str(match self { +SdnNameError::TooLong => "name too long", +SdnNameError::InvalidSymbols => "invalid symbols in name", +SdnNameError::InvalidSubnetCidr => "invalid cidr in name", +SdnNameError::InvalidSubnetFormat => "invalid format for subnet name", +SdnNameError::Empty => "name is empty", +}) +} +} + +fn validate_sdn_name(name: &str) -> Result<(), SdnNameError> { +if name.is_empty() { +return Err(SdnNameError::Empty); +} + +if name.len() > 8 { +return Err(SdnNameError::TooLong); +} + +// safe because of empty check +if !name.chars().next().unwrap().is_ascii_alphabetic() { +return Err(SdnNameError::InvalidSymbols); +} + +if !name.chars().all(|c| c.is_ascii_alphanumeric()) { +return Err(SdnNameError::InvalidSymbols); +} + +Ok(()) +} + +/// represents the name of an sdn zone +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct ZoneName(String); + +impl ZoneName { +/// construct a new zone name +/// +/// # Errors +/// +/// This function will return an error if the name is empty, too long (>8 characters), starts +/// with a non-alphabetic symbol or if there are non alphanumeric symbols contained in the name. +pub fn new(name: String) -> Result { +validate_sdn_name(&name)?; +Ok(ZoneName(name)) +} + +pub fn name(&self) -> &str { +&self.0 +} +} + +impl FromStr for ZoneName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +Self::new(s.to_owned()) +} +} + +impl Display for ZoneName { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +self.0.fmt(f) +} +} + +/// represents the name of an sdn vnet +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct VnetName(String); + +impl VnetName { +/// construct a new vnet name +/// +/// # Errors +/// +/// This function will return an error if the name is empty, too long (>8 characters), starts +/// with a non-alphabetic symbol or if there are non alphanumeric symbols contained in the name. +pub fn new(name: String) -> Result { +validate_sdn_name(&name)?; +Ok(VnetName(name)) +} + +pub fn name(&self) -> &str { +&self.0 +} +} + +impl FromStr for VnetName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +Self::new(s.to_owned()) +} +} + +impl Display for VnetName { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +self.0.fmt(f) +} +} + +/// represents the name of an sdn subnet +/// +/// # Textual representation +/// A subnet name has the form `{zone_id}-{cidr_ip}-{cidr_mask}` +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct SubnetName(ZoneName, Cidr); + +impl SubnetName { +pub fn new(zone: ZoneName, cidr: Cidr) -> Self { +SubnetName(zone, cidr) +} + +pub fn zone(&self) -> &ZoneName { +&self.0 +} + +pub fn cidr(&self) -> &Cidr { +&self.1 +} +} + +impl FromStr for SubnetName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +if let Some((name, cidr_part)) = s.split_once('-') { +if let Some((ip, netmask)) = cidr_part.split_once('-') { +let zone_name = ZoneName::from_str(name)?; + +
[pve-devel] [PATCH proxmox-ve-rs v2 14/25] sdn: ipam: add method for generating ipsets
For every guest that has at least one entry in the IPAM we generate an ipset with the name `+sdn/guest-ipam-{vmid}`. The ipset contains all IPs from all zones for a guest with {vmid}. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 9 proxmox-ve-config/src/sdn/ipam.rs | 54 ++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index a7bb6ad..e5a3709 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -108,6 +108,15 @@ impl From for Cidr { } } +impl From for Cidr { +fn from(value: IpAddr) -> Self { +match value { +IpAddr::V4(addr) => Ipv4Cidr::from(addr).into(), +IpAddr::V6(addr) => Ipv6Cidr::from(addr).into(), +} +} +} + const IPV4_LENGTH: u8 = 32; #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] diff --git a/proxmox-ve-config/src/sdn/ipam.rs b/proxmox-ve-config/src/sdn/ipam.rs index 682bbe7..075c0f3 100644 --- a/proxmox-ve-config/src/sdn/ipam.rs +++ b/proxmox-ve-config/src/sdn/ipam.rs @@ -8,7 +8,11 @@ use std::{ use serde::Deserialize; use crate::{ -firewall::types::Cidr, +common::Allowlist, +firewall::types::{ +ipset::{IpsetEntry, IpsetScope}, +Cidr, Ipset, +}, guest::{types::Vmid, vm::MacAddress}, sdn::{SdnNameError, SubnetName, ZoneName}, }; @@ -309,6 +313,54 @@ impl Ipam { } } +impl Ipam { +/// generates an [`Ipset`] for all guests with at least one entry in the IPAM +/// +/// # Arguments +/// * `filter` - A [`Allowlist`] for which IPsets should get returned +/// +/// It contains all IPs in all VNets, that a guest has stored in IPAM. +/// Ipset name is of the form `guest-ipam-` +pub fn ipsets<'a>( +&self, +filter: impl Into>>, +) -> impl Iterator + '_ { +let filter = filter.into(); + +self.entries +.iter() +.flat_map(|(_, entries)| entries.iter()) +.filter_map(|entry| { +if let IpamData::Vm(data) = &entry.data() { +if filter +.map(|list| list.is_allowed(&data.vmid)) +.unwrap_or(true) +{ +return Some(data); +} +} + +None +}) +.fold(HashMapnew(), |mut acc, entry| { +match acc.get_mut(&entry.vmid) { +Some(ipset) => { +ipset.push(IpsetEntry::from(entry.ip)); +} +None => { +let ipset_name = format!("guest-ipam-{}", entry.vmid); +let mut ipset = Ipset::from_parts(IpsetScope::Sdn, ipset_name); +ipset.push(IpsetEntry::from(entry.ip)); +acc.insert(entry.vmid, ipset); +} +}; + +acc +}) +.into_values() +} +} + impl TryFrom for Ipam { type Error = IpamError; -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 05/25] firewall: add ip range types
Currently we are using tuples to represent IP ranges which is suboptimal. Validation logic and invariant checking needs to happen at every site using the IP range rather than having a unified struct for enforcing those invariants. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 230 +- 1 file changed, 228 insertions(+), 2 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index e48ac1b..42ec1a1 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -1,9 +1,9 @@ -use std::fmt; +use std::fmt::{self, Display}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::ops::Deref; use anyhow::{bail, format_err, Error}; -use serde_with::DeserializeFromStr; +use serde_with::{DeserializeFromStr, SerializeDisplay}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Family { @@ -239,6 +239,202 @@ impl> From for Ipv6Cidr { } } +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] +pub enum IpRangeError { +MismatchedFamilies, +StartGreaterThanEnd, +InvalidFormat, +} + +impl std::error::Error for IpRangeError {} + +impl Display for IpRangeError { +fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +f.write_str(match self { +IpRangeError::MismatchedFamilies => "mismatched ip address families", +IpRangeError::StartGreaterThanEnd => "start is greater than end", +IpRangeError::InvalidFormat => "invalid ip range format", +}) +} +} + +/// Represents a range of IPv4 or IPv6 addresses +/// +/// For more information see [`AddressRange`] +#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +pub enum IpRange { +V4(AddressRange), +V6(AddressRange), +} + +impl IpRange { +/// returns the family of the IpRange +pub fn family(&self) -> Family { +match self { +IpRange::V4(_) => Family::V4, +IpRange::V6(_) => Family::V6, +} +} + +/// creates a new [`IpRange`] from two [`IpAddr`] +/// +/// # Errors +/// +/// This function will return an error if start and end IP address are not from the same family. +pub fn new(start: impl Into, end: impl Into) -> Result { +match (start.into(), end.into()) { +(IpAddr::V4(start), IpAddr::V4(end)) => Self::new_v4(start, end), +(IpAddr::V6(start), IpAddr::V6(end)) => Self::new_v6(start, end), +_ => Err(IpRangeError::MismatchedFamilies), +} +} + +/// construct a new Ipv4 Range +pub fn new_v4( +start: impl Into, +end: impl Into, +) -> Result { +Ok(IpRange::V4(AddressRange::new_v4(start, end)?)) +} + +pub fn new_v6( +start: impl Into, +end: impl Into, +) -> Result { +Ok(IpRange::V6(AddressRange::new_v6(start, end)?)) +} +} + +impl std::str::FromStr for IpRange { +type Err = IpRangeError; + +fn from_str(s: &str) -> Result { +if let Ok(range) = s.parse() { +return Ok(IpRange::V4(range)); +} + +if let Ok(range) = s.parse() { +return Ok(IpRange::V6(range)); +} + +Err(IpRangeError::InvalidFormat) +} +} + +impl fmt::Display for IpRange { +fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +match self { +IpRange::V4(range) => range.fmt(f), +IpRange::V6(range) => range.fmt(f), +} +} +} + +/// Represents a range of IP addresses from start to end +/// +/// This type is for encapsulation purposes for the [`IpRange`] enum and should be instantiated via +/// that enum. +/// +/// # Invariants +/// +/// * start and end have the same IP address family +/// * start is lesser than or equal to end +/// +/// # Textual representation +/// +/// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255` +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct AddressRange { +start: T, +end: T, +} + +impl AddressRange { +pub(crate) fn new_v4( +start: impl Into, +end: impl Into, +) -> Result, IpRangeError> { +let (start, end) = (start.into(), end.into()); + +if start > end { +return Err(IpRangeError::StartGreaterThanEnd); +} + +Ok(Self { start, end }) +} +} + +impl AddressRange { +pub(crate) fn new_v6( +start: impl Into, +end: impl Into, +) -> Result, IpRangeError> { +let (start, end) = (start.into(), end.into()); + +if start > end { +return Err(IpRangeError::StartGreaterThanEnd); +} + +Ok(Self { start, end }) +} +} + +impl AddressRange { +
[pve-devel] [PATCH proxmox-ve-rs v2 10/25] firewall: guest: derive traits according to rust api guidelines
Almost every type should implement them anyway, and many of them are required for those types to be used in BTreeMaps, which the nftables firewall uses for generating stable output. Additionally, we derive Serialize and Deserialize for a few types that occur in the sdn configuration. The following patches will use those for (de-)serialization. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 19 +++ proxmox-ve-config/src/firewall/types/alias.rs | 4 ++-- proxmox-ve-config/src/firewall/types/ipset.rs | 6 +++--- proxmox-ve-config/src/guest/types.rs | 7 --- proxmox-ve-config/src/guest/vm.rs | 7 --- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 9f4ad02..6978a8f 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -30,8 +30,9 @@ impl fmt::Display for Family { } } -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive( +Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash, SerializeDisplay, DeserializeFromStr, +)] pub enum Cidr { Ipv4(Ipv4Cidr), Ipv6(Ipv6Cidr), @@ -101,8 +102,7 @@ impl From for Cidr { const IPV4_LENGTH: u8 = 32; -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct Ipv4Cidr { addr: Ipv4Addr, mask: u8, @@ -176,8 +176,7 @@ impl fmt::Display for Ipv4Cidr { const IPV6_LENGTH: u8 = 128; -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct Ipv6Cidr { addr: Ipv6Addr, mask: u8, @@ -271,7 +270,9 @@ impl Display for IpRangeError { /// Represents a range of IPv4 or IPv6 addresses /// /// For more information see [`AddressRange`] -#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +#[derive( +Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] pub enum IpRange { V4(AddressRange), V6(AddressRange), @@ -364,7 +365,9 @@ impl fmt::Display for IpRange { /// # Textual representation /// /// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255` -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive( +Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] pub struct AddressRange { start: T, end: T, diff --git a/proxmox-ve-config/src/firewall/types/alias.rs b/proxmox-ve-config/src/firewall/types/alias.rs index e6aa30d..5dfaa41 100644 --- a/proxmox-ve-config/src/firewall/types/alias.rs +++ b/proxmox-ve-config/src/firewall/types/alias.rs @@ -2,7 +2,7 @@ use std::fmt::Display; use std::str::FromStr; use anyhow::{bail, format_err, Error}; -use serde_with::DeserializeFromStr; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use crate::firewall::parse::{match_name, match_non_whitespace}; use crate::firewall::types::address::Cidr; @@ -35,7 +35,7 @@ impl Display for AliasScope { } } -#[derive(Debug, Clone, DeserializeFromStr)] +#[derive(Debug, Clone, DeserializeFromStr, SerializeDisplay)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct AliasName { scope: AliasScope, diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 7511490..fe5a930 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -88,7 +88,7 @@ impl Display for IpsetName { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpsetAddress { Alias(AliasName), @@ -124,7 +124,7 @@ impl From for IpsetAddress { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpsetEntry { pub nomatch: bool, @@ -208,7 +208,7 @@ impl Ipfilter<'_> { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct Ipset { pub name: IpsetName, diff --git a/proxmox-ve-config/src/guest/types.rs b/proxmox-ve-config/src/guest/types.rs index 217c537..ed6a48c 100644 --- a/proxmox-ve-config/src/guest/types.rs +++ b/proxmox-ve-config/src/guest/types.rs @@ -2,8 +2,11 @@ use std::fmt; use std::str::FromStr; use anyhow::{format_err, Error}; +use serde_with::{DeserializeFromStr, SerializeDisplay}; -#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] +#[derive( +Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, SerializeDisplay, DeserializeFromStr, +)] pub struct Vmid(u32); impl Vmid { @@ -34,5 +37,3 @@ impl FromStr for Vmid { )) } } - -serde_plain::derive_deserialize_from_fromstr!(Vmid, "va
[pve-devel] [PATCH proxmox-ve-rs v2 06/25] firewall: address: use new iprange type for ip entries
Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 81 +++ proxmox-ve-config/src/firewall/types/rule.rs | 6 +- 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 42ec1a1..019884c 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -439,57 +439,30 @@ impl fmt::Display for AddressRange { #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpEntry { Cidr(Cidr), -Range(IpAddr, IpAddr), +Range(IpRange), } impl std::str::FromStr for IpEntry { type Err = Error; fn from_str(s: &str) -> Result { -if s.is_empty() { -bail!("Empty IP specification!") +if let Ok(cidr) = s.parse() { +return Ok(IpEntry::Cidr(cidr)); } -let entries: Vec<&str> = s -.split('-') -.take(3) // so we can check whether there are too many -.collect(); - -match entries.as_slice() { -[cidr] => Ok(IpEntry::Cidr(cidr.parse()?)), -[beg, end] => { -if let Ok(beg) = beg.parse::() { -if let Ok(end) = end.parse::() { -if beg < end { -return Ok(IpEntry::Range(beg.into(), end.into())); -} - -bail!("start address is greater than end address!"); -} -} - -if let Ok(beg) = beg.parse::() { -if let Ok(end) = end.parse::() { -if beg < end { -return Ok(IpEntry::Range(beg.into(), end.into())); -} - -bail!("start address is greater than end address!"); -} -} - -bail!("start and end are not valid IP addresses of the same type!") -} -_ => bail!("Invalid amount of elements in IpEntry!"), +if let Ok(range) = s.parse() { +return Ok(IpEntry::Range(range)); } + +bail!("Invalid IP entry: {s}"); } } impl fmt::Display for IpEntry { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { -Self::Cidr(ip) => write!(f, "{ip}"), -Self::Range(beg, end) => write!(f, "{beg}-{end}"), +Self::Cidr(ip) => ip.fmt(f), +Self::Range(range) => range.fmt(f), } } } @@ -498,19 +471,7 @@ impl IpEntry { fn family(&self) -> Family { match self { Self::Cidr(cidr) => cidr.family(), -Self::Range(start, end) => { -if start.is_ipv4() && end.is_ipv4() { -return Family::V4; -} - -if start.is_ipv6() && end.is_ipv6() { -return Family::V6; -} - -// should never be reached due to constructors validating that -// start type == end type -unreachable!("invalid IP entry") -} +Self::Range(range) => range.family(), } } } @@ -521,6 +482,12 @@ impl From for IpEntry { } } +impl From for IpEntry { +fn from(value: IpRange) -> Self { +IpEntry::Range(value) +} +} + #[derive(Clone, Debug, DeserializeFromStr)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpList { @@ -708,7 +675,9 @@ mod tests { assert_eq!( entry, -IpEntry::Range([192, 168, 0, 1].into(), [192, 168, 99, 255].into()) +IpRange::new_v4([192, 168, 0, 1], [192, 168, 99, 255]) +.expect("valid IP range") +.into() ); entry = "fe80::1".parse().expect("valid IP entry"); @@ -733,10 +702,12 @@ mod tests { assert_eq!( entry, -IpEntry::Range( -[0xFD80, 0, 0, 0, 0, 0, 0, 1].into(), -[0xFD80, 0, 0, 0, 0, 0, 0, 0x].into(), +IpRange::new_v6( +[0xFD80, 0, 0, 0, 0, 0, 0, 1], +[0xFD80, 0, 0, 0, 0, 0, 0, 0x], ) +.expect("valid IP range") +.into() ); "192.168.100.0-192.168.99.255" @@ -764,7 +735,9 @@ mod tests { entries: vec![ IpEntry::Cidr(Cidr::new_v4([192, 168, 0, 1], 32).unwrap()), IpEntry::Cidr(Cidr::new_v4([192, 168, 100, 0], 24).unwrap()), -IpEntry::Range([172, 16, 0, 0].into(), [172, 32, 255, 255].into()), +IpRa
[pve-devel] [PATCH proxmox-ve-rs v2 11/25] common: add allowlist
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/common/mod.rs | 31 + proxmox-ve-config/src/lib.rs| 1 + 2 files changed, 32 insertions(+) create mode 100644 proxmox-ve-config/src/common/mod.rs diff --git a/proxmox-ve-config/src/common/mod.rs b/proxmox-ve-config/src/common/mod.rs new file mode 100644 index 000..ef09791 --- /dev/null +++ b/proxmox-ve-config/src/common/mod.rs @@ -0,0 +1,31 @@ +use core::hash::Hash; +use std::cmp::Eq; +use std::collections::HashSet; + +#[derive(Clone, Debug, Default)] +pub struct Allowlist(HashSet); + +impl FromIterator for Allowlist { +fn from_iter(iter: A) -> Self +where +A: IntoIterator, +{ +Allowlist(HashSet::from_iter(iter)) +} +} + +/// returns true if [`value`] is in the allowlist or if allowlist does not exist +impl Allowlist { +pub fn is_allowed(&self, value: &T) -> bool { +self.0.contains(value) +} +} + +impl Allowlist { +pub fn new(iter: I) -> Self +where +I: IntoIterator, +{ +Self::from_iter(iter) +} +} diff --git a/proxmox-ve-config/src/lib.rs b/proxmox-ve-config/src/lib.rs index 856b14f..1b6feae 100644 --- a/proxmox-ve-config/src/lib.rs +++ b/proxmox-ve-config/src/lib.rs @@ -1,3 +1,4 @@ +pub mod common; pub mod firewall; pub mod guest; pub mod host; -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 07/25] ipset: add range variant to addresses
A range can be used to store multiple IP addresses in an ipset that do not neatly fit into a single CIDR. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/ipset.rs | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 6fbdca8..e8131b5 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -6,7 +6,7 @@ use anyhow::{bail, format_err, Error}; use serde_with::DeserializeFromStr; use crate::firewall::parse::match_non_whitespace; -use crate::firewall::types::address::Cidr; +use crate::firewall::types::address::{Cidr, IpRange}; use crate::firewall::types::alias::AliasName; use crate::guest::vm::NetworkConfig; @@ -93,6 +93,7 @@ impl Display for IpsetName { pub enum IpsetAddress { Alias(AliasName), Cidr(Cidr), +Range(IpRange), } impl FromStr for IpsetAddress { @@ -117,6 +118,12 @@ impl> From for IpsetAddress { } } +impl From for IpsetAddress { +fn from(range: IpRange) -> Self { +IpsetAddress::Range(range) +} +} + #[derive(Debug)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpsetEntry { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v2 01/25] debian: add files for packaging
Since we now have a standalone repository for Proxmox VE related crates, add the required files for packaging the crates contained in this repository. Signed-off-by: Stefan Hanreich --- .cargo/config.toml | 5 ++ .gitignore | 8 +++ Cargo.toml | 17 +++ Makefile | 69 ++ build.sh | 35 + bump.sh| 44 proxmox-ve-config/Cargo.toml | 16 +++--- proxmox-ve-config/debian/changelog | 5 ++ proxmox-ve-config/debian/control | 43 proxmox-ve-config/debian/copyright | 19 +++ proxmox-ve-config/debian/debcargo.toml | 4 ++ 11 files changed, 255 insertions(+), 10 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 .gitignore create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100755 build.sh create mode 100755 bump.sh create mode 100644 proxmox-ve-config/debian/changelog create mode 100644 proxmox-ve-config/debian/control create mode 100644 proxmox-ve-config/debian/copyright create mode 100644 proxmox-ve-config/debian/debcargo.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000..3b5b6e4 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,5 @@ +[source] +[source.debian-packages] +directory = "/usr/share/cargo/registry" +[source.crates-io] +replace-with = "debian-packages" diff --git a/.gitignore b/.gitignore new file mode 100644 index 000..d72b68b --- /dev/null +++ b/.gitignore @@ -0,0 +1,8 @@ +/target +/*/target +Cargo.lock +**/*.rs.bk +/*.buildinfo +/*.changes +/build +/*-deb diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 000..ab23d89 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,17 @@ +[workspace] +members = [ +"proxmox-ve-config", +] +exclude = [ +"build", +] +resolver = "2" + +[workspace.package] +authors = ["Proxmox Support Team "] +edition = "2021" +license = "AGPL-3" +homepage = "https://proxmox.com"; +exclude = [ "debian" ] +rust-version = "1.70" + diff --git a/Makefile b/Makefile new file mode 100644 index 000..0da9b74 --- /dev/null +++ b/Makefile @@ -0,0 +1,69 @@ +# Shortcut for common operations: + +CRATES != echo proxmox-*/Cargo.toml | sed -e 's|/Cargo.toml||g' + +# By default we just run checks: +.PHONY: all +all: check + +.PHONY: deb +deb: $(foreach c,$(CRATES), $c-deb) + echo $(foreach c,$(CRATES), $c-deb) + lintian build/*.deb + +.PHONY: dsc +dsc: $(foreach c,$(CRATES), $c-dsc) + echo $(foreach c,$(CRATES), $c-dsc) + lintian build/*.dsc + +.PHONY: autopkgtest +autopkgtest: $(foreach c,$(CRATES), $c-autopkgtest) + +.PHONY: dinstall +dinstall: + $(MAKE) clean + $(MAKE) deb + sudo -k dpkg -i build/librust-*.deb + +%-deb: + ./build.sh $* + touch $@ + +%-dsc: + BUILDCMD='dpkg-buildpackage -S -us -uc -d' ./build.sh $* + touch $@ + +%-autopkgtest: + autopkgtest build/$* build/*.deb -- null + touch $@ + +.PHONY: check +check: + cargo test + +# Prints a diff between the current code and the one rustfmt would produce +.PHONY: fmt +fmt: + cargo +nightly fmt -- --check + +# Doc without dependencies +.PHONY: doc +doc: + cargo doc --no-deps + +.PHONY: clean +clean: + cargo clean + rm -rf build/ + rm -f -- *-deb *-dsc *-autopkgtest *.build *.buildinfo *.changes + +.PHONY: update +update: + cargo update + +%-upload: %-deb + cd build; \ + dcmd --deb rust-$*_*.changes \ + | grep -v '.changes$$' \ + | tar -cf "$@.tar" -T-; \ + cat "$@.tar" | ssh -X repo...@repo.proxmox.com upload --product devel --dist bookworm diff --git a/build.sh b/build.sh new file mode 100755 index 000..39a8302 --- /dev/null +++ b/build.sh @@ -0,0 +1,35 @@ +#!/bin/sh + +set -eux + +export CARGO=/usr/bin/cargo +export RUSTC=/usr/bin/rustc + +CRATE=$1 +BUILDCMD=${BUILDCMD:-"dpkg-buildpackage -b -uc -us"} + +mkdir -p build +echo system >build/rust-toolchain +rm -rf "build/${CRATE}" + +CONTROL="$PWD/${CRATE}/debian/control" + +if [ -e "$CONTROL" ]; then +# check but only warn, debcargo fails anyway if crates are missing +dpkg-checkbuilddeps $PWD/${CRATE}/debian/control || true +# rm -f "$PWD/${CRATE}/debian/control" +fi + +debcargo package \ +--config "$PWD/${CRATE}/debian/debcargo.toml" \ +--changelog-ready \ +--no-overlay-write-back \ +--directory "$PWD/build/${CRATE}" \ +"${CRATE}" \ +"$(dpkg-parsechangelog -l "${CRATE}/debian/changelog" -SVersion | sed -e 's/-.*//')" + +cd "bui
[pve-devel] [PATCH docs/firewall/manager/proxmox{-ve-rs, -firewall, -perl-rs} v2 00/25] autogenerate ipsets for sdn objects
This patch series adds support for autogenerating ipsets for SDN objects. It autogenerates ipsets for every VNet as follows: * ipset containing all IP ranges of the VNet * ipset containing all gateways of the VNet * ipset containing all IP ranges of the subnet - except gateways * ipset containing all dhcp ranges of the vnet Additionally it generates an IPSet for every guest that has one or more IPAM entries in the pve IPAM. Those can then be used in the cluster / host / guest firewalls. Firewall rules automatically update on changes of the SDN / IPAM configuration. This patch series works for the old firewall as well as the new firewall. The ipsets in nftables currently get generated as named ipsets in every table, this means that the `nft list ruleset` output can get quite crowded for large SDN configurations or large IPAM databases. Another option would be to only include them as anonymous IPsets in the rules, which would make the nft output far less crowded but this way would use more memory when making extensive use of the sdn ipsets, since everytime it is used in a rule we create an entirely new ipset. This patch series is based on my private repositories that split the existing proxmox-firewall package into proxmox-firewall and proxmox-ve-rs. Those can be found in my staff repo: staff/s.hanreich/proxmox-ve-rs.git master staff/s.hanreich/proxmox-firewall.git no-config Please note that I included the debian packaging commit in this patch series, since it is new and should get reviewed as well, I suppose. It is already included when pulling from the proxmox-ve-rs repository. Dependencies: * proxmox-perl-rs and proxmox-firewall depend on proxmox-ve-rs * pve-firewall depends on proxmox-perl-rs Changes from RFC: * added documentation * added separate SDN scope for IPSets * rustfmt fixes proxmox-ve-rs: Fabian Grünbichler (1): bump serde_with to 3 Stefan Hanreich (17): debian: add files for packaging bump dependencies firewall: add sdn scope for ipsets firewall: add ip range types firewall: address: use new iprange type for ip entries ipset: add range variant to addresses iprange: add methods for converting an ip range to cidrs ipset: address: add helper methods firewall: guest: derive traits according to rust api guidelines common: add allowlist sdn: add name types sdn: add ipam module sdn: ipam: add method for generating ipsets sdn: add config module sdn: config: add method for generating ipsets tests: add sdn config tests tests: add ipam tests .cargo/config.toml|5 + .gitignore|8 + Cargo.toml| 17 + Makefile | 69 + build.sh | 35 + bump.sh | 44 + proxmox-ve-config/Cargo.toml | 18 +- proxmox-ve-config/debian/changelog|5 + proxmox-ve-config/debian/control | 43 + proxmox-ve-config/debian/copyright| 19 + proxmox-ve-config/debian/debcargo.toml|4 + proxmox-ve-config/src/common/mod.rs | 31 + .../src/firewall/types/address.rs | 1171 - proxmox-ve-config/src/firewall/types/alias.rs |4 +- proxmox-ve-config/src/firewall/types/ipset.rs | 32 +- proxmox-ve-config/src/firewall/types/rule.rs |6 +- proxmox-ve-config/src/guest/types.rs |7 +- proxmox-ve-config/src/guest/vm.rs | 11 +- proxmox-ve-config/src/lib.rs |2 + proxmox-ve-config/src/sdn/config.rs | 642 + proxmox-ve-config/src/sdn/ipam.rs | 382 ++ proxmox-ve-config/src/sdn/mod.rs | 243 proxmox-ve-config/tests/sdn/main.rs | 189 +++ proxmox-ve-config/tests/sdn/resources/ipam.db | 26 + .../tests/sdn/resources/running-config.json | 54 + 25 files changed, 2980 insertions(+), 87 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 .gitignore create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100755 build.sh create mode 100755 bump.sh create mode 100644 proxmox-ve-config/debian/changelog create mode 100644 proxmox-ve-config/debian/control create mode 100644 proxmox-ve-config/debian/copyright create mode 100644 proxmox-ve-config/debian/debcargo.toml create mode 100644 proxmox-ve-config/src/common/mod.rs create mode 100644 proxmox-ve-config/src/sdn/config.rs create mode 100644 proxmox-ve-config/src/sdn/ipam.rs create mode 100644 proxmox-ve-config/src/sdn/mod.rs create mode 100644 proxmox-ve-config/tests/sdn/main.rs create mode 100644 proxmox-ve-config/tests/sdn/resources/ipam.db create mode 100644 proxmox-ve-config/tests/sdn/resources/running-config.json proxmox-firewall: Stefan Hanreich (2): config: tests: add support for loading sdn and ipam config ipsets: autogenerate ipsets for
Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
On 10/7/24 12:02, Christoph Heiss wrote: > Sounds like a good idea tho. Shouldn't be that much work, other than > maybe a custom (small) de-/serializer for the answer file and config > output. Possibly even untagged and rename serde directive could suffice (together with a FromStr impl maybe?) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
On 10/7/24 11:52, Christoph Heiss wrote: > Thanks for the review! > > On Mon, Oct 07, 2024 at 11:49:02AM GMT, Stefan Hanreich wrote: >> On 10/7/24 11:22, Christoph Heiss wrote: > [..] >>> @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> >>> Result<()> { >>> } else if answer.global.root_password.is_none() && >>> answer.global.root_password_hashed.is_none() >>> { >>> bail!("One of `global.root_password` or >>> `global.root_password_hashed` must be set"); >>> +} else if answer.global.root_password.is_some() >>> +&& answer.global.root_password.as_ref().map(|s| s.len()) < >>> Some(ROOT_PASSWORD_MIN_LENGTH) >>> +{ >>> +bail!("`global.root_password` must be at least >>> {ROOT_PASSWORD_MIN_LENGTH} characters long"); >>> } else { >>> Ok(()) >>> } >> >> maybe match is better at this point? >> >> Something like >> >> match (answer.global.root_password, answer.global.root_password_hashed) { >> [..] >> (Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH, >> [..] >> } >> > > I'll rework it a bit, match definitely seems like the better fit here! I also thought about suggesting an Enum here like enum AnswerPassword { Raw(...) Hashed(...) } But that's probably a bit more involved and idk how it would work with the answer file format, so hard for me to tell if feasible with the current state of things ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 3/3] auto-installer: raise minimum root password length to 8 characters
On 10/7/24 11:22, Christoph Heiss wrote: > .. in accordance with current NIST recommendations [0]. > > It's 2024; so reasonable to expect an 8-character-password at the > minimum. > > [0] https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver > > Signed-off-by: Christoph Heiss > --- > proxmox-auto-installer/src/utils.rs | 5 + > .../tests/resources/parse_answer/disk_match.json | 2 +- > .../tests/resources/parse_answer/disk_match.toml | 2 +- > .../tests/resources/parse_answer/disk_match_all.json | 2 +- > .../tests/resources/parse_answer/disk_match_all.toml | 2 +- > .../tests/resources/parse_answer/disk_match_any.json | 2 +- > .../tests/resources/parse_answer/disk_match_any.toml | 2 +- > .../tests/resources/parse_answer/minimal.json| 2 +- > .../tests/resources/parse_answer/minimal.toml| 2 +- > .../tests/resources/parse_answer/nic_matching.json | 2 +- > .../tests/resources/parse_answer/nic_matching.toml | 2 +- > .../tests/resources/parse_answer/specific_nic.json | 2 +- > .../tests/resources/parse_answer/specific_nic.toml | 2 +- > proxmox-auto-installer/tests/resources/parse_answer/zfs.json | 2 +- > proxmox-auto-installer/tests/resources/parse_answer/zfs.toml | 2 +- > 15 files changed, 19 insertions(+), 14 deletions(-) > > diff --git a/proxmox-auto-installer/src/utils.rs > b/proxmox-auto-installer/src/utils.rs > index 45ad222..e0dd2ae 100644 > --- a/proxmox-auto-installer/src/utils.rs > +++ b/proxmox-auto-installer/src/utils.rs > @@ -13,6 +13,7 @@ use proxmox_installer_common::{ > setup::{ > InstallConfig, InstallRootPassword, InstallZfsOption, LocaleInfo, > RuntimeInfo, SetupInfo, > }, > +ROOT_PASSWORD_MIN_LENGTH, > }; > use serde::{Deserialize, Serialize}; > > @@ -309,6 +310,10 @@ fn verify_root_password_settings(answer: &Answer) -> > Result<()> { > } else if answer.global.root_password.is_none() && > answer.global.root_password_hashed.is_none() > { > bail!("One of `global.root_password` or > `global.root_password_hashed` must be set"); > +} else if answer.global.root_password.is_some() > +&& answer.global.root_password.as_ref().map(|s| s.len()) < > Some(ROOT_PASSWORD_MIN_LENGTH) > +{ > +bail!("`global.root_password` must be at least > {ROOT_PASSWORD_MIN_LENGTH} characters long"); > } else { > Ok(()) > } maybe match is better at this point? Something like match (answer.global.root_password, answer.global.root_password_hashed) { [..] (Some(password), _) if password.len() < ROOT_PASSWORD_MIN_LENGTH, [..] } ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints
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
Re: [pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
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, } 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
[pve-devel] [PATCH proxmox-firewall 06/15] sdn: create forward firewall rules
Signed-off-by: Stefan Hanreich --- .../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
[pve-devel] [PATCH pve-firewall 09/15] sdn: add vnet firewall configuration
Signed-off-by: Stefan Hanreich --- 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|DR
[pve-devel] [PATCH pve-manager 13/15] firewall: make base_url dynamically configurable in options component
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 --- 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
[pve-devel] [PATCH pve-network 15/15] firewall: add endpoints for vnet-level firewall
Signed-off-by: Stefan Hanreich --- 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
[pve-devel] [PATCH proxmox-ve-rs 02/15] firewall: add forward direction
This direction will be used for specifying rules on bridge-level firewalls as well as rules on the cluster / host level that are for forwarded network packets. Signed-off-by: Stefan Hanreich --- 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(input: R) -> Result { @@ -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, policy_out: Option, +policy_forward: Option, } #[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, log_level_out: Option, +log_level_forward: Option, #[serde(default, with = "serde_option_bool")] macfilter: Option, @@ -61,6 +64,8 @@ pub struct Options { #[serde(rename = "policy_out")] policy_out: Option, + +policy_forward: Option, } #[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, log_level_out: Option, +log_level_forward: Option, #[serd
[pve-devel] [RFC firewall/manager/network/proxmox{-ve-rs, -firewall} 00/15] add forward chain firewalling for hosts and bridges
## Introduction This patch series introduces a new direction for firewall rules: forward. Additionally this patch series introduces defining firewall rules on a vnet level. ## Use Cases For hosts: * hosts utilizing NAT can define firewall rules for NATed traffic * hosts utilizing EVPN zones can define rules for exit node traffic * hosts acting as gateway can firewall the traffic that passes through them For vnets: * can create firewall rules globally without having to attach/update security groups to every newly created VM This patch series is particularly useful when combined with my other current RFC 'autogenerate ipsets for sdn objects'. It enables users to quickly define rules like: on the host level: * only SNAT HTTP traffic from hosts in this vnet to a specific host * restricting traffic routed from hosts in one vnet to another vnet on the vnet level: * only allow DHCP/DNS traffic inside a bridge to the gateway Not only does this streamline creating firewall rules, it also enables users to create firewall rules that haven't been possible before and needed to rely on external firewall appliances. Since forwarded traffic goes *both* ways, you generally have to create two rules in case of bi-directional traffic. It might make sense to simplify this in the future by adding an additional option to the firewall config scheme that specifies that rules in the other direction should also get automatically generated. ## Usage For creating forward rules on the cluster/host level, you simply create a new rule with the new 'forward' direction. It uses the existing configuration files. For creating them on a vnet level, there are new firewall configuration files located under '/etc/pve/sdn/firewall/.fw'. It utilizes the same configuration format as the existing firewall configuration files. You can only define rules with direction 'forward' on a vnet-level. proxmox-ve-rs: Stefan Hanreich (4): cargo: bump dependencies firewall: add forward direction firewall: add bridge firewall config parser host: add struct representing bridge names proxmox-ve-config/Cargo.toml | 5 +- proxmox-ve-config/src/firewall/bridge.rs | 59 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 +++- proxmox-ve-config/src/host/mod.rs| 1 + proxmox-ve-config/src/host/types.rs | 46 +++ 9 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 proxmox-ve-config/src/firewall/bridge.rs create mode 100644 proxmox-ve-config/src/host/types.rs proxmox-firewall: Stefan Hanreich (4): sdn: add support for loading vnet-level firewall config sdn: create forward firewall rules use std::mem::take over drain() cargo: bump dependencies Cargo.toml| 3 + proxmox-firewall/Cargo.toml | 2 +- .../resources/proxmox-firewall.nft| 54 +++ proxmox-firewall/src/config.rs| 88 - proxmox-firewall/src/firewall.rs | 97 ++- proxmox-firewall/src/rule.rs | 7 +- proxmox-firewall/tests/integration_tests.rs | 12 +++ .../integration_tests__firewall.snap | 86 proxmox-nftables/Cargo.toml | 2 +- proxmox-nftables/src/expression.rs| 8 ++ proxmox-nftables/src/types.rs | 6 ++ 11 files changed, 355 insertions(+), 10 deletions(-) pve-firewall: Stefan Hanreich (2): sdn: add vnet firewall configuration api: add vnet endpoints src/PVE/API2/Firewall/Makefile | 1 + src/PVE/API2/Firewall/Rules.pm | 71 ++ src/PVE/API2/Firewall/Vnet.pm | 166 + src/PVE/Firewall.pm| 124 +++- src/PVE/Firewall/Helpers.pm| 12 +++ 5 files changed, 369 insertions(+), 5 deletions(-) create mode 100644 src/PVE/API2/Firewall/Vnet.pm pve-manager: Stefan Hanreich (4): firewall: add forward direction to rule panel firewall: add vnet to firewall options component firewall: make base_url dynamically configurable in options component sdn: add firewall panel www/manager6/Makefile| 2 + www/manager6/dc/Config.js| 9 www/manager6/dc/SecurityGroups.js| 1 + www/manager6/grid/FirewallOptions.js | 72 +- www/manager6/grid/FirewallRules.js | 32 ++-- www/manager6/lxc/Config.js | 1 + www/manager6/node/Config.js | 1 + www/manager6/qemu/Config.js | 1 + www/manager6/sdn/FirewallPanel.js| 48 + www/manager6/sdn/FirewallVnetView.js | 77 10 files change
[pve-devel] [PATCH proxmox-firewall 08/15] cargo: bump dependencies
Signed-off-by: Stefan Hanreich --- 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
[pve-devel] [PATCH pve-firewall 10/15] api: add vnet endpoints
Signed-off-by: Stefan Hanreich --- 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 000..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 $r
[pve-devel] [PATCH pve-manager 11/15] firewall: add forward direction to rule panel
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 --- 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: '' + gettext('Rules') + ':', 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,
[pve-devel] [PATCH pve-manager 14/15] sdn: add firewall panel
Expose the ability to create vnet-level firewalls in the PVE UI Signed-off-by: Stefan Hanreich --- 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 ..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 ..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: [ + {
[pve-devel] [PATCH pve-manager 12/15] firewall: add vnet to firewall options component
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 --- 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
[pve-devel] [PATCH proxmox-firewall 05/15] sdn: add support for loading vnet-level firewall config
Signed-off-by: Stefan Hanreich --- 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>, Error>; fn sdn_running_config(&self) -> Result>, Error>; fn ipam(&self) -> Result>, Error>; +fn bridge_list(&self) -> Result, Error>; +fn bridge_firewall_config( +&self, +bridge_name: &BridgeName, +) -> Result>, Error>; } #[derive(Default)] @@ -61,8 +68,31 @@ fn open_config_file(path: &str) -> Result, Error> { } } +fn open_config_folder(path: &str) -> Result, 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 { +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, 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>, 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; +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, +bridge_config: BTreeMap, nft_config: BTreeMap, sdn_config: Option, ipam_config: Option, @@ -284,6 +347,22 @@ impl FirewallConfig { Ok(chains) } +pub fn parse_bridges( +firewall_loader: &dyn FirewallConfigLoader, +) -> Result, 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) +} +
[pve-devel] [PATCH proxmox-firewall 07/15] use std::mem::take over drain()
This is more efficient than draining and collecting the Vec. It also fixes the respective clippy lint. Signed-off-by: Stefan Hanreich --- 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, env: &NftRuleEnv) -> Result<(), Error> { log::trace!("applying macro: {self:?}"); -let initial_rules: Vec = rules.drain(..).collect(); +let initial_rules: Vec = 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
[pve-devel] [PATCH proxmox-ve-rs 04/15] host: add struct representing bridge names
Signed-off-by: Stefan Hanreich --- 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 000..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 { +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::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 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
[pve-devel] [PATCH proxmox-ve-rs 03/15] firewall: add bridge firewall config parser
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 --- 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 000..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, +} + +/// 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(input: R) -> Result { +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 + '_ { +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, + +policy_forward: Option, +} -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs 01/15] cargo: bump dependencies
Signed-off-by: Stefan Hanreich --- 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
Re: [pve-devel] SDN Feature request - add IPAM support for Nautobot
Hi! Thanks for your interest in contributing to Proxmox VE. A good start would be to look at the existing Netbox plugin [1], as well as the base plugin [2]. I've shortly tried using the existing Netbox plugin with the Nautobot API, but it seems like the API already diverged too much for it to work. Nevertheless, I think some endpoints would still work 1:1 (was hard for me to test due to other endpoints failing). So a good starting point could be extending the Netbox plugin and overriding the methods that have changed. Sadly, the documentation for plugins is still a bit lacking wrt expected behaviour of the plugin functions - if you have any questions regarding that I'd be happy to assist you. Before submitting the patches, please also make sure to read our developer guidelines [3]. We also require contributors to sign a CLA, which you can find on our homepage [4]. Kind Regards Stefan [1] https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Ipams/NetboxPlugin.pm;h=d9232696e4cb638c788f94f3b01c8c13c6fb2d93;hb=HEAD [2] https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Ipams/Plugin.pm;h=05d1416c8cc725b759c17273b29b32bbceeaa156;hb=HEAD [3] https://pve.proxmox.com/wiki/Developer_Documentation [4] https://www.proxmox.com/en/about/developers ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type
On 7/30/24 20:01, Thomas Lamprecht wrote: > albeit, it might be slightly nicer to also disable the field when it's not > relevant, as then any validation handling cannot make the form invalid without > the user seeing why nor being able to fix it. Yes, thinking about it again that would have been the better way to be honest. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 1/1] fix #5619: honor link_down setting when hot-plugging nic
sent a v2: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064855.html On 7/23/24 15:14, Stefan Hanreich wrote: > When detaching and attaching the network device on update, the > link_down setting is not considered and the network device always gets > attached to the guest - even if link_down is set. > > Fixes: 3f14f206 ("nic online bridge/vlan change: link disconnect/reconnect") > > Signed-off-by: Stefan Hanreich > --- > PVE/QemuServer.pm | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 88c274d..19e59a8 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -5339,20 +5339,16 @@ sub vmconfig_update_net { > PVE::Network::tap_plug($iface, $newnet->{bridge}, > $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate}); > } > > - #set link_up in guest if bridge or vlan change to notify guest > (dhcp renew for example) > - if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) || > - safe_num_ne($oldnet->{tag}, $newnet->{tag}) > - ) { > - qemu_set_link_status($vmid, $opt, 1); > - } > - > } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) { > # Rate can be applied on its own but any change above needs to > # include the rate in tap_plug since OVS resets everything. > PVE::Network::tap_rate_limit($iface, $newnet->{rate}); > } > > - if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down})) { > + if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down}) > + || safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) > + || safe_num_ne($oldnet->{tag}, $newnet->{tag}) > + ) { > qemu_set_link_status($vmid, $opt, !$newnet->{link_down}); > } > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 1/1] fix #5619: honor link_down setting when hot-plugging nic
When detaching and attaching the network device on update, the link_down setting is not considered and the network device always gets attached to the guest - even if link_down is set. Fixes: 3f14f206 ("nic online bridge/vlan change: link disconnect/reconnect") Signed-off-by: Stefan Hanreich Reviewed-by: Fiona Ebner --- Changes from v1 -> v2: * Fixed trailers in commit message * Re-added comment describing the reason for setting link_down on changed bridge/tag Thanks @Fiona for your suggestions! PVE/QemuServer.pm | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 88c274d..39d729c 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -5339,20 +5339,18 @@ sub vmconfig_update_net { PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate}); } - #set link_up in guest if bridge or vlan change to notify guest (dhcp renew for example) - if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) || - safe_num_ne($oldnet->{tag}, $newnet->{tag}) - ) { - qemu_set_link_status($vmid, $opt, 1); - } - } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) { # Rate can be applied on its own but any change above needs to # include the rate in tap_plug since OVS resets everything. PVE::Network::tap_rate_limit($iface, $newnet->{rate}); } - if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down})) { + # set link_down on changed bridge/tag as well, because we detach the + # network device in the section above if the bridge or tag changed + if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down}) + || safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) + || safe_num_ne($oldnet->{tag}, $newnet->{tag}) + ) { qemu_set_link_status($vmid, $opt, !$newnet->{link_down}); } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 1/1] fix #5619: honor link_down setting when hot-plugging nic
On 7/23/24 16:00, Fiona Ebner wrote: > > Am 23.07.24 um 15:14 schrieb Stefan Hanreich: >> When detaching and attaching the network device on update, the >> link_down setting is not considered and the network device always gets >> attached to the guest - even if link_down is set. >> >> Fixes: 3f14f206 ("nic online bridge/vlan change: link disconnect/reconnect") >> > > Please don't use newlines between trailers. Yes, I only noticed after sending > >> Signed-off-by: Stefan Hanreich > > Reviewed-by: Fiona Ebner > >> --- >> PVE/QemuServer.pm | 12 >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index 88c274d..19e59a8 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -5339,20 +5339,16 @@ sub vmconfig_update_net { >> PVE::Network::tap_plug($iface, $newnet->{bridge}, >> $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate}); >> } >> >> -#set link_up in guest if bridge or vlan change to notify guest >> (dhcp renew for example) > > I'd prefer to not remove the comment completely, but adapt and move it > to below. Then nobody needs to wonder why link status is set if only > bridge or tag changes for example. Makes sense, I'll re-add a similar comment in a v2 > >> -if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) || >> -safe_num_ne($oldnet->{tag}, $newnet->{tag}) >> -) { >> -qemu_set_link_status($vmid, $opt, 1); >> -} >> - >> } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) { >> # Rate can be applied on its own but any change above needs to >> # include the rate in tap_plug since OVS resets everything. >> PVE::Network::tap_rate_limit($iface, $newnet->{rate}); >> } >> >> -if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down})) { >> +if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down}) >> +|| safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) >> +|| safe_num_ne($oldnet->{tag}, $newnet->{tag}) >> +) { >> qemu_set_link_status($vmid, $opt, !$newnet->{link_down}); >> } >> ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 1/1] fix #5619: honor link_down setting when hot-plugging nic
When detaching and attaching the network device on update, the link_down setting is not considered and the network device always gets attached to the guest - even if link_down is set. Fixes: 3f14f206 ("nic online bridge/vlan change: link disconnect/reconnect") Signed-off-by: Stefan Hanreich --- PVE/QemuServer.pm | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 88c274d..19e59a8 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -5339,20 +5339,16 @@ sub vmconfig_update_net { PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate}); } - #set link_up in guest if bridge or vlan change to notify guest (dhcp renew for example) - if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) || - safe_num_ne($oldnet->{tag}, $newnet->{tag}) - ) { - qemu_set_link_status($vmid, $opt, 1); - } - } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) { # Rate can be applied on its own but any change above needs to # include the rate in tap_plug since OVS resets everything. PVE::Network::tap_rate_limit($iface, $newnet->{rate}); } - if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down})) { + if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down}) + || safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) + || safe_num_ne($oldnet->{tag}, $newnet->{tag}) + ) { qemu_set_link_status($vmid, $opt, !$newnet->{link_down}); } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common, widget-toolkit, manager v3 0/3] fix #3893: make bridge vids configurable
Tested the patches on my machine and everything worked as advertised. It might make sense to note that this setting currently only applies to the bridge_ports specified in the configuration, not the bridge interface itself. Not sure if this is an ifupdown2 bug or intended. I think it is actually a bug when reading the docs of the bridge-vids parameter: > Denotes the space separated list of VLANs to be allowed tagged > ingress/egress on this interface. It doesn't make a practical difference for our use case though. It might make sense to note that this only applies to north-south traffic (due to the bridge_ports getting the VLAN tags set), but not east-west. One can still create two network devices on two guests with a tag that is not specified in the bridge-vids and they can still communicate (This is actually not a bug, but intended behavior of the linux bridge when vlan_filtering is on!). This behavior might be conterintuitive for users. Consider this: Tested-By: Stefan Hanreich On 7/3/24 10:01, Aaron Lauterer wrote: > this version reworks a few parts since v2. > > * renamed format in JSONSchema to a more generic `pve-vlan-id-or-range` > * explicitly use spaces when writing interfaces file. This is one > possible approach to deal with the fact, that the generic `-list` > format will accept quite a few delimiters and we need spaces. > * code style improvements such as naming the regex results. > * add parameter verification to the web ui > > With the changes to the JSONSchema we can then work on using it too for > the guest trunk option. This hasn't been started yet though. > > common: Aaron Lauterer (3): > tools: add check_list_empty function > fix #3893: network: add vlan id and range parameter definitions > inotify: interfaces: make sure bridge_vids use space as separator > > src/PVE/INotify.pm| 2 +- > src/PVE/JSONSchema.pm | 34 ++ > src/PVE/Tools.pm | 8 > 3 files changed, 43 insertions(+), 1 deletion(-) > > > widget-toolkit: Aaron Lauterer (1): > fix #3892: Network: add bridge vids field for bridge_vids > > src/node/NetworkEdit.js | 62 + > src/node/NetworkView.js | 5 > 2 files changed, 67 insertions(+) > > > manager: Aaron Lauterer (2): > fix #3893: api: network: add bridge_vids parameter > fix #3893: ui: network: enable bridge_vids field > > PVE/API2/Network.pm | 15 ++- > www/manager6/node/Config.js | 1 + > 2 files changed, 15 insertions(+), 1 deletion(-) > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
Hi, I tested the following things via the 'Test' option in the Notifications configuration panel in PBS and PVE: * Setting a custom header * Using a secret in body / URL * POST / PUT / GET methods I triggered notifications in PBS for the following things: * GC * Sync I triggered notifications in PVE for the following things: * Backup using the following template: ``` this is {{ secrets.test }}, can has webhook? {{ title }} {{ message }} {{ severity }} {{ timestamp }} {{ json fields }} {{ json secrets }} {{ url-encode secrets.encode }} {{ escape secrets.escape }} ``` I used dummyhttp [1] for debugging and displaying the HTTP requests. I got the following JS error when trying to save a secret/header/body template with the content: `qweqwe ßẞ` Uncaught DOMException: String contains an invalid character getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11924 each ExtJS getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11920 checkChange ExtJS itemChange https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:12009 ExtJS 22 proxmoxlib.js:11924 Seems like the issue is with btoa and uppercase scharfes S? When using Umlauts, I at least get an error message, but cannot save. Other than that everything worked fine - consider this: Tested-By: Stefan Hanreich ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v3 1/1] service: flush firewall rules on force disable
When disabling the nftables firewall again, there is a race condition where the nftables ruleset never gets flushed and persists after disabling. The nftables firewall update loop does a noop when the force disable file exists. It only flushes the ruleset when nftables is disabled in the configuration file but the force disable file does not yet exist. This can lead to the following situation: * nftables is activated and created its ruleset * user switches from nftables firewall back to iptables firewall * pve-firewall runs and creates the force disable file * proxmox-firewall sees that the file exists and does nothing Reported-by: Hannes Laimer Signed-off-by: Stefan Hanreich --- Changes from v2 to v3: * Use proper debug output formatter Changes from v1 to v2: * Removed misleading/wrong section about the probability of this happening * Added a detailed description of the scenario this commit prevents proxmox-firewall/src/bin/proxmox-firewall.rs | 4 1 file changed, 4 insertions(+) diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs index f7e816e..4732e51 100644 --- a/proxmox-firewall/src/bin/proxmox-firewall.rs +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs @@ -91,6 +91,10 @@ fn main() -> Result<(), std::io::Error> { while !term.load(Ordering::Relaxed) { if force_disable_flag.exists() { +if let Err(error) = remove_firewall() { +log::error!("unable to disable firewall: {error:?}"); +} + std::thread::sleep(Duration::from_secs(5)); continue; } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings
Currently custom mappings cannot be edited, due to them having no VMID value. The VMID parameter was always sent by the frontend to the update call - even if it was empty - leading to validation failure on the backend. Fix this by only sending the vmid parameter when it is actually set. Signed-off-by: Stefan Hanreich --- www/manager6/tree/DhcpTree.js | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js index d0b80803..6868bb60 100644 --- a/www/manager6/tree/DhcpTree.js +++ b/www/manager6/tree/DhcpTree.js @@ -156,15 +156,20 @@ Ext.define('PVE.sdn.DhcpTree', { openEditWindow: function(data) { let me = this; + let extraRequestParams = { + mac: data.mac, + zone: data.zone, + vnet: data.vnet, + }; + + if (data.vmid) { + extraRequestParams.vmid = data.vmid; + } + Ext.create('PVE.sdn.IpamEdit', { autoShow: true, mapping: data, - extraRequestParams: { - vmid: data.vmid, - mac: data.mac, - zone: data.zone, - vnet: data.vnet, - }, + extraRequestParams, listeners: { destroy: () => me.reload(), }, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update
Updating the NIC of a VM when the following conditions were met: * VM is turned off * NIC is on a bridge that uses automatic dhcp * Leave bridge unchanged led to duplicate IPAM entries for the same network device. This is due to the fact that the add_next_free_cidr always ran on applying pending network changes. Now we only add a new ipam entry if either: * the value of the bridge or mac address changed * the network device has been newly added This way no duplicate IPAM entries should get created. Signed-off-by: Stefan Hanreich --- PVE/QemuServer.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 88c274d..bf59b09 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -5254,10 +5254,11 @@ sub vmconfig_apply_pending { safe_string_ne($old_net->{macaddr}, $new_net->{macaddr}) )) { PVE::Network::SDN::Vnets::del_ips_from_mac($old_net->{bridge}, $old_net->{macaddr}, $conf->{name}); + PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1); } + } else { + PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1); } - #fixme: reuse ip if mac change && same bridge - PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1); } }; if (my $err = $@) { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC firewall/proxmox{-ve-rs, -firewall, -perl-rs} 00/21] autogenerate ipsets for sdn objects
On 6/28/24 15:46, Gabriel Goller wrote: > Already talked with Stefan offlist, but some major things I noted when > testing: > * It would be cool to have the generated IPSets visible in the IPSet > menu under Firewall (Datacenter). We could add a checkmark to hide > them (as there can be quite many) and make them read-only. As already discussed, this might be a bit tricky to do read-only, since we want to be able to override those IPSets (as is the case with management, ipfilter, ..). It might make more sense to just additionally display the IP sets and make them editable as any other. That way you can easily append / delete IP addresses. Maybe give an indicator if this is an auto-generated IPSet or an overridden one in the UI? Maybe I'll make it a separate patch series that also implements this for the other auto-generated IPsets. > * Zones can be restricted to specific Nodes, but we generate the > IPSets on every Node for all Zones. This means some IPSets are > useless and we could avoid generating them in the first place. Will try and add this. > > Otherwise the IPSet generation works fine. The algorithm for generating > iptables ipset ranges also works perfectly! > Thanks for the review! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-ve-rs 12/21] sdn: add config module
On 6/27/24 12:54, Gabriel Goller wrote: > On 26.06.2024 14:15, Stefan Hanreich wrote: >> diff --git a/proxmox-ve-config/src/sdn/config.rs >> b/proxmox-ve-config/src/sdn/config.rs >> new file mode 100644 >> index 000..8454adf >> --- /dev/null >> +++ b/proxmox-ve-config/src/sdn/config.rs >> @@ -0,0 +1,571 @@ >> [snip] >> +impl Display for DhcpType { >> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { >> + f.write_str(match self { >> + DhcpType::Dnsmasq => "dnsmasq", >> + }) >> + } >> +} >> + >> +/// struct for deserializing a zone entry of the SDN running config > > I think we usually begin doc-strings with a capital letter :) > >> +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, >> Ord)] >> +pub struct ZoneRunningConfig { >> + #[serde(rename = "type")] >> + ty: ZoneType, >> + dhcp: DhcpType, >> +} >> + >> +/// struct for deserializing the zones of the SDN running config >> +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Default)] >> +pub struct ZonesRunningConfig { >> + ids: HashMap, >> +} >> + >> +/// represents the dhcp-range property string used in the SDN >> configuration >> +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, >> Ord)] >> +pub struct DhcpRange { >> + #[serde(rename = "start-address")] >> + start: IpAddr, >> + #[serde(rename = "end-address")] >> + end: IpAddr, >> +} >> + >> +impl ApiType for DhcpRange { >> + const API_SCHEMA: proxmox_schema::Schema = ObjectSchema::new( >> + "DHCP range", >> + &[ >> + ( >> + "end-address", >> + false, >> + &StringSchema::new("start address of DHCP >> range").schema(), > > Shouldn't this be "end address..." or is this intended? > Same below. Good catch, seems like i messed up when copy-pasting. >> + ), >> + ( >> + "start-address", >> + false, >> + &StringSchema::new("end address of DHCP >> range").schema(), >> + ), >> + ], >> + ) >> + .schema(); >> +} >> + > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-ve-rs 09/21] sdn: add name types
On 6/27/24 12:56, Gabriel Goller wrote: > On 26.06.2024 14:15, Stefan Hanreich wrote: >> diff --git a/proxmox-ve-config/src/sdn/mod.rs >> b/proxmox-ve-config/src/sdn/mod.rs >> new file mode 100644 >> index 000..4e7c525 >> --- /dev/null >> +++ b/proxmox-ve-config/src/sdn/mod.rs >> @@ -0,0 +1,240 @@ >> +use std::{error::Error, fmt::Display, str::FromStr}; >> + >> +use serde_with::DeserializeFromStr; >> + >> +use crate::firewall::types::Cidr; >> + >> +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] >> +pub enum SdnNameError { >> + Empty, >> + TooLong, >> + InvalidSymbols, >> + InvalidSubnetCidr, >> + InvalidSubnetFormat, >> +} >> + >> +impl Error for SdnNameError {} >> + >> +impl Display for SdnNameError { >> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { >> + f.write_str(match self { >> + SdnNameError::TooLong => "name too long", >> + SdnNameError::InvalidSymbols => "invalid symbols in name", >> + SdnNameError::InvalidSubnetCidr => "invalid cidr in name", >> + SdnNameError::InvalidSubnetFormat => "invalid format for >> subnet name", >> + SdnNameError::Empty => "name is empty", >> + }) >> + } >> +} >> + > > Hmm, maybe we should pull in the `thiserror` crate here... > There are a few error-enums that could benefit from it: SdnNameError, > IpamError, SdnConfigError, IpRangeError. Thought about this as well, I guess we depend on it in quite a few crates already - so pulling it in here wouldn't be too bad. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism
Did a quick smoke test of this series by creating an ISO with an answer file baked in and checking the response via `nc -l`. Review is inline. Consider this: Tested-By: Stefan Hanreich Reviewed-By: Stefan Hanreich On 7/10/24 15:27, Christoph Heiss wrote: > This implements a mechanism for post-installation "notifications" via a > POST request [0] when using the auto-installer. > > It's implemented as a separate, small utility to facilitate separation > of concerns and make the information gathering easier by having it > isolated in one place. > > Patches #1 through #10 are simply clean-ups, refactors, etc. that were > done along the way, which do not impact functionality in any way. > > Most interesting here will be patch #12, which adds the actual > implementation of the post-hook. (Bind-)mounting the installed host > system is done using the existing `proxmox-chroot` utility, and the HTTP > POST functionality can fortunately be re-used 1:1 from > `proxmox-fetch-answer`. > > I've also included an example of how the JSON body (pretty-printed for > readability) of such a post-installation request would look like below, > for reference. > > Tested this with both PVE and PBS ISOs, PMG did not (yet) have a > release with an auto-installation capable ISO. The only product-specific > code is the version detection in `proxmox-post-hook`, which - since it > works the same for PVE and PMG - be no obstacle. > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536 > > { > "debian-version": "12.5", > "product-version": "pve-manager/8.2.2/9355359cd7afbae4", > "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed", > "boot-type": "bios", > "filesystem": "zfs (RAID1)", > "fqdn": "host.domain", > "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc", > "bootdisk": [ > { > "size": 8589934592, > "udev-properties": { > "DEVNAME": "/dev/vda", [..] > } > }, > { > "size": 8589934592, > "udev-properties": { > "DEVNAME": "/dev/vdb", [..] > } > } > ], > "management-nic": { > "mac": "de:ad:f0:0d:12:34", > "address": "10.0.0.10/24", > "udev-properties": { > "INTERFACE": "enp6s18", [..] > } > }, > "ssh-public-host-keys": { > "ecdsa": "ecdsa-sha2-nistp256 [..] root@host", > "ed25519": "ssh-ed25519 [..] root@host", > "rsa": "ssh-rsa [..] root@host", > } > } > > Christoph Heiss (14): chroot: print full anyhow message > tree-wide: fix some typos > tree-wide: collect hardcoded installer runtime directory strings into > constant > common: simplify filesystem type serializing & Display trait impl > common: setup: serialize `target_hd` as string explicitly > common: split out installer setup files loading functionality > debian: strip unused library dependencies > fetch-answer: move http-related code to gated module in > installer-common > tree-wide: convert some more crates to use workspace dependencies > auto-installer: tests: replace left/right with got/expected in output > auto-installer: answer: add `posthook` section > fix #5536: add post-hook utility for sending notifications after > auto-install > fix #5536: post-hook: add some unit tests > unconfigured.sh: run proxmox-post-hook after successful auto-install > > Cargo.toml| 11 + > Makefile | 8 +- > debian/control| 1 + > debian/install| 1 + > debian/rules | 9 + > proxmox-auto-install-assistant/Cargo.toml | 14 +- > proxmox-auto-installer/Cargo.toml | 15 +- > proxmox-auto-installer/src/answer.rs | 27 +- > .../src/bin/proxmox-auto-installer.rs | 15 +- > proxmox-auto-installer/src/sysinfo.rs | 10 +- > proxmox-auto-installer/src/utils.rs | 15 +- > proxmox-auto-installer/tests/parse-answer.rs | 42 +- > proxmox-chroot/Cargo.toml | 8 +- > proxmox-chroot/src/main.rs| 19 +- > proxmox-fetch-answer/Cargo.toml | 17 +- > .../src/fetch_plugins/http.rs | 100 +--- > .../src/fetch_plugins/partition.rs|
Re: [pve-devel] [PATCH installer 12/14] fix #5536: add post-hook utility for sending notifications after auto-install
On 7/10/24 15:27, Christoph Heiss wrote: > +impl Answer { > +pub fn from_reader(reader: impl BufRead) -> Result { > +let mut buffer = String::new(); > +let lines = reader.lines(); > +for line in lines { > +buffer.push_str(&line.unwrap()); > +buffer.push('\n'); > +} > + > +toml::from_str(&buffer).map_err(|err| format_err!("Failed parsing > answer file: {err}")) > +} > +} maybe better impl TryFrom for Answer ? Or at least call the method try_from_reader if it returns a result? > #[derive(Clone, Deserialize, Debug)] > #[serde(deny_unknown_fields)] > pub struct Global { > diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > index bf6f8fb..aab0f1f 100644 > --- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > +++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs > @@ -42,16 +42,7 @@ fn auto_installer_setup(in_test_mode: bool) -> > Result<(Answer, UdevInfo)> { > .map_err(|err| format_err!("Failed to retrieve udev info > details: {err}"))? > }; > > -let mut buffer = String::new(); > -let lines = std::io::stdin().lock().lines(); > -for line in lines { > -buffer.push_str(&line.unwrap()); > -buffer.push('\n'); > -} > - > -let answer: Answer = > -toml::from_str(&buffer).map_err(|err| format_err!("Failed parsing > answer file: {err}"))?; > - > +let answer = Answer::from_reader(std::io::stdin().lock())?; > Ok((answer, udev_info)) > } > > @@ -91,8 +82,6 @@ fn main() -> ExitCode { > } > } > > -// TODO: (optionally) do a HTTP post with basic system info, like host > SSH public key(s) here > - > ExitCode::SUCCESS > } > > diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs > b/proxmox-fetch-answer/src/fetch_plugins/http.rs > index a6a8de0..4317430 100644 > --- a/proxmox-fetch-answer/src/fetch_plugins/http.rs > +++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs > @@ -68,7 +68,7 @@ impl FetchFromHTTP { > let payload = SysInfo::as_json()?; > info!("Sending POST request to '{answer_url}'."); > let answer = > -proxmox_installer_common::http::post(answer_url, > fingerprint.as_deref(), payload)?; > +proxmox_installer_common::http::post(&answer_url, > fingerprint.as_deref(), payload)?; > Ok(answer) > } > > diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs > b/proxmox-fetch-answer/src/fetch_plugins/partition.rs > index 4472922..f07389b 100644 > --- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs > +++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs > @@ -31,7 +31,7 @@ impl FetchFromPartition { > } > > fn path_exists_logged(file_name: &str, search_path: &str) -> Option > { > -let path = Path::new(search_path).join(&file_name); > +let path = Path::new(search_path).join(file_name); > info!("Testing partition search path {path:?}"); > match path.try_exists() { > Ok(true) => Some(path), > diff --git a/proxmox-installer-common/src/http.rs > b/proxmox-installer-common/src/http.rs > index 4a5d444..b754ed8 100644 > --- a/proxmox-installer-common/src/http.rs > +++ b/proxmox-installer-common/src/http.rs > @@ -15,7 +15,7 @@ use ureq::{Agent, AgentBuilder}; > /// * `url` - URL to call > /// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should > be used. Optional. > /// * `payload` - The payload to send to the server. Expected to be a JSON > formatted string. > -pub fn post(url: String, fingerprint: Option<&str>, payload: String) -> > Result { > +pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> > Result { > let answer; > > if let Some(fingerprint) = fingerprint { > @@ -27,7 +27,7 @@ pub fn post(url: String, fingerprint: Option<&str>, > payload: String) -> Result let agent: Agent = > AgentBuilder::new().tls_config(Arc::new(tls_config)).build(); > > answer = agent > -.post(&url) > +.post(url) > .set("Content-Type", "application/json; charset=utf-8") > .send_string(&payload)? > .into_string()?; > @@ -47,7 +47,7 @@ pub fn post(url: String, fingerprint: Option<&str>, > payload: String) -> Result .tls_config(Arc::new(tls_config)) > .build(); > answer = agent > -.post(&url) > +.post(url) > .set("Content-Type", "application/json; charset=utf-8") > .timeout(std::time::Duration::from_secs(60)) > .send_string(&payload)? > diff --git a/proxmox-installer-common/src/options.rs > b/proxmox-installer-common/src/options.rs > index 9f6131b..b209587 100644 > --- a/proxmox-installer-common/src/options.rs > +++ b/proxmox-installer-common/src/options.rs > @@ -45,6 +45,10 @@ impl FsType { > pub fn is_b
Re: [pve-devel] [PATCH installer 06/14] common: split out installer setup files loading functionality
On 7/10/24 15:27, Christoph Heiss wrote: > diff --git a/proxmox-installer-common/src/setup.rs > b/proxmox-installer-common/src/setup.rs > index 9131ac9..29137bf 100644 > --- a/proxmox-installer-common/src/setup.rs > +++ b/proxmox-installer-common/src/setup.rs > @@ -163,24 +163,29 @@ pub fn installer_setup(in_test_mode: bool) -> > Result<(SetupInfo, LocaleInfo, Run > } else { > crate::RUNTIME_DIR.to_owned() > }; > -let path = PathBuf::from(base_path); > > +load_installer_setup_files(&PathBuf::from(base_path)) > +} > + > +pub fn load_installer_setup_files( > +base_path: &Path, Could use AsRef here since it's public API. In the test specific code we could also use it for parameters, but there it's not really important since it's not public API. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 10/14] auto-installer: tests: replace left/right with got/expected in output
On 7/10/24 15:27, Christoph Heiss wrote: > Makes more sense and makes debugging easier. > > Signed-off-by: Christoph Heiss > --- > proxmox-auto-installer/tests/parse-answer.rs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/proxmox-auto-installer/tests/parse-answer.rs > b/proxmox-auto-installer/tests/parse-answer.rs > index 450915a..81079b8 100644 > --- a/proxmox-auto-installer/tests/parse-answer.rs > +++ b/proxmox-auto-installer/tests/parse-answer.rs > @@ -77,7 +77,7 @@ fn test_parse_answers() { > let compare: Value = serde_json::from_str(&compare_raw).unwrap(); > if config != compare { > panic!( > -"Test {} failed:\nleft: {:#?}\nright: {:#?}\n", > +"Test {} failed:\ngot: {:#?}\nexpected: {:#?}\n", > name, config, compare > ); > } maybe use assert_eq!() here altogether? Also above in the file use assert!(!runtime_info.disks.is_empty()) ? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 04/14] common: simplify filesystem type serializing & Display trait impl
On 7/10/24 15:27, Christoph Heiss wrote: > +impl<'de> Deserialize<'de> for FsType { > +fn deserialize(deserializer: D) -> Result > +where > +D: serde::Deserializer<'de>, > +{ > +let fs: String = Deserialize::deserialize(deserializer)?; > + > +match fs.as_str() { > +"ext4" => Ok(FsType::Ext4), > +"xfs" => Ok(FsType::Xfs), > +"zfs (RAID0)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid0)), > +"zfs (RAID1)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid1)), > +"zfs (RAID10)" => Ok(FsType::Zfs(ZfsRaidLevel::Raid10)), > +"zfs (RAIDZ-1)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ)), > +"zfs (RAIDZ-2)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ2)), > +"zfs (RAIDZ-3)" => Ok(FsType::Zfs(ZfsRaidLevel::RaidZ3)), > +"btrfs (RAID0)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid0)), > +"btrfs (RAID1)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid1)), > +"btrfs (RAID10)" => Ok(FsType::Btrfs(BtrfsRaidLevel::Raid10)), > +_ => Err(serde::de::Error::custom("could not find file system: > {fs}")), > +} > +} > +} Maybe we could implement FromStr here and use serde_plain::derive_deserialize_from_fromstr ? We could even implement FromStr for BtrfsRaidLevel and ZfsRaidLevel and then use that here, but it might be a bit overkill for just this > +impl Serialize for FsType { > +fn serialize(&self, serializer: S) -> Result > +where > +S: serde::Serializer, > +{ > +let value = match self { > +// proxinstall::$fssetup > +FsType::Ext4 => "ext4", > +FsType::Xfs => "xfs", > +// proxinstall::get_zfs_raid_setup() > +FsType::Zfs(level) => &format!("zfs ({level})"), > +// proxinstall::get_btrfs_raid_setup() > +FsType::Btrfs(level) => &format!("btrfs ({level})"), > +}; > + > +serializer.collect_str(value) > +} > +} Same as above but with Display and serde_plain::derive_display_from_serialize ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-firewall 1/1] service: flush firewall rules on force disable
superseded by: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064439.html On 5/29/24 15:25, Stefan Hanreich wrote: > When disabling the nftables firewall again, there is a race condition > where the nftables ruleset never gets flushed and persists after > disabling. In practice this almost never happens due to pve-firewall > running every 10 seconds, and proxmox-firewall running every 5 > seconds, so the proxmox-firewall main loop almost always runs at least > once before the force disable file gets created and flushes the > ruleset. > > Reported-by: Hannes Laimer > Signed-off-by: Stefan Hanreich > --- > proxmox-firewall/src/bin/proxmox-firewall.rs | 4 > 1 file changed, 4 insertions(+) > > diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs > b/proxmox-firewall/src/bin/proxmox-firewall.rs > index f7e816e..5133cbf 100644 > --- a/proxmox-firewall/src/bin/proxmox-firewall.rs > +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs > @@ -91,6 +91,10 @@ fn main() -> Result<(), std::io::Error> { > > while !term.load(Ordering::Relaxed) { > if force_disable_flag.exists() { > +if let Err(error) = remove_firewall() { > +log::error!("unable to disable firewall: {error:#}"); > +} > + > std::thread::sleep(Duration::from_secs(5)); > continue; > } ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v2 1/1] service: flush firewall rules on force disable
When disabling the nftables firewall again, there is a race condition where the nftables ruleset never gets flushed and persists after disabling. The nftables firewall update loop does a noop when the force disable file exists. It only flushes the ruleset when nftables is disabled in the configuration file but the force disable file does not yet exist. This can lead to the following situation: * nftables is activated and created its ruleset * user switches from nftables firewall back to iptables firewall * pve-firewall runs and creates the force disable file * proxmox-firewall sees that the file exists and does nothing Reported-by: Hannes Laimer Signed-off-by: Stefan Hanreich --- Changes from v1 to v2: * Removed misleading/wrong section about the probability of this happening * Added a detailed description of the scenario this commit prevents proxmox-firewall/src/bin/proxmox-firewall.rs | 4 1 file changed, 4 insertions(+) diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs index f7e816e..5133cbf 100644 --- a/proxmox-firewall/src/bin/proxmox-firewall.rs +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs @@ -91,6 +91,10 @@ fn main() -> Result<(), std::io::Error> { while !term.load(Ordering::Relaxed) { if force_disable_flag.exists() { +if let Err(error) = remove_firewall() { +log::error!("unable to disable firewall: {error:#}"); +} + std::thread::sleep(Duration::from_secs(5)); continue; } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-firewall 1/1] service: flush firewall rules on force disable
On 7/4/24 12:49, Fabian Grünbichler wrote: > so if I understand this correctly, it should handle the following case: > > proxmox-firewall runs and sets up NFT rules > user disables NFT > pve-firewall runs and sets up legacy rules and force disable file > proxmox-firewall runs and disables NFT rules > > as opposed to the following sequence > > proxmox-firewall runs and sets up NFT rules > user disables NFT > proxmox-firewall runs and disables NFT rules > pve-firewall runs and sets up legacy rules and force disable file > > which is already handled.. correct > I don't see why the first cast should "almost never happen", just because the > loops have a different period - it all comes down to alignment of the periods > and timing of the user action? > > in other words, you have a sequence > > 0:N > 5:N > 5+X: L > 10: N > 15: N > 15+X: L > 20: N > 25: N > 25+X: L > > where the gap between N and N is 5 seconds, and the gap between N and L and L > and N together is also 5 seconds. on average (assuming random alignment of the > periods), there's an X=2.5s window (out of 10) that the user action must hit > to > trigger the issue (in the gap between L and N, since X can be between 0 and > 5s)? > > FWIW, the change itself looks good to me, but the commit message might need > some adaptation ;) You're correct, the reasoning in the commit message is wrong or at least badly worded and I'll try to improve upon that in a v2. I guess it might even be unnecessary to mention how often / rarely this can happen and focus more on describing the race condition itself. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH ifupdown2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
superseded by https://lists.proxmox.com/pipermail/pve-devel/2024-July/064404.html On 6/27/24 17:01, Stefan Hanreich wrote: > This can lead to issue when upgrading from ifupdown to ifupdown2. The > particular issue this fixes occurs in the following scenario: > > * Suppose there is a legacy Debian host with ifupdown and ifenslave > installed that has a bond configured in /etc/network/interfaces. > * ifenslave installs a script /etc/network/if-pre-up.d/ifenslave. > * Now, an upgrade creates a second script > /etc/network/if-pre-up.d/ifenslave.dpkg-new. As ifupdown executes > network scripts via run-parts which ignores scripts with . in their > name, ifenslave.dpkg-new has no effect. > * If the host switches over to ifupdown2 by installing it (removing > ifupdown, keeping ifenslave) and reboots, the network will not come > up: > /etc/network/if-pre-up.d/ifenslave still exists, but is ignored > by ifupdown2's bond addon [1] > /etc/network/if-pre-up.d/ifenslave.dpkg-new is executed by ifupdown2 > because it executes all scripts in /etc/network/if-pre-up.d, even if > their name contains a dot > > This leads to ifreload failing on upgrades, which in turn causes > issues with the networking of upgraded hosts. > > Also submitted upstream at [2] > > [1] > https://github.com/CumulusNetworks/ifupdown2/blob/ccdc386cfab70703b657fe7c0ffceb95448a9c2b/ifupdown2/addons/bond.py#L45 > [2] https://github.com/CumulusNetworks/ifupdown2/pull/304 > > Signed-off-by: Stefan Hanreich > --- > ...dpkg-files-when-running-hook-scripts.patch | 54 +++ > debian/patches/series | 1 + > 2 files changed, 55 insertions(+) > create mode 100644 > debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch > > diff --git > a/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch > > b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch > new file mode 100644 > index 000..eea615f > --- /dev/null > +++ > b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch > @@ -0,0 +1,54 @@ > +From dbb759a1383cf736a0fa769c5c5827e1e7f8145c Mon Sep 17 00:00:00 2001 > +From: Stefan Hanreich > +Date: Tue, 4 Jun 2024 16:17:54 +0200 > +Subject: [PATCH] main: ignore dpkg files when running hook scripts > + > +Currently ifupdown2 executes scripts that are backed up by dpkg (e.g. > +foo.dpkg-old). This can lead to issues with hook scripts getting > +executed after upgrading ifupdown2 via dpkg, even though they should > +not be executed. > + > +This also brings ifupdown2 closer on par with the behavior of > +ifupdown, which did not execute hook scripts with dpkg suffixes. > + > +Signed-off-by: Stefan Hanreich > +--- > + ifupdown2/ifupdown/ifupdownmain.py | 4 +++- > + ifupdown2/ifupdown/utils.py| 6 ++ > + 2 files changed, 9 insertions(+), 1 deletion(-) > + > +diff --git a/ifupdown2/ifupdown/ifupdownmain.py > b/ifupdown2/ifupdown/ifupdownmain.py > +index 51f5460..e6622f0 100644 > +--- a/ifupdown2/ifupdown/ifupdownmain.py > b/ifupdown2/ifupdown/ifupdownmain.py > +@@ -1540,7 +1540,9 @@ class ifupdownMain: > + try: > + module_list = os.listdir(msubdir) > + for module in module_list: > +-if self.modules.get(module) or module in > self.overridden_ifupdown_scripts: > ++if (self.modules.get(module) > ++or module in self.overridden_ifupdown_scripts > ++or utils.is_dpkg_file(module)): > + continue > + self.script_ops[op].append(msubdir + '/' + module) > + except Exception: > +diff --git a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py > +index 05c7e48..3085e82 100644 > +--- a/ifupdown2/ifupdown/utils.py > b/ifupdown2/ifupdown/utils.py > +@@ -212,6 +212,12 @@ class utils(): > + # what we have in the cache (data retrieved via a netlink dump by > + # nlmanager). nlmanager return all macs in lower-case > + > ++_dpkg_suffixes = (".dpkg-old", ".dpkg-dist", ".dpkg-new", ".dpkg-tmp") > ++ > ++@staticmethod > ++def is_dpkg_file(name): > ++return any(name.endswith(suffix) for suffix in utils._dpkg_suffixes) > ++ > + @classmethod > + def importName(cls, modulename, name): > + """ Import a named object """ > +-- > +2.39.2 > + > diff --git a/debian/patches/series b/debian/patches/series > index 557aa7f..d5772c9 100644 > --- a/debian/patches/series >
[pve-devel] [PATCH ifupdown2 v2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
This can lead to issue when upgrading from ifupdown to ifupdown2. The particular issue this fixes occurs in the following scenario: * Suppose there is a legacy Debian host with ifupdown and ifenslave installed that has a bond configured in /etc/network/interfaces. * ifenslave installs a script /etc/network/if-pre-up.d/ifenslave. * Now, an upgrade creates a second script /etc/network/if-pre-up.d/ifenslave.dpkg-new. As ifupdown executes network scripts via run-parts which ignores scripts with . in their name, ifenslave.dpkg-new has no effect. * If the host switches over to ifupdown2 by installing it (removing ifupdown, keeping ifenslave) and reboots, the network will not come up: /etc/network/if-pre-up.d/ifenslave still exists, but is ignored by ifupdown2's bond addon [1] /etc/network/if-pre-up.d/ifenslave.dpkg-new is executed by ifupdown2 because it executes all scripts in /etc/network/if-pre-up.d, even if their name contains a dot This leads to ifreload failing on upgrades, which in turn causes issues with the networking of upgraded hosts. Also submitted upstream at [2] [1] https://github.com/CumulusNetworks/ifupdown2/blob/ccdc386cfab70703b657fe7c0ffceb95448a9c2b/ifupdown2/addons/bond.py#L45 [2] https://github.com/CumulusNetworks/ifupdown2/pull/304 Signed-off-by: Stefan Hanreich Tested-by: Friedrich Weber Reviewed-by: Fabian Grünbichler --- Changes from v1 -> v2: * Improved commit message of patch (thanks @Fabian!) ...dpkg-files-when-running-hook-scripts.patch | 55 +++ debian/patches/series | 1 + 2 files changed, 56 insertions(+) create mode 100644 debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch diff --git a/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch new file mode 100644 index 000..4298c76 --- /dev/null +++ b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch @@ -0,0 +1,55 @@ +From 1c4ef3796e18625f8f93aa49f071e759120a72ea Mon Sep 17 00:00:00 2001 +From: Stefan Hanreich +Date: Tue, 4 Jun 2024 16:17:54 +0200 +Subject: [PATCH] main: ignore dpkg files when running hook scripts + +Currently ifupdown2 executes scripts that are backed up by dpkg (e.g. +foo.dpkg-old). This can lead to issues with hook scripts getting +executed after upgrading ifupdown2 or packages that ship hook scripts +(e.g. ifenslave). + +This also brings the behavior of ifupdown2 more in line with the +behavior of ifupdown. ifupdown used run-parts for executing the hook +scripts, which ignores dpkg-created files (among others). + +Signed-off-by: Stefan Hanreich +--- + ifupdown2/ifupdown/ifupdownmain.py | 4 +++- + ifupdown2/ifupdown/utils.py| 6 ++ + 2 files changed, 9 insertions(+), 1 deletion(-) + +diff --git a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py +index 51f5460..e6622f0 100644 +--- a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py +@@ -1540,7 +1540,9 @@ class ifupdownMain: + try: + module_list = os.listdir(msubdir) + for module in module_list: +-if self.modules.get(module) or module in self.overridden_ifupdown_scripts: ++if (self.modules.get(module) ++or module in self.overridden_ifupdown_scripts ++or utils.is_dpkg_file(module)): + continue + self.script_ops[op].append(msubdir + '/' + module) + except Exception: +diff --git a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py +index 05c7e48..3085e82 100644 +--- a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py +@@ -212,6 +212,12 @@ class utils(): + # what we have in the cache (data retrieved via a netlink dump by + # nlmanager). nlmanager return all macs in lower-case + ++_dpkg_suffixes = (".dpkg-old", ".dpkg-dist", ".dpkg-new", ".dpkg-tmp") ++ ++@staticmethod ++def is_dpkg_file(name): ++return any(name.endswith(suffix) for suffix in utils._dpkg_suffixes) ++ + @classmethod + def importName(cls, modulename, name): + """ Import a named object """ +-- +2.39.2 + diff --git a/debian/patches/series b/debian/patches/series index 557aa7f..d5772c9 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,6 +7,7 @@ pve/0006-openvswitch-ovs-ports-condone-regex-exclude-tap-veth.patch pve/0007-allow-vlan-tag-inside-vxlan-tunnel.patch pve/0008-lacp-bond-remove-bond-min-links-0-warning.patch pve/0009-gvgeb-fix-python-interpreter-shebang.patch +pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch upstream/0001-add-ipv6-slaac-support-inet6-auto-accept_ra.patch upstream/0001-addons-eth
[pve-devel] [PATCH proxmox-firewall 3/3] guest: match arp packets via meta
When matching via ether type, VLAN packets are not matched. This can cause ARP packets encapsulated in VLAN frames to be dropped. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/firewall.rs | 2 +- .../tests/snapshots/integration_tests__firewall.snap | 10 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 4ea81df..941aa20 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -538,7 +538,7 @@ impl Firewall { // we allow outgoing ARP, except if blocked by the MAC filter above let arp_rule = vec![ -Match::new_eq(Payload::field("ether", "type"), Expression::from("arp")).into(), +Match::new_eq(Meta::new("protocol"), Expression::from("arp")).into(), Statement::make_accept(), ]; diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index e1953e0..40d4405 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -2961,9 +2961,8 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "match": { "op": "==", "left": { - "payload": { -"protocol": "ether", -"field": "type" + "meta": { +"key": "protocol" } }, "right": "arp" @@ -3623,9 +3622,8 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" "match": { "op": "==", "left": { - "payload": { -"protocol": "ether", -"field": "type" + "meta": { +"key": "protocol" } }, "right": "arp" -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall 1/3] cargo: bump dependencies of proxmox-ve-config
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index cc689c8..b0f3434 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -20,6 +20,6 @@ serde_json = "1" serde_plain = "1" serde_with = "2.3.3" -proxmox-schema = "3.1.0" -proxmox-sys = "0.5.3" +proxmox-schema = "3.1.1" +proxmox-sys = "0.5.8" 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
[pve-devel] [PATCH proxmox-firewall 2/3] conntrack: arp: move handling to guest chains
In order to make sure we are only affecting VM traffic and no host interfaces that are bridged, move the rules into a chain that gets executed inside the guest chain, rather than setting the rules globally. Since ether type matches on the respective Ethernet header, it doesn't work for packets with VLAN header. Matching via meta protocol ensures that VLAN encapsulated ARP packets are matched as well. Otherwise ARP traffic inside VLANs gets dropped, due to them having conntrack state invalid. Signed-off-by: Stefan Hanreich --- .../resources/proxmox-firewall.nft| 16 - proxmox-firewall/src/firewall.rs | 11 +++- .../integration_tests__firewall.snap | 64 +++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft index 537ba88..968fb93 100644 --- a/proxmox-firewall/resources/proxmox-firewall.nft +++ b/proxmox-firewall/resources/proxmox-firewall.nft @@ -33,7 +33,9 @@ add chain bridge proxmox-firewall-guests block-ndp-out add chain bridge proxmox-firewall-guests allow-ra-out add chain bridge proxmox-firewall-guests block-ra-out add chain bridge proxmox-firewall-guests do-reject +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;} flush chain inet proxmox-firewall do-reject @@ -64,7 +66,9 @@ flush chain bridge proxmox-firewall-guests block-ndp-out flush chain bridge proxmox-firewall-guests allow-ra-out flush chain bridge proxmox-firewall-guests block-ra-out flush chain bridge proxmox-firewall-guests do-reject +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 table inet proxmox-firewall { @@ -295,16 +299,22 @@ table bridge proxmox-firewall-guests { drop } +chain pre-vm-out { +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +} + chain vm-out { type filter hook prerouting priority 0; policy accept; -ether type != arp ct state vmap { established : accept, related : accept, invalid : drop } iifname vmap @vm-map-out } +chain pre-vm-in { +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +meta protocol arp accept +} + chain vm-in { type filter hook postrouting priority 0; policy accept; -ether type != arp ct state vmap { established : accept, related : accept, invalid : drop } -ether type arp accept oifname vmap @vm-map-in } } diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 4c85ea2..4ea81df 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -758,7 +758,16 @@ impl Firewall { let chain = Self::guest_chain(direction, vmid); -commands.append(&mut vec![Add::chain(chain.clone()), Flush::chain(chain)]); +let pre_chain = match direction { +Direction::In => "pre-vm-in", +Direction::Out => "pre-vm-out", +}; + +commands.append(&mut vec![ +Add::chain(chain.clone()), +Flush::chain(chain.clone()), +Add::rule(AddRule::from_statement(chain, Statement::jump(pre_chain))), +]); Ok(()) } diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 669bad9..e1953e0 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -2543,6 +2543,22 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, +{ + "add": { +"rule": { + "family": "bridge", + "table": "proxmox-firewall-guests", + "chain": "guest-100-in", + "expr": [ +{ + "jump": { +"target": "pre-vm-in" + } +} + ] +} + } +}, { "add": { "chain": { @@ -2561,6 +2577,22 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, +{ + "add": { +
Re: [pve-devel] [PATCH pve-network/pve-common/pve-manager] fix #4300 : sdn: add bridge ports isolation
On 6/27/24 18:23, DERUMIER, Alexandre wrote: > isolated on or isolated off > Controls whether a given port will be isolated, which means it will be > able to communicate with non-isolated ports only. By default this flag > is off." Yeah, makes sense this way. I thought since one can set this on a per-port basis it might make sense to expose it as such but there's probably not a lot of use cases for that. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-network/pve-common/pve-manager] fix #4300 : sdn: add bridge ports isolation
Hi! I gave this a quick test on my machine and everything worked well. Would we maybe want to expose this setting on the NIC level as well? Also I think 'Isolate Ports' or 'Port Isolation' would be the better label, 'Ports Isolation' sounds a bit wrong to me. Otherwise, consider this: Tested-By: Stefan Hanreich Reviewed-By: Stefan Hanreich 4/25/24 16:43, Alexandre Derumier via pve-devel wrote: > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH ifupdown2 1/1] fix #5197: do not run scripts ending with .dpkg-{old, new, tmp, dist}
This can lead to issue when upgrading from ifupdown to ifupdown2. The particular issue this fixes occurs in the following scenario: * Suppose there is a legacy Debian host with ifupdown and ifenslave installed that has a bond configured in /etc/network/interfaces. * ifenslave installs a script /etc/network/if-pre-up.d/ifenslave. * Now, an upgrade creates a second script /etc/network/if-pre-up.d/ifenslave.dpkg-new. As ifupdown executes network scripts via run-parts which ignores scripts with . in their name, ifenslave.dpkg-new has no effect. * If the host switches over to ifupdown2 by installing it (removing ifupdown, keeping ifenslave) and reboots, the network will not come up: /etc/network/if-pre-up.d/ifenslave still exists, but is ignored by ifupdown2's bond addon [1] /etc/network/if-pre-up.d/ifenslave.dpkg-new is executed by ifupdown2 because it executes all scripts in /etc/network/if-pre-up.d, even if their name contains a dot This leads to ifreload failing on upgrades, which in turn causes issues with the networking of upgraded hosts. Also submitted upstream at [2] [1] https://github.com/CumulusNetworks/ifupdown2/blob/ccdc386cfab70703b657fe7c0ffceb95448a9c2b/ifupdown2/addons/bond.py#L45 [2] https://github.com/CumulusNetworks/ifupdown2/pull/304 Signed-off-by: Stefan Hanreich --- ...dpkg-files-when-running-hook-scripts.patch | 54 +++ debian/patches/series | 1 + 2 files changed, 55 insertions(+) create mode 100644 debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch diff --git a/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch new file mode 100644 index 000..eea615f --- /dev/null +++ b/debian/patches/pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch @@ -0,0 +1,54 @@ +From dbb759a1383cf736a0fa769c5c5827e1e7f8145c Mon Sep 17 00:00:00 2001 +From: Stefan Hanreich +Date: Tue, 4 Jun 2024 16:17:54 +0200 +Subject: [PATCH] main: ignore dpkg files when running hook scripts + +Currently ifupdown2 executes scripts that are backed up by dpkg (e.g. +foo.dpkg-old). This can lead to issues with hook scripts getting +executed after upgrading ifupdown2 via dpkg, even though they should +not be executed. + +This also brings ifupdown2 closer on par with the behavior of +ifupdown, which did not execute hook scripts with dpkg suffixes. + +Signed-off-by: Stefan Hanreich +--- + ifupdown2/ifupdown/ifupdownmain.py | 4 +++- + ifupdown2/ifupdown/utils.py| 6 ++ + 2 files changed, 9 insertions(+), 1 deletion(-) + +diff --git a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py +index 51f5460..e6622f0 100644 +--- a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py +@@ -1540,7 +1540,9 @@ class ifupdownMain: + try: + module_list = os.listdir(msubdir) + for module in module_list: +-if self.modules.get(module) or module in self.overridden_ifupdown_scripts: ++if (self.modules.get(module) ++or module in self.overridden_ifupdown_scripts ++or utils.is_dpkg_file(module)): + continue + self.script_ops[op].append(msubdir + '/' + module) + except Exception: +diff --git a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py +index 05c7e48..3085e82 100644 +--- a/ifupdown2/ifupdown/utils.py b/ifupdown2/ifupdown/utils.py +@@ -212,6 +212,12 @@ class utils(): + # what we have in the cache (data retrieved via a netlink dump by + # nlmanager). nlmanager return all macs in lower-case + ++_dpkg_suffixes = (".dpkg-old", ".dpkg-dist", ".dpkg-new", ".dpkg-tmp") ++ ++@staticmethod ++def is_dpkg_file(name): ++return any(name.endswith(suffix) for suffix in utils._dpkg_suffixes) ++ + @classmethod + def importName(cls, modulename, name): + """ Import a named object """ +-- +2.39.2 + diff --git a/debian/patches/series b/debian/patches/series index 557aa7f..d5772c9 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,6 +7,7 @@ pve/0006-openvswitch-ovs-ports-condone-regex-exclude-tap-veth.patch pve/0007-allow-vlan-tag-inside-vxlan-tunnel.patch pve/0008-lacp-bond-remove-bond-min-links-0-warning.patch pve/0009-gvgeb-fix-python-interpreter-shebang.patch +pve/0010-main-ignore-dpkg-files-when-running-hook-scripts.patch upstream/0001-add-ipv6-slaac-support-inet6-auto-accept_ra.patch upstream/0001-addons-ethtool-add-rx-vlan-filter.patch upstream/0001-scheduler-import-traceback.patch -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-firewall 20/21] api: load sdn ipsets
Seems like I regenerated the patches once, after writing a comment so I'll leave it here: This is certainly the minimally invasive way to go about this, but it has the downside of having to load the cluster configuration twice. Once for validating all rules properly and once for providing the methods with a cluster config dictionary that doesn't contain the SDN ipsets, so it can be saved. This didn't pose too much of an issue in my tests, the API calls were still quite fast. Passing the configurations from load_conf certainly is a bit hacky, since we're passing almost the same config twice - but works quite well for this use case. We only have to do this for the cluster configuration, since this is the only place where the cluster configuration gets saved. On 6/26/24 14:15, Stefan Hanreich wrote: > Signed-off-by: Stefan Hanreich > --- > src/PVE/API2/Firewall/Cluster.pm | 3 ++- > src/PVE/API2/Firewall/Rules.pm | 18 +++--- > src/PVE/API2/Firewall/VM.pm | 3 ++- > 3 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/src/PVE/API2/Firewall/Cluster.pm > b/src/PVE/API2/Firewall/Cluster.pm > index 48ad90d..3f48431 100644 > --- a/src/PVE/API2/Firewall/Cluster.pm > +++ b/src/PVE/API2/Firewall/Cluster.pm > @@ -214,6 +214,7 @@ __PACKAGE__->register_method({ > permissions => { > check => ['perm', '/', [ 'Sys.Audit' ]], > }, > +protected => 1, > parameters => { > additionalProperties => 0, > properties => { > @@ -253,7 +254,7 @@ __PACKAGE__->register_method({ > code => sub { > my ($param) = @_; > > - my $conf = PVE::Firewall::load_clusterfw_conf(); > + my $conf = PVE::Firewall::load_clusterfw_conf(undef, 1); > > return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, > "dc"); > }}); > diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm > index 9fcfb20..ebb51af 100644 > --- a/src/PVE/API2/Firewall/Rules.pm > +++ b/src/PVE/API2/Firewall/Rules.pm > @@ -72,6 +72,7 @@ sub register_get_rules { > path => '', > method => 'GET', > description => "List rules.", > + protected => 1, > permissions => PVE::Firewall::rules_audit_permissions($rule_env), > parameters => { > additionalProperties => 0, > @@ -120,6 +121,7 @@ sub register_get_rule { > path => '{pos}', > method => 'GET', > description => "Get single rule data.", > + protected => 1, > permissions => PVE::Firewall::rules_audit_permissions($rule_env), > parameters => { > additionalProperties => 0, > @@ -412,11 +414,12 @@ sub lock_config { > sub load_config { > my ($class, $param) = @_; > > +my $sdn_conf = PVE::Firewall::load_clusterfw_conf(undef, 1); > my $fw_conf = PVE::Firewall::load_clusterfw_conf(); > -my $rules = $fw_conf->{groups}->{$param->{group}}; > +my $rules = $sdn_conf->{groups}->{$param->{group}}; > die "no such security group '$param->{group}'\n" if !defined($rules); > > -return (undef, $fw_conf, $rules); > +return ($sdn_conf, $fw_conf, $rules); > } > > sub save_rules { > @@ -488,10 +491,11 @@ sub lock_config { > sub load_config { > my ($class, $param) = @_; > > +my $sdn_conf = PVE::Firewall::load_clusterfw_conf(undef, 1); > my $fw_conf = PVE::Firewall::load_clusterfw_conf(); > -my $rules = $fw_conf->{rules}; > +my $rules = $sdn_conf->{rules}; > > -return (undef, $fw_conf, $rules); > +return ($sdn_conf, $fw_conf, $rules); > } > > sub save_rules { > @@ -528,7 +532,7 @@ sub lock_config { > sub load_config { > my ($class, $param) = @_; > > -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); > +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1); > my $fw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf); > my $rules = $fw_conf->{rules}; > > @@ -572,7 +576,7 @@ sub lock_config { > sub load_config { > my ($class, $param) = @_; > > -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); > +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1); > my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'vm', > $param->{vmid}); > my $rules = $fw_conf->{rules}; > > @@ -616,7 +620,7 @@ sub lock_config { > sub load_config { > my ($class, $param) = @_; > > -my $cluster_conf = PVE::Firewall::load
[pve-devel] [PATCH proxmox-firewall 18/21] ipsets: autogenerate ipsets for vnets and ipam
They act like virtual ipsets, similar to ipfilter-net, that can be used for defining firewall rules for sdn objects dynamically. The changes in proxmox-ve-config also introduced a dedicated struct for representing ip ranges, so we update the existing code, so that it uses that struct as well. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/firewall.rs | 22 +- proxmox-firewall/src/object.rs| 41 +- .../integration_tests__firewall.snap | 1288 + proxmox-nftables/src/expression.rs| 17 +- 4 files changed, 1354 insertions(+), 14 deletions(-) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 4c85ea2..9c19580 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -197,6 +197,27 @@ impl Firewall { self.reset_firewall(&mut commands); let cluster_host_table = Self::cluster_table(); +let guest_table = Self::guest_table(); + +if let Some(sdn_config) = self.config.sdn() { +let ipsets = sdn_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} + +if let Some(ipam_config) = self.config.ipam() { +let ipsets = ipam_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} if self.config.host().is_enabled() { log::info!("creating cluster / host configuration"); @@ -242,7 +263,6 @@ impl Firewall { commands.push(Delete::table(TableName::from(Self::cluster_table(; } -let guest_table = Self::guest_table(); let enabled_guests: BTreeMap<&Vmid, &GuestConfig> = self .config .guests() diff --git a/proxmox-firewall/src/object.rs b/proxmox-firewall/src/object.rs index 32c4ddb..cf7e773 100644 --- a/proxmox-firewall/src/object.rs +++ b/proxmox-firewall/src/object.rs @@ -72,20 +72,37 @@ impl ToNftObjects for Ipset { let mut nomatch_elements = Vec::new(); for element in self.iter() { -let cidr = match &element.address { -IpsetAddress::Cidr(cidr) => cidr, -IpsetAddress::Alias(alias) => env -.alias(alias) -.ok_or(format_err!("could not find alias {alias} in environment"))? -.address(), +let expression = match &element.address { +IpsetAddress::Range(range) => { +if family != range.family() { +continue; +} + +Expression::from(range) +} +IpsetAddress::Cidr(cidr) => { +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} +IpsetAddress::Alias(alias) => { +let cidr = env +.alias(alias) +.ok_or_else(|| { +format_err!("could not find alias {alias} in environment") +})? +.address(); + +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} }; -if family != cidr.family() { -continue; -} - -let expression = Expression::from(Prefix::from(cidr)); - if element.nomatch { nomatch_elements.push(expression); } else { diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 669bad9..aa8ab64 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -202,6 +202,1294 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, +{ + "add": { +"set": { + "family&q
[pve-devel] [PATCH proxmox-firewall 16/21] cargo: update dependencies
Signed-off-by: Stefan Hanreich --- proxmox-firewall/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-firewall/Cargo.toml b/proxmox-firewall/Cargo.toml index 4246f18..c0ce579 100644 --- a/proxmox-firewall/Cargo.toml +++ b/proxmox-firewall/Cargo.toml @@ -25,4 +25,4 @@ proxmox-ve-config = "0.1.0" [dev-dependencies] insta = { version = "1.21", features = ["json"] } -proxmox-sys = "0.5.3" +proxmox-sys = "0.5.8" -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel