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

Pavel Reichl preichl at redhat.com
Wed Aug 12 11:52:33 UTC 2015


On 08/10/2015 07:30 AM, Lukas Slebodnik wrote:
> On (07/08/15 13:58), Pavel Březina wrote:
>> On 08/07/2015 06:28 AM, Lukas Slebodnik wrote:
>>> On (06/08/15 15:04), Pavel Reichl wrote:
>>>>
>>>> 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.
>>> Yes, that's true. I checked RFC4514 and RFC2253
>>>
>>> However, I do not understand why full dn cannot be escaped and just rdn part of
>>> dn.
>>>
>>> You can use hexadecimal representatin even for normal letters.
>>> [root at host db]# ldbsearch -H cache_example.com.ldb -b name=pcp,cn=groups,cn=example.com,cn=sysdb '(name=\70\63\70)'
>>> # record 1
>>> dn: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>> createTimestamp: 1438338481
>>> gidNumber: 967
>>> name: pcp
>>> objectClass: group
>>> isPosix: TRUE
>>> lastUpdate: 1438673281
>>> dataExpireTimestamp: 1438678681
>>> distinguishedName: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>>
>>> # returned 1 records
>>> # 1 entries
>>> # 0 referrals
>>> [root at host db]# ldbsearch -H cache_example.com.ldb -b name=pcp,cn=groups,cn=example.com,cn=sysdb '(name=pcp)'
>>> # record 1
>>> dn: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>> createTimestamp: 1438338481
>>> gidNumber: 967
>>> name: pcp
>>> objectClass: group
>>> isPosix: TRUE
>>> lastUpdate: 1438673281
>>> dataExpireTimestamp: 1438678681
>>> distinguishedName: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>>
>>>
>>>
>>> And it works also with dn
>>> [root at host db]# ldbsearch -H cache_example.com.ldb -b name=pcp,cn=groups,cn=example.com,cn=sysdb '(distinguishedName=name\3dpcp\2ccn\3dgroups\2ccn\3dexample.com\2ccn\3dsysdb)'
>>> # record 1
>>> dn: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>> createTimestamp: 1438338481
>>> gidNumber: 967
>>> name: pcp
>>> objectClass: group
>>> isPosix: TRUE
>>> lastUpdate: 1438673281
>>> dataExpireTimestamp: 1438678681
>>> distinguishedName: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>>
>>> [root at host db]# ldbsearch -H cache_example.com.ldb -b name=pcp,cn=groups,cn=example.com,cn=sysdb '(distinguishedName=name=pcp,cn=groups,cn=example.com,cn=sysdb)'
>>> # record 1
>>> dn: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>> createTimestamp: 1438338481
>>> gidNumber: 967
>>> name: pcp
>>> objectClass: group
>>> isPosix: TRUE
>>> lastUpdate: 1438673281
>>> dataExpireTimestamp: 1438678681
>>> distinguishedName: name=pcp,cn=groups,cn=example.com,cn=sysdb
>>>
>>>
>>> Please try to fix it properly in general way without nasty hacks.
>>> We are not in a hurry.
>>>
>>> LS
>> Yes, full dn may be escaped. The problem may be double escaping.
>>
> There is no problem with double escaping.
>
>> If you get dn through sysdb_group_dn, it returns dn sanitized with
>> sysdb_dn_sanitize as you noted.
> Yes, you need to escape some character if you create DN.
> @see RFC4514 and RFC2253 for special characters.
> However, we already have ldb_dn. So it contains valid/escaped DN characters.
>
>> However, it requires more characters to be escaped in order to be used in filter.
> I need to correct you here. We do not need to escape more characters.
> We need to escape different one.
> @see rfc4515 and rfc2254.
> In teory, we can even escape all characters as I showed in previous example,
> but it's not necessary and it would be just an overhead.
>
>> Therefore sysdb_filter_sanitize is the
>> proper function to be used, but you can't run in on data that was already
>> sanitized (though not completely for the purpose of filter).
>>
> That's not true. @see my previous two paragrpahs and mentioned RFC.
> RFCs are for LDAP but ldb should behave the same. The only exceptions
> is that ldb is schema less.
>
>> Thus Pavel needs to built dn himself either way and here I prefer only the
>> name to be sanitized to not obfuscate dn in logs.
> And I prefer less alocations. Clean-up task is already slow.
>
> Here is an example how it should look.
>
> If you have just a name (not dn).
> then you need to escape name and create ldb_dn with base dn (We need to
> escape base dn as well if it is just a string and not ldb_dn.
> Now, you need to filter entries in ldb using dn.
> So you serialize ldb_dn to string. In ideal world,
> you would use somthing like prepared statements in SQL world.
>
> However we do not have something like this in ldb. So we need to explicitly
> escape strings ourselves and then you can use it for creating string with
> filter or to call "{sss_,}ldb_search" with template filter + escaped args.
>
> HTH
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Pavel do you share Lukas' concerns? Do we need to update the patches? 
Should we ask other developers for opinions?



More information about the sssd-devel mailing list