public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager v3] fix #6735: api: pci: allow mdevscan access via mapping permissions
@ 2026-09-03 12:11 Elias Huhsovitz
  2026-09-04  7:17 ` Dominik Csapak
  2026-09-04  9:49 ` superseded: " Elias Huhsovitz
  0 siblings, 2 replies; 3+ messages in thread
From: Elias Huhsovitz @ 2026-09-03 12:11 UTC (permalink / raw)
  To: pve-devel; +Cc: 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.

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

Changes v2->v3
--------------
* re-introduce else-statement in API handler
* move permissions checks into respective logical branches
  (previously sepate sesection before core code)
* update commit message

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

Changes
-------
 PVE/API2/Hardware/PCI.pm | 44 ++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Hardware/PCI.pm b/PVE/API2/Hardware/PCI.pm
index 36b9741b..bfcd1fc5 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,20 +228,37 @@ __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
+        my $id = $param->{'pci-id-or-mapping'};
+        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 $rpcenv = PVE::RPCEnvironment::get();
+        my $authuser = $rpcenv->get_user();
+
+        my $has_global_perms =
+            $rpcenv->check_any($authuser, '/', ['Sys.Audit', 'Sys.Modify'], 1);
+
+        if ($is_pci_id) {
+            raise_perm_exc("/, " . join("|", ['Sys.Audit', 'Sys.Modify']))
+                if !$has_global_perms;
+
+            return PVE::QemuServer::PCI::Mdev::get_mdev_types($id);
         } else {
-            my $mapping = $param->{'pci-id-or-mapping'};
+            if (!$has_global_perms) {
+                $rpcenv->check_any(
+                    $authuser,
+                    "/mapping/pci/$id",
+                    ['Mapping.Use', 'Mapping.Modify', 'Mapping.Audit'],
+                );
+            }
 
             my $types = {};
-            my $devices = PVE::Mapping::PCI::find_on_current_node($mapping);
+            my $devices = PVE::Mapping::PCI::find_on_current_node($id);
             for my $device ($devices->@*) {
-                my $id = $device->{path};
-                next if $id =~ m/;/; # mdev not supported for multifunction 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($id);
+                my $device_types = PVE::QemuServer::PCI::Mdev::get_mdev_types($dev_id);
 
                 for my $type_definition ($device_types->@*) {
                     my $type = $type_definition->{type};
@@ -247,6 +270,5 @@ __PACKAGE__->register_method({
 
             return [sort { $a->{type} cmp $b->{type} } values($types->%*)];
         }
-
     },
 });
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-04  9:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 12:11 [PATCH manager v3] fix #6735: api: pci: allow mdevscan access via mapping permissions Elias Huhsovitz
2026-09-04  7:17 ` Dominik Csapak
2026-09-04  9:49 ` superseded: " Elias Huhsovitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal