[PATCH manager v2] fix #6735: api: pci: allow mdevscan access via mapping permissions.

[PATCH manager v2] fix #6735: api: pci: allow mdevscan access via mapping permissions.

[Elias Huhsovitz]

The mdevscan endpoint requires Sys.Audit or Sys.Modify on '/'. This
blocks non-admin users from listing mediated device types for a PCI
mapping, even when they hold Mapping.Use on that mapping.

Check the permission in the API handler based on the parameter type: For
a raw PCI ID, require Sys.Audit or Sys.Modify on '/'. For a mapping,
require Sys.Audit or Sys.Modify on '/', or fall back to requiring
Mapping.Use, Mapping.Modify or Mapping.Audit on the specific mapping
path.

Set the endpoint permission to 'user => all' so the handler performs the
type-dependent check. This keeps raw PCI IDs, which are not valid ACL
paths, out of the declarative ACL evaluation.

Reduce indentation by removing redundant else statement. (return
provides implicit branching)

Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
v1: https://lore.proxmox.com/pve-devel/20260824112610.148089-1-e.huhsovitz@proxmox.com/

Changes v1->v2
--------------
* Allow all users in the declarative API permissions.
* Implement ACL check in API handler by calling
  check_any and raise_perm_exc.
* Remove else statement in API handler, since return statement
  provides implicit branching
* update commit message

Side Note
---------
Since this pattern of manual permission checks appear quite frequently,
it might be of interest to provide generic way to deal with such cases
inside of the permissions block (Similar to overriding the
hasPermission method in Java Spring Boot)

For example like this:
permissions => {
        user => 'all',
        custom => my_perm_sub(),
},

my_perm_sub would simply return a boolean. This would be an optional
extension, without requiring major changes in ACL Logic.

Let me know if this is something you would consider worthwhile.

Changes
-------
 PVE/API2/Hardware/PCI.pm | 71 +++++++++++++++++++++++++++-------------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm
index 36b9741b..47c4b29b 100644
--- a/PVE/API2/Hardware/PCI.pm
+++ b/PVE/API2/Hardware/PCI.pm
@@ -3,10 +3,12 @@ package PVE::API2::Hardware::PCI;
 use strict;
 use warnings;
 
+use PVE::Exception qw(raise raise_perm_exc);
 use PVE::JSONSchema qw(get_standard_option);
 
 use PVE::QemuServer::PCI::Mdev;
 use PVE::RESTHandler;
+use PVE::RPCEnvironment;
 
 use base qw(PVE::RESTHandler);
 
@@ -180,7 +182,11 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => "node",
     permissions => {
-        check => ['perm', '/', ['Sys.Audit', 'Sys.Modify'], any => 1],
+        description =>
+            "For a PCI ID, requires 'Sys.Audit' or 'Sys.Modify' on '/'. For a mapping,"
+            . " requires the same global permissions, or 'Mapping.Use', 'Mapping.Modify'"
+            . ", or'Mapping.Audit' on '/mapping/pci/<id>'.",
+        user => 'all',
     },
     parameters => {
         additionalProperties => 0,
@@ -222,31 +228,50 @@ __PACKAGE__->register_method({
     code => sub {
         my ($param) = @_;
 
-        if ($param->{'pci-id-or-mapping'} =~
-            m/^(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]$/
-        ) {
-            return PVE::QemuServer::PCI::Mdev::get_mdev_types($param->{'pci-id-or-mapping'}); # PCI ID
-        } else {
-            my $mapping = $param->{'pci-id-or-mapping'};
-
-            my $types = {};
-            my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
-            for my $device ($devices->@*) {
-                my $id = $device->{path};
-                next if $id =~ m/;/; # mdev not supported for multifunction devices
-
-                my $device_types = PVE::QemuServer::PCI::Mdev::get_mdev_types($id);
-
-                for my $type_definition ($device_types->@*) {
-                    my $type = $type_definition->{type};
-                    if (!defined($types->{$type})) {
-                        $types->{$type} = $type_definition;
-                    }
+        my $id = $param->{'pci-id-or-mapping'};
+        my $rpcenv = PVE::RPCEnvironment::get();
+        my $authuser = $rpcenv->get_user();
+
+        my $is_pci_id =
+            $id =~ m/^(?:[0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\.[0-9a-fA-F]$/;
+        my $has_global_perms =
+            $rpcenv->check_any($authuser, '/', ['Sys.Audit', 'Sys.Modify'], 1);
+
+        if (!$has_global_perms) {
+            raise_perm_exc("/, " . join("|", ['Sys.Audit', 'Sys.Modify'])) if ($is_pci_id);
+            $rpcenv->check_any(
+                $authuser,
+                "/mapping/pci/$id",
+                ['Mapping.Use', 'Mapping.Modify', 'Mapping.Audit'],
+            );
+        }
+
+        return PVE::QemuServer::PCI::Mdev::get_mdev_types($id) if ($is_pci_id);
+
+        if (!$rpcenv->check_any($authuser, '/', ['Sys.Audit', 'Sys.Modify'], 1)) {
+            $rpcenv->check_any(
+                $authuser,
+                "/mapping/pci/$id",
+                ['Mapping.Use', 'Mapping.Modify', 'Mapping.Audit'],
+            );
+        }
+
+        my $types = {};
+        my $devices = PVE::Mapping::PCI::find_on_current_node($id);
+        for my $device ($devices->@*) {
+            my $dev_id = $device->{path};
+            next if $dev_id =~ m/;/; # mdev not supported for multifunction devices
+
+            my $device_types = PVE::QemuServer::PCI::Mdev::get_mdev_types($dev_id);
+
+            for my $type_definition ($device_types->@*) {
+                my $type = $type_definition->{type};
+                if (!defined($types->{$type})) {
+                    $types->{$type} = $type_definition;
                 }
             }
-
-            return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
         }
 
+        return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
     },
 });
-- 
2.47.3