On 11/12/2012 4:12 AM, Jakub Hrozek wrote:
Thank you for the contribution! I will take a look at the patch..can you just send the patch as output of git format-patch in the future? It's easier for us to handle that way.
As per the documentation and the configAPI -- you can either let us
Sorry; the diff was just a quickie to get some initial feedback. I've attached a patch that should include everything except possible changes to cache handling.
Yes, that's my understanding, too. I think the way the code is built now would treat the attributes as missing and delete them during a getgrnam or getgrgid call.
[...]
The SSSD always retrieves initgroups data during an authentication request that comes through the PAM stack for exactly the reason you cited. For lookups that only occur through the name-service-switch module, the SSSD caches the data for entry_cache_user_timeout by default.
I'm not completely clear on what you mean about retrieving initgroups data via the PAM stack. My understanding is that the supplementary groups are set via the initgroups(3) call, which uses nss to look up what groups a particular user belongs to and then calls setgroups to set them. Or are you talking about access control in the pam stack provided for example by simple_allow_groups/simple_deny_groups?
So maybe the code could be modifed to read the existing group memberships and fake the results it got from LDAP to also include these memberships?
Hmm, look up the current cached groups and spoof them in the response so the cache handling code won't delete them? That sounds a bit kludgy :). The mechanism mentioned in the original ticket of having the cache update code only activate for initgroups lookups and not getgr lookups seems cleaner.
I guess I need to dig a bit deeper in the code and try to understand the scenario where not adding this additional cache handling exception will cause a problem. With the new option enabled, it would seem the only time group membership is relevant is for initgroups() calls.
Hmm, okay, I found an anomaly:
# id -a henson uid=1000(henson) gid=1000(henson) groups=1000(henson),1866(cppnet-admin),1402(iit),3285(iit_staff),14988(iit_operations),18445(nt-mgr),22212(iit_ex_spam_out-admin),1865(cppnet),1020(intranet),14528(iit_systems),17730(unxadmin),21016(eoc_essential),23306(iit_vmware_admin),23380(netadmin),1406(staff),15379(noc),23358(employees),23359(members),19289(idm_sysadmin),19295(idm_restricted),19317(iit_systems_staff),20795(campus_techs),23668(iit_ops_systems)
# id -a henson uid=1000(henson) gid=1000(henson) groups=1000(henson)
With the new option enabled, the first time I do an 'id -a', it lists all of my groups; the next time (and subsequent times), they're gone 8-/. Presumably the getgr lookups of each individual group after my group list was obtained wiped out the memberships as warned about in the ticket.
I'll see about fixing it up so this doesn't happen.
Thanks