[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 at proxmox.com
Wed Jul 27 11:06:01 CEST 2022


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 at proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas at 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
> 
> 





More information about the pve-devel mailing list