[pve-devel] [PATCH access-control] parse_user_cfg: correctly parse group names in ACLs

[Thomas Lamprecht]

On 10/3/19 10:33 AM, Fabian Grünbichler wrote:
> usernames are allowed to start with '@', so adding a user '@test at pve'
> and adding it to an ACL should work, instead of ignoring that part of
> the ACL entry.
> 
> note: there is no potential for user and group to be confused, since a
> username must end with '@REALM', and a group reference in an ACL can
> only contain one '@' (as first character).
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> 
> Notes:
>     alternatively, we could also disallow usernames starting with '@', but those
>     are currently working as long as they just have ACLs via groups, and not
>     directly..
> 
>  PVE/AccessControl.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
> index 44f4a01..6ea0b85 100644
> --- a/PVE/AccessControl.pm
> +++ b/PVE/AccessControl.pm
> @@ -974,8 +974,9 @@ sub parse_user_config {
>  		    }
>  
>  		    foreach my $ug (split_list($uglist)) {
> -			if ($ug =~ m/^@(\S+)$/) {
> -			    my $group = $1;
> +			my ($group) = $ug =~ m/^@(\S+)$/;
> +
> +			if ($group && verify_groupname($group, 1)) {
>  			    if ($cfg->{groups}->{$group}) { # group exists
>  				$cfg->{acl}->{$path}->{groups}->{$group}->{$role} = $propagate;
>  			    } else {
> 

applied, thanks, added: 
> So use verify_groupname to additionally enforce that the group name we
> extracted does not include an additional @, as then it cannot be a
> group.

to the commit message, though, as I was slightly confused