[pve-devel] [RFC qemu-server 4/4] api: use common helper for checking root privileges
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Jan 10 14:45:08 CET 2022
this patch mixes a lot of "check for 'root at pam' as short cut" with
"check for 'root at pam' for stuff limited to 'root at pam'". as per the
discussion in the cover letter - if we make Sys.Root imply all other
privileges like 'root at pam' currently does, then all the short cut
instances can just remain as is..
On January 5, 2022 4:22 pm, Oguz Bektas wrote:
> to allow API users with the 'Sys.Root' permission to call these
> endpoints.
>
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
> PVE/API2/Qemu.pm | 74 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 6992f6f..e846a1e 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 PVE::Tools::check_for_root("/vms/$vmid");
>
> 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 PVE::Tools::check_for_root("/vms/$vmid");
>
> 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 PVE::Tools::check_for_root("/vms/$vmid");
>
> foreach my $opt (@$key_list) {
> # some checks (e.g., disk, serial port, usb) need to be done somewhere
> @@ -1105,6 +1105,8 @@ my $update_vm_api = sub {
>
> my $background_delay = extract_param($param, 'background_delay');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> if (defined(my $cipassword = $param->{cipassword})) {
> # Same logic as in cloud-init (but with the regex fixed...)
> $param->{cipassword} = PVE::Tools::encrypt_pw($cipassword)
> @@ -1119,7 +1121,7 @@ my $update_vm_api = sub {
>
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $delete_str = extract_param($param, 'delete');
>
> @@ -1340,7 +1342,7 @@ 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') {
> + } elsif (!$is_root) {
> die "only root can delete '$opt' config for real devices\n";
> }
> PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> @@ -1348,7 +1350,7 @@ my $update_vm_api = sub {
> } 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') {
> + } elsif (!$is_root) {
> die "only root can delete '$opt' config for real devices\n";
> }
> PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
> @@ -1392,14 +1394,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 at pam') {
> + } elsif (!$is_root) {
> die "only root can modify '$opt' config for real devices\n";
> }
> $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') {
> + } elsif (!$is_root) {
> die "only root can modify '$opt' config for real devices\n";
> }
> $conf->{pending}->{$opt} = $param->{$opt};
> @@ -1644,9 +1646,11 @@ __PACKAGE__->register_method({
> my $authuser = $rpcenv->get_user();
> my $vmid = $param->{vmid};
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = $param->{skiplock};
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $early_checks = sub {
> # test if VM exists
> @@ -2291,10 +2295,12 @@ __PACKAGE__->register_method({
> my $machine = extract_param($param, 'machine');
> my $force_cpu = extract_param($param, 'force-cpu');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> 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';
> + if $value && !$is_root;
> return $value;
> };
>
> @@ -2436,17 +2442,19 @@ __PACKAGE__->register_method({
> my $node = extract_param($param, 'node');
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $keepActive = extract_param($param, 'keepActive');
> raise_param_exc({ keepActive => "Only root may use this option." })
> - if $keepActive && $authuser ne 'root at pam';
> + if $keepActive && !$is_root;
>
> my $migratedfrom = extract_param($param, 'migratedfrom');
> raise_param_exc({ migratedfrom => "Only root may use this option." })
> - if $migratedfrom && $authuser ne 'root at pam';
> + if $migratedfrom && !$is_root;
>
>
> my $storecfg = PVE::Storage::config();
> @@ -2513,9 +2521,11 @@ __PACKAGE__->register_method({
>
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
>
> @@ -2580,13 +2590,15 @@ __PACKAGE__->register_method({
> my $node = extract_param($param, 'node');
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $keepActive = extract_param($param, 'keepActive');
> raise_param_exc({ keepActive => "Only root may use this option." })
> - if $keepActive && $authuser ne 'root at pam';
> + if $keepActive && !$is_root;
>
> my $storecfg = PVE::Storage::config();
>
> @@ -2735,13 +2747,15 @@ __PACKAGE__->register_method({
> my $node = extract_param($param, 'node');
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $todisk = extract_param($param, 'todisk') // 0;
>
> my $statestorage = extract_param($param, 'statestorage');
>
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
>
> @@ -2811,13 +2825,15 @@ __PACKAGE__->register_method({
>
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $nocheck = extract_param($param, 'nocheck');
> raise_param_exc({ nocheck => "Only root may use this option." })
> - if $nocheck && $authuser ne 'root at pam';
> + if $nocheck && !$is_root;
>
> my $to_disk_suspended;
> eval {
> @@ -2883,9 +2899,11 @@ __PACKAGE__->register_method({
>
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $skiplock = extract_param($param, 'skiplock');
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> PVE::QemuServer::vm_sendkey($vmid, $skiplock, $param->{key});
>
> @@ -3392,6 +3410,8 @@ __PACKAGE__->register_method({
>
> my $storecfg = PVE::Storage::config();
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> my $move_updatefn = sub {
> my $conf = PVE::QemuConfig->load_config($vmid);
> PVE::QemuConfig->check_lock($conf);
> @@ -3671,7 +3691,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 at pam';
> + if !$is_root;
>
> raise_param_exc({ 'target-vmid' => "must be different than source VMID to reassign disk" })
> if $vmid eq $target_vmid;
> @@ -3910,15 +3930,17 @@ __PACKAGE__->register_method({
>
> my $vmid = extract_param($param, 'vmid');
>
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> raise_param_exc({ force => "Only root may use this option." })
> - if $param->{force} && $authuser ne 'root at pam';
> + if $param->{force} && !$is_root;
>
> raise_param_exc({ migration_type => "Only root may use this option." })
> - if $param->{migration_type} && $authuser ne 'root at pam';
> + if $param->{migration_type} && !$is_root;
>
> # 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';
> + if $param->{migration_network} && !$is_root;
>
> # test if VM exists
> my $conf = PVE::QemuConfig->load_config($vmid);
> @@ -4099,8 +4121,10 @@ __PACKAGE__->register_method({
> my $sizestr = extract_param($param, 'size');
>
> my $skiplock = extract_param($param, 'skiplock');
> + my $is_root = PVE::Tools::check_for_root("/vms/$vmid");
> +
> raise_param_exc({ skiplock => "Only root may use this option." })
> - if $skiplock && $authuser ne 'root at pam';
> + if $skiplock && !$is_root;
>
> my $storecfg = PVE::Storage::config();
>
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list