[pve-devel] [PATCH container v2 1/3] add old config and unprivileged to check_ct_modify_config_perm

[Dominik Csapak <d.csapak@proxmox.com>]

we'll need that for checking the features more granularly
for it to work correctly, we have to move the permission checks
into the 'lock_config' sub, since we now also need to check the current
config and it could change between the permission check and the lock

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/API2/LXC.pm        |  6 +--
 src/PVE/API2/LXC/Config.pm | 95 +++++++++++++++++++-------------------
 src/PVE/LXC.pm             |  2 +-
 3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index b929481..afef7ec 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -254,7 +254,7 @@ __PACKAGE__->register_method({
 	my $ostemplate = extract_param($param, 'ostemplate');
 	my $storage = extract_param($param, 'storage') // 'local';
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, $param, []);
+	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, undef, $param, [], $unprivileged);
 
 	my $storage_cfg = cfs_read_file("storage.cfg");
 
@@ -1679,8 +1679,6 @@ __PACKAGE__->register_method({
 
 	die "no options specified\n" if !scalar(keys %$param);
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
-
 	my $storage_cfg = cfs_read_file("storage.cfg");
 
 	my $code = sub {
@@ -1688,6 +1686,8 @@ __PACKAGE__->register_method({
 	    my $conf = PVE::LXC::Config->load_config($vmid);
 	    PVE::LXC::Config->check_lock($conf);
 
+	    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, $param, [], $conf->{unprivileged});
+
 	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
 
 	    my $running = PVE::LXC::check_running($vmid);
diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 73fec36..1fec048 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -144,62 +144,63 @@ __PACKAGE__->register_method({
 	my $revert_str = extract_param($param, 'revert');
 	my @revert = PVE::Tools::split_list($revert_str);
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@delete]);
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@revert]);
+	my $code = sub {
 
-	foreach my $opt (@revert) {
-	    raise_param_exc({ revert => "unknown option '$opt'" })
-		if !PVE::LXC::Config->option_exists($opt);
+	    my $conf = PVE::LXC::Config->load_config($vmid);
+	    PVE::LXC::Config->check_lock($conf);
 
-	    raise_param_exc({ revert => "you can't use '-$opt' and '-revert $opt' at the same time" })
-		if defined($param->{$opt});
-	}
+	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
 
-	foreach my $opt (@delete) {
-	    raise_param_exc({ delete => "unknown option '$opt'" })
-		if !PVE::LXC::Config->option_exists($opt);
+	    my $unprivileged = $conf->{unprivileged};
+	    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, {}, [@delete], $unprivileged);
+	    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, {}, [@revert], $unprivileged);
 
-	    raise_param_exc({ delete => "you can't use '-$opt' and -delete $opt' at the same time" })
-		if defined($param->{$opt});
+	    foreach my $opt (@revert) {
+		raise_param_exc({ revert => "unknown option '$opt'" })
+		    if !PVE::LXC::Config->option_exists($opt);
 
-	    raise_param_exc({ delete => "you can't use '-delete $opt' and '-revert $opt' at the same time" })
-		if grep(/^$opt$/, @revert);
-	}
+		raise_param_exc({ revert => "you can't use '-$opt' and '-revert $opt' at the same time" })
+		    if defined($param->{$opt});
+	    }
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
-
-	my $storage_cfg = PVE::Storage::config();
-
-	my $repl_conf = PVE::ReplicationConfig->new();
-	my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
-	if ($is_replicated) {
-	    PVE::LXC::Config->foreach_volume($param, sub {
-		my ($opt, $mountpoint) = @_;
-		my $volid = $mountpoint->{volume};
-		return if !$volid || !($mountpoint->{replicate}//1);
-		if ($mountpoint->{type} eq 'volume') {
-		    my ($storeid, $format);
-		    if ($volid =~ $PVE::LXC::NEW_DISK_RE) {
-			$storeid = $1;
-			$format = $mountpoint->{format} || PVE::Storage::storage_default_format($storage_cfg, $storeid);
-		    } else {
-			($storeid, undef) = PVE::Storage::parse_volume_id($volid, 1);
-			$format = (PVE::Storage::parse_volname($storage_cfg, $volid))[6];
-		    }
-		    return if PVE::Storage::storage_can_replicate($storage_cfg, $storeid, $format);
-		    my $scfg = PVE::Storage::storage_config($storage_cfg, $storeid);
-		    return if $scfg->{shared};
-		}
-		die "cannot add non-replicatable volume to a replicated VM\n";
-	    });
-	}
+	    foreach my $opt (@delete) {
+		raise_param_exc({ delete => "unknown option '$opt'" })
+		    if !PVE::LXC::Config->option_exists($opt);
 
-	my $code = sub {
+		raise_param_exc({ delete => "you can't use '-$opt' and -delete $opt' at the same time" })
+		    if defined($param->{$opt});
 
-	    my $conf = PVE::LXC::Config->load_config($vmid);
-	    PVE::LXC::Config->check_lock($conf);
+		raise_param_exc({ delete => "you can't use '-delete $opt' and '-revert $opt' at the same time" })
+		    if grep(/^$opt$/, @revert);
+	    }
 
-	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
+	    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, $param, [], $unprivileged);
+
+	    my $storage_cfg = PVE::Storage::config();
+
+	    my $repl_conf = PVE::ReplicationConfig->new();
+	    my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
+	    if ($is_replicated) {
+		PVE::LXC::Config->foreach_volume($param, sub {
+		    my ($opt, $mountpoint) = @_;
+		    my $volid = $mountpoint->{volume};
+		    return if !$volid || !($mountpoint->{replicate}//1);
+		    if ($mountpoint->{type} eq 'volume') {
+			my ($storeid, $format);
+			if ($volid =~ $PVE::LXC::NEW_DISK_RE) {
+			    $storeid = $1;
+			    $format = $mountpoint->{format} || PVE::Storage::storage_default_format($storage_cfg, $storeid);
+			} else {
+			    ($storeid, undef) = PVE::Storage::parse_volume_id($volid, 1);
+			    $format = (PVE::Storage::parse_volname($storage_cfg, $volid))[6];
+			}
+			return if PVE::Storage::storage_can_replicate($storage_cfg, $storeid, $format);
+			my $scfg = PVE::Storage::storage_config($storage_cfg, $storeid);
+			return if $scfg->{shared};
+		    }
+		    die "cannot add non-replicatable volume to a replicated VM\n";
+		});
+	    }
 
 	    my $running = PVE::LXC::check_running($vmid);
 
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 139f901..32a2127 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1242,7 +1242,7 @@ sub template_create {
 }
 
 sub check_ct_modify_config_perm {
-    my ($rpcenv, $authuser, $vmid, $pool, $newconf, $delete) = @_;
+    my ($rpcenv, $authuser, $vmid, $pool, $oldconf, $newconf, $delete, $unprivileged) = @_;
 
     return 1 if $authuser eq 'root@pam';
     my $storage_cfg = PVE::Storage::config();
-- 
2.30.2