From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; Thu, 29 Apr 2021 16:07:18 +0200 (CEST) Message-ID: 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 , Proxmox VE development discussion , Dominik Csapak References: <901234855.1836.1619703982799@webmail.proxmox.com> From: Thomas Lamprecht 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 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.