[pve-devel] [PATCH v2 access-control++ 00/12] SuperUser privilege

[pve-devel] [PATCH v2 access-control++ 00/12] SuperUser privilege

[Oguz Bektas]

v1->v2:
* added some basic docs

changes in rest of the patches are in the separate mails.

big thanks to Fabian G. for the reviews and answering my questions
throughout the series :)

it's a complicated series so if i forgot something i'm sorry!

note: all the patches on the other repositories depend on the
access-control patches to be applied and installed first

 docs: Oguz Bektas (1):
  pveum: add SU privilege and SA role

 pveum.adoc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

 qemu-server: Oguz Bektas (2):
  api: allow SU privileged users to edit root-only options for VM
    configs
  api: allow 'skiplock' option to be used by SU privileged users

 PVE/API2/Qemu.pm | 122 ++++++++++++++++++++++++++++-------------------
 1 file changed, 72 insertions(+), 50 deletions(-)

 manager: Oguz Bektas (4):
  api: backup: allow SUs to use 'tmpdir', 'dumpdir' and 'script' options
  api: vzdump: allow SUs to use 'bwlimit' and 'ionice' parameters
  api: update comment about login prompt for non-root users
  ui: adapt sensible 'root@pam' checks to SU privilege

 PVE/API2/Backup.pm             | 11 ++++++++---
 PVE/API2/Nodes.pm              |  2 +-
 PVE/API2/VZDump.pm             |  8 +++++---
 www/manager6/dc/Config.js      |  2 +-
 www/manager6/lxc/Options.js    |  2 +-
 www/manager6/lxc/Resources.js  |  2 +-
 www/manager6/window/Migrate.js |  4 ++--
 7 files changed, 19 insertions(+), 12 deletions(-)

 container: Oguz Bektas (1):
  fix #2582: api: add checks for 'SuperUser' privilege for root-only
    options

 src/PVE/API2/LXC.pm        | 15 +++++++--------
 src/PVE/API2/LXC/Config.pm |  2 +-
 src/PVE/API2/LXC/Status.pm | 12 ++++++++----
 src/PVE/LXC.pm             | 21 ++++++++++++---------
 4 files changed, 28 insertions(+), 22 deletions(-)

 storage: Oguz Bektas (1):
  check_volume_access: allow superusers to pass arbitrary fs paths

 PVE/Storage.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

 access-control: Oguz Bektas (3):
  add "SuperAdministrator" role with the new "SuperUser" privilege
  api: allow superusers to edit tfa and password settings
  api: acl: only allow granting SU privilege if user already has it

 src/PVE/API2/ACL.pm           | 9 +++++++++
 src/PVE/API2/AccessControl.pm | 6 ++++++
 src/PVE/API2/TFA.pm           | 7 +++++--
 src/PVE/AccessControl.pm      | 9 ++++++---
 src/PVE/RPCEnvironment.pm     | 2 +-
 5 files changed, 27 insertions(+), 6 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v2 docs 01/12] pveum: add SU privilege and SA role

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 pveum.adoc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pveum.adoc b/pveum.adoc
index a5c8906..5ad111a 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -684,7 +684,8 @@ Roles
 A role is simply a list of privileges. Proxmox VE comes with a number
 of predefined roles, which satisfy most requirements.
 
-* `Administrator`: has full privileges
+* `SuperAdministrator`: has full privileges (equivalent to 'root@pam', be careful when giving this role to a user!)
+* `Administrator`: has all privileges except `SuperUser`
 * `NoAccess`: has no privileges (used to forbid access)
 * `PVEAdmin`: can do most tasks, but has no rights to modify system settings (`Sys.PowerMgmt`, `Sys.Modify`, `Realm.Allocate`)
 * `PVEAuditor`: has read only access
@@ -727,6 +728,7 @@ We currently support the following privileges:
 
 Node / System related privileges::
 
+* `SuperUser`: modify root-only configuration options (dangerous! don't give this privilege to untrusted users)
 * `Permissions.Modify`: modify access permissions
 * `Sys.PowerMgmt`: node power management (start, stop, reset, shutdown, ...)
 * `Sys.Console`: console access to node
-- 
2.30.2

Re: [pve-devel] [PATCH v2 docs 01/12] pveum: add SU privilege and SA role

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  pveum.adoc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/pveum.adoc b/pveum.adoc
> index a5c8906..5ad111a 100644
> --- a/pveum.adoc
> +++ b/pveum.adoc
> @@ -684,7 +684,8 @@ Roles
>  A role is simply a list of privileges. Proxmox VE comes with a number
>  of predefined roles, which satisfy most requirements.
>  
> -* `Administrator`: has full privileges
> +* `SuperAdministrator`: has full privileges (equivalent to 'root@pam', be careful when giving this role to a user!)
> +* `Administrator`: has all privileges except `SuperUser`

I'd make the descriptions shorter and add the warnings as proper 
warnings.

* `SuperAdministrator`: has full privileges including `SuperUser`

>  * `NoAccess`: has no privileges (used to forbid access)
>  * `PVEAdmin`: can do most tasks, but has no rights to modify system settings (`Sys.PowerMgmt`, `Sys.Modify`, `Realm.Allocate`)
>  * `PVEAuditor`: has read only access
> @@ -727,6 +728,7 @@ We currently support the following privileges:
>  
>  Node / System related privileges::
>  
> +* `SuperUser`: modify root-only configuration options (dangerous! don't give this privilege to untrusted users)
>  * `Permissions.Modify`: modify access permissions
>  * `Sys.PowerMgmt`: node power management (start, stop, reset, shutdown, ...)
>  * `Sys.Console`: console access to node

SuperUser is not Node/System related though? it also affects guest 
operations for example, so I'd add it either up front or last on its 
own, with a warning and longer description (allows root stuff, might 
require other basic privs in addition to SuperUser, danger danger, 
certain actions on users with this privs are restricted, ..)

[pve-devel] [PATCH v2 qemu-server 02/12] api: allow SU privileged users to edit root-only options for VM configs

[Oguz Bektas]

we now allow users with SU privilege to edit real device configurations
for VMs.

they still need the required privilege to edit the corresponding
configuration options (such as `VM.Config.HWType`), as well as the SU
privilege.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
* add comments at the shortcuts for root@pam
* remove wrong shortcut for SU, instead check required privileges + SU
* revert migration-only parameters and vzdump internal ones


 PVE/API2/Qemu.pm | 63 ++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 68077cc..21fc82b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -352,16 +352,17 @@ my $cloudinitoptions = {
 my $check_vm_create_serial_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
 
+    # no need to check permissions for root@pam
     return 1 if $authuser eq 'root@pam';
 
+    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^serial\d+$/;
 
-	if ($param->{$opt} eq 'socket') {
-	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
-	} else {
-	    die "only root can set '$opt' config for real devices\n";
-	}
+	$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+	die "only superusers can set '$opt' config for real devices\n"
+	    if !($param->{$opt} eq 'socket' || $is_superuser);
     }
 
     return 1;
@@ -370,16 +371,17 @@ my $check_vm_create_serial_perm = sub {
 my $check_vm_create_usb_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
 
+    # no need to check permissions for root@pam
     return 1 if $authuser eq 'root@pam';
 
+    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
     foreach my $opt (keys %{$param}) {
 	next if $opt !~ m/^usb\d+$/;
 
-	if ($param->{$opt} =~ m/spice/) {
-	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
-	} else {
-	    die "only root can set '$opt' config for real devices\n";
-	}
+	$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
+	die "only superusers can set '$opt' config for real devices\n"
+	    if !($param->{$opt} =~ m/spice/ || $is_superuser);
     }
 
     return 1;
@@ -388,8 +390,11 @@ my $check_vm_create_usb_perm = sub {
 my $check_vm_modify_config_perm = sub {
     my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
 
+    # no need to check permissions for root@pam
     return 1 if $authuser eq 'root@pam';
 
+    my $is_superuser = $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
 	# else, as there the permission can be value dependend
@@ -425,7 +430,8 @@ my $check_vm_modify_config_perm = sub {
 	} else {
 	    # catches hostpci\d+, args, lock, etc.
 	    # new options will be checked here
-	    die "only root can set '$opt' config\n";
+	    die "only superusers can set '$opt' config\n"
+		if !$is_superuser;
 	}
     }
 
@@ -1117,6 +1123,8 @@ my $update_vm_api  = sub {
 	push @paramarr, "-$key", $value;
     }
 
+    my $is_superuser = $authuser eq 'root@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@pam';
@@ -1338,19 +1346,15 @@ my $update_vm_api  = sub {
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
 		} elsif ($opt =~ m/^serial\d+$/) {
-		    if ($val eq 'socket') {
-			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
-		    } elsif ($authuser ne 'root@pam') {
-			die "only root can delete '$opt' config for real devices\n";
-		    }
+		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    die "only superusers can delete '$opt' config for real devices\n"
+			if !($val eq 'socket' || $is_superuser);
 		    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@pam') {
-			die "only root can delete '$opt' config for real devices\n";
-		    }
+		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    die "only superusers can delete '$opt' config for real devices\n"
+			if !($val =~ m/spice/ || $is_superuser);
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
 		} else {
@@ -1362,6 +1366,7 @@ my $update_vm_api  = sub {
 	    foreach my $opt (keys %$param) { # add/change
 		$modified->{$opt} = 1;
 		$conf = PVE::QemuConfig->load_config($vmid); # update/reload
+		my $val = $conf->{$opt} // $conf->{pending}->{$opt};
 		next if defined($conf->{pending}->{$opt}) && ($param->{$opt} eq $conf->{pending}->{$opt}); # skip if nothing changed
 
 		my $arch = PVE::QemuServer::get_vm_arch($conf);
@@ -1390,18 +1395,14 @@ 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@pam') {
-			die "only root can modify '$opt' config for real devices\n";
-		    }
+		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    die "only superusers can modify '$opt' config for real devices\n"
+			if !($val eq 'socket' || $is_superuser);
 		    $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@pam') {
-			die "only root can modify '$opt' config for real devices\n";
-		    }
+		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
+		    die "only superusers can modify '$opt' config for real devices\n"
+			if !($val =~ m/spice/ || $is_superuser);
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
-- 
2.30.2

Re: [pve-devel] [PATCH v2 qemu-server 02/12] api: allow SU privileged users to edit root-only options for VM configs

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> we now allow users with SU privilege to edit real device configurations
> for VMs.
> 
> they still need the required privilege to edit the corresponding
> configuration options (such as `VM.Config.HWType`), as well as the SU
> privilege.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * add comments at the shortcuts for root@pam
> * remove wrong shortcut for SU, instead check required privileges + SU
> * revert migration-only parameters and vzdump internal ones
> 
> 
>  PVE/API2/Qemu.pm | 63 ++++++++++++++++++++++++------------------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 68077cc..21fc82b 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -352,16 +352,17 @@ my $cloudinitoptions = {
>  my $check_vm_create_serial_perm = sub {
>      my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
>  
> +    # no need to check permissions for root@pam
>      return 1 if $authuser eq 'root@pam';
>  
> +    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
> +
>      foreach my $opt (keys %{$param}) {
>  	next if $opt !~ m/^serial\d+$/;
>  
> -	if ($param->{$opt} eq 'socket') {
> -	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> -	} else {
> -	    die "only root can set '$opt' config for real devices\n";
> -	}
> +	$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> +	die "only superusers can set '$opt' config for real devices\n"
> +	    if !($param->{$opt} eq 'socket' || $is_superuser);

check and message match, but not really. IMHO

    if $param->{$opt} ne 'socket' && !$is_superuser;

matches the text of the message in a more readable fashion

>      }
>  
>      return 1;
> @@ -370,16 +371,17 @@ my $check_vm_create_serial_perm = sub {
>  my $check_vm_create_usb_perm = sub {
>      my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
>  
> +    # no need to check permissions for root@pam
>      return 1 if $authuser eq 'root@pam';
>  
> +    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
> +
>      foreach my $opt (keys %{$param}) {
>  	next if $opt !~ m/^usb\d+$/;
>  
> -	if ($param->{$opt} =~ m/spice/) {
> -	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> -	} else {
> -	    die "only root can set '$opt' config for real devices\n";
> -	}
> +	$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.HWType']);
> +	die "only superusers can set '$opt' config for real devices\n"
> +	    if !($param->{$opt} =~ m/spice/ || $is_superuser);

same here

>      }
>  
>      return 1;
> @@ -388,8 +390,11 @@ my $check_vm_create_usb_perm = sub {
>  my $check_vm_modify_config_perm = sub {
>      my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
>  
> +    # no need to check permissions for root@pam
>      return 1 if $authuser eq 'root@pam';
>  
> +    my $is_superuser = $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
>  	# else, as there the permission can be value dependend
> @@ -425,7 +430,8 @@ my $check_vm_modify_config_perm = sub {
>  	} else {
>  	    # catches hostpci\d+, args, lock, etc.
>  	    # new options will be checked here
> -	    die "only root can set '$opt' config\n";
> +	    die "only superusers can set '$opt' config\n"
> +		if !$is_superuser;
>  	}
>      }
>  
> @@ -1117,6 +1123,8 @@ my $update_vm_api  = sub {
>  	push @paramarr, "-$key", $value;
>      }
>  
> +    my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

nit: line too long (I know, there are others as well, but that doesn't 
mean we want to introduce even more mess ;))

not sure whether the shortcut here is worth it anyway, this is a single 
call and we have a few others that are not skipped either.

> +
>      my $skiplock = extract_param($param, 'skiplock');
>      raise_param_exc({ skiplock => "Only root may use this option." })
>  	if $skiplock && $authuser ne 'root@pam';
> @@ -1338,19 +1346,15 @@ my $update_vm_api  = sub {
>  		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} elsif ($opt =~ m/^serial\d+$/) {
> -		    if ($val eq 'socket') {
> -			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> -		    } elsif ($authuser ne 'root@pam') {
> -			die "only root can delete '$opt' config for real devices\n";
> -		    }
> +		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    die "only superusers can delete '$opt' config for real devices\n"
> +			if !($val eq 'socket' || $is_superuser);

condition style here

>  		    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@pam') {
> -			die "only root can delete '$opt' config for real devices\n";
> -		    }
> +		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    die "only superusers can delete '$opt' config for real devices\n"
> +			if !($val =~ m/spice/ || $is_superuser);

and here

>  		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
>  		    PVE::QemuConfig->write_config($vmid, $conf);
>  		} else {
> @@ -1362,6 +1366,7 @@ my $update_vm_api  = sub {
>  	    foreach my $opt (keys %$param) { # add/change
>  		$modified->{$opt} = 1;
>  		$conf = PVE::QemuConfig->load_config($vmid); # update/reload
> +		my $val = $conf->{$opt} // $conf->{pending}->{$opt};

no explanation for this change here, but $val now has the current value 
if one is set, or the pending value if that is set.

it seems to be copied from the code handling deletions (where this makes 
sense - there is no new value in that case), but we want to ensure the 
thing we remove is something we are allowed to add back/in the first 
place.

>  		next if defined($conf->{pending}->{$opt}) && ($param->{$opt} eq $conf->{pending}->{$opt}); # skip if nothing changed
>  
>  		my $arch = PVE::QemuServer::get_vm_arch($conf);
> @@ -1390,18 +1395,14 @@ my $update_vm_api  = sub {
>  			}
>  		    }
>  		} elsif ($opt =~ m/^serial\d+/) {
> -		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {

but here we have totally different rules applying, and can't just 
apply the ones for deleting? this here checks both the old value if one 
is set (which we remove) and the new value (which we set)

> -			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> -		    } elsif ($authuser ne 'root@pam') {
> -			die "only root can modify '$opt' config for real devices\n";
> -		    }
> +		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    die "only superusers can modify '$opt' config for real devices\n"
> +			if !($val eq 'socket' || $is_superuser);

so this completely changes the semantics, no longer checking the new 
value at all.. so again I wonder how did you test this?  this allows 
skipping the SU check as long as I first set the allowed value, and then 
replace it with the high-privilege one..

>  		    $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@pam') {
> -			die "only root can modify '$opt' config for real devices\n";
> -		    }
> +		    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> +		    die "only superusers can modify '$opt' config for real devices\n"
> +			if !($val =~ m/spice/ || $is_superuser);

same applies here..

>  		    $conf->{pending}->{$opt} = $param->{$opt};
>  		} else {
>  		    $conf->{pending}->{$opt} = $param->{$opt};
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 qemu-server 03/12] api: allow 'skiplock' option to be used by SU privileged users

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/API2/Qemu.pm | 59 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 21fc82b..95cc46d 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1126,8 +1126,8 @@ my $update_vm_api  = sub {
     my $is_superuser = $authuser eq 'root@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@pam';
+    raise_param_exc({ skiplock => "Only superusers may use this option." })
+	if $skiplock && !$is_superuser;
 
     my $delete_str = extract_param($param, 'delete');
 
@@ -1645,9 +1645,11 @@ __PACKAGE__->register_method({
 	my $authuser = $rpcenv->get_user();
 	my $vmid = $param->{vmid};
 
+	my $is_superuser = $authuser eq 'root@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@pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	my $early_checks = sub {
 	    # test if VM exists
@@ -2290,6 +2292,12 @@ __PACKAGE__->register_method({
 	my $timeout = extract_param($param, 'timeout');
 	my $machine = extract_param($param, 'machine');
 
+	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
+	my $skiplock = extract_param($param, 'skiplock');
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
+
 	my $get_root_param = sub {
 	    my $value = extract_param($param, $_[0]);
 	    raise_param_exc({ "$_[0]" => "Only root may use this option." })
@@ -2298,7 +2306,6 @@ __PACKAGE__->register_method({
 	};
 
 	my $stateuri = $get_root_param->('stateuri');
-	my $skiplock = $get_root_param->('skiplock');
 	my $migratedfrom = $get_root_param->('migratedfrom');
 	my $migration_type = $get_root_param->('migration_type');
 	my $migration_network = $get_root_param->('migration_network');
@@ -2436,9 +2443,11 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@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." })
@@ -2513,9 +2522,11 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@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,9 +2591,11 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@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." })
@@ -2739,9 +2752,11 @@ __PACKAGE__->register_method({
 
 	my $statestorage = extract_param($param, 'statestorage');
 
+	my $is_superuser = $authuser eq 'root@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@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,9 +2826,11 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@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." })
@@ -2883,9 +2900,11 @@ __PACKAGE__->register_method({
 
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	PVE::QemuServer::vm_sendkey($vmid, $skiplock, $param->{key});
 
@@ -4114,9 +4133,11 @@ __PACKAGE__->register_method({
 
 	my $sizestr = extract_param($param, 'size');
 
+	my $is_superuser = $authuser eq 'root@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@pam';
+        raise_param_exc({ skiplock => "Only superusers may use this option." })
+            if $skiplock && !$is_superuser;
 
         my $storecfg = PVE::Storage::config();
 
-- 
2.30.2

Re: [pve-devel] [PATCH v2 qemu-server 03/12] api: allow 'skiplock' option to be used by SU privileged users

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 59 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 40 insertions(+), 19 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 21fc82b..95cc46d 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1126,8 +1126,8 @@ my $update_vm_api  = sub {
>      my $is_superuser = $authuser eq 'root@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@pam';
> +    raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	if $skiplock && !$is_superuser;
>  
>      my $delete_str = extract_param($param, 'delete');
>  
> @@ -1645,9 +1645,11 @@ __PACKAGE__->register_method({
>  	my $authuser = $rpcenv->get_user();
>  	my $vmid = $param->{vmid};
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

nit: line too long

> +
>  	my $skiplock = $param->{skiplock};
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@pam';
> +	raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	    if $skiplock && !$is_superuser;
>  
>  	my $early_checks = sub {
>  	    # test if VM exists
> @@ -2290,6 +2292,12 @@ __PACKAGE__->register_method({
>  	my $timeout = extract_param($param, 'timeout');
>  	my $machine = extract_param($param, 'machine');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
> +	my $skiplock = extract_param($param, 'skiplock');
> +	raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	    if $skiplock && !$is_superuser;
> +
>  	my $get_root_param = sub {
>  	    my $value = extract_param($param, $_[0]);
>  	    raise_param_exc({ "$_[0]" => "Only root may use this option." })
> @@ -2298,7 +2306,6 @@ __PACKAGE__->register_method({
>  	};
>  

a comment here that this are intentionally still root@pam because they 
are only used for migration-internal flows (and marking them as such in 
the parameter description) would be nice..

>  	my $stateuri = $get_root_param->('stateuri');
> -	my $skiplock = $get_root_param->('skiplock');
>  	my $migratedfrom = $get_root_param->('migratedfrom');
>  	my $migration_type = $get_root_param->('migration_type');
>  	my $migration_network = $get_root_param->('migration_network');
> @@ -2436,9 +2443,11 @@ __PACKAGE__->register_method({
>  	my $node = extract_param($param, 'node');
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@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." })

and same for these here (keepactive -> vzdump, migratedfrom -> 
migration)

> @@ -2513,9 +2522,11 @@ __PACKAGE__->register_method({
>  
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@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,9 +2591,11 @@ __PACKAGE__->register_method({
>  	my $node = extract_param($param, 'node');
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@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." })

again, comment here and in schema description would be nice

> @@ -2739,9 +2752,11 @@ __PACKAGE__->register_method({
>  
>  	my $statestorage = extract_param($param, 'statestorage');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

again

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@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,9 +2826,11 @@ __PACKAGE__->register_method({
>  
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@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." })

and comment here again (migration?)

> @@ -2883,9 +2900,11 @@ __PACKAGE__->register_method({
>  
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@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@pam';
> +	raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	    if $skiplock && !$is_superuser;
>  
>  	PVE::QemuServer::vm_sendkey($vmid, $skiplock, $param->{key});
>  
> @@ -4114,9 +4133,11 @@ __PACKAGE__->register_method({
>  
>  	my $sizestr = extract_param($param, 'size');
>  
> +	my $is_superuser = $authuser eq 'root@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@pam';
> +        raise_param_exc({ skiplock => "Only superusers may use this option." })
> +            if $skiplock && !$is_superuser;
>  
>          my $storecfg = PVE::Storage::config();
>  
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 manager 04/12] api: backup: allow SUs to use 'tmpdir', 'dumpdir' and 'script' options

[Oguz Bektas]

previously limited to root@pam; we can allow SUs to use these options if
they have the privilege on the whole API path.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/API2/Backup.pm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 9953a704..142eddd1 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -43,8 +43,13 @@ my $assert_param_permission = sub {
     my ($param, $user) = @_;
     return if $user eq 'root@pam'; # always OK
 
+    my $rpcenv = PVE::RPCEnvironment::get();
+    # we need to have SU privs on / path for these options to be used safely
+    my $is_superuser = $rpcenv->check($user, "/", ['SuperUser'], 1);
+    return if $is_superuser;
+
     for my $key (qw(tmpdir dumpdir script)) {
-	raise_param_exc({ $key => "Only root may set this option."}) if exists $param->{$key};
+	raise_param_exc({ $key => "Only superusers may set this option."}) if exists $param->{$key};
     }
 };
 
@@ -142,7 +147,7 @@ __PACKAGE__->register_method({
     description => "Create new vzdump backup job.",
     permissions => {
 	check => ['perm', '/', ['Sys.Modify']],
-	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to the 'root\@pam' user.",
+	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to superusers.",
     },
     parameters => {
     	additionalProperties => 0,
@@ -344,7 +349,7 @@ __PACKAGE__->register_method({
     description => "Update vzdump backup job definition.",
     permissions => {
 	check => ['perm', '/', ['Sys.Modify']],
-	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to the 'root\@pam' user.",
+	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to superusers.",
     },
     parameters => {
     	additionalProperties => 0,
-- 
2.30.2

Re: [pve-devel] [PATCH v2 manager 04/12] api: backup: allow SUs to use 'tmpdir', 'dumpdir' and 'script' options

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> previously limited to root@pam; we can allow SUs to use these options if
> they have the privilege on the whole API path.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  PVE/API2/Backup.pm | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
> index 9953a704..142eddd1 100644
> --- a/PVE/API2/Backup.pm
> +++ b/PVE/API2/Backup.pm
> @@ -43,8 +43,13 @@ my $assert_param_permission = sub {
>      my ($param, $user) = @_;
>      return if $user eq 'root@pam'; # always OK
>  
> +    my $rpcenv = PVE::RPCEnvironment::get();
> +    # we need to have SU privs on / path for these options to be used safely

that comment is bogus - having some privileges doesn't make this 
options 'safe', it's the other way round - we require this privilege 
because the options are not safe. the check call itself and the messages 
below are already very clear, I don't think we need a comment here at 
all? but if you think we do, a simple 

# SuperUser always OK

like for root above would suffice..

> +    my $is_superuser = $rpcenv->check($user, "/", ['SuperUser'], 1);
> +    return if $is_superuser;

return if $rpcenv...

> +
>      for my $key (qw(tmpdir dumpdir script)) {
> -	raise_param_exc({ $key => "Only root may set this option."}) if exists $param->{$key};
> +	raise_param_exc({ $key => "Only superusers may set this option."}) if exists $param->{$key};
>      }
>  };
>  
> @@ -142,7 +147,7 @@ __PACKAGE__->register_method({
>      description => "Create new vzdump backup job.",
>      permissions => {
>  	check => ['perm', '/', ['Sys.Modify']],
> -	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to the 'root\@pam' user.",
> +	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to superusers.",
>      },
>      parameters => {
>      	additionalProperties => 0,
> @@ -344,7 +349,7 @@ __PACKAGE__->register_method({
>      description => "Update vzdump backup job definition.",
>      permissions => {
>  	check => ['perm', '/', ['Sys.Modify']],
> -	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to the 'root\@pam' user.",
> +	description => "The 'tmpdir', 'dumpdir' and 'script' parameters are additionally restricted to superusers.",
>      },
>      parameters => {
>      	additionalProperties => 0,
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 manager 05/12] api: vzdump: allow SUs to use 'bwlimit' and 'ionice' parameters

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/API2/VZDump.pm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 2c0df4c3..3b87ef38 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -27,7 +27,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	description => "The user needs 'VM.Backup' permissions on any VM, and 'Datastore.AllocateSpace'"
 	    ." on the backup storage. The 'maxfiles', 'prune-backups', 'tmpdir', 'dumpdir', 'script',"
-	    ." 'bwlimit' and 'ionice' parameters are restricted to the 'root\@pam' user.",
+	    ." 'bwlimit' and 'ionice' parameters are restricted to the superusers.",
 	user => 'all',
     },
     protected => 1,
@@ -52,6 +52,8 @@ __PACKAGE__->register_method ({
 
 	my $nodename = PVE::INotify::nodename();
 
+	my $is_superuser = $user eq 'root@pam' || $rpcenv->check($user, "/", ['SuperUser'], 1);
+
 	if ($rpcenv->{type} ne 'cli') {
 	    raise_param_exc({ node => "option is only allowed on the command line interface."})
 		if $param->{node} && $param->{node} ne $nodename;
@@ -61,8 +63,8 @@ __PACKAGE__->register_method ({
 	}
 
 	foreach my $key (qw(maxfiles prune-backups tmpdir dumpdir script bwlimit ionice)) {
-	    raise_param_exc({ $key => "Only root may set this option."})
-		if defined($param->{$key}) && ($user ne 'root@pam');
+	    raise_param_exc({ $key => "Only superusers may set this option."})
+		if defined($param->{$key}) && !$is_superuser;
 	}
 
 	PVE::VZDump::verify_vzdump_parameters($param, 1);
-- 
2.30.2

[pve-devel] [PATCH v2 manager 06/12] api: update comment about login prompt for non-root users

[Oguz Bektas]

we have a SU privilege now, but we still drop to a login prompt for such
users.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/API2/Nodes.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 655493a3..0c3de231 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -870,7 +870,7 @@ sub get_shell_command  {
 	    $cmd = [ '/bin/login', '-f', 'root' ];
 	}
     } else {
-	# non-root must always login for now, we do not have a superuser role!
+	# non-root must always login, even with SU privilege
 	$cmd = [ '/bin/login' ];
     }
     return $cmd;
-- 
2.30.2

Re: [pve-devel] [PATCH v2 manager 06/12] api: update comment about login prompt for non-root users

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> we have a SU privilege now, but we still drop to a login prompt for such
> users.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  PVE/API2/Nodes.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index 655493a3..0c3de231 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -870,7 +870,7 @@ sub get_shell_command  {
>  	    $cmd = [ '/bin/login', '-f', 'root' ];
>  	}
>      } else {
> -	# non-root must always login for now, we do not have a superuser role!
> +	# non-root must always login, even with SU privilege

it would be nicer to check this early on as well with a proper error 
message - all of temrproxy, vncshell, spiceshell allow passing in a cmd 
('login', 'upgrade', or 'ceph_install'), and only 'upgrade' is checked 
there for being root@pam only. so if a user calls those with 
'ceph_install', they'd be dropped in a login prompt instead without any 
indication why..

>  	$cmd = [ '/bin/login' ];
>      }
>      return $cmd;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 manager 07/12] ui: adapt sensible 'root@pam' checks to SU privilege

[Oguz Bektas]

so that SUs can perform some root-only actions over the GUI

also silence eslint's warning about the access notation for these lines
only.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
* silence eslint warnings
* correct the conditional in Migrate.js
* remove the unneeded/wrong ones (see fabian's previous review)

 www/manager6/dc/Config.js      | 2 +-
 www/manager6/lxc/Options.js    | 2 +-
 www/manager6/lxc/Resources.js  | 2 +-
 www/manager6/window/Migrate.js | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 9c54b19d..0f70e33e 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -197,7 +197,7 @@ Ext.define('PVE.dc.Config', {
 		});
 	    }
 
-	    if (Proxmox.UserName === 'root@pam') {
+	    if (caps.dc['SuperUser']) { // eslint-disable-line
 		me.items.push({
 		    xtype: 'pveACMEClusterView',
 		    title: 'ACME',
diff --git a/www/manager6/lxc/Options.js b/www/manager6/lxc/Options.js
index f2661dfc..dce41505 100644
--- a/www/manager6/lxc/Options.js
+++ b/www/manager6/lxc/Options.js
@@ -136,7 +136,7 @@ Ext.define('PVE.lxc.Options', {
 	    features: {
 		header: gettext('Features'),
 		defaultValue: Proxmox.Utils.noneText,
-		editor: Proxmox.UserName === 'root@pam' || caps.vms['VM.Allocate']
+		editor: caps.vms['SuperUser'] || caps.vms['VM.Allocate'] // eslint-disable-line
 		    ? 'PVE.lxc.FeaturesEdit' : undefined,
 	    },
 	    hookscript: {
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 15ee3c67..26e1bd36 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -257,7 +257,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    var isUsedDisk = isDisk && !isUnusedDisk;
 
 	    var noedit = rec.data.delete || !rowdef.editor;
-	    if (!noedit && Proxmox.UserName !== 'root@pam' && key.match(/^mp\d+$/)) {
+	    if (!noedit && !caps.vms['SuperUser'] && key.match(/^mp\d+$/)) { // eslint-disable-line
 		var mp = PVE.Parser.parseLxcMountPoint(value);
 		if (mp.type !== 'volume') {
 		    noedit = true;
diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 1c23abb3..597e3b0b 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -52,8 +52,8 @@ Ext.define('PVE.window.Migrate', {
 		    }
 	    },
 	    setLocalResourceCheckboxHidden: function(get) {
-		if (get('running') || !get('migration.hasLocalResources') ||
-		    Proxmox.UserName !== 'root@pam') {
+		let caps = Ext.state.Manager.get('GuiCap');
+		if (get('running') || !get('migration.hasLocalResources') || !caps.vms['SuperUser']) { // eslint-disable-line
 		    return true;
 		} else {
 		    return false;
-- 
2.30.2

Re: [pve-devel] [PATCH v2 manager 07/12] ui: adapt sensible 'root@pam' checks to SU privilege

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> so that SUs can perform some root-only actions over the GUI
> 
> also silence eslint's warning about the access notation for these lines
> only.
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * silence eslint warnings
> * correct the conditional in Migrate.js
> * remove the unneeded/wrong ones (see fabian's previous review)
> 
>  www/manager6/dc/Config.js      | 2 +-
>  www/manager6/lxc/Options.js    | 2 +-
>  www/manager6/lxc/Resources.js  | 2 +-
>  www/manager6/window/Migrate.js | 4 ++--
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
> index 9c54b19d..0f70e33e 100644
> --- a/www/manager6/dc/Config.js
> +++ b/www/manager6/dc/Config.js
> @@ -197,7 +197,7 @@ Ext.define('PVE.dc.Config', {
>  		});
>  	    }
>  
> -	    if (Proxmox.UserName === 'root@pam') {
> +	    if (caps.dc['SuperUser']) { // eslint-disable-line
>  		me.items.push({
>  		    xtype: 'pveACMEClusterView',

that one is still root only though in the backend?

(and I am still missing some sort of overview of things you checked but 
left as root-only intentional!)

>  		    title: 'ACME',
> diff --git a/www/manager6/lxc/Options.js b/www/manager6/lxc/Options.js
> index f2661dfc..dce41505 100644
> --- a/www/manager6/lxc/Options.js
> +++ b/www/manager6/lxc/Options.js
> @@ -136,7 +136,7 @@ Ext.define('PVE.lxc.Options', {
>  	    features: {
>  		header: gettext('Features'),
>  		defaultValue: Proxmox.Utils.noneText,
> -		editor: Proxmox.UserName === 'root@pam' || caps.vms['VM.Allocate']
> +		editor: caps.vms['SuperUser'] || caps.vms['VM.Allocate'] // eslint-disable-line
>  		    ? 'PVE.lxc.FeaturesEdit' : undefined,

this one requires a (trivial) rebase

>  	    },
>  	    hookscript: {
> diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
> index 15ee3c67..26e1bd36 100644
> --- a/www/manager6/lxc/Resources.js
> +++ b/www/manager6/lxc/Resources.js
> @@ -257,7 +257,7 @@ Ext.define('PVE.lxc.RessourceView', {
>  	    var isUsedDisk = isDisk && !isUnusedDisk;
>  
>  	    var noedit = rec.data.delete || !rowdef.editor;
> -	    if (!noedit && Proxmox.UserName !== 'root@pam' && key.match(/^mp\d+$/)) {
> +	    if (!noedit && !caps.vms['SuperUser'] && key.match(/^mp\d+$/)) { // eslint-disable-line
>  		var mp = PVE.Parser.parseLxcMountPoint(value);
>  		if (mp.type !== 'volume') {
>  		    noedit = true;
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
> index 1c23abb3..597e3b0b 100644
> --- a/www/manager6/window/Migrate.js
> +++ b/www/manager6/window/Migrate.js
> @@ -52,8 +52,8 @@ Ext.define('PVE.window.Migrate', {
>  		    }
>  	    },
>  	    setLocalResourceCheckboxHidden: function(get) {
> -		if (get('running') || !get('migration.hasLocalResources') ||
> -		    Proxmox.UserName !== 'root@pam') {
> +		let caps = Ext.state.Manager.get('GuiCap');
> +		if (get('running') || !get('migration.hasLocalResources') || !caps.vms['SuperUser']) { // eslint-disable-line
>  		    return true;
>  		} else {
>  		    return false;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 container 08/12] fix #2582: api: add checks for 'SuperUser' privilege for root-only options

[Oguz Bektas]

this way we can allow regular users to act as superuser on specific
paths by giving them the (new) builtin 'SuperAdministrator' role or a
custom role with the 'SuperUser' privilege

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
* update the messages to reflect superuser instead of root@pam


 src/PVE/API2/LXC.pm        | 15 +++++++--------
 src/PVE/API2/LXC/Config.pm |  2 +-
 src/PVE/API2/LXC/Status.pm | 12 ++++++++----
 src/PVE/LXC.pm             | 21 ++++++++++++---------
 4 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 84712f7..4631d0b 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -304,7 +304,7 @@ __PACKAGE__->register_method({
 
 	my $conf = {};
 
-	my $is_root = $authuser eq 'root@pam';
+	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
 
 	my $no_disk_param = {};
 	my $mp_param = {};
@@ -339,8 +339,8 @@ __PACKAGE__->register_method({
 	    my $mp = $mountpoint->{mp};
 
 	    if ($mountpoint->{type} ne 'volume') { # bind or device
-		die "Only root can pass arbitrary filesystem paths.\n"
-		    if !$is_root;
+		die "Only superusers can pass arbitrary filesystem paths.\n"
+		    if !$is_superuser;
 	    } else {
 		my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
 		&$check_and_activate_storage($sid);
@@ -380,7 +380,7 @@ __PACKAGE__->register_method({
 			# causing it to restore the raw lxc entries, among which there may be
 			# 'lxc.idmap' entries. We need to make sure that the extracted contents
 			# of the container match up with the restored configuration afterwards:
-			$conf->{lxc} = $orig_conf->{lxc} if $is_root;
+			$conf->{lxc} = $orig_conf->{lxc} if $is_superuser;
 
 			$conf->{unprivileged} = $orig_conf->{unprivileged}
 			    if !defined($unprivileged) && defined($orig_conf->{unprivileged});
@@ -414,8 +414,7 @@ __PACKAGE__->register_method({
 				my $type = $mountpoint->{type};
 				die "restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
 				    if ($ms eq 'rootfs');
-				die "restoring '$ms' to $type mount is only possible for root\n"
-				    if !$is_root;
+				die "restoring '$ms' to $type mount is only possible for superusers\n" if !$is_superuser;
 
 				if ($mountpoint->{backup}) {
 				    warn "WARNING - unsupported configuration!\n";
@@ -456,7 +455,7 @@ __PACKAGE__->register_method({
 
 		    if ($restore) {
 			print "merging backed-up and given configuration..\n";
-			PVE::LXC::Create::restore_configuration($vmid, $storage_cfg, $archive, $rootdir, $conf, !$is_root, $unique, $skip_fw_config_restore);
+			PVE::LXC::Create::restore_configuration($vmid, $storage_cfg, $archive, $rootdir, $conf, !$is_superuser, $unique, $skip_fw_config_restore);
 			my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
 			$lxc_setup->template_fixup($conf);
 		    } else {
@@ -2216,7 +2215,7 @@ __PACKAGE__->register_method({
 	    raise_param_exc({ 'target-vmid' => $msg, 'storage' => $msg });
 	} elsif ($target_vmid) {
 	    $rpcenv->check_vm_perm($authuser, $target_vmid, undef, ['VM.Config.Disk'])
-		if $authuser ne 'root@pam';
+		if $authuser ne 'root@pam'; # no need to check for root@pam
 
 	    if ($vmid eq $target_vmid) {
 		my $msg = "must be different than source VMID to move disk to another container";
diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 1fec048..6278b8a 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -99,7 +99,7 @@ __PACKAGE__->register_method({
     description => "Set container options.",
     permissions => {
 	check => ['perm', '/vms/{vmid}', $vm_config_perm_list, any => 1],
-	description => 'non-volume mount points in rootfs and mp[n] are restricted to root@pam',
+	description => 'non-volume mount points in rootfs and mp[n] are restricted to superusers',
     },
     parameters => {
 	additionalProperties => 0,
diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
index f7e3128..92e37f8 100644
--- a/src/PVE/API2/LXC/Status.pm
+++ b/src/PVE/API2/LXC/Status.pm
@@ -150,9 +150,11 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	die "CT $vmid already running\n" if PVE::LXC::check_running($vmid);
 
@@ -234,9 +236,11 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
+	my $is_superuser = $authuser eq 'root@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@pam';
+	raise_param_exc({ skiplock => "Only superusers may use this option." })
+	    if $skiplock && !$is_superuser;
 
 	die "CT $vmid not running\n" if !PVE::LXC::check_running($vmid);
 
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index b07d986..60c4fce 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1254,7 +1254,10 @@ sub template_create {
 sub check_ct_modify_config_perm {
     my ($rpcenv, $authuser, $vmid, $pool, $oldconf, $newconf, $delete, $unprivileged) = @_;
 
-    return 1 if $authuser eq 'root@pam';
+    return 1 if $authuser eq 'root@pam'; # early exit for root@pam
+
+    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
     my $storage_cfg = PVE::Storage::config();
 
     my $check = sub {
@@ -1265,8 +1268,8 @@ sub check_ct_modify_config_perm {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
 	    return if $delete;
 	    my $data = PVE::LXC::Config->parse_volume($opt, $newconf->{$opt});
-	    raise_perm_exc("mount point type $data->{type} is only allowed for root\@pam")
-		if $data->{type} ne 'volume';
+	    raise_perm_exc("mount point type $data->{type} is only allowed for superusers")
+		if $data->{type} ne 'volume' && !$is_superuser;
 	    my $volid = $data->{volume};
 	    if ($volid =~ $NEW_DISK_RE) {
 		my $sid = $1;
@@ -1280,8 +1283,8 @@ sub check_ct_modify_config_perm {
 		 $opt eq 'searchdomain' || $opt eq 'hostname') {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
 	} elsif ($opt eq 'features') {
-	    raise_perm_exc("changing feature flags for privileged container is only allowed for root\@pam")
-		if !$unprivileged;
+	    raise_perm_exc("changing feature flags for privileged container is only allowed for superusers")
+		if !$unprivileged && !$is_superuser;
 
 	    my $nesting_changed = 0;
 	    my $other_changed = 0;
@@ -1319,13 +1322,13 @@ sub check_ct_modify_config_perm {
 		    $other_changed = 1;
 		}
 	    }
-	    raise_perm_exc("changing feature flags (except nesting) is only allowed for root\@pam")
-		if $other_changed;
+	    raise_perm_exc("changing feature flags (except nesting) is only allowed for superusers")
+		if $other_changed && !$is_superuser;
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Allocate'])
 		if $nesting_changed;
 	} elsif ($opt eq 'hookscript') {
-	    # For now this is restricted to root@pam
-	    raise_perm_exc("changing the hookscript is only allowed for root\@pam");
+	    raise_perm_exc("changing the hookscript is only allowed for superusers")
+		if !$is_superuser;
 	} else {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
 	}
-- 
2.30.2

Re: [pve-devel] [PATCH v2 container 08/12] fix #2582: api: add checks for 'SuperUser' privilege for root-only options

[Fabian Grünbichler]

some more stuff:
- PVE::API2::LXC mentions root-only code paths in line 261
- message in PVE::LXC::Create mentions root in line 334

On March 11, 2022 12:25 pm, Oguz Bektas wrote:
> this way we can allow regular users to act as superuser on specific
> paths by giving them the (new) builtin 'SuperAdministrator' role or a
> custom role with the 'SuperUser' privilege
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * update the messages to reflect superuser instead of root@pam
> 
> 
>  src/PVE/API2/LXC.pm        | 15 +++++++--------
>  src/PVE/API2/LXC/Config.pm |  2 +-
>  src/PVE/API2/LXC/Status.pm | 12 ++++++++----
>  src/PVE/LXC.pm             | 21 ++++++++++++---------
>  4 files changed, 28 insertions(+), 22 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 84712f7..4631d0b 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -304,7 +304,7 @@ __PACKAGE__->register_method({
>  
>  	my $conf = {};
>  
> -	my $is_root = $authuser eq 'root@pam';
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

nit: line too long..

>  
>  	my $no_disk_param = {};
>  	my $mp_param = {};
> @@ -339,8 +339,8 @@ __PACKAGE__->register_method({
>  	    my $mp = $mountpoint->{mp};
>  
>  	    if ($mountpoint->{type} ne 'volume') { # bind or device
> -		die "Only root can pass arbitrary filesystem paths.\n"
> -		    if !$is_root;
> +		die "Only superusers can pass arbitrary filesystem paths.\n"
> +		    if !$is_superuser;
>  	    } else {
>  		my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
>  		&$check_and_activate_storage($sid);
> @@ -380,7 +380,7 @@ __PACKAGE__->register_method({

comment above this context line is still referencing root..

>  			# causing it to restore the raw lxc entries, among which there may be
>  			# 'lxc.idmap' entries. We need to make sure that the extracted contents
>  			# of the container match up with the restored configuration afterwards:
> -			$conf->{lxc} = $orig_conf->{lxc} if $is_root;
> +			$conf->{lxc} = $orig_conf->{lxc} if $is_superuser;
>  
>  			$conf->{unprivileged} = $orig_conf->{unprivileged}
>  			    if !defined($unprivileged) && defined($orig_conf->{unprivileged});
> @@ -414,8 +414,7 @@ __PACKAGE__->register_method({
>  				my $type = $mountpoint->{type};
>  				die "restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
>  				    if ($ms eq 'rootfs');
> -				die "restoring '$ms' to $type mount is only possible for root\n"
> -				    if !$is_root;
> +				die "restoring '$ms' to $type mount is only possible for superusers\n" if !$is_superuser;
>  
>  				if ($mountpoint->{backup}) {
>  				    warn "WARNING - unsupported configuration!\n";
> @@ -456,7 +455,7 @@ __PACKAGE__->register_method({
>  
>  		    if ($restore) {
>  			print "merging backed-up and given configuration..\n";
> -			PVE::LXC::Create::restore_configuration($vmid, $storage_cfg, $archive, $rootdir, $conf, !$is_root, $unique, $skip_fw_config_restore);
> +			PVE::LXC::Create::restore_configuration($vmid, $storage_cfg, $archive, $rootdir, $conf, !$is_superuser, $unique, $skip_fw_config_restore);
>  			my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
>  			$lxc_setup->template_fixup($conf);
>  		    } else {
> @@ -2216,7 +2215,7 @@ __PACKAGE__->register_method({
>  	    raise_param_exc({ 'target-vmid' => $msg, 'storage' => $msg });
>  	} elsif ($target_vmid) {
>  	    $rpcenv->check_vm_perm($authuser, $target_vmid, undef, ['VM.Config.Disk'])
> -		if $authuser ne 'root@pam';
> +		if $authuser ne 'root@pam'; # no need to check for root@pam

why leave this one in and add a comment, but the others don't get the 
short cut? (e.g., there are two plain rpcenv->check calls jumping out 
even when just looking at the location of this change!)

>  
>  	    if ($vmid eq $target_vmid) {
>  		my $msg = "must be different than source VMID to move disk to another container";
> diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
> index 1fec048..6278b8a 100644
> --- a/src/PVE/API2/LXC/Config.pm
> +++ b/src/PVE/API2/LXC/Config.pm
> @@ -99,7 +99,7 @@ __PACKAGE__->register_method({
>      description => "Set container options.",
>      permissions => {
>  	check => ['perm', '/vms/{vmid}', $vm_config_perm_list, any => 1],
> -	description => 'non-volume mount points in rootfs and mp[n] are restricted to root@pam',
> +	description => 'non-volume mount points in rootfs and mp[n] are restricted to superusers',
>      },
>      parameters => {
>  	additionalProperties => 0,
> diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
> index f7e3128..92e37f8 100644
> --- a/src/PVE/API2/LXC/Status.pm
> +++ b/src/PVE/API2/LXC/Status.pm
> @@ -150,9 +150,11 @@ __PACKAGE__->register_method({
>  	my $node = extract_param($param, 'node');
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

you can guess the nit ;)

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@pam';
> +	raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	    if $skiplock && !$is_superuser;
>  
>  	die "CT $vmid already running\n" if PVE::LXC::check_running($vmid);
>  
> @@ -234,9 +236,11 @@ __PACKAGE__->register_method({
>  	my $node = extract_param($param, 'node');
>  	my $vmid = extract_param($param, 'vmid');
>  
> +	my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);

same

> +
>  	my $skiplock = extract_param($param, 'skiplock');
> -	raise_param_exc({ skiplock => "Only root may use this option." })
> -	    if $skiplock && $authuser ne 'root@pam';
> +	raise_param_exc({ skiplock => "Only superusers may use this option." })
> +	    if $skiplock && !$is_superuser;
>  
>  	die "CT $vmid not running\n" if !PVE::LXC::check_running($vmid);
>  
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index b07d986..60c4fce 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1254,7 +1254,10 @@ sub template_create {
>  sub check_ct_modify_config_perm {
>      my ($rpcenv, $authuser, $vmid, $pool, $oldconf, $newconf, $delete, $unprivileged) = @_;
>  
> -    return 1 if $authuser eq 'root@pam';
> +    return 1 if $authuser eq 'root@pam'; # early exit for root@pam
> +
> +    my $is_superuser = $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
> +
>      my $storage_cfg = PVE::Storage::config();
>  
>      my $check = sub {
> @@ -1265,8 +1268,8 @@ sub check_ct_modify_config_perm {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
>  	    return if $delete;
>  	    my $data = PVE::LXC::Config->parse_volume($opt, $newconf->{$opt});
> -	    raise_perm_exc("mount point type $data->{type} is only allowed for root\@pam")
> -		if $data->{type} ne 'volume';
> +	    raise_perm_exc("mount point type $data->{type} is only allowed for superusers")
> +		if $data->{type} ne 'volume' && !$is_superuser;
>  	    my $volid = $data->{volume};
>  	    if ($volid =~ $NEW_DISK_RE) {
>  		my $sid = $1;
> @@ -1280,8 +1283,8 @@ sub check_ct_modify_config_perm {
>  		 $opt eq 'searchdomain' || $opt eq 'hostname') {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
>  	} elsif ($opt eq 'features') {
> -	    raise_perm_exc("changing feature flags for privileged container is only allowed for root\@pam")
> -		if !$unprivileged;
> +	    raise_perm_exc("changing feature flags for privileged container is only allowed for superusers")
> +		if !$unprivileged && !$is_superuser;
>  
>  	    my $nesting_changed = 0;
>  	    my $other_changed = 0;
> @@ -1319,13 +1322,13 @@ sub check_ct_modify_config_perm {
>  		    $other_changed = 1;
>  		}
>  	    }
> -	    raise_perm_exc("changing feature flags (except nesting) is only allowed for root\@pam")
> -		if $other_changed;
> +	    raise_perm_exc("changing feature flags (except nesting) is only allowed for superusers")
> +		if $other_changed && !$is_superuser;
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Allocate'])
>  		if $nesting_changed;
>  	} elsif ($opt eq 'hookscript') {
> -	    # For now this is restricted to root@pam
> -	    raise_perm_exc("changing the hookscript is only allowed for root\@pam");
> +	    raise_perm_exc("changing the hookscript is only allowed for superusers")
> +		if !$is_superuser;
>  	} else {
>  	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
>  	}
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 storage 09/12] check_volume_access: allow superusers to pass arbitrary fs paths

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 PVE/Storage.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index b1d31bb..762933f 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -467,6 +467,11 @@ sub parse_volume_id {
 sub check_volume_access {
     my ($rpcenv, $user, $cfg, $vmid, $volid) = @_;
 
+    return if $user eq 'root@pam'; # root@pam always OK
+
+    # SU on "/" are needed for passing arbitrary filesystem paths
+    my $is_superuser = $rpcenv->check($user, "/", ['SuperUser'], 1);
+
     my ($sid, $volname) = parse_volume_id($volid, 1);
     if ($sid) {
 	my ($vtype, undef, $ownervm) = parse_volname($cfg, $volid);
@@ -483,8 +488,8 @@ sub check_volume_access {
 	    $rpcenv->check($user, "/storage/$sid", ['Datastore.Allocate']);
 	}
     } else {
-	die "Only root can pass arbitrary filesystem paths."
-	    if $user ne 'root@pam';
+	die "Only superusers can pass arbitrary filesystem paths."
+	    if !$is_superuser;
     }
 
     return undef;
-- 
2.30.2

[pve-devel] [PATCH v2 access-control 10/12] add "SuperAdministrator" role with the new "SuperUser" privilege

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
no changes

 src/PVE/AccessControl.pm  | 9 ++++++---
 src/PVE/RPCEnvironment.pm | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 1306576..1137756 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1008,6 +1008,7 @@ my $privgroups = {
     },
     Sys => {
 	root => [
+	    'SuperUser',
 	    'Sys.PowerMgmt',
 	    'Sys.Modify', # edit/change node settings
 	],
@@ -1074,7 +1075,8 @@ my $valid_privs = {};
 
 my $special_roles = {
     'NoAccess' => {}, # no privileges
-    'Administrator' => $valid_privs, # all privileges
+    'Administrator' => {}, # populated with all privs except superuser
+    'SuperAdministrator' => $valid_privs, # can do everything
 };
 
 sub create_roles {
@@ -1101,6 +1103,7 @@ sub create_roles {
     }
 
     $special_roles->{"PVETemplateUser"} = { 'VM.Clone' => 1, 'VM.Audit' => 1 };
+    $special_roles->{"Administrator"} =  { map { $_ => 1 } grep { $_ ne 'SuperUser'  } keys %$valid_privs };
 };
 
 create_roles();
@@ -1581,7 +1584,7 @@ sub write_user_config {
 
 	$collect_rolelist_members->($d->{'groups'}, $rolelist_members, '@');
 
-	# no need to save 'root@pam', it is always 'Administrator'
+	# no need to save 'root@pam', it is always 'SuperAdministrator'
 	$collect_rolelist_members->($d->{'users'}, $rolelist_members, '', 'root@pam');
 
 	$collect_rolelist_members->($d->{'tokens'}, $rolelist_members, '');
@@ -1635,7 +1638,7 @@ sub roles {
     # corresponding user has.
     # Use $rpcenv->permission() for any actual permission checks!
 
-    return 'Administrator' if $user eq 'root@pam'; # root can do anything
+    return 'SuperAdministrator' if $user eq 'root@pam'; # root can do anything
 
     if (pve_verify_tokenid($user, 1)) {
 	my $tokenid = $user;
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index b1348fa..3afb311 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -101,7 +101,7 @@ sub permissions {
 
     if ($user eq 'root@pam') { # root can do anything
 	my $cfg = $self->{user_cfg};
-	return { map { $_ => 1 } keys %{$cfg->{roles}->{'Administrator'}} };
+	return { map { $_ => 1 } keys %{$cfg->{roles}->{'SuperAdministrator'}} };
     }
 
     if (PVE::AccessControl::pve_verify_tokenid($user, 1)) {
-- 
2.30.2

[pve-devel] [PATCH v2 access-control 11/12] api: allow superusers to edit tfa and password settings

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
* also adapt change_password
* didn't remove the comments in TFA.pm since it was still confusing without them

 src/PVE/API2/AccessControl.pm | 6 ++++++
 src/PVE/API2/TFA.pm           | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
index 5d78c6f..1b471f6 100644
--- a/src/PVE/API2/AccessControl.pm
+++ b/src/PVE/API2/AccessControl.pm
@@ -378,9 +378,15 @@ __PACKAGE__->register_method ({
 
 	$rpcenv->check_user_exist($userid);
 
+	# check for SU privs on user groups
+	my $is_superuser = $rpcenv->check($authuser, "/access/groups", ['SuperUser'], 1);
+
 	if ($authuser eq 'root@pam') {
 	    # OK - root can change anything
 	} else {
+	    if ($is_superuser && $userid ne 'root@pam') {
+		# OK - superusers can change any user's password except root@pam
+	    }
 	    if ($authuser eq $userid) {
 		$rpcenv->check_user_enabled($userid);
 		# OK - each user can change its own password
diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm
index bee4dee..bab78ea 100644
--- a/src/PVE/API2/TFA.pm
+++ b/src/PVE/API2/TFA.pm
@@ -102,13 +102,16 @@ my $TFA_UPDATE_INFO_SCHEMA = {
 my sub root_permission_check : prototype($$$$) {
     my ($rpcenv, $authuser, $userid, $password) = @_;
 
+    # authuser = the user making the change
+    # userid = the user to be changed
     ($userid, undef, my $realm) = PVE::AccessControl::verify_username($userid);
     $rpcenv->check_user_exist($userid);
 
-    raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam';
+    my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/access/groups", ['SuperUser'], 1);
+    raise_perm_exc() if $userid eq 'root@pam' && !$is_superuser; # root@pam can be only edited by superusers
 
     # Regular users need to confirm their password to change TFA settings.
-    if ($authuser ne 'root@pam') {
+    if ($authuser ne 'root@pam') { # we still do password confirmation for superusers like the other users
 	raise_param_exc({ 'password' => 'password is required to modify TFA data' })
 	    if !defined($password);
 
-- 
2.30.2

Re: [pve-devel] [PATCH v2 access-control 11/12] api: allow superusers to edit tfa and password settings

[Fabian Grünbichler]

On March 11, 2022 12:25 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * also adapt change_password
> * didn't remove the comments in TFA.pm since it was still confusing without them
> 
>  src/PVE/API2/AccessControl.pm | 6 ++++++
>  src/PVE/API2/TFA.pm           | 7 +++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
> index 5d78c6f..1b471f6 100644
> --- a/src/PVE/API2/AccessControl.pm
> +++ b/src/PVE/API2/AccessControl.pm
> @@ -378,9 +378,15 @@ __PACKAGE__->register_method ({
>  
>  	$rpcenv->check_user_exist($userid);
>  
> +	# check for SU privs on user groups
> +	my $is_superuser = $rpcenv->check($authuser, "/access/groups", ['SuperUser'], 1);

I know this is modelled after userid-group, but it's a lot more 
restrictive? I'd either use full userid-group semantics, or just 
'/access' as path, the current one doesn't make much sense as a middle 
ground.. I think I'd prefer '/access', given the issue further below..

but also, this check here is with $noerr set.. (ctd)

> +
>  	if ($authuser eq 'root@pam') {
>  	    # OK - root can change anything
>  	} else {
> +	    if ($is_superuser && $userid ne 'root@pam') {
> +		# OK - superusers can change any user's password except root@pam
> +	    }

and here you add an empty `if` with a side-effect free condition as only 
place where the result of the check above is used.. so effectively, 
nothing about this API path changes at all? how did you test this? what 
was the expected change, "adapt change_password" doesn't really give me 
a clue ;)

one could argue that if a user is SU anywhere, changing their password 
should require SU as well (on '/access' ?) or be limited to the user 
itself, similar to the semantics of 'root@pam'..

the 'user is SU anywhere' part doesn't exist yet and is a bit tricky 
(handling of groups/..) or expensive (get list of all currently defined 
ACL paths, query actual permissions on all of them).

changing another user's password already requires fairly high privileges:
- realm != pam
- Realm.AllocateUser on realm
- User.Modify on user (i.e., either User.Modify on all groups or a group 
  the user belongs to, like the other user-modification API endpoints)

there obviously is a pit-fall there though with the last part (add a SU 
to some group without knowing about the implication that other users 
with User.Modify on that group can now modify the SU, including changing 
password if the other conditions are met).

>  	    if ($authuser eq $userid) {
>  		$rpcenv->check_user_enabled($userid);
>  		# OK - each user can change its own password
> diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm
> index bee4dee..bab78ea 100644
> --- a/src/PVE/API2/TFA.pm
> +++ b/src/PVE/API2/TFA.pm
> @@ -102,13 +102,16 @@ my $TFA_UPDATE_INFO_SCHEMA = {

comment above this sub is now wrong..

>  my sub root_permission_check : prototype($$$$) {
>      my ($rpcenv, $authuser, $userid, $password) = @_;
>  
> +    # authuser = the user making the change
> +    # userid = the user to be changed
>      ($userid, undef, my $realm) = PVE::AccessControl::verify_username($userid);
>      $rpcenv->check_user_exist($userid);
>  
> -    raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam';
> +    my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/access/groups", ['SuperUser'], 1);

same as above applies here as well w.r.t. the checked path / semantics.

> +    raise_perm_exc() if $userid eq 'root@pam' && !$is_superuser; # root@pam can be only edited by superusers

if we add a check for the other direction (special restriction of who 
can change a SU's password) above, the same semantics might make sense 
here (i.e., changing a SU's TFA settings requires SU as well).

>  
>      # Regular users need to confirm their password to change TFA settings.
> -    if ($authuser ne 'root@pam') {
> +    if ($authuser ne 'root@pam') { # we still do password confirmation for superusers like the other users
>  	raise_param_exc({ 'password' => 'password is required to modify TFA data' })
>  	    if !defined($password);
>  
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] [PATCH v2 access-control 12/12] api: acl: only allow granting SU privilege if user already has it

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v1->v2:
* added new after discussion with fabian about security implications of
allowing SU privilege to be granted by users with Permissions.Modify

 src/PVE/API2/ACL.pm | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
index 857c672..d415334 100644
--- a/src/PVE/API2/ACL.pm
+++ b/src/PVE/API2/ACL.pm
@@ -134,6 +134,10 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $is_superuser = $rpcenv->check($authuser, $param->{path}, ['SuperUser'], 1);
+
 	if (!($param->{users} || $param->{groups} || $param->{tokens})) {
 	    raise_param_exc({ map { $_ => "either 'users', 'groups' or 'tokens' is required." } qw(users groups tokens) });
 	}
@@ -160,6 +164,11 @@ __PACKAGE__->register_method ({
 		    die "role '$role' does not exist\n"
 			if !$cfg->{roles}->{$role};
 
+		    my $role_privs = $cfg->{roles}->{$role};
+		    my $role_contains_superuser = grep { $_ eq 'SuperUser' } keys %$role_privs;
+		    die "only superusers can grant this role!\n"
+			if !$is_superuser && $role_contains_superuser;
+
 		    foreach my $group (split_list($param->{groups})) {
 
 			die "group '$group' does not exist\n"
-- 
2.30.2

Re: [pve-devel] [PATCH v2 access-control 12/12] api: acl: only allow granting SU privilege if user already has it

[Fabian Grünbichler]

a similar patch for adding/editing roles is missing (else, this is 
trivially worked around by giving myself 'CustomRole' that  doesn't have 
SU, then editing that role to add SU to it).

On March 11, 2022 12:25 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * added new after discussion with fabian about security implications of
> allowing SU privilege to be granted by users with Permissions.Modify
> 
>  src/PVE/API2/ACL.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
> index 857c672..d415334 100644
> --- a/src/PVE/API2/ACL.pm
> +++ b/src/PVE/API2/ACL.pm
> @@ -134,6 +134,10 @@ __PACKAGE__->register_method ({
>      code => sub {
>  	my ($param) = @_;
>  
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $authuser = $rpcenv->get_user();
> +	my $is_superuser = $rpcenv->check($authuser, $param->{path}, ['SuperUser'], 1);

this does not include checking whether propagate is set or not, but this 
API path allows setting the propagate flag on an ACL (so if $authuser 
has SU on $param->{path} *without propagation*, it could now give away SU 
on $param->{path} *with propagation*, thus extending SU to arbitrary sub 
paths).

> +
>  	if (!($param->{users} || $param->{groups} || $param->{tokens})) {
>  	    raise_param_exc({ map { $_ => "either 'users', 'groups' or 'tokens' is required." } qw(users groups tokens) });
>  	}
> @@ -160,6 +164,11 @@ __PACKAGE__->register_method ({
>  		    die "role '$role' does not exist\n"
>  			if !$cfg->{roles}->{$role};
>  
> +		    my $role_privs = $cfg->{roles}->{$role};
> +		    my $role_contains_superuser = grep { $_ eq 'SuperUser' } keys %$role_privs;
> +		    die "only superusers can grant this role!\n"
> +			if !$is_superuser && $role_contains_superuser;
> +
>  		    foreach my $group (split_list($param->{groups})) {
>  
>  			die "group '$group' does not exist\n"
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH v2 access-control++ 00/12] SuperUser privilege

[Fabian Grünbichler]

On March 11, 2022 12:24 pm, Oguz Bektas wrote:
> v1->v2:
> * added some basic docs

still missing even though I requested this a few times already:
- list of API paths that are root-only still and why/plans on how to 
  proceed there
- list of other things which are root-only still and why/plans on how to 
  proceed there

or alternatively, comments where it makes sense for stuff that is and 
remains root-only intentionally (forever, or for the time being).

it's really hard otherwise for me to know what was missed/not looked 
at/skipped for $reasons which are valid/skipped for $reasons which are 
wrong.

> 
> changes in rest of the patches are in the separate mails.
> 
> big thanks to Fabian G. for the reviews and answering my questions
> throughout the series :)
> 
> it's a complicated series so if i forgot something i'm sorry!
> 
> note: all the patches on the other repositories depend on the
> access-control patches to be applied and installed first
> 
>  docs: Oguz Bektas (1):
>   pveum: add SU privilege and SA role
> 
>  pveum.adoc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
>  qemu-server: Oguz Bektas (2):
>   api: allow SU privileged users to edit root-only options for VM
>     configs
>   api: allow 'skiplock' option to be used by SU privileged users

missing in qemu-server (besides stuff noted in reply to the patches):
- an unmarked shortcut in $parse_backup_hints

> 
>  PVE/API2/Qemu.pm | 122 ++++++++++++++++++++++++++++-------------------
>  1 file changed, 72 insertions(+), 50 deletions(-)
> 
>  manager: Oguz Bektas (4):
>   api: backup: allow SUs to use 'tmpdir', 'dumpdir' and 'script' options
>   api: vzdump: allow SUs to use 'bwlimit' and 'ionice' parameters
>   api: update comment about login prompt for non-root users
>   ui: adapt sensible 'root@pam' checks to SU privilege
> 
>  PVE/API2/Backup.pm             | 11 ++++++++---
>  PVE/API2/Nodes.pm              |  2 +-
>  PVE/API2/VZDump.pm             |  8 +++++---
>  www/manager6/dc/Config.js      |  2 +-
>  www/manager6/lxc/Options.js    |  2 +-
>  www/manager6/lxc/Resources.js  |  2 +-
>  www/manager6/window/Migrate.js |  4 ++--
>  7 files changed, 19 insertions(+), 12 deletions(-)
> 
>  container: Oguz Bektas (1):
>   fix #2582: api: add checks for 'SuperUser' privilege for root-only
>     options
> 
>  src/PVE/API2/LXC.pm        | 15 +++++++--------
>  src/PVE/API2/LXC/Config.pm |  2 +-
>  src/PVE/API2/LXC/Status.pm | 12 ++++++++----
>  src/PVE/LXC.pm             | 21 ++++++++++++---------
>  4 files changed, 28 insertions(+), 22 deletions(-)
> 
>  storage: Oguz Bektas (1):
>   check_volume_access: allow superusers to pass arbitrary fs paths
> 
>  PVE/Storage.pm | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
>  access-control: Oguz Bektas (3):
>   add "SuperAdministrator" role with the new "SuperUser" privilege
>   api: allow superusers to edit tfa and password settings
>   api: acl: only allow granting SU privilege if user already has it
> 
>  src/PVE/API2/ACL.pm           | 9 +++++++++
>  src/PVE/API2/AccessControl.pm | 6 ++++++
>  src/PVE/API2/TFA.pm           | 7 +++++--
>  src/PVE/AccessControl.pm      | 9 ++++++---
>  src/PVE/RPCEnvironment.pm     | 2 +-
>  5 files changed, 27 insertions(+), 6 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>