[PATCH manager/access-control 0/3] fix #7513: fine-grained ACL for node certificates

[PATCH manager/access-control 0/3] fix #7513: fine-grained ACL for node certificates

[Jakob Klocker]

Managing node certificates currently requires broad /nodes/{node}
permissions, which violates the least-privilege principle. This
series adds /nodes/{node}/certificates as a dedicated ACL path and
moves the relevant permission checks onto it, so certificate
management can be done without granting node-wide rights.

The pve-access-control patch registers the new ACL path and must be
applied first. The pve-manager patches depend on it for the API
permission checks and the GUI path selector.


pve-access-control:

Jakob Klocker (1):
  fix #7513: acl: allow ACL access to /nodes/{node}/certificates path

  src/PVE/AccessControl.pm           |  1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

pve-manager:

Jakob Klocker (2):
  fix #7513: api: certs: change ACL on certificates path
  fix #7513: ui: perm path store: add per-node certificates ACL path

 PVE/API2/ACME.pm                   |  6 +++---
 PVE/API2/Certificates.pm           |  4 ++--
 www/manager6/data/PermPathStore.js | 11 +++++++++--
 3 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.47.3

[PATCH access-control 1/3] fix #7513: acl: allow ACL access to /nodes/{node}/certificates path

[Jakob Klocker]

Currently access to /nodes/{node}/certificates requires /nodes/{node}
rights, this doesn't follow least-privilege best practice. Instead
of introducing a specific privilege, allow ACL access
to /nodes/{node}/certificates.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7513
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/AccessControl.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 0d632b3..f005786 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1289,6 +1289,7 @@ sub check_path {
 	|/access/realm/[[:alnum:]\.\-\_]+
 	|/nodes
 	|/nodes/[[:alnum:]\.\-\_]+
+	|/nodes/[[:alnum:]\.\-\_]+/certificates
 	|/pool
 	|/pool/[A-Za-z0-9\.\-_]+(?:/[A-Za-z0-9\.\-_]+){0,2}
 	|/sdn
-- 
2.47.3

[PATCH manager 2/3] fix #7513: api: certs: change ACL on certificates path

[Jakob Klocker]

Endpoints under /nodes/{node}/certificates/* required
/nodes/{node} permission. To allow least-privileged for certification
changes, check the permissions on /nodes/{node}/certificates path
instead.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7513
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 PVE/API2/ACME.pm         | 6 +++---
 PVE/API2/Certificates.pm | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/ACME.pm b/PVE/API2/ACME.pm
index a948a72a..3f6679a0 100644
--- a/PVE/API2/ACME.pm
+++ b/PVE/API2/ACME.pm
@@ -160,7 +160,7 @@ __PACKAGE__->register_method({
     path => 'certificate',
     method => 'POST',
     permissions => {
-        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
+        check => ['perm', '/nodes/{node}/certificates', ['Sys.Modify']],
     },
     description => "Order a new certificate from ACME-compatible CA.",
     protected => 1,
@@ -232,7 +232,7 @@ __PACKAGE__->register_method({
     path => 'certificate',
     method => 'PUT',
     permissions => {
-        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
+        check => ['perm', '/nodes/{node}/certificates', ['Sys.Modify']],
     },
     description => "Renew existing certificate from CA.",
     protected => 1,
@@ -319,7 +319,7 @@ __PACKAGE__->register_method({
     path => 'certificate',
     method => 'DELETE',
     permissions => {
-        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
+        check => ['perm', '/nodes/{node}/certificates', ['Sys.Modify']],
     },
     description => "Revoke existing certificate from CA.",
     protected => 1,
diff --git a/PVE/API2/Certificates.pm b/PVE/API2/Certificates.pm
index de8762c5..8da1e93b 100644
--- a/PVE/API2/Certificates.pm
+++ b/PVE/API2/Certificates.pm
@@ -87,7 +87,7 @@ __PACKAGE__->register_method({
     path => 'custom',
     method => 'POST',
     permissions => {
-        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
+        check => ['perm', '/nodes/{node}/certificates', ['Sys.Modify']],
     },
     description => 'Upload or update custom certificate chain and key.',
     protected => 1,
@@ -165,7 +165,7 @@ __PACKAGE__->register_method({
     path => 'custom',
     method => 'DELETE',
     permissions => {
-        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
+        check => ['perm', '/nodes/{node}/certificates', ['Sys.Modify']],
     },
     description => 'DELETE custom certificate chain and key.',
     protected => 1,
-- 
2.47.3

[PATCH manager 3/3] fix #7513: ui: perm path store: add per-node certificates ACL path

[Jakob Klocker]

Expose /nodes/{node}/certificates as a selectable path in the
permission path store, so the more fine-grained certificate ACL
can be assigned through the GUI.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7513
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 www/manager6/data/PermPathStore.js | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/www/manager6/data/PermPathStore.js b/www/manager6/data/PermPathStore.js
index 7210e0cd..bc2112ad 100644
--- a/www/manager6/data/PermPathStore.js
+++ b/www/manager6/data/PermPathStore.js
@@ -34,9 +34,16 @@ Ext.define('PVE.data.PermPathStore', {
         PVE.data.ResourceStore.each(function (record) {
             let path;
             switch (record.get('type')) {
-                case 'node':
-                    path = '/nodes/' + record.get('text');
+                case 'node': {
+                    let nodename = record.get('text');
+                    path = '/nodes/' + nodename;
+                    let certpath = '/nodes/' + nodename + '/certificates';
+                    if (!donePaths[certpath]) {
+                        me.add({ value: certpath });
+                        donePaths[certpath] = 1;
+                    }
                     break;
+                }
                 case 'network':
                     path = '/sdn/' + record.get('network-type') + 's/' + record.get('network');
                     break;
-- 
2.47.3

Re: [PATCH manager/access-control 0/3] fix #7513: fine-grained ACL for node certificates

[Michael Köppl]

Thanks for these patches! Gave this a quick spin by creating a new user
and assigning the now more specific permissions.

- Checked that assigning the CertManager Role to the new user for path
  /nodes/<node>/certificates was possible through both the UI and pveum
- Checked that the user then was able to view, upload, and remove
  certificates for the nodes for which the path was added (and that it
  was not possible for other nodes)
- Checked that assigning the CertManager role for path /nodes/<node>
  still allowed the user to manipulate certificates
- Also checked that the user could not access any other functionality
  even if assigned a role with more privileges

The patches seemed to work as expected for me. I also had a look at the
code and the changes look simple enough and good to me. I quickly
checked if any other parts of PVE might be affected by these changes,
but I don't think that's the case.

Consider this:
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Tested-by: Michael Köppl <m.koeppl@proxmox.com>

On Thu May 28, 2026 at 1:35 PM CEST, Jakob Klocker wrote:
> Managing node certificates currently requires broad /nodes/{node}
> permissions, which violates the least-privilege principle. This
> series adds /nodes/{node}/certificates as a dedicated ACL path and
> moves the relevant permission checks onto it, so certificate
> management can be done without granting node-wide rights.

[snip]