[pve-devel] [PATCH docs/firewall/manager v5 0/5] autogenerate ipsets for sdn objects

[pve-devel] [PATCH docs/firewall/manager v5 0/5] autogenerate ipsets for sdn objects

[Stefan Hanreich]

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.

Dependencies:
* proxmox-perl-rs and proxmox-firewall depend on proxmox-ve-rs
* pve-firewall depends on proxmox-perl-rs
* pve-manager depends on pve-firewall

Changes from v4 to v5:
* extracted the API changes setting protected into a separate commit and put
  them up front
* fixed perl style issues - thanks @Thomas

Changes from v3 to v4:
* omitted proxmox-ve-rs since it is merged
* always load SDN configuration now when loading cluster config
* adapt is_nftables to check the flag file instead of reading the config
* gracefully fail when RPCEnvironment is not available

Changes from v2:
* rename end in IpRange to last to avoid confusion - thanks @Wolfgang
* bump Rust to 1.82 - thanks @Wolfgang
* improvements to the code generating IPSets - thanks @Wolfgang
* implement AsRef<str> for SDN name types - thanks @Wolfgang
* improve docstrings (proper capitalization and punctuation) - thanks @Wolfgang
* included a patch that removes proxmox-ve-config from proxmox-firewall

Changes from RFC:
* added documentation
* added separate SDN scope for IPSets
* rustfmt fixes

pve-firewall:

Stefan Hanreich (3):
  api: add protected flag to endpoints
  add support for loading sdn firewall configuration
  ipsets: return sdn ipsets from api

 src/PVE/API2/Firewall/Aliases.pm |  2 +
 src/PVE/API2/Firewall/Cluster.pm |  7 +++-
 src/PVE/API2/Firewall/Groups.pm  |  1 +
 src/PVE/API2/Firewall/Host.pm    |  1 +
 src/PVE/API2/Firewall/IPSet.pm   |  2 +
 src/PVE/API2/Firewall/Rules.pm   |  2 +
 src/PVE/API2/Firewall/VM.pm      |  5 ++-
 src/PVE/Firewall.pm              | 64 ++++++++++++++++++++++++++++----
 src/PVE/Service/pve_firewall.pm  |  4 +-
 9 files changed, 76 insertions(+), 12 deletions(-)


pve-manager:

Stefan Hanreich (1):
  firewall: add sdn scope to IPRefSelector

 www/manager6/form/IPRefSelector.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


pve-docs:

Stefan Hanreich (1):
  sdn: add documentation for firewall integration

 pvesdn.adoc | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)


Summary over all repositories:
  11 files changed, 175 insertions(+), 13 deletions(-)

-- 
Generated by git-murpp 0.6.0

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

[pve-devel] [PATCH pve-firewall v5 1/5] api: add protected flag to endpoints

[Stefan Hanreich]

In preparation for loading the SDN configuration during
load_clusterfw_conf. Since we read /etc/pve/priv/ipam.db there, we
require the protected flag to be set.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Firewall/Aliases.pm | 2 ++
 src/PVE/API2/Firewall/Cluster.pm | 2 ++
 src/PVE/API2/Firewall/Groups.pm  | 1 +
 src/PVE/API2/Firewall/Host.pm    | 1 +
 src/PVE/API2/Firewall/IPSet.pm   | 2 ++
 src/PVE/API2/Firewall/Rules.pm   | 2 ++
 src/PVE/API2/Firewall/VM.pm      | 2 ++
 7 files changed, 12 insertions(+)

diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm
index 33ac669..2f947aa 100644
--- a/src/PVE/API2/Firewall/Aliases.pm
+++ b/src/PVE/API2/Firewall/Aliases.pm
@@ -87,6 +87,7 @@ sub register_get_aliases {
 	path => '',
 	method => 'GET',
 	description => "List aliases",
+	protected => 1,
 	permissions => PVE::Firewall::rules_audit_permissions($class->rule_env()),
 	parameters => {
 	    additionalProperties => 0,
@@ -177,6 +178,7 @@ sub register_read_alias {
 	path => '{name}',
 	method => 'GET',
 	description => "Read alias.",
+	protected => 1,
 	permissions => PVE::Firewall::rules_audit_permissions($class->rule_env()),
 	parameters => {
 	    additionalProperties => 0,
diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm
index 48ad90d..f91257e 100644
--- a/src/PVE/API2/Firewall/Cluster.pm
+++ b/src/PVE/API2/Firewall/Cluster.pm
@@ -88,6 +88,7 @@ __PACKAGE__->register_method({
     path => 'options',
     method => 'GET',
     description => "Get Firewall options.",
+    protected => 1,
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit' ]],
     },
@@ -214,6 +215,7 @@ __PACKAGE__->register_method({
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit' ]],
     },
+    protected => 1,
     parameters => {
 	additionalProperties => 0,
 	properties => {
diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm
index ffdc45c..98b0747 100644
--- a/src/PVE/API2/Firewall/Groups.pm
+++ b/src/PVE/API2/Firewall/Groups.pm
@@ -44,6 +44,7 @@ __PACKAGE__->register_method({
     path => '',
     method => 'GET',
     description => "List security groups.",
+    protected => 1,
     permissions => { user => 'all' },
     parameters => {
     	additionalProperties => 0,
diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm
index 0432de2..8bd5da1 100644
--- a/src/PVE/API2/Firewall/Host.pm
+++ b/src/PVE/API2/Firewall/Host.pm
@@ -68,6 +68,7 @@ __PACKAGE__->register_method({
     path => 'options',
     method => 'GET',
     description => "Get host firewall options.",
+    protected => 1,
     proxyto => 'node',
     permissions => {
 	check => ['perm', '/nodes/{node}', [ 'Sys.Audit' ]],
diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index ed92d87..98c5443 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -91,6 +91,7 @@ sub register_get_ipset {
 	path => '',
 	method => 'GET',
 	description => "List IPSet content",
+	protected => 1,
 	permissions => PVE::Firewall::rules_audit_permissions($class->rule_env()),
 	parameters => {
 	    additionalProperties => 0,
@@ -586,6 +587,7 @@ sub register_index {
 	path => '',
 	method => 'GET',
 	description => "List IPSets",
+	protected => 1,
 	permissions => PVE::Firewall::rules_audit_permissions($class->rule_env()),
 	parameters => {
 	    additionalProperties => 0,
diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 9fcfb20..9e903d4 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,
diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm
index 4222103..3400375 100644
--- a/src/PVE/API2/Firewall/VM.pm
+++ b/src/PVE/API2/Firewall/VM.pm
@@ -69,6 +69,7 @@ sub register_handlers {
 	path => 'options',
 	method => 'GET',
 	description => "Get VM firewall options.",
+	protected => 1,
 	proxyto => 'node',
 	permissions => {
 	    check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
@@ -234,6 +235,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' ]],
 	},
-- 
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 v5 2/5] add support for loading sdn firewall configuration

[Stefan Hanreich]

This also includes support for parsing rules referencing IPSets in the
new SDN scope and generating those IPSets in the firewall. We always
load the new configuration, since loading the configuration always
includes validating the loaded rules. Validation fails without
including the SDN ipsets, leading to syslog error messages.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-By: Gabriel Goller <g.goller@proxmox.com>
Tested-By: Hannes Dürr <h.duerr@proxmox.com>
---
 src/PVE/Firewall.pm             | 64 ++++++++++++++++++++++++++++-----
 src/PVE/Service/pve_firewall.pm |  4 +--
 2 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index a69d5dd..d5b711d 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -20,11 +20,13 @@ use PVE::INotify;
 use PVE::JSONSchema qw(register_standard_option get_standard_option);
 use PVE::Network;
 use PVE::ProcFSTools;
+use PVE::RPCEnvironment;
 use PVE::SafeSyslog;
 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,10 +1691,15 @@ sub verify_rule {
 
 	if (my $value = $rule->{$name}) {
 	    if ($value =~ m/^\+/) {
-		if ($value =~ m@^\+(guest/|dc/)?(${ipset_name_pattern})$@) {
-		    &$add_error($name, "no such ipset '$2'")
-			if !($cluster_conf->{ipset}->{$2} || ($fw_conf && $fw_conf->{ipset}->{$2}));
-
+		if ($value =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) {
+		    if (
+			!($cluster_conf->{ipset}->{$2})
+			&& !($fw_conf && $fw_conf->{ipset}->{$2})
+			&& !($cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$2})
+			&& !($fw_conf->{sdn} && $fw_conf->{sdn}->{ipset}->{$2})
+		    ) {
+			$add_error->($name, "no such ipset '$2'")
+		    }
 		} else {
 		    &$add_error($name, "invalid ipset name '$value'");
 		}
@@ -2108,13 +2115,18 @@ 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}) {
+
+	    my $is_scope = sub { return !$scope || $scope eq "$_[0]/" };
+
+	    if ($is_scope->('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 ($is_scope->('dc') && $cluster_conf && $cluster_conf->{ipset}->{$name}) {
+		$ipset_chain = compute_ipset_chain_name(0, $name, $ipversion);
+	    } elsif ($is_scope->('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";
@@ -3655,6 +3667,7 @@ sub load_clusterfw_conf {
 	group_comments => {},
 	ipset => {} ,
 	ipset_comments => {},
+	sdn => load_sdn_conf(),
     };
 
     my $cluster_conf = generic_fw_config_parser($filename, $empty_conf, $empty_conf, 'cluster');
@@ -3663,6 +3676,40 @@ sub load_clusterfw_conf {
     return $cluster_conf;
 }
 
+sub load_sdn_conf {
+    my $rpcenv = eval { PVE::RPCEnvironment::get() };
+
+    if ($@) {
+	warn "could not load SDN configuration because RPCEnvironment is not initialized.";
+	return {};
+    }
+
+    my $authuser = $rpcenv->get_user();
+
+    my $guests = PVE::Cluster::get_vmlist();
+    my $allowed_vms = [
+	grep { $rpcenv->check($authuser, "/vms/$_", [ 'VM.Audit' ], 1) } sort keys $guests->{ids}->%*
+    ];
+
+    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 $empty_sdn_config = { ipset => {} , ipset_comments => {} };
+
+    my $sdn_config = eval {
+	PVE::RS::Firewall::SDN::config($allowed_vnets, $allowed_vms)
+    };
+    warn $@ if $@;
+
+    return $sdn_config // $empty_sdn_config;
+}
+
 sub save_clusterfw_conf {
     my ($cluster_conf) = @_;
 
@@ -3768,7 +3815,7 @@ sub compile {
 
 	$vmfw_configs = read_vm_firewall_configs($cluster_conf, $vmdata, $testdir);
     } else { # normal operation
-	$cluster_conf = load_clusterfw_conf(undef) if !$cluster_conf;
+	$cluster_conf = load_clusterfw_conf() if !$cluster_conf;
 
 	$hostfw_conf = load_hostfw_conf($cluster_conf, undef) if !$hostfw_conf;
 
@@ -4043,6 +4090,7 @@ sub compile_ipsets {
     }
 
     generate_ipset_chains($ipset_ruleset, undef, $cluster_conf, undef, $cluster_conf->{ipset});
+    generate_ipset_chains($ipset_ruleset, undef, $cluster_conf, undef, $cluster_conf->{sdn}->{ipset});
 
     return $ipset_ruleset;
 }
diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
index 65cb2b8..02b507a 100755
--- a/src/PVE/Service/pve_firewall.pm
+++ b/src/PVE/Service/pve_firewall.pm
@@ -158,7 +158,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Firewall::set_verbose(1); # show syntax errors
 
-	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef);
+	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 	    $res->{enable} = $cluster_conf->{options}->{enable} ? 1 : 0;
 
 	    if ($status eq 'running') {
@@ -202,7 +202,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Firewall::set_verbose(1);
 
-	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef);
+	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 	    my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = PVE::Firewall::compile($cluster_conf, undef, undef);
 
 	    print "ipset cmdlist:\n";
-- 
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 v5 3/5] ipsets: return sdn ipsets from api

[Stefan Hanreich]

In order for the new SDN ipsets to show up we need to adapt the
existing API endpoints so they read the SDN configuration.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Firewall/Cluster.pm | 5 ++++-
 src/PVE/API2/Firewall/VM.pm      | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm
index f91257e..e519ab9 100644
--- a/src/PVE/API2/Firewall/Cluster.pm
+++ b/src/PVE/API2/Firewall/Cluster.pm
@@ -257,7 +257,10 @@ __PACKAGE__->register_method({
 
 	my $conf = PVE::Firewall::load_clusterfw_conf();
 
-	return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc");
+	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 [@$sdn_refs, @$cluster_refs];
     }});
 
 1;
diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm
index 3400375..75b4345 100644
--- a/src/PVE/API2/Firewall/VM.pm
+++ b/src/PVE/API2/Firewall/VM.pm
@@ -284,9 +284,10 @@ sub register_handlers {
 	    my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid});
 
 	    my $dc_refs = PVE::Firewall::Helpers::collect_refs($cluster_conf, $param->{type}, 'dc');
+	    my $sdn_refs = PVE::Firewall::Helpers::collect_refs($cluster_conf->{sdn}, $param->{type}, 'sdn');
 	    my $vm_refs = PVE::Firewall::Helpers::collect_refs($fw_conf, $param->{type}, 'guest');
 
-	    return [@$dc_refs, @$vm_refs];
+	    return [@$dc_refs, @$sdn_refs, @$vm_refs];
 	}});
 }
 
-- 
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-manager v5 4/5] firewall: add sdn scope to IPRefSelector

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Gabriel Goller <g.goller@proxmox.com>
Tested-by: Hannes Dürr <h.duerr@proxmox.com>
---
 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 pve-docs v5 5/5] sdn: add documentation for firewall integration

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 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] [PATCH pve-firewall v5 2/5] add support for loading sdn firewall configuration

[Stefan Hanreich]

On 11/18/24 12:41, Stefan Hanreich wrote:
> +sub load_sdn_conf {
> +    my $rpcenv = eval { PVE::RPCEnvironment::get() };

After some additional consideration and testing, I think it is a bad
idea to have the permission filtering in the core firewall code.
Particularly because loading and validating is a single step in
pve-firewall and you cannot opt out of it. As soon as we limit the
amount of IPSets returned here, we run into the possibility of
validation errors. Also, it's generally bad w.r.t. separation of concerns.

I think we should *always* load the whole configuration here and filter
which IPSets we output in the API methods instead (by invoking
load_sdn_conf there explicitly and updating the cluster_conf hash).

Consider the following situation:

A user has permissions for one (out of many) SDN subnet and one (out of
many) VM and wants to edit the VM firewall config from the Web UI. When
editing the VM Firewall configuration, as soon as we load the cluster
config in the code of that endpoint and the cluster configuration
references an IPSet the user cannot see, there are validation errors in
the syslog (although they do not pose any functional issue - the
firewall works correctly).

The only way to avoid this I see is:
* always load the *full* SDN IPsets initially, so cfg validation always
  sees the big pictrue
* filter the output of IPSets in the API endpoints returning valid
  IPsets for rule creation
* use the filtered IPSet list when validating rules on creation / update

When returning a firewall configuration from the API, and it contains
IPSets that actually exist, but cannot be seen by the user, we're in a
bit of a pickle.
Currently we show them as errors in the UI (since the referenced IPSet
is not loaded because of the filtering we do currently load_sdn_conf).
With the proposed changes, there would be no errors displayed in the
frontend unless the user tries to edit the rule (since then we're
validating against the filtered IPSet list). I'm not sure what would be
the best behavior here.
I don't think filtering the rules is a good option, since it can lead to
unexpected behavior of the firewall rules and is generally just
intransparent. I think just showing them as valid but not letting the
user actually use them in rules is fine.

Currently we show errors in the syslog in this situation (could not find
ipset ...), but the functionality of the firewall is not affected in any
way! So we can also decide that this is okay for now and fix this with a
subsequent patch.

> +
> +    if ($@) {
> +	warn "could not load SDN configuration because RPCEnvironment is not initialized.";
> +	return {};
> +    }
> +
> +    my $authuser = $rpcenv->get_user();
> +
> +    my $guests = PVE::Cluster::get_vmlist();
> +    my $allowed_vms = [
> +	grep { $rpcenv->check($authuser, "/vms/$_", [ 'VM.Audit' ], 1) } sort keys $guests->{ids}->%*
> +    ];
> +
> +    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 $empty_sdn_config = { ipset => {} , ipset_comments => {} };
> +
> +    my $sdn_config = eval {
> +	PVE::RS::Firewall::SDN::config($allowed_vnets, $allowed_vms)
> +    };
> +    warn $@ if $@;
> +
> +    return $sdn_config // $empty_sdn_config;
> +}
> +




_______________________________________________
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 v5 2/5] add support for loading sdn firewall configuration

[Thomas Lamprecht]

Am 18.11.24 um 15:24 schrieb Stefan Hanreich:
> I think we should *always* load the whole configuration here and filter
> which IPSets we output in the API methods instead (by invoking
> load_sdn_conf there explicitly and updating the cluster_conf hash).

sounds good to me, are you already working on that for a v8?


_______________________________________________
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 v5 1/5] api: add protected flag to endpoints

[Thomas Lamprecht]

Am 18.11.24 um 12:41 schrieb Stefan Hanreich:
> In preparation for loading the SDN configuration during
> load_clusterfw_conf. Since we read /etc/pve/priv/ipam.db there, we
> require the protected flag to be set.
> 

should be obsolete now due to

https://git.proxmox.com/?p=pve-network.git;a=commitdiff;h=0f48bc6561f2fd901f2665387b4954c8105614e0
(and a small fix-up commit on top that I forgot to squash in)


_______________________________________________
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 v5 2/5] add support for loading sdn firewall configuration

[Stefan Hanreich]

On 11/18/24 17:09, Thomas Lamprecht wrote:
> Am 18.11.24 um 15:24 schrieb Stefan Hanreich:
>> I think we should *always* load the whole configuration here and filter
>> which IPSets we output in the API methods instead (by invoking
>> load_sdn_conf there explicitly and updating the cluster_conf hash).
> 
> sounds good to me, are you already working on that for a v8?

yes, indeed


_______________________________________________
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 v5 2/5] add support for loading sdn firewall configuration

[Thomas Lamprecht]

Am 18.11.24 um 17:11 schrieb Stefan Hanreich:
> On 11/18/24 17:09, Thomas Lamprecht wrote:
>> Am 18.11.24 um 15:24 schrieb Stefan Hanreich:
>>> I think we should *always* load the whole configuration here and filter
>>> which IPSets we output in the API methods instead (by invoking
>>> load_sdn_conf there explicitly and updating the cluster_conf hash).
>>
>> sounds good to me, are you already working on that for a v8?
> 
> yes, indeed

great (and I obv. meant v6 ^^)


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