[pve-devel] [PATCH v1 qemu-server 5/5] add SuperUser privilege checks for root-only options

Oguz Bektas o.bektas at proxmox.com
Tue Feb 8 14:10:11 CET 2022


analogous to the changes in container.

we now allow users with SU privilege to edit real device configurations,
provided that they also have the necessary VM privileges.

note that root at pam is still able to do everything as usual

---
 PVE/API2/Qemu.pm | 119 +++++++++++++++++++++++++++++------------------
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 6992f6f..9d403b4 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -352,7 +352,7 @@ my $cloudinitoptions = {
 my $check_vm_create_serial_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
 
-    return 1 if $authuser eq 'root at pam';
+    return 1 if $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
 
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^serial\d+$/;
@@ -370,7 +370,7 @@ my $check_vm_create_serial_perm = sub {
 my $check_vm_create_usb_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
 
-    return 1 if $authuser eq 'root at pam';
+    return 1 if $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
 
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^usb\d+$/;
@@ -388,7 +388,7 @@ my $check_vm_create_usb_perm = sub {
 my $check_vm_modify_config_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
 
-    return 1 if $authuser eq 'root at pam';
+    return 1 if $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
 
     foreach my $opt (@$key_list) {
 	# some checks (e.g., disk, serial port, usb) need to be done somewhere
@@ -1117,9 +1117,10 @@ my $update_vm_api  = sub {
 	push @paramarr, "-$key", $value;
     }
 
+    my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
     my $skiplock = extract_param($param, 'skiplock');
-    raise_param_exc({ skiplock => "Only root may use this option." })
-	if $skiplock && $authuser ne 'root at pam';
+    raise_param_exc({ skiplock => "Only superusers may use this option." })
+	if $skiplock && !$is_superuser;
 
     my $delete_str = extract_param($param, 'delete');
 
@@ -1340,16 +1341,18 @@ my $update_vm_api  = sub {
 		} elsif ($opt =~ m/^serial\d+$/) {
 		    if ($val eq 'socket') {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
-		    } elsif ($authuser ne 'root at pam') {
-			die "only root can delete '$opt' config for real devices\n";
+		    } elsif (!$is_superuser) {
+			die "only superusers can delete '$opt' config for real devices\n"
+			    if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
 		} elsif ($opt =~ m/^usb\d+$/) {
 		    if ($val =~ m/spice/) {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
-		    } elsif ($authuser ne 'root at pam') {
-			die "only root can delete '$opt' config for real devices\n";
+		    } elsif (!$is_superuser) {
+			die "only superusers can delete '$opt' config for real devices\n"
+			    if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
@@ -1392,15 +1395,17 @@ my $update_vm_api  = sub {
 		} elsif ($opt =~ m/^serial\d+/) {
 		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
-		    } elsif ($authuser ne 'root at pam') {
-			die "only root can modify '$opt' config for real devices\n";
+		    } elsif (!$is_superuser) {
+			die "only superuser can modify '$opt' config for real devices\n"
+			    if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt =~ m/^usb\d+/) {
 		    if ((!defined($conf->{$opt}) || $conf->{$opt} =~ m/spice/) && $param->{$opt} =~ m/spice/) {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
-		    } elsif ($authuser ne 'root at pam') {
-			die "only root can modify '$opt' config for real devices\n";
+		    } elsif (!$is_superuser) {
+			die "only superuser can modify '$opt' config for real devices\n"
+			    if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} else {
@@ -1644,9 +1649,11 @@ __PACKAGE__->register_method({
 	my $authuser = $rpcenv->get_user();
 	my $vmid = $param->{vmid};
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = $param->{skiplock};
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	my $early_checks = sub {
 	    # test if VM exists
@@ -2291,10 +2298,12 @@ __PACKAGE__->register_method({
 	my $machine = extract_param($param, 'machine');
 	my $force_cpu = extract_param($param, 'force-cpu');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $get_root_param = sub {
 	    my $value = extract_param($param, $_[0]);
-	    raise_param_exc({ "$_[0]" => "Only root may use this option." })
-		if $value && $authuser ne 'root at pam';
+	    raise_param_exc({ "$_[0]" => "Only superusers may use this option." })
+		if $value && !$is_superuser;
 	    return $value;
 	};
 
@@ -2436,17 +2445,19 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	my $keepActive = extract_param($param, 'keepActive');
-	raise_param_exc({ keepActive => "Only root may use this option." })
-	    if $keepActive && $authuser ne 'root at pam';
+	raise_param_exc({ keepActive => "Only superusers may use this option." })
+	    if $keepActive && !$is_superuser;
 
 	my $migratedfrom = extract_param($param, 'migratedfrom');
-	raise_param_exc({ migratedfrom => "Only root may use this option." })
-	    if $migratedfrom && $authuser ne 'root at pam';
+	raise_param_exc({ migratedfrom => "Only superusers may use this option." })
+	    if $migratedfrom && !$is_superuser;
 
 
 	my $storecfg = PVE::Storage::config();
@@ -2513,9 +2524,11 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
 
@@ -2580,13 +2593,15 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	my $keepActive = extract_param($param, 'keepActive');
-	raise_param_exc({ keepActive => "Only root may use this option." })
-	    if $keepActive && $authuser ne 'root at pam';
+	raise_param_exc({ keepActive => "Only superusers may use this option." })
+	    if $keepActive && !$is_superuser;
 
 	my $storecfg = PVE::Storage::config();
 
@@ -2739,9 +2754,11 @@ __PACKAGE__->register_method({
 
 	my $statestorage = extract_param($param, 'statestorage');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
 
@@ -2811,13 +2828,15 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	my $nocheck = extract_param($param, 'nocheck');
-	raise_param_exc({ nocheck => "Only root may use this option." })
-	    if $nocheck && $authuser ne 'root at pam';
+	raise_param_exc({ nocheck => "Only superusers may use this option." })
+	    if $nocheck && !$is_superuser;
 
 	my $to_disk_suspended;
 	eval {
@@ -2883,9 +2902,11 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-	raise_param_exc({ skiplock => "Only root may use this option." })
-	    if $skiplock && $authuser ne 'root at pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	PVE::QemuServer::vm_sendkey($vmid, $skiplock, $param->{key});
 
@@ -3392,6 +3413,8 @@ __PACKAGE__->register_method({
 
 	my $storecfg = PVE::Storage::config();
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $move_updatefn =  sub {
 	    my $conf = PVE::QemuConfig->load_config($vmid);
 	    PVE::QemuConfig->check_lock($conf);
@@ -3856,7 +3879,7 @@ __PACKAGE__->register_method({
 	    },
 	    force => {
 		type => 'boolean',
-		description => "Allow to migrate VMs which use local devices. Only root may use this option.",
+		description => "Allow to migrate VMs which use local devices. Only superusers may use this option.",
 		optional => 1,
 	    },
 	    migration_type => {
@@ -3910,15 +3933,17 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
-	raise_param_exc({ force => "Only root may use this option." })
-	    if $param->{force} && $authuser ne 'root at pam';
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
 
-	raise_param_exc({ migration_type => "Only root may use this option." })
-	    if $param->{migration_type} && $authuser ne 'root at pam';
+	raise_param_exc({ force => "Only superusers may use this option." })
+	    if $param->{force} && !$is_superuser;
+
+	raise_param_exc({ migration_type => "Only superusers may use this option." })
+	    if $param->{migration_type} && !$is_superuser;
 
 	# allow root only until better network permissions are available
-	raise_param_exc({ migration_network => "Only root may use this option." })
-	    if $param->{migration_network} && $authuser ne 'root at pam';
+	raise_param_exc({ migration_network => "Only superusers may use this option." })
+	    if $param->{migration_network} && !$is_superuser;
 
 	# test if VM exists
 	my $conf = PVE::QemuConfig->load_config($vmid);
@@ -4098,9 +4123,11 @@ __PACKAGE__->register_method({
 
 	my $sizestr = extract_param($param, 'size');
 
+	my $is_superuser = $authuser eq 'root at pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
 	my $skiplock = extract_param($param, 'skiplock');
-        raise_param_exc({ skiplock => "Only root may use this option." })
-            if $skiplock && $authuser ne 'root at pam';
+        raise_param_exc({ skiplock => "Only superusers may use this option." })
+            if $skiplock && !$is_superuser;
 
         my $storecfg = PVE::Storage::config();
 
-- 
2.30.2






More information about the pve-devel mailing list