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

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

a similar patch for adding/editing roles is missing (else, this is 
trivially worked around by giving myself 'CustomRole' that  doesn't have 
SU, then editing that role to add SU to it).

On March 11, 2022 12:25 pm, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v1->v2:
> * added new after discussion with fabian about security implications of
> allowing SU privilege to be granted by users with Permissions.Modify
> 
>  src/PVE/API2/ACL.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
> index 857c672..d415334 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);

this does not include checking whether propagate is set or not, but this 
API path allows setting the propagate flag on an ACL (so if $authuser 
has SU on $param->{path} *without propagation*, it could now give away SU 
on $param->{path} *with propagation*, thus extending SU to arbitrary sub 
paths).

> +
>  	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,11 @@ __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;
> +		    die "only superusers can grant this role!\n"
> +			if !$is_superuser && $role_contains_superuser;
> +
>  		    foreach my $group (split_list($param->{groups})) {
>  
>  			die "group '$group' does not exist\n"
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>