Re: [pve-devel] [PATCH storage 1/1] fix #1710: add retrieve method for storage

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 29.04.21 15:46, Lorenz Stechauner wrote:
>> On 29.04.21 15:22 Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
>>> i am not quite sure if it is a good idea to have this feature
>>> unrestricted for everybody who can download a template
>>>
>>> it possibly gives access to an internal network to which
>>> the users does not have access otherwise...
>>>
>>> maybe we want to give the admin control over allow- and/or blocklists ?
>>
>> I do not want such lists, PITA to manage for everybody.
>>
>> Maybe we can just allow it only for users with Sys.Modify + Sys.Audit on / ?
>>
>> We could also enforce that it needs to be a hostname (no IP) and/or resolve
>> to something out of the priv. network ranges, at least if the aforementioned
>> privs are not set.
>>
>> Another idea would be enforcing the URL to match something like /\.(iso|img)$/ 
>> and being not to informative on errors to avoid allowing to see which hsot are
>> on/off line in a network. With that one could make this pretty safe I think.
>>
> Another idea would be to introduce two new permissions:
> Sys.RetrieveLocal - only local/private ip addresses allowed
> Sys.RetrieveGlobal - all other ip addresses allowed (means only non-private)

First, we try to avoid top-posting on the mailing list as it makes reading
replies unnecessarily hard without any real benefit. Interleaved style is
recommended.

IMO that is not a really good solution here due to:

* very specialized permission for a specific API call, makes the permission
  system crowded and confusing fast.

* Themself they have again a very broad reach, which would then lead to a
  situation where an admin needs to fully trust them again in the net as
  once enabled they have full access to everything.

I'd avoid adding new permissions for this, especially such overly specific
ones, rather:

* enforce the content/type or file name limits (must end with a valid type
  for the ISO or CT template type anyway, else it won't be returned by the
  content API)
* enforce existing Sys. poermissions if the hostname is a LAN address, public
  ones are not really problematic, anybody with allocate permissions can
  already upload them by indirection.
* suppress timeout vs. 404 distinction in error, possibly even adding a
  response delay so that they cannot be differed by timing either.

If that is deemed to complicated then the simple solution for now should be
to require Sys.Modify on / + Datastore.Allocate on the storage.