[SSSD] [PATCH] LDAP: retain external members

Pavel Reichl preichl at redhat.com
Thu Dec 11 17:07:07 UTC 2014


On 12/10/2014 11:28 PM, Lukas Slebodnik wrote:
> On (10/12/14 21:32), Sumit Bose wrote:
>> On Wed, Dec 10, 2014 at 05:43:37PM +0100, Lukas Slebodnik wrote:
>>> On (10/12/14 11:26), Sumit Bose wrote:
>>>> On Wed, Dec 10, 2014 at 10:44:32AM +0100, Pavel Reichl wrote:
>>>>> On 12/09/2014 01:36 PM, Sumit Bose wrote:
>>>>>> On Tue, Nov 25, 2014 at 03:42:14PM +0100, Pavel Reichl wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> please see attached patch for https://fedorahosted.org/sssd/ticket/2492
>>>>>>>
>>>>>>> Thanks!
>>>>>> Hi Pavel,
>>>>>>
>>>>>> thank you for the patch, it works well in my tests and I didn't see any
>>>>>> regressions in IPA setup with and without trsut to AD, so ACK.
>>>>>>
>>>>>> I would just like to ask you to add a comment to
>>>>>>
>>>>>>> @@ -842,6 +913,23 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
>>>>>>>               goto fail;
>>>>>>>           }
>>>>>>>       }
>>>>>>> +    if (opts->schema_type == SDAP_SCHEMA_IPA_V1) {
>>>>>>> +        ret = sysdb_attrs_get_string(attrs, SYSDB_SID_STR, &group_sid);
>>>>>>> +        if (ret != EOK) {
>>>>>>> +            DEBUG(SSSDBG_TRACE_FUNC, "Failed to get group sid\n");
>>>>>>> +            group_sid = NULL;
>>>>>>> +        }
>>>>>>> +
>>>>>>> +        if (group_sid != NULL) {
>>>>>>> +            ret = retain_extern_members(memctx, dom, group_name, group_sid,
>>>>>>> +                                        &userdns, &nuserdns);
>>>>>>> +            if (ret != EOK) {
>>>>>>> +                DEBUG(SSSDBG_MINOR_FAILURE,
>>>>>>> +                      "retain_extern_members failed: %d:[%s].\n",
>>>>>>> +                      ret, sss_strerror(ret));
>>>>>>> +            }
>>>>>>> +        }
>>>>>>> +    }
>>>>>> which explains that this is a temporary solution until the IPA provider
>>>>>> can resolve external group membership. I have created
>>>>>> https://fedorahosted.org/sssd/ticket/2522 for this. Feel free to
>>>>>> explicitly add the ticket URL into the comment.
>>>>>>
>>>>>> bye,
>>>>>> Sumit
>>>>> Thanks for review.
>>>>> Please see updated patch.
>>>>>
>>>> Thank you, ACK.
>>> NACK, sssd crashed with this patch.
>>> How to reproduce:
>>> sssd with ipa server mode:
>>>
>>> id admin at rdustv1911.test  //ipa admin
>>> id aduser1 at ipaad2012r2.test // aduser
>>> id aduser2 at ipaad2012r2.test
>>> sss_cache -E
>>> id aduser1 at ipaad2012r2.test // aduser
>> good catch, thank you. Looks like I always only tested with one external
>> member. Does
>>
>> diff --git a/src/providers/ldap/sdap_async_groups.c
>> b/src/providers/ldap/sdap_async_groups.c
>> index 43744ac..4bc2aa7 100644
>> --- a/src/providers/ldap/sdap_async_groups.c
>> +++ b/src/providers/ldap/sdap_async_groups.c
>> @@ -861,7 +861,7 @@ retain_extern_members(TALLOC_CTX *mem_ctx,
>>
>>              (*_nuserdns)++;
>>              *_userdns = talloc_realloc(mem_ctx, *_userdns, char*, *_nuserdns);
>> -            *_userdns[(*_nuserdns)-1] = talloc_steal(mem_ctx, dns[i]);
>> +            (*_userdns)[(*_nuserdns)-1] = talloc_steal(mem_ctx, dns[i]);
>>          }
>>      }
>>
>>
>> fix the issue for you?
> Yet, it fixed.
> I could see the mistake in code and I was looking at the code at least fo 10
> minutes. Someone should write 100 times I will learn C operator precedence
> http://en.cppreference.com/w/c/language/operator_precedence
>
>> Maybe this should be better written with local variables to avoid this
>> kind of issues?
> +1
>
>> Additionally there is a NULL check missing after talloc_realloc().
> +1
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I'm really sorry about the crash.
Please see if the new patch is crash free.

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-LDAP-retain-external-members.patch
Type: text/x-patch
Size: 8149 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141211/a683ca66/attachment.bin>


More information about the sssd-devel mailing list