On Thursday 09 September 2010 17:48:09 Simo Sorce wrote:
On Thu, 9 Sep 2010 16:46:35 +0200
Ralf Haferkamp <rhafer(a)suse.de> wrote:
[..]
>
> Then it 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.
It depends on the point of view. For the specific logged in user the
groups he is interested in are fully update wrt his own membership.
It is true you may not see other members unless they log in, but why
would that be a problem ? If they do not log in they are not affected
by group memberships anyway.
The user currently logged in might need to see who
else is member of the
groups he is member of.
> 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).
Initgroups will always call the LDAP server, and will make sure all
groups the user is member of are available.
Ok, there something else seems broken
here:
1. When initgroups is called for a user that is currently not cached no
LDAP groups are returned. (I guess that's inline with your idea about
how sssd should behave)
2. When the user is already cached and rfc2307bis+memberof is used on the
Serverside, nothing is returned upon the first initgroups call, but
the cache is correctly populated with the groups, a second initgroups
call then returns the correct results. So I guess there's a bug here
somewhere. Haven't investigated this deeper yet.
3. When using rfc2307 or rfc2307bis (without memberof Attributes)
initgroups() seems completely broken.
I'll create bug reports after I was able to take a deeper look. (It seems
some of the issues are already reported. See e.g.
https://fedorahosted.org/sssd/ticket/595)
> > Not only that but it would be an inefficient111 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)
In order to save a user you have to look it up. so if you have a group
with 50k users in it you will have to do 50k lookups. Multiply this
for N groups.
Users that were already looked up when looking up the first group
will
not be looked up again (they are already cached) when resolving the
second group. If the groups really have 50k distinct members you are of
course right that a lot of LDAP lookups will need to be done. But I
wonder how common such a scenario is and if sssd's default configuration
should be optimized for such a usecases. I'd guess most enviroments that
sssd operates in are much smaller. And there you'll confuse the
users/admins when groups show up incomplete.
If you need enumeration completeness then you should just have
enumerations turned on IMO.
Now thinking a bit more there is probably a way to avoid the lookups.
You can avoid the 50k lookups by actually *not* fetching each member
from LDAP but create fake entries that are marked as expired so they
are immediately refreshed with the right data if they are ever
requested from the user space.
Right, this how I was thinking about implementing it
for rfc2307 groups,
but I haven't come to that yet.
But this technique has some pretty sever limits when using
rfc2307bis
and nested groups. (should work just fine for classic rfc2307)
One limit is that you don't know whether a DN is a user or a Group.
It is true that we can adopt some heuristics like examining the DN
components to get a clue, but they would be pretty much schema
dependent and would fail consistently in some scenarios. So you may
fail to properly unroll nested groups.
The other is that the actual user name may not be contained within the
DN. if the DN includes the uid attribute as part of it does, but if
the CN is used (like against AD where the samAccoutnName attribute is
used as the OS user name) then you do not have the user name you need
to present in the group as member. So, again, it would work only in
some setups.
I think it is not worth to go that path for rfc2307bis groups. It
wrong
to rely too much on such specific setups.
Another concern is about how to properly do caching.
When we did the initgroups call I used to fully resolve groups, we
removed that code because it would make initgroups unbearably slow as
it would end up doing way too many lookups.
Agreed. For initgroups the groups
should not be completely resolved. OTOH
when dealing with nested groups (in a scenario without "memberof") you'd
have to look up all group members anyways. (IMO a reason why nested
groups should be disabled by default when using a standard rfc2307bis
setup.)
So we need to make sure
initgroups can create/manipulate groups and know when a group can be
considered up to date, and yet at the same time mark it as
"incomplete" or something like that for getent group <group> calls
(and vice versa ?)
Yes. Probably something like this: After initgroups has finished
successfully for a specific user. The sysdb object of that user should be
"marked" that the initgroups result for that user in completely cached.
All groups that needed to be loaded from LDAP during that initgroups call
need to be marked as incomplete. When getgr(gid|nam) is called for such a
group the group will be resolved and the "imcomplete" mark can be
removed.
I guess we could have a third option to manage this, and allow
people
full group (all member users/groups(recursive)) lookups if they really
want/need it. But it has a lot of subtle interactions we need to care
about to avoid perf. side effects.
May be something like:
fully_resolve_groups defaulting to false ?
Simo.
regards,
Ralf