Re: [pve-devel] [PATCH cluster v8 4/4] DataCenterConfig: add tag rights control to the datacenter config

[Aaron Lauterer <a.lauterer@proxmox.com>]

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 <d.csapak@proxmox.com>
> ---
>   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 => "<tag>[;<tag>=...]",
> +	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.