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.
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to