all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege
@ 2025-07-18 15:03 Fiona Ebner
  2025-07-30 13:11 ` Fabian Grünbichler
  0 siblings, 1 reply; 4+ messages in thread
From: Fiona Ebner @ 2025-07-18 15:03 UTC (permalink / raw)
  To: pve-devel

The check_volume_access() method is for checking read access to a
volume. Users should be able to list the images, e.g. to check backup
health via monitoring like reported in #5492 comment 3, with just an
audit privilege.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Storage/Content.pm | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index 1fe7303..c1f9a1f 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -154,12 +154,6 @@ __PACKAGE__->register_method({
 
         my $res = [];
         foreach my $item (@$vollist) {
-            eval {
-                PVE::Storage::check_volume_access(
-                    $rpcenv, $authuser, $cfg, undef, $item->{volid},
-                );
-            };
-            next if $@;
             $item->{vmid} = int($item->{vmid}) if defined($item->{vmid});
             $item->{size} = int($item->{size}) if defined($item->{size});
             $item->{used} = int($item->{used}) if defined($item->{used});
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege
  2025-07-18 15:03 [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege Fiona Ebner
@ 2025-07-30 13:11 ` Fabian Grünbichler
  2025-07-30 14:20   ` Fiona Ebner
  0 siblings, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2025-07-30 13:11 UTC (permalink / raw)
  To: Proxmox VE development discussion

On July 18, 2025 5:03 pm, Fiona Ebner wrote:
> The check_volume_access() method is for checking read access to a
> volume. Users should be able to list the images, e.g. to check backup
> health via monitoring like reported in #5492 comment 3, with just an
> audit privilege.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/API2/Storage/Content.pm | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
> index 1fe7303..c1f9a1f 100644
> --- a/src/PVE/API2/Storage/Content.pm
> +++ b/src/PVE/API2/Storage/Content.pm
> @@ -154,12 +154,6 @@ __PACKAGE__->register_method({
>  
>          my $res = [];
>          foreach my $item (@$vollist) {
> -            eval {
> -                PVE::Storage::check_volume_access(
> -                    $rpcenv, $authuser, $cfg, undef, $item->{volid},
> -                );
> -            };
> -            next if $@;

the data here also contains things like the notes content for that
volume, which might be sensitive..

should we maybe limit the returned information if there is no volume
access? e.g., just return volid, format, type, owner, and size
information?

>              $item->{vmid} = int($item->{vmid}) if defined($item->{vmid});
>              $item->{size} = int($item->{size}) if defined($item->{size});
>              $item->{used} = int($item->{used}) if defined($item->{used});
> -- 
> 2.47.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege
  2025-07-30 13:11 ` Fabian Grünbichler
@ 2025-07-30 14:20   ` Fiona Ebner
  2025-07-30 14:27     ` Fabian Grünbichler
  0 siblings, 1 reply; 4+ messages in thread
From: Fiona Ebner @ 2025-07-30 14:20 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 30.07.25 um 3:11 PM schrieb Fabian Grünbichler:
> On July 18, 2025 5:03 pm, Fiona Ebner wrote:
>> The check_volume_access() method is for checking read access to a
>> volume. Users should be able to list the images, e.g. to check backup
>> health via monitoring like reported in #5492 comment 3, with just an
>> audit privilege.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>  src/PVE/API2/Storage/Content.pm | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
>> index 1fe7303..c1f9a1f 100644
>> --- a/src/PVE/API2/Storage/Content.pm
>> +++ b/src/PVE/API2/Storage/Content.pm
>> @@ -154,12 +154,6 @@ __PACKAGE__->register_method({
>>  
>>          my $res = [];
>>          foreach my $item (@$vollist) {
>> -            eval {
>> -                PVE::Storage::check_volume_access(
>> -                    $rpcenv, $authuser, $cfg, undef, $item->{volid},
>> -                );
>> -            };
>> -            next if $@;
> 
> the data here also contains things like the notes content for that
> volume, which might be sensitive..
> 
> should we maybe limit the returned information if there is no volume
> access? e.g., just return volid, format, type, owner, and size
> information?

Good catch! But should information like 'verification', 'protected',
'encrypted' really be limited as well (maybe mapping a fingerprint to
just 1 and updating the docs)? The feature request is precisely for
backup monitoring, where those would be important. 'parent' and 'ctime'
seem also useful for auditing a storage.


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege
  2025-07-30 14:20   ` Fiona Ebner
@ 2025-07-30 14:27     ` Fabian Grünbichler
  0 siblings, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2025-07-30 14:27 UTC (permalink / raw)
  To: Fiona Ebner, Proxmox VE development discussion

On July 30, 2025 4:20 pm, Fiona Ebner wrote:
> Am 30.07.25 um 3:11 PM schrieb Fabian Grünbichler:
>> On July 18, 2025 5:03 pm, Fiona Ebner wrote:
>>> The check_volume_access() method is for checking read access to a
>>> volume. Users should be able to list the images, e.g. to check backup
>>> health via monitoring like reported in #5492 comment 3, with just an
>>> audit privilege.
>>>
>>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>>> ---
>>>  src/PVE/API2/Storage/Content.pm | 6 ------
>>>  1 file changed, 6 deletions(-)
>>>
>>> diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
>>> index 1fe7303..c1f9a1f 100644
>>> --- a/src/PVE/API2/Storage/Content.pm
>>> +++ b/src/PVE/API2/Storage/Content.pm
>>> @@ -154,12 +154,6 @@ __PACKAGE__->register_method({
>>>  
>>>          my $res = [];
>>>          foreach my $item (@$vollist) {
>>> -            eval {
>>> -                PVE::Storage::check_volume_access(
>>> -                    $rpcenv, $authuser, $cfg, undef, $item->{volid},
>>> -                );
>>> -            };
>>> -            next if $@;
>> 
>> the data here also contains things like the notes content for that
>> volume, which might be sensitive..
>> 
>> should we maybe limit the returned information if there is no volume
>> access? e.g., just return volid, format, type, owner, and size
>> information?
> 
> Good catch! But should information like 'verification', 'protected',
> 'encrypted' really be limited as well (maybe mapping a fingerprint to
> just 1 and updating the docs)? The feature request is precisely for
> backup monitoring, where those would be important. 'parent' and 'ctime'
> seem also useful for auditing a storage.

yes, probably we should take a look at all the returned members and make
a list of allowed-for-audit-purposes and drop the rest.


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-30 14:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-18 15:03 [pve-devel] [PATCH storage] close #5492: api: content: allow listing volumes with Datastore.Audit privilege Fiona Ebner
2025-07-30 13:11 ` Fabian Grünbichler
2025-07-30 14:20   ` Fiona Ebner
2025-07-30 14:27     ` Fabian Grünbichler

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal