From: Elias Huhsovitz <e.huhsovitz@proxmox.com>
To: d.csapak@proxmox.com
Cc: e.huhsovitz@proxmox.com, pve-devel@lists.proxmox.com
Subject: [PATCH access-control] rpc: do not check malformed acl path
Date: Wed, 26 Aug 2026 12:37:52 +0200 [thread overview]
Message-ID: <20260826103752.115037-1-e.huhsovitz@proxmox.com> (raw)
In-Reply-To: <8eff67e8-ac88-4b7d-a90d-de32644d1d95@proxmox.com>
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
Thank you for the feedback. I looked into this a bit more and
provided an experimental patch as a reference.
The Problem
-----------
The core problem is indeed the colon ':', which makes sense.
I did some further testing and the same error occurs when sending a
request to the enpoint:
GET /cluster/mapping/pci/{id}
It outputs the errors:
pveproxy[42711]: Failed to normalize '/mapping/pci/0000:01:00.0'
pveproxy[42711]: internal error: ACL check called for undefined ACL path!
pveproxy[42711]: Use of uninitialized value $path in concatenation (.) or string at /usr/share/perl5/PVE/RPCEnvironment.pm line 325.
Mismatch
---------
I think explicitely checking the permissions in the api handler itself
will work, but to my understanding there is an underlying mismatch:
The API schema explicitly allows colons (for raw PCI IDs), but the
ACL path normalizer explicitly rejects them.
Proposed solution
-----------------
IMHO providing explicit permissions on a raw PCI ID like 0000:01:00.0
instead of a mapping should not happen (i think) and should not be
checked.
What is your opinion on simply not checking invalid ACL
paths?
This would cause a more graceful failure.
Caveats
-------
(I fear that such a check might causes problems in existing systems,
that rely on "invalid" ACL paths. But i would love to hear your input
on this)
Side note
---------
What aboud allowing encoded PCI IDs instead? e.g.,
GET /nodes/lisa/hardware/pci/0000%3A01%3A00.0
Summary of changes
------------------
src/PVE/RPCEnvironment.pm | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index 7591aa9..61ef28b 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -487,7 +487,11 @@ sub exec_api2_perm_check {
}
my $path = PVE::Tools::template_replace($tmplpath, $param);
my $normpath = PVE::AccessControl::normalize_path($path);
- warn "Failed to normalize '$path'\n" if !defined($normpath) && defined($path);
+
+ if (!defined($normpath)) {
+ warn "Failed to normalize ACL path '$path'\n" if defined($path);
+ return 0;
+ }
return $self->check_full($username, $normpath, $privs, $any, $noerr);
} elsif ($test eq 'userid-group') {
--
2.47.3
next prev parent reply other threads:[~2026-08-26 10:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-24 11:26 [PATCH manager] fix #6735: api: pci: extend mdevscan persmissions for mappings Elias Huhsovitz
2026-08-26 7:12 ` Dominik Csapak
2026-08-26 10:37 ` Elias Huhsovitz [this message]
2026-08-26 12:37 ` [PATCH access-control] rpc: do not check malformed acl path Fiona Ebner
2026-08-26 12:53 ` Elias Huhsovitz
2026-08-26 13:03 ` Fiona Ebner
2026-08-26 13:04 ` Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260826103752.115037-1-e.huhsovitz@proxmox.com \
--to=e.huhsovitz@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.