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 65C088DB82 for ; Wed, 9 Nov 2022 15:42:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 46EB31DC87 for ; Wed, 9 Nov 2022 15:42:23 +0100 (CET) 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 for ; Wed, 9 Nov 2022 15:42:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 36F784203A for ; Wed, 9 Nov 2022 15:42:22 +0100 (CET) Message-ID: <1a3009d9-8232-bd8f-067f-466ff1b9d3ec@proxmox.com> Date: Wed, 9 Nov 2022 15:42:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Content-Language: en-US To: Proxmox VE development discussion , Dominik Csapak References: <20221018140226.598710-1-d.csapak@proxmox.com> <20221018140226.598710-5-d.csapak@proxmox.com> From: Aaron Lauterer In-Reply-To: <20221018140226.598710-5-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.044 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datacenterconfig.pm] Subject: Re: [pve-devel] [PATCH cluster v8 4/4] DataCenterConfig: add tag rights control to the datacenter config 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: Wed, 09 Nov 2022 14:42:23 -0000 On 10/18/22 16:02, Dominik Csapak wrote: > by adding a 'user-tag-privileges' and 'admin-tags' option. > The first sets the policy by which "normal" users (with > 'VM.Config.Options' on the respective guest) can create/delete tags > and the second is a list of tags only settable by 'admins' > ('Sys.Modify' on '/') > > also add a helper 'get_user_admin_tags' that returns two hashmaps that > determines the allowed user tags and admin tags that require elevated > permissions > > Signed-off-by: Dominik Csapak > --- > data/PVE/DataCenterConfig.pm | 93 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 93 insertions(+) > > diff --git a/data/PVE/DataCenterConfig.pm b/data/PVE/DataCenterConfig.pm > index bb29d26..e2140ff 100644 > --- a/data/PVE/DataCenterConfig.pm > +++ b/data/PVE/DataCenterConfig.pm > @@ -154,6 +154,26 @@ my $tag_style_format = { > }, > }; > > +my $user_tag_privs_format = { > + 'usable' => { > + optional => 1, > + type => 'string', > + enum => ['none', 'list', 'existing', 'free'], > + default => 'free', > + dscription => "Determines which tags a user without Sys.Modify on '/' can set and delete. ". s/dscription/description/ > + "'none' means no tags are settable.'list' allows tags from the given list. ". > + "'existing' means only already existing tags or from the given list. ". > + "And 'free' means users can assign any tags." > + }, > + 'list' => { > + optional => 1, > + type => 'string', > + pattern => "${PVE::JSONSchema::PVE_TAG_RE}(?:\;${PVE::JSONSchema::PVE_TAG_RE})*", > + typetext => "[;=...]", > + description => "List of tags users are allowd to set and delete (semicolon separated).", > + }, > +}; > + > my $datacenter_schema = { > type => "object", > additionalProperties => 0, > @@ -285,12 +305,60 @@ my $datacenter_schema = { > description => "Tag style options.", > format => $tag_style_format, > }, > + 'user-tag-privileges' => { > + optional => 1, > + type => 'string', > + description => "Privilege options for user settable tags", > + format => $user_tag_privs_format, > + }, > + 'admin-tags' => { > + optional => 1, > + type => 'string', > + description => "A list of tags only admins (Sys.Modify on '/') are allowed to set/delete", > + pattern => "(?:${PVE::JSONSchema::PVE_TAG_RE};)*${PVE::JSONSchema::PVE_TAG_RE}", > + }, > }, > }; > Is it possible to add a "typetext" for admin-tags as well? The `pvesh usage --verbose` output for the parameter looks rather confusing. > # make schema accessible from outside (for documentation) > sub get_datacenter_schema { return $datacenter_schema }; > > +# returns two hashmaps of tags, the first is the list of tags that can > +# be used by users with 'VM.Config.Options', and the second is a list > +# that needs 'Sys.Modify' on '/' > +# > +# If the first map is 'undef', it means there is generally no restriction > +# besides the tags defined in the second map. > +# > +# CAUTION: this function may include tags from *all* guest configs, > +# regardless of the current authuser > +sub get_user_admin_tags { > + my $user_tags = {}; > + my $admin_tags = {}; > + > + my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg'); > + if (my $user_tag_privs = $dc->{'user-tag-privileges'}) { > + my $usable = $user_tag_privs->{usable} // 'free'; > + if ($usable eq 'free') { > + $user_tags = undef; > + } elsif ($usable eq 'existing') { > + map { $user_tags->{$_} = 1 } ($user_tag_privs->{list} // [])->@*; > + my $props = PVE::Cluster::get_guest_config_properties(['tags']); > + for my $vmid (keys $props->%*) { > + map { $user_tags->{$_} = 1 } PVE::Tools::split_list($props->{$vmid}->{tags}); Am I right that a permission check to only add the tags from guests for which the user has the necessary permissions is computationally quite expensive? I see two potential use cases here. One, a large organization that splits up access for mgmt but would like to use the same tags throughout for consistency. So getting all tags is fine. The other would be one cluster with direct access for mgmt by different customers. Seeing all the tags configured in the cluster could leak some private information, depending on what tags have been assigned.