From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 582F0783BB
 for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 16:07:20 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 4E4BE1E30C
 for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 16:07:20 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 5B12C1E2FE
 for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 16:07:19 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AC82046464
 for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 16:07:18 +0200 (CEST)
Message-ID: <ba4be725-724b-cb4d-09f5-86ffd7b8802f@proxmox.com>
Date: Thu, 29 Apr 2021 16:07:17 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101
 Thunderbird/89.0
Content-Language: en-US
To: Lorenz Stechauner <l.stechauner@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Dominik Csapak <d.csapak@proxmox.com>
References: <901234855.1836.1619703982799@webmail.proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <901234855.1836.1619703982799@webmail.proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.005 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -0.001 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH storage 1/1] fix #1710: add retrieve method
 for storage
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 29 Apr 2021 14:07:20 -0000

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.