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

Michal Židek mzidek at redhat.com
Tue Sep 2 14:37:00 UTC 2014


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 :)

>>
>> 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




More information about the sssd-devel mailing list