[pve-devel] applied: [PATCH v2 qemu-server] API: update_vm_api: check for CDROM on disk delete

[Thomas Lamprecht]

On 12.02.21 16:57, Aaron Lauterer wrote:
> Since CDRoms and disks share the same config keys, we need to check if
> it actually is a CDRom and then check the permissions accordingly.
> 
> Otherwise it is possible for someone without VM.Config.CDROM
> permissions, but with VM.Config.Disk permissions to remove a CD drive
> while being unable to create a CDRom drive.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
> ---
> 
> Since it is possible to delete a CDRom while not having the permissions
> to create them, I consider this a bug.
> 
> With this patch it is also possible to now remove a CDRom drive with
> only the VM.Config.CDROM permissions which needed VM.Config.Disk
> permissions before. Creating them with the CDRom permissions has already
> been possible before.
> 
>  PVE/API2/Qemu.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

For the record: technically this is, in the widest sense, a backward incompatible
change and I even had a commit here prepared which would allow any of those
permissions (see below), but I dropped it, this is such a niche use case that I just
cannot believe that anybody will be affected by it - one needs to have CDROM for
adding, so basically a CDROM dev would need to be there, then a user with a role
containing 'VM.Config.Disk' but *not* 'VM.Config.CDROM', whom only needs to remove
CDROM devices but not add them, cannot do that anymore, yeah, no, really not worth
the hassle.


obsoleted dropped diff for the record only:

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index feb9ea8..c932571 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1237,7 +1237,8 @@ my $update_vm_api  = sub {
                    PVE::QemuConfig->check_protection($conf, "can't remove drive '$opt'");
                    my $drive = PVE::QemuServer::parse_drive($opt, $val);
                    if (PVE::QemuServer::drive_is_cdrom($drive)) {
-                       $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.CDROM']);
+                       # FIXME: remove 'VM.Config.Disk' and $any flag for PVE 7
+                       $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.CDROM', 'VM.Config.Disk'], 1);
                    } else {
                        $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
                    }