[Freeipa-devel] [PATCH] 21 Escape LDAP characters in member and memberof searches

[Rob Crittenden]

JR Aquino wrote:
> On Mar 30, 2011, at 3:03 PM, Rob Crittenden wrote:
>
>> JR Aquino wrote:
>>> On Mar 30, 2011, at 1:01 PM, Stephen Gallagher wrote:
>>>
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> On 03/30/2011 03:53 PM, JR Aquino wrote:
>>>>>
>>>>> On Mar 30, 2011, at 12:05 PM, JR Aquino wrote:
>>>>>
>>>>>> The FreeIPA framework performs unescaped searches to enumerate group membership.
>>>>>>
>>>>>> The following patch corrects this behavior.
>>>>>>
>>>>>> -JR
>>>>>>
>>>>>> <freeipa-jraquino-0021-Escape-LDAP-characters-in-member-and-memberof-search.patch>_______________________________________________
>>>>>> Freeipa-devel mailing list
>>>>>> Freeipa-devel at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>
>>>>> Self NACK
>>>>>
>>>>> Attached is the corrected patch.
>>>>>
>>>>> search_group_dn = _ldap_filter.escape_filter_chars(search_group_dn)
>>>>>
>>>>> Is now correctly changed to:
>>>>>
>>>>> search_group_dn = _ldap_filter.escape_filter_chars(group_dn)
>>>>>
>>>>
>>>> Nack. This is a step in the right direction, but you're not actually
>>>> using this value anywhere.
>>>>
>>>> I think you wanted to have the next line changed to:
>>>>
>>>> searchfilter = "(memberof=%s)" % search_group_dn
>>>>
>>>> - --
>>>> Stephen Gallagher
>>>> RHCE 804006346421761
>>>
>>> Oh! You are right.
>>>
>>> Attached is the corrected patch.
>>
>> I don't think you need a new variable for search_group_dn. The value is passed in from a tuple so any changes will be silently lost anyway.
>>
>> Or you can leave it, I think it's probably safer this way (since we can't predict how it will be called in the future), but you should then do the same in get_memberof().
>>
>> rob
>
> I agree with you. For the sake of equality, I have adjusted the patch to address entry_dn with search_entry_dn.

ack, pushed to master and ipa-2-0

rob