From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id AC91E1FF0E1 for ; Thu, 27 Aug 2026 13:25:38 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E348721450; Thu, 27 Aug 2026 13:25:37 +0200 (CEST) From: Elias Huhsovitz To: pve-devel@lists.proxmox.com Subject: [PATCH manager v2] fix #6735: api: pci: allow mdevscan access via mapping permissions. Date: Thu, 27 Aug 2026 13:25:25 +0200 Message-ID: <20260827112525.154445-1-e.huhsovitz@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787829921302 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.722 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 6HQECFRTOFWYUAUISY3RN2Z6DUFSWP2L X-Message-ID-Hash: 6HQECFRTOFWYUAUISY3RN2Z6DUFSWP2L X-MailFrom: e.huhsovitz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Elias Huhsovitz X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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/'.", + 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