On Thursday 09 September 2010 15:59:46 Simo Sorce wrote:
On Thu, 09 Sep 2010 09:18:12 -0400
Stephen Gallagher <sgallagh(a)redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> On 09/09/2010 09:14 AM, Ralf Haferkamp wrote:
> > Hi,
> > Is it really the intended behaviour of the sssd LDAP backend (I am
> > running the current code from the master branch) to only return
> > the group members that are already cached in sysdb and to
> > silently ignore everything else? E.g. when I start sssd with
> > empty caches and do a "getent group <random-ldap-group>" I
> > only get back the group without any members. Somehow I think this
> > can't be intended :)
> > I have started working on a patch to let sssd look up the
> > non-cached users via LDAP (and save them into the cache). Find it
> > attached. Note: That patch is not really complete (e.g. it doesn't
> > handle rfc2307 groups correctly). But before putting more effort
> > into this I like to make sure that I am not trying to fix a
> > "feature" here.
> No, it is not intentional that groups should be missing users. This
> is definitely a bug. Please file a ticket upstream.
It is intended if enumerations are off.
Thee reason is that you may end up effectively doing full user
enumerations otherwise if you have a big group that contain all users.
should probably be possible to disable that feature separately
from enumeration. While, turning enumerations off by default makes sense
to me, I think returning incomplete results when resolving groups by
default goes a bit too far.
BTW, I just found out that the behaviour of getgrouplist()/initgroups()
is similar currently. It will only return the groups that are already
present in the cache. That means in many cases it will return nothing.
(Or just the gid you supplied via the group argument).
Not only that but it would be an inefficient enumeration as it would
be repeated multiple times for each group.
Why is that? Users should of course be
saved in the sysdb after being
read from LDAP. If you read another group which has that user as a member
the result is taken from the cache of course. (That's what my patch tries
to do at least, though I admit there is still room for optimization)
And if you have a *lot* of users this would defeat the point of
disabling enumerations, making performances actually worse.
So please do not change this without proper discussion.