[Freeipa-devel] Sudorules user validation help

[Drew Erny]

Ok, so should I write a regex that matches that broader pattern, and 
only allow sudorules users to be added that follow those broader 
restrictions?

On 05/28/2015 02:09 PM, Alexander Bokovoy wrote:
> On Thu, 28 May 2015, Martin Kosek wrote:
>> On 05/28/2015 04:27 PM, Drew Erny wrote:
>>> In the ticket, however, it's stated that if the user wants to use any
>>> combination of weird characters, they should be able to. Would it be 
>>> better to
>>> just define a function like
>>>
>>> def validate_username(username, ignore_pattern=False):
>>>
>>> and have it ignore all username validation?
>>
>> Tough question. FreeIPA in general tries to sanitize user input and 
>> does not
>> allow everything user wants and try to advise the best practices, as 
>> we see it.
>>
>> In your case, if we allow only the 2 mentioned user login formats, it 
>> should
>> work for AD use case just fine and add some level of sanity check of 
>> the user
>> name. BTW, given we run an LDAP search later on this user name, isn't 
>> there a
>> possibility of "LDAP injection" if we choose to allow all characters, 
>> including
>> "(" and ")"? :-)
>>
>> In any case, if we choose to ignore the pattern, we do not need the 
>> extra
>> validator function at all. We would just skip validation in the pre 
>> callback if
>> a user is being added.
> I still think we should run the validator exactly for the reasons
> outlined above. There are few limiting factors for Active Directory and
> Linux environments -- while user and group objects names are specified
> in 'cn' attirbute in Active Directory, in POSIX environment we get the
> real name from sAMAccountName attribute for users:
>
> * Certain characters in the Relative Distinguished Names of objects must
>  be escaped using the backslash, "\", escape character. The characters
>  that must be escaped are:
>    , \ # + < > ; " =
>  In addition, any leading or trailing spaces in the RDN must be escaped.
>
> * The following characters are not allowed in sAMAccountName values:
>    " [ ] : ; | = + * ? < > / \ ,
>
> * In Windows Server 2003 domains and above, if you do not assign a
> value for sAMAccountName, the system will create a semi-random value for
> your. This value will be similar to:
>    $KJK000-H4GJL6AQOV1I
>
> * The schema allows 256 characters in sAMAccountName values. However,
> the system limits sAMAccountName to 20 characters for user and group 
> objects and
> 16 characters for computer objects.
> As you can see, group names may have "(" and ")", also "!" and few more
> characters which you have to escape properly before making them part of
> the LDAP filter.
>
> Additionally, we actually have to allow UTF-8 characters, not just
> ASCII as syntax for DirectoryString (OID 1.3.6.1.4.1.1466.115.121.1.15)
> requires that.
>
>>
>>>
>>> On 05/28/2015 09:40 AM, Drew Erny wrote:
>>>> OK, I see now what you mean by that. That is a simpler solution. 
>>>> I'll do it
>>>> that way.
>>>>
>>>> On 05/28/2015 04:44 AM, Martin Kosek wrote:
>>>>> On 05/27/2015 08:41 PM, Drew Erny wrote:
>>>>>> Hey, Freeipa-devel,
>>>>>>
>>>>>> I'm working on ticket #3226 
>>>>>> (https://fedorahosted.org/freeipa/ticket/3226)
>>>>>>
>>>>>> I've identified the problem. The sudorules add user command adds 
>>>>>> the user
>>>>>> validations at the end of it's pre-callback using 
>>>>>> add_external_pre_callback.
>>>>>> However, the "user" plugin pattern-matches a string for the "uid" 
>>>>>> param,
>>>>>> because it only allows certain characters.
>>>>>>
>>>>>> I've been picking through the codebase and I think I have enough 
>>>>>> understanding
>>>>>> to ask this: What if we changed the user "uid" validation to a 
>>>>>> standalone
>>>>>> "rule" function (you can do that, right? pass in a function as a 
>>>>>> validation
>>>>>> rule?) that would normally just assert that the pattern matches, 
>>>>>> but could
>>>>>> have
>>>>>> that pattern matching validation overridden in some cases. I 
>>>>>> think that's the
>>>>>> easiest, cleanest way to change user so that sudorules and other 
>>>>>> plugins can
>>>>>> ignore this validation if necessary (I'm trying to figure out 
>>>>>> exactly how to
>>>>>> implement this without changing any APIs).
>>>>>>
>>>>>> Am I understanding the plugin params API correctly, and can I do 
>>>>>> this? Is this
>>>>>> the best way to do this?
>>>>>>
>>>>>> The only other solution I see is to write sudorules-specific code 
>>>>>> in some
>>>>>> plugin-related (either user.py or baseldap.py module, which would 
>>>>>> create
>>>>>> unwanted coupling.
>>>>>>
>>>>>> Most specifically, this would be a change to the object 
>>>>>> instantiated at
>>>>>> ipalib/plugins/user.py line 467
>>>>>>
>>>>>> Thoughts and suggestions?
>>>>> I think it would make sense to follow the example with 
>>>>> validate_hostname and
>>>>> prepare a function validate_username(username, upn=False,
>>>>> netbios_name=False) [1].
>>>>>
>>>>> where upn would allow using "@." on top of current validator (i.e.
>>>>> user at Domain.test) and netbios_name would allow the "DOMAIN\user" 
>>>>> style. I would
>>>>> just suggest making sure the standard user validation error 
>>>>> message is still
>>>>> the same to avoid unnecessary QE fail positives.
>>>>>
>>>>> In add_external_pre_callback you could then just simply call
>>>>>
>>>>> validate_username(user, True, True)
>>>>>
>>>>> just like it is already done with hostname.
>>>>>
>>>>> My 2 cents.
>>>>>
>>>>> Martin
>>>>>
>>>>> [1]
>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa380525(v=vs.85).aspx 
>>>>>
>>>>
>>>
>>
>> -- 
>> Manage your subscription for the Freeipa-devel mailing list:
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>> Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
>