[pve-devel] [RFC manager 0/3] #fix 6094: relax permissions of informational api endpoints

[pve-devel] [RFC manager 0/3] #fix 6094: relax permissions of informational api endpoints

[Daniel Kral]

Small patch series which relaxes the permissions needed for purely
informational API endpoints (i.e. do not modify system state, but only
read it) from "Sys.Modify" to accept any of "Sys.Modify" or "Sys.Audit".
The same has been done for other, similar API endpoints as well [0].

The initial bug report only included the API endpoint for fetching a
list of available updates on a node (GET `/nodes/{node}/apt/update`),
but it made sense to include other endpoints where this applies as well,
at least as an RFC.

This does preserve backwards compatibility for the
odd case where a user/token has only the "Sys.Modify" but not the
"Sys.Audit" permission on the previous paths.

As for the other commit [0], I didn't bother to adapt the WebGUI as
Resource Mappings, Updates and ACME are not accessible there now. If
there is more user interest to e.g. read the list of available updates
without being able to refresh or upgrade them, it should be an easy fix,
but should be done together with PMG (and possibly PBS) as those
components are defined in proxmox-widget-toolkit.

[0] https://git.proxmox.com/?p=pve-manager.git;a=commit;h=65cbfe79e49638c10acce4354182dcaeae96e74d

Daniel Kral (3):
  fix #6094: api: apt: allow to get packages info with Sys.Audit
  fix #6094: api: node HW: allow to get usb info with Sys.Audit on /
  fix #6094: api: acme: allow to get plugin info with Sys.Audit on /

 PVE/API2/ACMEPlugin.pm   | 4 ++--
 PVE/API2/APT.pm          | 4 ++--
 PVE/API2/Hardware/USB.pm | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC manager 1/3] fix #6094: api: apt: allow to get packages info with Sys.Audit

[Daniel Kral]

Relax the required permissions to query the current list of available
package updates and the changelog of a package on a node. Both API
endpoints do not modify any system state except caching outputs.

Those were probably only "Sys.Modify" before, since both are used in
conjunction with upgrading packages in the WebGUI, which is only visible
with that permission, but some users might be interested in this
information outside of this and/or without being able to upgrade.

Keep Sys.Modify for backwards compatibility.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 PVE/API2/APT.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 47c50961..2a1de1e6 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -202,7 +202,7 @@ __PACKAGE__->register_method({
     method => 'GET',
     description => "List available updates.",
     permissions => {
-	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
+	check => ['perm', '/nodes/{node}', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
     },
     protected => 1,
     proxyto => 'node',
@@ -379,7 +379,7 @@ __PACKAGE__->register_method({
     method => 'GET',
     description => "Get package changelogs.",
     permissions => {
-	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
+	check => ['perm', '/nodes/{node}', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
     },
     proxyto => 'node',
     parameters => {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC manager 1/3] fix #6094: api: apt: allow to get packages info with Sys.Audit

[Fiona Ebner]

Am 17.02.25 um 13:19 schrieb Daniel Kral:
> Relax the required permissions to query the current list of available
> package updates and the changelog of a package on a node. Both API
> endpoints do not modify any system state except caching outputs.
> 
> Those were probably only "Sys.Modify" before, since both are used in
> conjunction with upgrading packages in the WebGUI, which is only visible
> with that permission, but some users might be interested in this
> information outside of this and/or without being able to upgrade.
> 
> Keep Sys.Modify for backwards compatibility.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>

The 'changelog' endpoint does trigger the download of the changelog of
course, but I fail to see how that could be abused for anything, so:

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
>  PVE/API2/APT.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
> index 47c50961..2a1de1e6 100644
> --- a/PVE/API2/APT.pm
> +++ b/PVE/API2/APT.pm
> @@ -202,7 +202,7 @@ __PACKAGE__->register_method({
>      method => 'GET',
>      description => "List available updates.",
>      permissions => {
> -	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> +	check => ['perm', '/nodes/{node}', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
>      },
>      protected => 1,
>      proxyto => 'node',
> @@ -379,7 +379,7 @@ __PACKAGE__->register_method({
>      method => 'GET',
>      description => "Get package changelogs.",
>      permissions => {
> -	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> +	check => ['perm', '/nodes/{node}', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
>      },
>      proxyto => 'node',
>      parameters => {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC manager 2/3] fix #6094: api: node HW: allow to get usb info with Sys.Audit on /

[Daniel Kral]

Relax the required permissions to query the current list of locally
available USB devices. The API endpoint does only read the USB topology
from the sysfs but does not modify any system state.

Keep Sys.Modify for backwards compatibility.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 PVE/API2/Hardware/USB.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Hardware/USB.pm b/PVE/API2/Hardware/USB.pm
index d7cb6607..049d1170 100644
--- a/PVE/API2/Hardware/USB.pm
+++ b/PVE/API2/Hardware/USB.pm
@@ -17,7 +17,7 @@ __PACKAGE__->register_method({
     protected => 1,
     proxyto => "node",
     permissions => {
-	check => ['perm', '/', ['Sys.Modify']],
+	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
     },
     parameters => {
 	additionalProperties => 0,
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC manager 2/3] fix #6094: api: node HW: allow to get usb info with Sys.Audit on /

[Fiona Ebner]

Am 17.02.25 um 13:19 schrieb Daniel Kral:
> Relax the required permissions to query the current list of locally
> available USB devices. The API endpoint does only read the USB topology
> from the sysfs but does not modify any system state.
> 
> Keep Sys.Modify for backwards compatibility.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
>  PVE/API2/Hardware/USB.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Hardware/USB.pm b/PVE/API2/Hardware/USB.pm
> index d7cb6607..049d1170 100644
> --- a/PVE/API2/Hardware/USB.pm
> +++ b/PVE/API2/Hardware/USB.pm
> @@ -17,7 +17,7 @@ __PACKAGE__->register_method({
>      protected => 1,
>      proxyto => "node",
>      permissions => {
> -	check => ['perm', '/', ['Sys.Modify']],
> +	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
>      },
>      parameters => {
>  	additionalProperties => 0,



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC manager 3/3] fix #6094: api: acme: allow to get plugin info with Sys.Audit on /

[Daniel Kral]

Relax the required permissions to query the list of ACME plugins and
their configurations. Both API endpoints do only read the ACME plugins
configuration file but does not modify any system state.

Keep Sys.Modify for backwards compatibility.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 PVE/API2/ACMEPlugin.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/ACMEPlugin.pm b/PVE/API2/ACMEPlugin.pm
index 30616625..ad5625fa 100644
--- a/PVE/API2/ACMEPlugin.pm
+++ b/PVE/API2/ACMEPlugin.pm
@@ -51,7 +51,7 @@ __PACKAGE__->register_method ({
     path => '',
     method => 'GET',
     permissions => {
-	check => ['perm', '/', [ 'Sys.Modify' ]],
+	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
     },
     description => "ACME plugin index.",
     protected => 1,
@@ -98,7 +98,7 @@ __PACKAGE__->register_method({
     method => 'GET',
     description => "Get ACME plugin configuration.",
     permissions => {
-	check => ['perm', '/', [ 'Sys.Modify' ]],
+	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
     },
     protected => 1,
     parameters => {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC manager 3/3] fix #6094: api: acme: allow to get plugin info with Sys.Audit on /

[Fiona Ebner]

Am 17.02.25 um 13:19 schrieb Daniel Kral:
> Relax the required permissions to query the list of ACME plugins and
> their configurations. Both API endpoints do only read the ACME plugins
> configuration file but does not modify any system state.

Can't there be secrets in there that should not leak? I.e. the plugin
config file is in /etc/pve/priv, so I'm not sure this should be relaxed.
Even if it doesn't modify the state, it might be too sensitive for
Sys.Audit.

> Keep Sys.Modify for backwards compatibility.
> 
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  PVE/API2/ACMEPlugin.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/ACMEPlugin.pm b/PVE/API2/ACMEPlugin.pm
> index 30616625..ad5625fa 100644
> --- a/PVE/API2/ACMEPlugin.pm
> +++ b/PVE/API2/ACMEPlugin.pm
> @@ -51,7 +51,7 @@ __PACKAGE__->register_method ({
>      path => '',
>      method => 'GET',
>      permissions => {
> -	check => ['perm', '/', [ 'Sys.Modify' ]],
> +	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
>      },
>      description => "ACME plugin index.",
>      protected => 1,
> @@ -98,7 +98,7 @@ __PACKAGE__->register_method({
>      method => 'GET',
>      description => "Get ACME plugin configuration.",
>      permissions => {
> -	check => ['perm', '/', [ 'Sys.Modify' ]],
> +	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ], any => 1],
>      },
>      protected => 1,
>      parameters => {


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC manager 3/3] fix #6094: api: acme: allow to get plugin info with Sys.Audit on /

[Fabian Grünbichler]

On May 6, 2025 3:52 pm, Fiona Ebner wrote:
> Am 17.02.25 um 13:19 schrieb Daniel Kral:
>> Relax the required permissions to query the list of ACME plugins and
>> their configurations. Both API endpoints do only read the ACME plugins
>> configuration file but does not modify any system state.
> 
> Can't there be secrets in there that should not leak? I.e. the plugin
> config file is in /etc/pve/priv, so I'm not sure this should be relaxed.
> Even if it doesn't modify the state, it might be too sensitive for
> Sys.Audit.

we could maybe do what we do in other index API calls, and restrict the
returned information in case Sys.Modify is missing? this would basically
entail stripping the 'data' option for DNS plugins (which might contain
credentials), everything else should not be sensitive AFAICT..

OTOH, I am not sure there's much benefit to it either ;)

the ACME API parts which are still root only are probably more
interesting cleanup targets!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC manager 3/3] fix #6094: api: acme: allow to get plugin info with Sys.Audit on /

[Daniel Kral]

On 5/7/25 11:15, Fabian Grünbichler wrote:
> On May 6, 2025 3:52 pm, Fiona Ebner wrote:
>> Am 17.02.25 um 13:19 schrieb Daniel Kral:
>>> Relax the required permissions to query the list of ACME plugins and
>>> their configurations. Both API endpoints do only read the ACME plugins
>>> configuration file but does not modify any system state.
>>
>> Can't there be secrets in there that should not leak? I.e. the plugin
>> config file is in /etc/pve/priv, so I'm not sure this should be relaxed.
>> Even if it doesn't modify the state, it might be too sensitive for
>> Sys.Audit.
> 
> we could maybe do what we do in other index API calls, and restrict the
> returned information in case Sys.Modify is missing? this would basically
> entail stripping the 'data' option for DNS plugins (which might contain
> credentials), everything else should not be sensitive AFAICT..
> 
> OTOH, I am not sure there's much benefit to it either ;)
> 
> the ACME API parts which are still root only are probably more
> interesting cleanup targets!

I agree, there's not much benefit to lower that here and would just 
complicate what is exposed to the API without a user requesting this. 
Let's drop this patch then :)


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel