[SSSD] Re: Nested netgroups with IPA provider

[Michal Židek]

On 11/10/2016 02:13 PM, Lukas Slebodnik wrote:
>  On (10/11/16 13:57), Michal Židek wrote:
> > On 11/10/2016 12:29 PM, Jakub Hrozek wrote:
> >> On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:
> >>> Hi,
> >>>
> >>> this is continuation of discussion about pull
> >>> request 51 and associated tickets.
> >>>
> >>> For context, see:
> >>> https://github.com/SSSD/sssd/pull/59
> >>> https://fedorahosted.org/sssd/ticket/3159
> >>> https://fedorahosted.org/sssd/ticket/3116
> >>>
> >>> The FreeIPA UQE guys added upstream test for this issue
> >>> because we do not have upstream CI tests in SSSD with
> >>> IPA provider yet and this bug is not present in the
> >>> plain LDAP.
> >>>
> >>> We use hash tables to store members of netgroups while
> >>> processing netgroups (and creating the netgroup triples).
> >>> The netgroup names are lowercased before they are stored
> >>> in the hash table. The reason for this normalization is
> >>> unknown to me.
> >>
> >> I also don't know the reason behind this normalization. There is a
> >> comment above the code that lowercases the DN that says:
> >> 646         /* Transform the DN to lower case.
> >> 647          * this is important, as the member/memberof attributes
> >> 648          * have the value also in lower-case
> >> 649          */
> >>
> >> But I really have no idea what this means. We're using
> SYSDB_NETGROUP_MEMBER,
> >> but we're storing names in that attribute and the IPA code lowercases
> >> original DNs, so I'm quite confused about it.
> >>
> >>> FreeIPA only creates lowercased netgroup
> >>> names, so lowercasing only affects the attribute name
> >>> (that is stored as prefix to the netgroup name in the hash table,
> >>> and maybe it can happen that the attribute name can be
> >>> stored in different cases at some point, which would
> >>> explain why we lower case it, however I was not able
> >>> to confirm if this is the case).
> >>
> >> I really think this is about the original DN values. This is what we
> >> enter:
> >>     (gdb) p key.str
> >>    
> "ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"
> >>
> >>>
> >>> When we read the hash table, we do not lowercase the keys,
> >>> so the nested netgroups are not found and this is the
> >>> reason why the bug appears. The patch in PR 51 lower cases
> >>> the keys before reading the hash table and the bug does not
> >>> appear after that. Lukas thinks however that this is not
> >>> good approach, because there should be no need for the lower
> >>> casing in the first place.
> >>>
> >>> Patch that removes the lower casing before adding the keys
> >>> to htable should also fix the issue. I did not send the patch
> >>> with this approach, because I was not sure why the lowercasing
> >>> happens in the first place and I know that lowercasing is not
> >>> harmful for the IPA netgroups, so I find it safer to use
> >>> the approach in the PR 51 especially while we do not have good
> >>> code coverage for IPA provider, however as I mentioned in the
> >>> PR 51, I am looking for opinions on this.
> >>
> >> I'm fine with removing the lowercasing if this moves fixing the issue
> >> forward.
> >
> > I just quickly tried it without the lowercasing and it does work
> > for me.
> >
>  awesome
>
>  LS 
The patch for master is in attachment.

Michal