[pve-devel] [PATCH-SERIES access-control/qemu-server/manager/docs] replace ambiguously named VM.Monitor privilege

[pve-devel] [PATCH-SERIES access-control/qemu-server/manager/docs] replace ambiguously named VM.Monitor privilege

[Fiona Ebner]

The privilege VM.Monitor has a very ambiguous name and is dropped.
Most of the API endpoints using it are for the QEMU guest agent
commands, the only other place is access to the QEMU HMP monitor.

1. Introduce dedicated, more fine-grained privileges for the guest
agent commands:

There is a basic VM.GuestAgent.Audit privilege for read-only,
informational commands.

There are dedicated privileges VM.GuestAgent.File{Read,Write} for the
file-{read,write} commands. There is a separate
VM.GuestAgent.FileSystemMgmt privilege for filesystem freeze, thaw and
trim.

The VM.GuestAgent.Unrestricted privilege is to allow all guest agent
operations, in particular also execution of arbitrary commands with
guest-exec.

2. For access to the QEMU HMP monitor, only the 'info' and 'help'
commands were usable without an additional Sys.Modify privilege. Since
the information accessible via 'info' is very low-level and often
related to the QEMU process on the system, requiring Sys.Audit seems
natural.

These are breaking changes. A check in pve8to9 is provided.


qemu-server patch "api: monitor: improve permission handling" and
manager patch "pve8to9: remove outdated checks for user roles" can
be applied independently from the rest of the series.


New qemu-server depends on new access-control, new access-control
breaks old qemu-server.


access-control:

Fiona Ebner (2):
  add VM.GuestAgent privileges
  privileges: drop VM.Monitor

 src/PVE/AccessControl.pm | 7 +++++--
 src/test/perm-test1.pl   | 8 ++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)


qemu-server:

Fiona Ebner (3):
  api: agent: use more specific guest agent privileges
  api: monitor: improve permission handling
  api: monitor: require Sys.Audit or Sys.Modify privilege

 src/PVE/API2/Qemu.pm          |  34 ++++--
 src/PVE/API2/Qemu/Agent.pm    |  66 +++++++++--
 src/PVE/API2/Qemu/HMPPerms.pm | 207 ++++++++++++++++++++++++++++++++++
 src/PVE/API2/Qemu/Makefile    |   2 +-
 4 files changed, 289 insertions(+), 20 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/HMPPerms.pm


manager:

Fiona Ebner (2):
  pve8to9: remove outdated checks for user roles
  pve8to9: check for to-be-dropped VM.Monitor privilege in custom roles

 PVE/CLI/pve8to9.pm | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)


docs:

Fiona Ebner (2):
  user management: privileges: document new VM guest agent privileges
  user management: privileges: remove reference to dropped VM.Monitor
    privilege

 pveum.adoc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


Summary over all repositories:
  8 files changed, 322 insertions(+), 49 deletions(-)

-- 
Generated by git-murpp 0.5.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH access-control 1/9] add VM.GuestAgent privileges

[Fiona Ebner]

The privilege VM.Monitor has a very ambiguous name and is planned to
be dropped. Most of the API endpoints using it are for the QEMU guest
agent commands. Introduce dedicated, more fine-grained privileges for
those.

There is a basic VM.GuestAgent.Audit privilege for read-only,
informational commands.

There are dedicated privileges VM.GuestAgent.File{Read,Write} for
the file-{read,write} commands. There is a separate
VM.GuestAgent.FileSystemMgmt privilege for filesystem freeze, thaw
and trim.

The VM.GuestAgent.Unrestricted privilege is to allow all guest agent
operations, in particular also execution of arbitrary commands with
guest-exec.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/AccessControl.pm | 6 +++++-
 src/test/perm-test1.pl   | 8 ++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index cebb76f..3e12850 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1059,6 +1059,7 @@ my $privgroups = {
             'VM.Config.Options', # covers all other things
             'VM.Allocate',
             'VM.Clone',
+            'VM.GuestAgent.Unrestricted',
             'VM.Migrate',
             'VM.Monitor',
             'VM.Snapshot',
@@ -1069,10 +1070,13 @@ my $privgroups = {
             'VM.Config.Cloudinit',
             'VM.Console',
             'VM.Backup',
+            'VM.GuestAgent.FileRead',
+            'VM.GuestAgent.FileSystemMgmt',
+            'VM.GuestAgent.FileWrite',
             'VM.PowerMgmt',
         ],
         audit => [
-            'VM.Audit',
+            'VM.Audit', 'VM.GuestAgent.Audit',
         ],
     },
     Sys => {
diff --git a/src/test/perm-test1.pl b/src/test/perm-test1.pl
index 69b396e..bf01ac6 100755
--- a/src/test/perm-test1.pl
+++ b/src/test/perm-test1.pl
@@ -65,7 +65,9 @@ check_permission(
     '' # sorted, comma-separated expected privilege string
         . 'VM.Allocate,VM.Audit,VM.Backup,VM.Clone,VM.Config.CDROM,VM.Config.CPU,VM.Config.Cloudinit,'
         . 'VM.Config.Disk,VM.Config.HWType,VM.Config.Memory,VM.Config.Network,VM.Config.Options,'
-        . 'VM.Console,VM.Migrate,VM.Monitor,VM.PowerMgmt,VM.Snapshot,VM.Snapshot.Rollback',
+        . 'VM.Console,VM.GuestAgent.Audit,VM.GuestAgent.FileRead,VM.GuestAgent.FileSystemMgmt,'
+        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.Monitor,VM.PowerMgmt,'
+        . 'VM.Snapshot,VM.Snapshot.Rollback',
 );
 # Administrator -> Permissions.Modify!
 check_permission(
@@ -83,7 +85,9 @@ check_permission(
         . 'User.Modify,'
         . 'VM.Allocate,VM.Audit,VM.Backup,VM.Clone,VM.Config.CDROM,VM.Config.CPU,VM.Config.Cloudinit,'
         . 'VM.Config.Disk,VM.Config.HWType,VM.Config.Memory,VM.Config.Network,VM.Config.Options,'
-        . 'VM.Console,VM.Migrate,VM.Monitor,VM.PowerMgmt,VM.Snapshot,VM.Snapshot.Rollback',
+        . 'VM.Console,VM.GuestAgent.Audit,VM.GuestAgent.FileRead,VM.GuestAgent.FileSystemMgmt,'
+        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.Monitor,VM.PowerMgmt,'
+        . 'VM.Snapshot,VM.Snapshot.Rollback',
 );
 
 check_roles('max@pve', '/vms/200', 'storage_manager');
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH access-control 2/9] privileges: drop VM.Monitor

[Fiona Ebner]

The name VM.Monitor is ambiguous and makes it hard to guess what the
privilege is for. The privilege was used for two things:
1. QEMU guest agent operations, for which dedicated privileges were
   introduced, see commit "add VM.GuestAgent privileges".
2. Access to the QEMU HMP monitor, where only the 'info' and 'help'
   commands were usable without an additional Sys.Modify privilege.
   Access to the monitor will be guarded with Sys.Audit.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/AccessControl.pm | 1 -
 src/test/perm-test1.pl   | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 3e12850..e48e647 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1061,7 +1061,6 @@ my $privgroups = {
             'VM.Clone',
             'VM.GuestAgent.Unrestricted',
             'VM.Migrate',
-            'VM.Monitor',
             'VM.Snapshot',
             'VM.Snapshot.Rollback',
         ],
diff --git a/src/test/perm-test1.pl b/src/test/perm-test1.pl
index bf01ac6..50b46e2 100755
--- a/src/test/perm-test1.pl
+++ b/src/test/perm-test1.pl
@@ -66,8 +66,8 @@ check_permission(
         . 'VM.Allocate,VM.Audit,VM.Backup,VM.Clone,VM.Config.CDROM,VM.Config.CPU,VM.Config.Cloudinit,'
         . 'VM.Config.Disk,VM.Config.HWType,VM.Config.Memory,VM.Config.Network,VM.Config.Options,'
         . 'VM.Console,VM.GuestAgent.Audit,VM.GuestAgent.FileRead,VM.GuestAgent.FileSystemMgmt,'
-        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.Monitor,VM.PowerMgmt,'
-        . 'VM.Snapshot,VM.Snapshot.Rollback',
+        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.PowerMgmt,VM.Snapshot,'
+        . 'VM.Snapshot.Rollback',
 );
 # Administrator -> Permissions.Modify!
 check_permission(
@@ -86,8 +86,8 @@ check_permission(
         . 'VM.Allocate,VM.Audit,VM.Backup,VM.Clone,VM.Config.CDROM,VM.Config.CPU,VM.Config.Cloudinit,'
         . 'VM.Config.Disk,VM.Config.HWType,VM.Config.Memory,VM.Config.Network,VM.Config.Options,'
         . 'VM.Console,VM.GuestAgent.Audit,VM.GuestAgent.FileRead,VM.GuestAgent.FileSystemMgmt,'
-        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.Monitor,VM.PowerMgmt,'
-        . 'VM.Snapshot,VM.Snapshot.Rollback',
+        . 'VM.GuestAgent.FileWrite,VM.GuestAgent.Unrestricted,VM.Migrate,VM.PowerMgmt,VM.Snapshot,'
+        . 'VM.Snapshot.Rollback',
 );
 
 check_roles('max@pve', '/vms/200', 'storage_manager');
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 3/9] api: agent: use more specific guest agent privileges

[Fiona Ebner]

Make the 'perms' property in the command schema required.

Replace the misleadingly named 'VM.Monitor' privilege with more
precise guest-agent-specific privileges and, for power managment,
the pre-existing 'VM.PowerMgmt' privilege.

There is a basic VM.GuestAgent.Audit privilege for read-only,
informational commands.

There are dedicated privileges VM.GuestAgent.File{Read,Write} for
the file-{read,write} commands. There is a separate
VM.GuestAgent.FileSystemMgmt privilege for filesystem freeze, thaw
and trim. Querying the filesystem freeze status is also allowed with
VM.GuestAgent.Audit.

The VM.GuestAgent.Unrestricted privilege is allowed to do all
guest agent operations, in particular also execution of arbitrary
commands with guest-exec. Querying the result of the guest-exec
command via guest-exec-status is read-only, but it only makes sense in
combination with guest-exec, so it also requires
VM.GuestAgent.Unrestricted.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu/Agent.pm | 66 ++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 9 deletions(-)

diff --git a/src/PVE/API2/Qemu/Agent.pm b/src/PVE/API2/Qemu/Agent.pm
index 8a9b9264..05ef4f50 100644
--- a/src/PVE/API2/Qemu/Agent.pm
+++ b/src/PVE/API2/Qemu/Agent.pm
@@ -18,68 +18,99 @@ my $MAX_READ_SIZE = 16 * 1024 * 1024; # 16 MiB
 
 # list of commands
 # will generate one api endpoint per command
-# needs a 'method' property and optionally a 'perms' property (default VM.Monitor)
+# needs a 'method' property and a 'perms' property
 my $guest_agent_commands = {
     'ping' => {
         method => 'POST',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-time' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'info' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'fsfreeze-status' => {
         method => 'POST',
+        perms => {
+            check => [
+                'perm',
+                '/vms/{vmid}',
+                [
+                    'VM.GuestAgent.Audit',
+                    'VM.GuestAgent.FileSystemMgmt',
+                    'VM.GuestAgent.Unrestricted',
+                ],
+                any => 1,
+            ],
+        },
     },
     'fsfreeze-freeze' => {
         method => 'POST',
+        perms => 'VM.GuestAgent.FileSystemMgmt',
     },
     'fsfreeze-thaw' => {
         method => 'POST',
+        perms => 'VM.GuestAgent.FileSystemMgmt',
     },
     'fstrim' => {
         method => 'POST',
+        perms => 'VM.GuestAgent.FileSystemMgmt',
     },
     'network-get-interfaces' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-vcpus' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-fsinfo' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-memory-blocks' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-memory-block-info' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'suspend-hybrid' => {
         method => 'POST',
+        perms => 'VM.PowerMgmt',
     },
     'suspend-ram' => {
         method => 'POST',
+        perms => 'VM.PowerMgmt',
     },
     'suspend-disk' => {
         method => 'POST',
+        perms => 'VM.PowerMgmt',
     },
     'shutdown' => {
         method => 'POST',
+        perms => 'VM.PowerMgmt',
     },
     # added since qemu 2.9
     'get-host-name' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-osinfo' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-users' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
     'get-timezone' => {
         method => 'GET',
+        perms => 'VM.GuestAgent.Audit',
     },
 };
 
@@ -144,8 +175,11 @@ sub register_command {
     if (ref($perm) eq 'HASH') {
         $permission = $perm;
     } else {
-        $perm //= 'VM.Monitor';
-        $permission = { check => ['perm', '/vms/{vmid}', [$perm]] };
+        die "internal error: missing permission for $command" if !$perm;
+
+        $permission = {
+            check => ['perm', '/vms/{vmid}', [$perm, 'VM.GuestAgent.Unrestricted'], any => 1],
+        };
     }
 
     my $parameters = {
@@ -206,7 +240,7 @@ sub register_command {
 }
 
 # old {vmid}/agent POST endpoint, here for compatibility
-__PACKAGE__->register_command('', 'POST');
+__PACKAGE__->register_command('', 'POST', 'VM.GuestAgent.Unrestricted');
 
 for my $cmd (sort keys %$guest_agent_commands) {
     my $props = $guest_agent_commands->{$cmd};
@@ -221,7 +255,7 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => 'node',
     description => "Sets the password for the given user to the given password",
-    permissions => { check => ['perm', '/vms/{vmid}', ['VM.Monitor']] },
+    permissions => { check => ['perm', '/vms/{vmid}', ['VM.GuestAgent.Unrestricted']] },
     parameters => {
         additionalProperties => 0,
         properties => {
@@ -280,7 +314,7 @@ __PACKAGE__->register_method({
     proxyto => 'node',
     description =>
         "Executes the given command in the vm via the guest-agent and returns an object with the pid.",
-    permissions => { check => ['perm', '/vms/{vmid}', ['VM.Monitor']] },
+    permissions => { check => ['perm', '/vms/{vmid}', ['VM.GuestAgent.Unrestricted']] },
     parameters => {
         additionalProperties => 0,
         properties => {
@@ -335,7 +369,7 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => 'node',
     description => "Gets the status of the given pid started by the guest-agent",
-    permissions => { check => ['perm', '/vms/{vmid}', ['VM.Monitor']] },
+    permissions => { check => ['perm', '/vms/{vmid}', ['VM.GuestAgent.Unrestricted']] },
     parameters => {
         additionalProperties => 0,
         properties => {
@@ -411,7 +445,14 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => 'node',
     description => "Reads the given file via guest agent. Is limited to $MAX_READ_SIZE bytes.",
-    permissions => { check => ['perm', '/vms/{vmid}', ['VM.Monitor']] },
+    permissions => {
+        check => [
+            'perm',
+            '/vms/{vmid}',
+            ['VM.GuestAgent.FileRead', 'VM.GuestAgent.Unrestricted'],
+            any => 1,
+        ],
+    },
     parameters => {
         additionalProperties => 0,
         properties => {
@@ -490,7 +531,14 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => 'node',
     description => "Writes the given file via guest agent.",
-    permissions => { check => ['perm', '/vms/{vmid}', ['VM.Monitor']] },
+    permissions => {
+        check => [
+            'perm',
+            '/vms/{vmid}',
+            ['VM.GuestAgent.FileWrite', 'VM.GuestAgent.Unrestricted'],
+            any => 1,
+        ],
+    },
     parameters => {
         additionalProperties => 0,
         properties => {
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 4/9] api: monitor: improve permission handling

[Fiona Ebner]

While HMP (human monitor protocol) commands beside 'info' and 'help'
already require the 'Sys.Modify' permission on '/', certain commands
are better further restricted to be root-only.

Command list and descriptions taken from the output of 'help' and
shortened the descriptions where appropriate.

Many related commands for root-only commands were also made root-only,
for example 'drive_del', because 'drive_add' is or the NBD commands,
because 'nbd_server_start' is. That is being able to only do certain
parts of command groups that are not that useful by themselves. An
exception here is 'qom-get' which is just too useful to be root-only.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm          |  32 ++++--
 src/PVE/API2/Qemu/HMPPerms.pm | 207 ++++++++++++++++++++++++++++++++++
 src/PVE/API2/Qemu/Makefile    |   2 +-
 3 files changed, 231 insertions(+), 10 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/HMPPerms.pm

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index dbc08737..82cdc742 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -56,6 +56,7 @@ use PVE::Network;
 use PVE::Firewall;
 use PVE::API2::Firewall::VM;
 use PVE::API2::Qemu::Agent;
+use PVE::API2::Qemu::HMPPerms;
 use PVE::VZDump::Plugin;
 use PVE::DataCenterConfig;
 use PVE::ProcFSTools;
@@ -5582,8 +5583,7 @@ __PACKAGE__->register_method({
     proxyto => 'node',
     description => "Execute QEMU monitor commands.",
     permissions => {
-        description =>
-            "Sys.Modify is required for (sub)commands which are not read-only ('info *' and 'help')",
+        description => PVE::API2::Qemu::HMPPerms::generate_description(),
         check => ['perm', '/vms/{vmid}', ['VM.Monitor']],
     },
     parameters => {
@@ -5604,14 +5604,28 @@ __PACKAGE__->register_method({
         my $rpcenv = PVE::RPCEnvironment::get();
         my $authuser = $rpcenv->get_user();
 
-        my $is_ro = sub {
-            my $command = shift;
-            return $command =~ m/^\s*info(\s+|$)/
-                || $command =~ m/^\s*help\s*$/;
-        };
+        my $command = $param->{command} or die "no command specified\n";
+        die "unexpected command '$command'\n" if $command !~ m/^\s*(\S+)/;
+        my $command_name = $1;
+        my $required_perm = $PVE::API2::Qemu::HMPPerms::hmp_command_perms->{$command_name};
+        if (!$required_perm) {
+            my $msg =
+                "command '$command_name' non-existent or not assigned a required permission"
+                . " yet, limiting to root user\n";
+            die $msg if $authuser ne 'root@pam';
+            warn $msg;
+            $required_perm = 'root';
+        }
 
-        $rpcenv->check_full($authuser, "/", ['Sys.Modify'])
-            if !&$is_ro($param->{command});
+        if ($required_perm eq 'root') {
+            die "root-only command '$command_name'\n" if $authuser ne 'root@pam';
+        } elsif ($required_perm eq 'Sys.Modify') {
+            $rpcenv->check_full($authuser, "/", ['Sys.Modify']);
+        } elsif ($required_perm eq 'none') {
+            # nothing to check
+        } else {
+            die "unexpected required permission '$required_perm' for command '$command_name'\n";
+        }
 
         my $vmid = $param->{vmid};
 
diff --git a/src/PVE/API2/Qemu/HMPPerms.pm b/src/PVE/API2/Qemu/HMPPerms.pm
new file mode 100644
index 00000000..f6b32891
--- /dev/null
+++ b/src/PVE/API2/Qemu/HMPPerms.pm
@@ -0,0 +1,207 @@
+package PVE::API2::Qemu::HMPPerms;
+
+use strict;
+use warnings;
+
+# List of monitor commands and associated required permission. Listed explicitly to be future-proof.
+#
+# Currently permissions are:
+# 'root' - for root-only commands
+# 'Sys.Modify' - commands that can be issued with 'Sys.Modify' on '/'
+# 'none' - no permissions required (i.e. help and info)
+our $hmp_command_perms = {
+    help => 'none', # show the help
+    '?' => 'none', # short-form of 'help'
+    info => 'none', # show various information about the system state
+
+    # root-only: backup to arbitrary target file (although currently, not overwriting existing file)
+    backup => 'root', # create a VM backup (VMA format).
+    # root-only: requires the stream source in the backing chain currently, but better be safe
+    block_stream => 'root', # copy data from a backing file into a block device
+    # root-only: allows changing the path a removable medium points to
+    change => 'root', # change a removable medium
+    # root-only: among others, there is a 'file' driver
+    'chardev-add' => 'root', # add chardev
+    # root-only: among others, there is a 'file' driver (e.g. modify backend for serial device)
+    'chardev-change' => 'root', # change chardev
+    # root-only: because chardev-add is
+    'chardev-remove' => 'root', # remove chardev
+    # root-only: after migration SPICE client will attempt to connect to arbitrarily set host
+    client_migrate_info => 'root', # set migration information for remote display
+    # root-only: like '-device' on the commandline
+    device_add => 'root', # add device, like -device on the command line
+    # root-only: because device_add is
+    device_del => 'root', # remove device
+    # root-only: like '-drive' on the commandline
+    drive_add => 'root', # add drive to PCI storage controller
+    # root-only: backup to arbitrary target file
+    drive_backup => 'root', # initiates a point-in-time copy for a device.
+    # root-only: because drive_add is
+    drive_del => 'root', # remove host block device
+    # root-only: mirror to arbitrary target file
+    drive_mirror => 'root', # initiates live storage migration for a device.
+    # root-only: dump guest memory into arbitrary target file
+    'dump-guest-memory' => 'root', # dump guest memory into file 'filename'.
+    # root-only: dumps into arbitrary target file
+    dumpdtb => 'root', # dump the FDT in dtb format to 'filename'
+    # root-only: starts GDB server on the host
+    gdbserver => 'root', # start gdbserver on given device (default 'tcp::1234'), stop with 'none'
+    # root-only: host information leak
+    gpa2hpa => 'Sys.Modify', # print the host physical address corresponding to a guest physical address
+    # root-only: host information leak
+    gpa2hva => 'Sys.Modify', # print the host virtual address corresponding to a guest physical address
+    # root-only: redirect TCP or UDP connections from host to guest
+    hostfwd_add => 'root', # redirect TCP or UDP connections from host to guest (requires -net user)
+    # root-only: because hostfwd_add is
+    hostfwd_remove => 'root', # remove host-to-guest TCP or UDP redirection
+    # root-only: read from IO adress space (e.g. PCI devices)
+    i => 'Sys.Modify', # I/O port read
+    # root-only: log to arbitrary target file
+    logfile => 'root', # output logs to 'filename'
+    # root-only: no guarantee there are no KVM bugs that could afffect the real CPU
+    mce => 'root', # inject a MCE on the given CPU [and broadcast to other CPUs with -b option]
+    # root-only: allows to save to arbitrary file
+    memsave => 'root', # save to disk virtual memory dump starting at 'addr' of size 'size'
+    # root-only: could specify arbitrary host, also there is 'exec' and 'file' migrations
+    migrate => 'root', # migrate to URI (using -d to not wait for completion)
+    # root-only: allows setting arbitrary URI
+    migrate_incoming => 'root', # Continue an incoming migration from an -incoming defer
+    # root-only: allows setting arbitrary URI
+    migrate_recover => 'root', # Continue a paused incoming postcopy migration
+    # root-only: because nbd_server_start is
+    nbd_server_add => 'root', # export a block device via NBD
+    # root-only: because nbd_server_start is
+    nbd_server_remove => 'root', # remove an export previously exposed via NBD
+    # root-only: start NBD server on the host
+    nbd_server_start => 'root', # serve block devices on the given host and port
+    # root-only: because nbd_server_start is
+    nbd_server_stop => 'root', # stop serving block devices using the NBD protocol
+    # root-only: add host network device
+    netdev_add => 'root', # add host network device
+    # root-only: because netdev_add is
+    netdev_del => 'root', # remove host network device
+    # root-only: no guarantee there are no KVM bugs that could afffect the real CPU
+    nmi => 'root', # inject an NMI
+    # root-only: write to IO adress space (e.g. PCI devices)
+    o => 'root', # I/O port write
+    # root-only: create arbitrary objects, e.g. serial
+    object_add => 'root', # create QOM object
+    # root-only: because object_del is
+    object_del => 'root', # destroy QOM object
+    # root-only: inject error on PCIe devices
+    pcie_aer_inject_error => 'root', # inject pcie aer error
+    # root-only: save to arbitrary file
+    pmemsave => 'root', # save to disk physical memory dump starting at 'addr' of size 'size'
+    # root-only: modify arbitrary object properties
+    'qom-set' => 'root', # set QOM property.
+    # root-only: because savevm-start is
+    'savevm-end' => 'root', # Resume VM after snaphot.
+    # root-only: save VM state to arbitrary target file
+    'savevm-start' => 'root', # Prepare for snapshot and halt VM. Save VM state to statefile.
+    # root-only: dump to arbitrary target file
+    screendump => 'root', # save screen
+    # root-only: allows specifying arbitrary target file
+    snapshot_blkdev => 'root', # initiates a live snapshot of device
+    # root-only: allows inject-nmi
+    watchdog_action => 'root', # change watchdog action
+    # root-only: saves to arbitrary target file
+    wavcapture => 'root', # capture audio to a wave file
+    # root-only: not relevant for Proxmox VE
+    'xen-event-inject' => 'root', # inject event channel
+    # root-only: not relevant for Proxmox VE
+    'xen-event-list' => 'root', # list event channel state
+
+    announce_self => 'Sys.Modify', # Trigger GARP/RARP announcements
+    backup_cancel => 'Sys.Modify', # cancel the current VM backup
+    balloon => 'Sys.Modify', # request VM to change its memory allocation (in MB)
+    block_job_cancel => 'Sys.Modify', # stop an active background block operation
+    block_job_complete => 'Sys.Modify', # stop an active background block operation
+    block_job_pause => 'Sys.Modify', # pause an active background block operation
+    block_job_resume => 'Sys.Modify', # resume a paused background block operation
+    block_job_set_speed => 'Sys.Modify', # set maximum speed for a background block operation
+    block_resize => 'Sys.Modify', # resize a block image
+    block_set_io_throttle => 'Sys.Modify', # change I/O throttle limits for a block drive
+    boot_set => 'Sys.Modify', # define new values for the boot device list
+    calc_dirty_rate => 'Sys.Modify', # start a round of guest dirty rate measurement
+    cancel_vcpu_dirty_limit => 'Sys.Modify', # cancel dirty page rate limit
+    'chardev-send-break' => 'Sys.Modify', # send a break on chardev
+    closefd => 'Sys.Modify', # close a file descriptor previously passed via SCM rights
+    commit => 'Sys.Modify', # commit changes to the disk images or backing files
+    cont => 'Sys.Modify', # resume emulation
+    c => 'Sys.Modify', # short-form of 'cont'
+    cpu => 'Sys.Modify', # set the default CPU
+    delvm => 'Sys.Modify', # delete a VM snapshot from its tag
+    eject => 'Sys.Modify', # eject a removable medium (use -f to force it)
+    exit_preconfig => 'Sys.Modify', # exit the preconfig state
+    expire_password => 'Sys.Modify', # set spice/vnc password expire-time
+    getfd => 'Sys.Modify', # receive a file descriptor via SCM rights and assign it a name
+    gva2gpa => 'Sys.Modify', # print the guest physical address corresponding to a guest virtual address
+    loadvm => 'Sys.Modify', # restore a VM snapshot from its tag
+    log => 'Sys.Modify', # activate logging of the specified items
+    migrate_cancel => 'Sys.Modify', # cancel the current VM migration
+    migrate_continue => 'Sys.Modify', # Continue migration from the given paused state
+    migrate_pause => 'Sys.Modify', # Pause an ongoing migration (postcopy-only)
+    migrate_set_capability => 'Sys.Modify', # Enable/Disable the usage of a capability for migration
+    migrate_set_parameter => 'Sys.Modify', # Set the parameter for migration
+    migrate_start_postcopy => 'Sys.Modify', # Switch the migration to postcopy mode.
+    mouse_button => 'Sys.Modify', # change mouse button state (1=L, 2=M, 4=R)
+    mouse_move => 'Sys.Modify', # send mouse move events
+    mouse_set => 'Sys.Modify', # set which mouse device receives events
+    'one-insn-per-tb' => 'Sys.Modify', # run emulation with one guest instruction per translation block
+    print => 'Sys.Modify', # print expression value (use $reg for CPU register access)
+    p => 'Sys.Modify', # alias for 'print'
+    'qemu-io' => 'Sys.Modify', # run a qemu-io command on a block device
+    # decidedly not root-only even if qom-set ist, because it is just too useful
+    'qom-get' => 'Sys.Modify', # print QOM property
+    'qom-list' => 'Sys.Modify', # list QOM properties
+    quit => 'Sys.Modify', # quit the emulator
+    q => 'Sys.Modify', # short-form of 'quit'
+    replay_break => 'Sys.Modify', # set breakpoint at the specified instruction count
+    replay_delete_break => 'Sys.Modify', # remove replay breakpoint
+    replay_seek => 'Sys.Modify', # replay execution to the specified instruction count
+    ringbuf_read => 'Sys.Modify', # Read from a ring buffer character device
+    ringbuf_write => 'Sys.Modify', # Write to a ring buffer character device
+    savevm => 'Sys.Modify', # save a VM snapshot. If no tag is provided, a new snapshot is created
+    sendkey => 'Sys.Modify', # send keys to the VM
+    set_link => 'Sys.Modify', # change the link status of a network adapter
+    set_password => 'Sys.Modify', # set spice/vnc password
+    set_vcpu_dirty_limit => 'Sys.Modify', # set dirty page rate limit
+    snapshot_blkdev_internal => 'Sys.Modify', # take an internal snapshot of device.
+    snapshot_delete_blkdev_internal => 'Sys.Modify', # delete an internal snapshot of device.
+    stopcapture => 'Sys.Modify', # stop capture
+    stop => 'Sys.Modify', # stop emulation
+    s => 'Sys.Modify', # short-form of 'stop'
+    sum => 'Sys.Modify', # compute the checksum of a memory region
+    'sync-profile' => 'Sys.Modify', # enable, disable or reset synchronization profiling.
+    system_powerdown => 'Sys.Modify', # send system power down event
+    system_reset => 'Sys.Modify', # reset the system
+    system_wakeup => 'Sys.Modify', # wakeup guest from suspend
+    'trace-event' => 'Sys.Modify', # changes status of a specific trace event
+    x => 'Sys.Modify', # virtual memory dump starting at 'addr'
+    x_colo_lost_heartbeat => 'Sys.Modify', # Tell COLO that heartbeat is lost
+    xp => 'Sys.Modify', # physical memory dump starting at 'addr'
+};
+
+sub generate_description {
+    my $cmd_by_priv = {};
+    for my $cmd (sort keys $hmp_command_perms->%*) {
+        push $cmd_by_priv->{$hmp_command_perms->{$cmd}}->@*, $cmd;
+    }
+    my $none_cmds = delete($cmd_by_priv->{none})
+        or die "internal error - no commands for 'none' found";
+    my $root_only_cmds = delete($cmd_by_priv->{'root'})
+        or die "internal error no commands for 'root' found";
+
+    my $text = '';
+    $text .= "The following commands do not require any additional privilege: "
+        . join(', ', $none_cmds->@*) . "\n\n";
+
+    for my $priv (sort keys $cmd_by_priv->%*) {
+        $text .= "The following commands require '$priv': "
+            . join(', ', $cmd_by_priv->{$priv}->@*) . "\n\n";
+    }
+
+    $text .= "The following commands are root-only: " . join(', ', $root_only_cmds->@*) . "\n";
+}
+
+1;
diff --git a/src/PVE/API2/Qemu/Makefile b/src/PVE/API2/Qemu/Makefile
index e64aa278..7c539702 100644
--- a/src/PVE/API2/Qemu/Makefile
+++ b/src/PVE/API2/Qemu/Makefile
@@ -2,7 +2,7 @@ DESTDIR=
 PREFIX=/usr
 PERLDIR=$(PREFIX)/share/perl5
 
-SOURCES=Agent.pm CPU.pm Machine.pm
+SOURCES=Agent.pm CPU.pm HMPPerms.pm Machine.pm
 
 .PHONY: install
 install:
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 5/9] api: monitor: require Sys.Audit or Sys.Modify privilege

[Fiona Ebner]

The name VM.Monitor is ambiguous and made it hard to guess what the
privilege is for. The privilege was only used here and for guest agent
operations, where dedicated privileges were introduced. Since only the
'info' and 'help' commands were usable without an additional
Sys.Modify privilege, the VM.Monitor privilege seems superfluous and
was dropped in pve-access-control. As the information accessible via
'info' is very low-level and often related to the QEMU process on the
system, requiring Sys.Audit seems natural.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 82cdc742..09d4411b 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -5584,7 +5584,7 @@ __PACKAGE__->register_method({
     description => "Execute QEMU monitor commands.",
     permissions => {
         description => PVE::API2::Qemu::HMPPerms::generate_description(),
-        check => ['perm', '/vms/{vmid}', ['VM.Monitor']],
+        check => ['perm', '/vms/{vmid}', ['Sys.Audit', 'Sys.Modify'], any => 1],
     },
     parameters => {
         additionalProperties => 0,
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 6/9] pve8to9: remove outdated checks for user roles

[Fiona Ebner]

These checks were only relevant for the upgrade to PVE 8 and the
messages talking about a new PVE namespace or dropped
Permission.Modify privilege do not apply anymore.

Keep the infrastructure for checking custom roles intact for future
checks.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/CLI/pve8to9.pm | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/PVE/CLI/pve8to9.pm b/PVE/CLI/pve8to9.pm
index eb6d67e5..0c7cb97f 100644
--- a/PVE/CLI/pve8to9.pm
+++ b/PVE/CLI/pve8to9.pm
@@ -760,41 +760,21 @@ sub check_custom_pool_roles {
             for my $priv (split_list($privlist)) {
                 $roles->{$role}->{$priv} = 1;
             }
-        } elsif ($et eq 'acl') {
-            my ($propagate, $pathtxt, $uglist, $rolelist) = @data;
-            for my $role (split_list($rolelist)) {
-                if ($role eq 'PVESysAdmin' || $role eq 'PVEAdmin') {
-                    log_warn(
-                        "found ACL entry on '$pathtxt' for '$uglist' with role '$role' - this role"
-                            . " will no longer have 'Permissions.Modify' after the upgrade!");
-                }
-            }
         }
     }
 
-    log_info("Checking custom role IDs for clashes with new 'PVE' namespace..");
-    my ($custom_roles, $pve_namespace_clashes) = (0, 0);
+    log_info("Checking custom role IDs");
+    my ($custom_roles, $need_handling) = (0, 0);
     for my $role (sort keys %{$roles}) {
         next if PVE::AccessControl::role_is_special($role);
         $custom_roles++;
-
-        if ($role =~ /^PVE/i) {
-            log_warn("custom role '$role' clashes with 'PVE' namespace for built-in roles");
-            $pve_namespace_clashes++;
-        }
     }
-    if ($pve_namespace_clashes > 0) {
-        log_fail(
-            "$pve_namespace_clashes custom role(s) will clash with 'PVE' namespace for built-in roles enforced in Proxmox VE 8"
-        );
+    if ($need_handling > 0) {
+        log_fail("$need_handling custom role(s) need handling");
     } elsif ($custom_roles > 0) {
-        log_pass(
-            "none of the $custom_roles custom roles will clash with newly enforced 'PVE' namespace"
-        );
+        log_pass("none of the $custom_roles custom roles need handling");
     } else {
-        log_pass(
-            "no custom roles defined, so no clash with 'PVE' role ID namespace enforced in Proxmox VE 8"
-        );
+        log_pass("no custom roles defined");
     }
 }
 
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 7/9] pve8to9: check for to-be-dropped VM.Monitor privilege in custom roles

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/CLI/pve8to9.pm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/PVE/CLI/pve8to9.pm b/PVE/CLI/pve8to9.pm
index 0c7cb97f..b0e09c6d 100644
--- a/PVE/CLI/pve8to9.pm
+++ b/PVE/CLI/pve8to9.pm
@@ -768,9 +768,21 @@ sub check_custom_pool_roles {
     for my $role (sort keys %{$roles}) {
         next if PVE::AccessControl::role_is_special($role);
         $custom_roles++;
+
+        $need_handling++ if $roles->{$role}->{'VM.Monitor'};
     }
     if ($need_handling > 0) {
-        log_fail("$need_handling custom role(s) need handling");
+        log_notice(
+            "Proxmox VE 9 replaced the ambiguously named 'VM.Monitor' privilege with 'Sys.Audit'"
+                . " for QEMU HMP monitor access and new dedicated 'VM.GuestAgent.*' privileges"
+                . " for access to a VM's guest agent. The guest agent sub-privileges are 'Audit'"
+                . " for all informational commands, 'FileRead' and 'FileWrite' for file-read and"
+                . " file-write, 'FileSystemMgmt' for filesystem freeze, thaw and trim, and"
+                . " 'Unrestricted' for everything, including command execution. Operations that"
+                . " affect the VM runstate require 'VM.PowerMgmt' or 'VM.GuestAgent.Unrestricted'");
+        log_fail(
+            "$need_handling custom role(s) use the to-be-dropped 'VM.Monitor' privilege and need"
+                . " to be adapted after the upgrade");
     } elsif ($custom_roles > 0) {
         log_pass("none of the $custom_roles custom roles need handling");
     } else {
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs 8/9] user management: privileges: document new VM guest agent privileges

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 pveum.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/pveum.adoc b/pveum.adoc
index 8e0a5f7..d7b84d7 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -906,6 +906,12 @@ Virtual machine related privileges::
 * `VM.Config.Network`: add/modify/remove network devices
 * `VM.Config.Options`: modify any other VM configuration
 * `VM.Console`: console access to VM
+* `VM.GuestAgent.Audit`: issue informational QEMU guest agent commands
+* `VM.GuestAgent.FileRead`: read files from the guest via QEMU guest agent
+* `VM.GuestAgent.FileSystemMgmt`: freeze/thaw/trim file systems via QEMU guest
+  agent
+* `VM.GuestAgent.FileWrite`: write files in the guest via QEMU guest agent
+* `VM.GuestAgent.Unrestricted`: issue arbitrary QEMU guest agent commands
 * `VM.Migrate`: migrate VM to alternate server on cluster
 * `VM.Monitor`: access to VM monitor (kvm)
 * `VM.PowerMgmt`: power management (start, stop, reset, shutdown, ...)
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs 9/9] user management: privileges: remove reference to dropped VM.Monitor privilege

[Fiona Ebner]

The privilege was replaced by Sys.Audit for access to the QEMU HMP
monitor and by VM.GuestAgent.* privileges for access to the QEMU guest
agent.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 pveum.adoc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pveum.adoc b/pveum.adoc
index d7b84d7..40a2a62 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -913,7 +913,6 @@ Virtual machine related privileges::
 * `VM.GuestAgent.FileWrite`: write files in the guest via QEMU guest agent
 * `VM.GuestAgent.Unrestricted`: issue arbitrary QEMU guest agent commands
 * `VM.Migrate`: migrate VM to alternate server on cluster
-* `VM.Monitor`: access to VM monitor (kvm)
 * `VM.PowerMgmt`: power management (start, stop, reset, shutdown, ...)
 * `VM.Snapshot.Rollback`: rollback VM to one of its snapshots
 * `VM.Snapshot`: create/delete VM snapshots
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied-series: [PATCH-SERIES access-control/qemu-server/manager/docs] replace ambiguously named VM.Monitor privilege

[Thomas Lamprecht]

Am 17.07.25 um 15:36 schrieb Fiona Ebner:
> The privilege VM.Monitor has a very ambiguous name and is dropped.
> Most of the API endpoints using it are for the QEMU guest agent
> commands, the only other place is access to the QEMU HMP monitor.
> 
> 1. Introduce dedicated, more fine-grained privileges for the guest
> agent commands:
> 
> There is a basic VM.GuestAgent.Audit privilege for read-only,
> informational commands.
> 
> There are dedicated privileges VM.GuestAgent.File{Read,Write} for the
> file-{read,write} commands. There is a separate
> VM.GuestAgent.FileSystemMgmt privilege for filesystem freeze, thaw and
> trim.
> 
> The VM.GuestAgent.Unrestricted privilege is to allow all guest agent
> operations, in particular also execution of arbitrary commands with
> guest-exec.
> 
> 2. For access to the QEMU HMP monitor, only the 'info' and 'help'
> commands were usable without an additional Sys.Modify privilege. Since
> the information accessible via 'info' is very low-level and often
> related to the QEMU process on the system, requiring Sys.Audit seems
> natural.
> 
> These are breaking changes. A check in pve8to9 is provided.
> 
> 
> qemu-server patch "api: monitor: improve permission handling" and
> manager patch "pve8to9: remove outdated checks for user roles" can
> be applied independently from the rest of the series.
> 
> 
> New qemu-server depends on new access-control, new access-control
> breaks old qemu-server.

I did not record the breaks for now, no big reason and we can still
do a re-bump of access-control if you prefer doing so, it *would* be
slightly cleaner. 
> access-control:
> 
> Fiona Ebner (2):
>   add VM.GuestAgent privileges
>   privileges: drop VM.Monitor
> 
>  src/PVE/AccessControl.pm | 7 +++++--
>  src/test/perm-test1.pl   | 8 ++++++--
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> 
> qemu-server:
> 
> Fiona Ebner (3):
>   api: agent: use more specific guest agent privileges
>   api: monitor: improve permission handling
>   api: monitor: require Sys.Audit or Sys.Modify privilege
> 
>  src/PVE/API2/Qemu.pm          |  34 ++++--
>  src/PVE/API2/Qemu/Agent.pm    |  66 +++++++++--
>  src/PVE/API2/Qemu/HMPPerms.pm | 207 ++++++++++++++++++++++++++++++++++
>  src/PVE/API2/Qemu/Makefile    |   2 +-
>  4 files changed, 289 insertions(+), 20 deletions(-)
>  create mode 100644 src/PVE/API2/Qemu/HMPPerms.pm
> 
> 
> manager:
> 
> Fiona Ebner (2):
>   pve8to9: remove outdated checks for user roles
>   pve8to9: check for to-be-dropped VM.Monitor privilege in custom roles
> 
>  PVE/CLI/pve8to9.pm | 40 ++++++++++++++++------------------------
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> 
> docs:
> 
> Fiona Ebner (2):
>   user management: privileges: document new VM guest agent privileges
>   user management: privileges: remove reference to dropped VM.Monitor
>     privilege
> 
>  pveum.adoc | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 
> Summary over all repositories:
>   8 files changed, 322 insertions(+), 49 deletions(-)
> 


applied, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel