[Freeipa-devel] [PATCH] 0032 Validate sudo RunAsUser/RunAsGroup arguments

[Rob Crittenden]

Alexander Bokovoy wrote:
> On Fri, 02 Dec 2011, Rob Crittenden wrote:
>> Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> FreeIPA SUDO rules use --usercat/--groupcat to specify that rule
>>> applies to all users or groups. Thus, sudorule-add-runasuser and
>>> sudorule-add-runasgroup accept specific groups and users and do not
>>> accept ALL reserved word.
>>>
>>> The patch validates user and group passed to these commands and
>>> reports appropriate errors when these are ALL or all arguments
>>> are empty.
>>>
>>> Ticket #1496
>>> https://fedorahosted.org/freeipa/ticket/1496
>>>
>>> One thing I'm not sure about is blocking all variants of the reserved
>>> word 'ALL'. The patch blocks them all due to the fact that most likely
>>> any of 'all', 'All', 'ALL', 'aLL', and so on are mistyping but there
>>> are might be valid cases when group or user is called 'all'.
>>
>> Then runasuser check reports runas-group as the attribute name, I
>> think it should still be runas-user even though it is a group of
>> users.
> Ok. Changed.
>
>
>> Other member commands don't consider it an error to provide any
>> actual members, it treats it as a no-op. We should probably be
>> consistent.
> Don't understand. Did you mean 'to not provide any actual members'?
>
> In case you did, attached patch removes remaining checks for
> runas_{user,group) to be False.
>
>
>> It would probably be better to return the value as passed in by the
>> user rather than user[0].value.
> The issue here is that names come to the callback already as DNs from
> LDAPAddMember's execute() method. Strictly speaking it is already
> different to what user has entered as we do expansion by default to
> add $SUFFIX and appropriate container.
>
> In the updated patch I tried to reduce DN to something reasonable by
> relying on known containers and only showing full DN for cases when
> these are not users/groups containers.
>

ACK on this patch.

Do we need to add similar to HBAC plugin and sudorule-add-user, 
add-command, etc?

rob