From: Oguz Bektas <o.bektas@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v1 qemu-server 5/5] add SuperUser privilege checks for root-only options
Date: Tue, 8 Feb 2022 14:10:11 +0100 [thread overview]
Message-ID: <20220208131011.752134-6-o.bektas@proxmox.com> (raw)
In-Reply-To: <20220208131011.752134-1-o.bektas@proxmox.com>
analogous to the changes in container.
we now allow users with SU privilege to edit real device configurations,
provided that they also have the necessary VM privileges.
note that root@pam is still able to do everything as usual
---
PVE/API2/Qemu.pm | 119 +++++++++++++++++++++++++++++------------------
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 6992f6f..9d403b4 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -352,7 +352,7 @@ my $cloudinitoptions = {
my $check_vm_create_serial_perm = sub {
my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
- return 1 if $authuser eq 'root@pam';
+ return 1 if $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
foreach my $opt (keys %{$param}) {
next if $opt !~ m/^serial\d+$/;
@@ -370,7 +370,7 @@ my $check_vm_create_serial_perm = sub {
my $check_vm_create_usb_perm = sub {
my ($rpcenv, $authuser, $vmid, $pool, $param) = @_;
- return 1 if $authuser eq 'root@pam';
+ return 1 if $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
foreach my $opt (keys %{$param}) {
next if $opt !~ m/^usb\d+$/;
@@ -388,7 +388,7 @@ my $check_vm_create_usb_perm = sub {
my $check_vm_modify_config_perm = sub {
my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
- return 1 if $authuser eq 'root@pam';
+ return 1 if $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
foreach my $opt (@$key_list) {
# some checks (e.g., disk, serial port, usb) need to be done somewhere
@@ -1117,9 +1117,10 @@ my $update_vm_api = sub {
push @paramarr, "-$key", $value;
}
+ my $is_superuser = $authuser eq 'root@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');
@@ -1340,16 +1341,18 @@ my $update_vm_api = sub {
} elsif ($opt =~ m/^serial\d+$/) {
if ($val eq 'socket') {
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
- } elsif ($authuser ne 'root@pam') {
- die "only root can delete '$opt' config for real devices\n";
+ } elsif (!$is_superuser) {
+ die "only superusers can delete '$opt' config for real devices\n"
+ if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
}
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
} elsif ($opt =~ m/^usb\d+$/) {
if ($val =~ m/spice/) {
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
- } elsif ($authuser ne 'root@pam') {
- die "only root can delete '$opt' config for real devices\n";
+ } elsif (!$is_superuser) {
+ die "only superusers can delete '$opt' config for real devices\n"
+ if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
}
PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
PVE::QemuConfig->write_config($vmid, $conf);
@@ -1392,15 +1395,17 @@ my $update_vm_api = sub {
} elsif ($opt =~ m/^serial\d+/) {
if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
- } elsif ($authuser ne 'root@pam') {
- die "only root can modify '$opt' config for real devices\n";
+ } elsif (!$is_superuser) {
+ die "only superuser can modify '$opt' config for real devices\n"
+ if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
}
$conf->{pending}->{$opt} = $param->{$opt};
} elsif ($opt =~ m/^usb\d+/) {
if ((!defined($conf->{$opt}) || $conf->{$opt} =~ m/spice/) && $param->{$opt} =~ m/spice/) {
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
- } elsif ($authuser ne 'root@pam') {
- die "only root can modify '$opt' config for real devices\n";
+ } elsif (!$is_superuser) {
+ die "only superuser can modify '$opt' config for real devices\n"
+ if !$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
}
$conf->{pending}->{$opt} = $param->{$opt};
} else {
@@ -1644,9 +1649,11 @@ __PACKAGE__->register_method({
my $authuser = $rpcenv->get_user();
my $vmid = $param->{vmid};
+ my $is_superuser = $authuser eq 'root@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
@@ -2291,10 +2298,12 @@ __PACKAGE__->register_method({
my $machine = extract_param($param, 'machine');
my $force_cpu = extract_param($param, 'force-cpu');
+ my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
my $get_root_param = sub {
my $value = extract_param($param, $_[0]);
- raise_param_exc({ "$_[0]" => "Only root may use this option." })
- if $value && $authuser ne 'root@pam';
+ raise_param_exc({ "$_[0]" => "Only superusers may use this option." })
+ if $value && !$is_superuser;
return $value;
};
@@ -2436,17 +2445,19 @@ __PACKAGE__->register_method({
my $node = extract_param($param, 'node');
my $vmid = extract_param($param, 'vmid');
+ my $is_superuser = $authuser eq 'root@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." })
- if $keepActive && $authuser ne 'root@pam';
+ raise_param_exc({ keepActive => "Only superusers may use this option." })
+ if $keepActive && !$is_superuser;
my $migratedfrom = extract_param($param, 'migratedfrom');
- raise_param_exc({ migratedfrom => "Only root may use this option." })
- if $migratedfrom && $authuser ne 'root@pam';
+ raise_param_exc({ migratedfrom => "Only superusers may use this option." })
+ if $migratedfrom && !$is_superuser;
my $storecfg = PVE::Storage::config();
@@ -2513,9 +2524,11 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid');
+ my $is_superuser = $authuser eq 'root@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,13 +2593,15 @@ __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." })
- if $keepActive && $authuser ne 'root@pam';
+ raise_param_exc({ keepActive => "Only superusers may use this option." })
+ if $keepActive && !$is_superuser;
my $storecfg = PVE::Storage::config();
@@ -2739,9 +2754,11 @@ __PACKAGE__->register_method({
my $statestorage = extract_param($param, 'statestorage');
+ my $is_superuser = $authuser eq 'root@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,13 +2828,15 @@ __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." })
- if $nocheck && $authuser ne 'root@pam';
+ raise_param_exc({ nocheck => "Only superusers may use this option." })
+ if $nocheck && !$is_superuser;
my $to_disk_suspended;
eval {
@@ -2883,9 +2902,11 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid');
+ my $is_superuser = $authuser eq 'root@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});
@@ -3392,6 +3413,8 @@ __PACKAGE__->register_method({
my $storecfg = PVE::Storage::config();
+ my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
+
my $move_updatefn = sub {
my $conf = PVE::QemuConfig->load_config($vmid);
PVE::QemuConfig->check_lock($conf);
@@ -3856,7 +3879,7 @@ __PACKAGE__->register_method({
},
force => {
type => 'boolean',
- description => "Allow to migrate VMs which use local devices. Only root may use this option.",
+ description => "Allow to migrate VMs which use local devices. Only superusers may use this option.",
optional => 1,
},
migration_type => {
@@ -3910,15 +3933,17 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid');
- raise_param_exc({ force => "Only root may use this option." })
- if $param->{force} && $authuser ne 'root@pam';
+ my $is_superuser = $authuser eq 'root@pam' || $rpcenv->check($authuser, "/vms/$vmid", ['SuperUser'], 1);
- raise_param_exc({ migration_type => "Only root may use this option." })
- if $param->{migration_type} && $authuser ne 'root@pam';
+ raise_param_exc({ force => "Only superusers may use this option." })
+ if $param->{force} && !$is_superuser;
+
+ raise_param_exc({ migration_type => "Only superusers may use this option." })
+ if $param->{migration_type} && !$is_superuser;
# allow root only until better network permissions are available
- raise_param_exc({ migration_network => "Only root may use this option." })
- if $param->{migration_network} && $authuser ne 'root@pam';
+ raise_param_exc({ migration_network => "Only superusers may use this option." })
+ if $param->{migration_network} && !$is_superuser;
# test if VM exists
my $conf = PVE::QemuConfig->load_config($vmid);
@@ -4098,9 +4123,11 @@ __PACKAGE__->register_method({
my $sizestr = extract_param($param, 'size');
+ my $is_superuser = $authuser eq 'root@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
next prev parent reply other threads:[~2022-02-08 13:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-08 13:10 [pve-devel] [PATCH v1 access-control++ 0/5] SuperUser privilege Oguz Bektas
2022-02-08 13:10 ` [pve-devel] [PATCH v1 access-control 1/5] add default "SuperAdministrator" role with the new "SuperUser" privilege Oguz Bektas
2022-02-08 13:10 ` [pve-devel] [PATCH v1 access-control 2/5] tfa: allow superusers to edit root@pam tfa Oguz Bektas
[not found] ` <<20220208131011.752134-3-o.bektas@proxmox.com>
2022-02-10 15:30 ` Fabian Grünbichler
2022-02-08 13:10 ` [pve-devel] [PATCH v1 container 3/5] fix #2582: api: add checks for 'SuperUser' privilege for root-only options Oguz Bektas
[not found] ` <<20220208131011.752134-4-o.bektas@proxmox.com>
2022-02-10 15:30 ` Fabian Grünbichler
2022-02-08 13:10 ` [pve-devel] [PATCH v1 manager 4/5] change 'root@pam' checks with 'SuperUser' capability check Oguz Bektas
[not found] ` <<20220208131011.752134-5-o.bektas@proxmox.com>
2022-02-10 15:29 ` Fabian Grünbichler
2022-02-25 10:13 ` Dominik Csapak
2022-02-25 12:24 ` Thomas Lamprecht
2022-02-08 13:10 ` Oguz Bektas [this message]
[not found] ` <<20220208131011.752134-6-o.bektas@proxmox.com>
2022-02-10 15:29 ` [pve-devel] [PATCH v1 qemu-server 5/5] add SuperUser privilege checks for root-only options Fabian Grünbichler
2022-02-10 15:28 ` [pve-devel] [PATCH v1 access-control++ 0/5] SuperUser privilege Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220208131011.752134-6-o.bektas@proxmox.com \
--to=o.bektas@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.