On 11/10/2016 03:18 PM, Michal Židek wrote:
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
Lukas wanted a new PR, so here it is:
https://github.com/SSSD/sssd/pull/78
Michal
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
