Re: [pve-devel] [PATCH v4 access-control 03/18] api: acl: only allow granting SU privilege if user already has it

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On June 2, 2022 9:24 am, Oguz Bektas wrote:
> also check for 'propagate' bit on the target path to verify if the
> user can grant SU privileges on there.
> 
> Co-authored-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  src/PVE/API2/ACL.pm | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
> index 857c672..f8d4914 100644
> --- a/src/PVE/API2/ACL.pm
> +++ b/src/PVE/API2/ACL.pm
> @@ -134,6 +134,10 @@ __PACKAGE__->register_method ({
>      code => sub {
>  	my ($param) = @_;
>  
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $authuser = $rpcenv->get_user();
> +	my $is_superuser = $rpcenv->check($authuser, $param->{path}, ['SuperUser'], 1);
> +
>  	if (!($param->{users} || $param->{groups} || $param->{tokens})) {
>  	    raise_param_exc({ map { $_ => "either 'users', 'groups' or 'tokens' is required." } qw(users groups tokens) });
>  	}
> @@ -160,6 +164,18 @@ __PACKAGE__->register_method ({
>  		    die "role '$role' does not exist\n"
>  			if !$cfg->{roles}->{$role};
>  
> +		    my $role_privs = $cfg->{roles}->{$role};
> +		    my $role_contains_superuser = grep { $_ eq 'SuperUser' } keys %$role_privs;
> +		    if ($role_contains_superuser) {
> +			die "only superusers can grant/remove this role!\n"
> +			    if !$is_superuser;
> +
> +			my $user_perms = $rpcenv->permissions($authuser, $param->{path});
> +			my $has_propagate = $user_perms->{SuperUser}; # check if user has SU with propagate bit on the target path
> +			die "cannot grant SU on '$param->{path}' without having 'propagate' bit!\n"
> +			    if !$has_propagate;

this only needs to be checked if the updated ACL also has the propagate 
bit set. also, it doesn't need to be checked for every role that is 
referenced by the ACL (it's not dependent on the role at all, but just 
on the combination of authid and path and propagate parameter).

> +		    }
> +
>  		    foreach my $group (split_list($param->{groups})) {
>  
>  			die "group '$group' does not exist\n"
> -- 
> 2.30.2
> 
>