[SSSD] [PATCHES]: incorrect expansion of group membership when encountering a non-POSIX group

Pavel Reichl preichl at redhat.com
Tue Sep 2 15:31:50 UTC 2014


On 09/02/2014 04:37 PM, Michal Židek wrote:
> On 09/02/2014 04:03 PM, Pavel Reichl wrote:
>> Thank you Michal for review.
>>
>> On 09/02/2014 03:38 PM, Michal Židek wrote:
>>> On 08/25/2014 03:40 PM, Pavel Reichl wrote:
>>>> Hello,
>>>>
>>>> I have prepared patches for https://fedorahosted.org/sssd/ticket/2343
>>>>
>>>> Patches actually do not work due to same bug which is causing
>>>> https://fedorahosted.org/sssd/ticket/2407.
>>>>
>>>> So my patches are effectively blocked by #2407, for testing purposes
>>>> following command may be used (which reverts patches introducing the
>>>> bug):
>>>>
>>>> git revert 983983dd1629ab33eab340a40d9ee83965a339c6
>>>> ed346bcc84b8a326996e5550771773d8e63f17c2
>>>>
>>>> Thanks,
>>>>
>>>> Pavel Reichl
>>>
>>> Hi,
>>>
>>> I just have one question.
>>>
>>> Patch 0001:
>>>>
>>>> -               state->group_dns[state->cur], count);
>>>> +              state->group_dns[state->cur], count);
>>>>
>>> Btw. I do not think this change was necessary, but OK, we can keep 
>>> it :)
>>> Ack.
>> Necessary? No. Was it change to better? I hope so.
>
> Indeed, but it is the only change you made in that file with the first
> patch. And in the 2nd patch were you actually touch the file,
> the changes you made are like 500 lines away. So it looked funny
> to me :)

Oh, I see now, that was silly of me.

>
>>>
>>> Patch 0002:
>>>> -        /* When not ID-mapping, make sure there is a non-NULL UID */
>>>> -        state->base_filter = 
>>>> talloc_asprintf_append(state->base_filter,
>>>> - "(&(%s=*)(!(%s=0))))",
>>>> - opts->group_map[SDAP_AT_GROUP_GID].name,
>>>> - opts->group_map[SDAP_AT_GROUP_GID].name);
>>>> +        state->base_filter =
>>>> talloc_asprintf_append(state->base_filter, ")");
>>>
>>> This is the change I was worried about most. It did not break anything
>>> I tested, but the comment seem to be there for a reason. Do you know
>>> why the comment was there in the first place? I would like to Ack this
>>> patch, but I am not sure if we are not breaking something with this.
>> Well, I simply assume that the comment described the filter which was
>> filtering out the non-posix groups. To achieve saving non-posix group I
>> don't see any way around then to make filter 'accept' non-posix groups.
>> Do you see any other option?
>
> The thing I did not understand about it is, why did we do this
> in the first place? Why would we not want to include non-posix
> groups? But now, when I think about it again it probably was
> just optimization where we originally did not realize that
> posix groups can be members of non-posix groups, so we skipped
> them.
>
> I think this answered my question.
>
> Ack.
>
>>>
>>> Patch 0003: Ack.
>>>
>>> The problem with not listing all members of a group still remains in
>>> trusted scenario (IPA<->AD trust, SSSD is client of IPA). If the
>>> group hierarchy is located on AD we do not get proper results.
>>> But this should be solved separately.
>>>
>>> Michal
>
>
> So ACK to all.
>
> Michal

Thanks!
>
> _______________________________________________
> 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