[SSSD] [PATCHSET] LDAP: sanitize group name when used in filter

Pavel Reichl preichl at redhat.com
Thu Aug 6 13:04:25 UTC 2015



On 08/06/2015 02:55 PM, Lukas Slebodnik wrote:
> On (06/08/15 14:31), Pavel Reichl wrote:
>> On 08/05/2015 02:44 PM, Pavel Březina wrote:
>>> On 08/05/2015 12:11 PM, Pavel Reichl wrote:
>>>>
>>>> On 08/05/2015 11:34 AM, Pavel Březina wrote:
>>>>> On 08/04/2015 03:52 PM, Pavel Reichl wrote:
>>>>>> Hello,
>>>>>>
>>>>>> please see 2 simple patches attached.
>>>>>>
>>>>>> I could not find function to sanitize DN so it could be used as part
>>>>>> of
>>>>>> filter (sanitize ()*/\...) so I had to write one.
>>>>>>
>>>>>>   sysdb_dn_sanitize is not the right choice,
>>>>>>
>>>>>> sysdb_dn_sanitize("name=expired-group(2016),cn=groups,cn=LOCAL,cn=sysdb")
>>>>>>
>>>>>> ->
>>>>>> "name\\3Dexpired-group(2016)\\,cn\\3Dgroups\\,cn\\3DLOCAL\\,cn\\3Dsysdb"
>>>>>>
>>>>>>
>>>>>> Thanks!
>>>>> Hi, I did just a quick read of your patches... can you take one more
>>>>> step with creating a sanitized dn and create a more generic function
>>>>> for that?
>>>>>
>>>>> Have you considered to modify sysdb_dn_sanitize to also escape
>>>>> parentheses (that's what is misssing, isn't it)?
>>>> no because sysdb_dn_sanitize escapes also ',' and '=' and I need them to
>>>> stat as they are
>>>>
>>>> This is what I have:
>>>> "name=expired-group(2016),cn=groups,cn=LOCAL,cn=sysdb"
>>>> This is what I need:
>>>> "name=expired-group\282016\29,cn=groups,cn=LOCAL,cn=sysdb"  // just
>>>> escape '(' and ')'
>>>> This is what sysdb_dn_sanitize returns:
>>>> "name\\3Dexpired-group(2016)\\,cn\\3Dgroups\\,cn\\3DLOCAL\\,cn\\3Dsysdb"
>>>>
>>>> Failing filter:
>>>> (&(objectClass=user)(|(memberOf=name=VDI-US02_Corporate-Environment(2013),cn=groups,cn=qut.edu.au,cn=sysdb)
>>>>
>>>>
>>>> Corrent filter
>>>> (&(objectClass=user)(|(memberOf=name=VDI-US02_Corporate-Environment\282013\29,cn=groups,cn=qut.edu.au,cn=sysdb)
>>>>
>>>>
>>>>
>>>> I hope it's clearer now.
>>> Of course... sysdb_dn_sanitize is not supposed to be called on the whole
>>> dn. Just on the name part. It mean "sanitize value so it can be used in
>>> dn". But changing it to also escape parentheses would require sysdb and
>>> code update, so it is not worth it.
>>>
>>>> +static errno_t
>>>> +get_group_dn_with_filter_sanitized_name(TALLOC_CTX *mem_ctx,
>>>> +                                        struct sss_domain_info *domain,
>>>> +                                        const char *grp_name,
>>>> +                                        const char **_grp_dn);
>>> Can you use group_name and _group_dn? Two characters won't kill anybody :-)
>>> Otherwise we can keep the code as is. I have just one recommendation for
>>> tests:
>> Sure, done.
>>>> +    /* let records to expire */
>>>> +    usleep(1100000);
>>> It will be better to expire the records manually by setting expiration time
>>> to zero. I'm not sure if we have already a function for that, if not,
>>> please write one. It may be quite useful for tests.
>> I agree with you and I know that you would prefer the function to be generic
>> and part of sysdb. But I am afraid that It would take too much time to do it
>> properly and we should also handle code duplication that would be introduced
>> to sss_cache.c. Would static function in this test be sufficient temporal
>> solution for now? I would also file a ticket for proper solution. Is this OK
>> with you?
>>
> I didn't try but I have an idea.
>
> sysdb_group_dn calls sysdb_dn_sanitize to sanitize name and then
> it creates "struct ldb_dn".
>
> It might be goot to try use sysdb_group_dn + convert dn to string.
> I hope it should be properly escaped.
Lukas, sysdb_dn_sanitize() does not escape '(',')','*',... as they are 
valid characters in DN AFAIK. But they have a special meaning when used 
as a part of the filter.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list