[pve-devel] [PATCH container] network permissions: implement checks

[pve-devel] [PATCH container] network permissions: implement checks

[Fabian Grünbichler]

when creating a new container
when restoring a backup
when cloning a container
and obviously, when changing the nics of an existing container

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/API2/LXC.pm   | 12 +++++++++++-
 src/PVE/LXC.pm        | 17 ++++++++++++++++-
 src/PVE/LXC/Create.pm |  5 +++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 2d67997..ed68390 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -388,6 +388,14 @@ __PACKAGE__->register_method({
 			print "recovering backed-up configuration from '$archive'\n";
 			($orig_conf, $orig_mp_param) = PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
 
+			for my $opt (keys %$orig_conf) {
+			    # early check before disks are created
+			    # the "real" check is in later on when actually merging the configs
+			    if ($opt =~ /^net\d+$/ && !$param->{opt}) {
+				PVE::LXC::check_bridge_access($rpcenv, $authuser, $orig_conf->{$opt});
+			    }
+			}
+
 			$was_template = delete $orig_conf->{template};
 
 			# When we're root call 'restore_configuration' with restricted=0,
@@ -1532,7 +1540,7 @@ __PACKAGE__->register_method({
 	description => "You need 'VM.Clone' permissions on /vms/{vmid}, " .
 	    "and 'VM.Allocate' permissions " .
 	    "on /vms/{newid} (or on the VM pool /pool/{pool}). You also need " .
-	    "'Datastore.AllocateSpace' on any used storage.",
+	    "'Datastore.AllocateSpace' on any used storage, and 'SDN.Use' on any bridge.",
 	check =>
 	[ 'and',
 	  ['perm', '/vms/{vmid}', [ 'VM.Clone' ]],
@@ -1724,6 +1732,8 @@ __PACKAGE__->register_method({
 		    my $net = PVE::LXC::Config->parse_lxc_network($value);
 		    $net->{hwaddr} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
 		    $newconf->{$opt} = PVE::LXC::Config->print_lxc_network($net);
+
+		    PVE::LXC::check_bridge_access($rpcenv, $authuser, $newconf->{$opt});
 		} else {
 		    # copy everything else
 		    $newconf->{$opt} = $value;
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 92c1b60..9642f2e 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -18,7 +18,7 @@ use PVE::AccessControl;
 use PVE::CGroup;
 use PVE::CpuSet;
 use PVE::Exception qw(raise_perm_exc);
-use PVE::GuestHelpers qw(safe_string_ne safe_num_ne safe_boolean_ne);
+use PVE::GuestHelpers qw(check_vnet_access safe_string_ne safe_num_ne safe_boolean_ne);
 use PVE::INotify;
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Network;
@@ -1317,6 +1317,7 @@ sub check_ct_modify_config_perm {
 	} elsif ($opt =~ m/^net\d+$/ || $opt eq 'nameserver' ||
 		 $opt eq 'searchdomain' || $opt eq 'hostname') {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
+	    PVE::LXC::check_bridge_access($rpcenv, $authuser, $newconf->{$opt});
 	} elsif ($opt eq 'features') {
 	    raise_perm_exc("changing feature flags for privileged container is only allowed for root\@pam")
 		if !$unprivileged;
@@ -1383,6 +1384,20 @@ sub check_ct_modify_config_perm {
     return 1;
 }
 
+sub check_bridge_access {
+    my ($rpcenv, $authuser, $raw) = @_;
+
+    return 1 if $authuser eq 'root@pam';
+
+    my $net = PVE::LXC::Config->parse_lxc_network($raw);
+    my $bridge = $net->{bridge};
+    my $tag = $net->{tag};
+    my $trunks = $net->{trunks};
+    check_vnet_access($rpcenv, $authuser, $bridge, $tag, $trunks);
+
+    return 1;
+};
+
 sub umount_all {
     my ($vmid, $storage_cfg, $conf, $noerr) = @_;
 
diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index b2e3d00..981f92d 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -325,6 +325,7 @@ sub sanitize_and_merge_config {
     my ($conf, $oldconf, $restricted, $unique) = @_;
 
     my $rpcenv = PVE::RPCEnvironment::get();
+    my $authuser = $rpcenv->get_user();
 
     foreach my $key (keys %$oldconf) {
 	next if $key eq 'digest' || $key eq 'rootfs' || $key eq 'snapshots' || $key eq 'unprivileged' || $key eq 'parent';
@@ -354,6 +355,10 @@ sub sanitize_and_merge_config {
 	    next;
 	}
 
+	if ($key =~ /^net\d+$/ && !defined($conf->{$key})) {
+	    PVE::LXC::check_bridge_access($rpcenv, $authuser, $oldconf->{$key});
+	}
+
 	if ($unique && $key =~ /^net\d+$/) {
 	    my $net = PVE::LXC::Config->parse_lxc_network($oldconf->{$key});
 	    my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
-- 
2.39.2

[pve-devel] applied: Re: [PATCH container] network permissions: implement checks

[Thomas Lamprecht]

On 09/06/2023 09:51, Fabian Grünbichler wrote:
> when creating a new container
> when restoring a backup
> when cloning a container
> and obviously, when changing the nics of an existing container
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/PVE/API2/LXC.pm   | 12 +++++++++++-
>  src/PVE/LXC.pm        | 17 ++++++++++++++++-
>  src/PVE/LXC/Create.pm |  5 +++++
>  3 files changed, 32 insertions(+), 2 deletions(-)
> 
>

applied, with $param->{opt} -> $param->{$opt} fix up, thanks!