URL:
https://github.com/SSSD/sssd/pull/57
Title: #57: LDAP/AD: resolve domain local groups for remote users
jhrozek commented:
"""
On Wed, Oct 19, 2016 at 02:45:02AM -0700, sumit-bose wrote:
This set of patches should solve
https://fedorahosted.org/sssd/ticket/3206. To
test this you have to join SSSD into an AD forest with at least two domains.
Assuming the domain SSSD is joined to is called A and there is another domain
in the forest called B you have to create a domain local group in domain A,
e.g. A\dom_loc. Then create e.g. a universal group in domain B, e.g. B\uni and
make B\uni a member of A\dom_loc. Any user from domain B which is a member of
B\uni should now be a member of A\dom_loc as well because SSSD is joined to
domain A and any domain local group from domain A is valid in this case.
If SSSD is joined to domain B A\dom_loc is not a valid group in this domain and
any user which is a member of B\uni should _not_ be a member of A\dom_loc.
The lookup to the domain local groups is added as a second step after the
initial initgroups lookup and should work with and without the tokenGroups
request.
There is one issue I currently do not have a solution for. It is possible to
add the user from domain B to A\dom_loc directly. It this user is an indirect
member of A\dom_loc as well, e.g. via B\uni, and this direct membership is
removed the initgroups request will not return A\dom_loc. This is due to the
nature of SSSD's cache. The memberOf attribute will be removed from the user
entry during the initial lookup because this lookup is not aware of the domain
local groups and will remove the missing direct parent. During the domain local
lookup no changes are detected, because the direct membership is already
removed and there are no changes in the objects related to the indirect
membership. Even adding all nested memberships again won't help because the
sysdb code will detect that the membership is already present and skip it. And
unconditionally removing and adding all nested membership again sounds too
costly to me for this special case especially since the plan to use the PAC to
determine the memberships in the next release even for user which are not
already logged in.
You can view, comment on, or merge this pull request online at:
https://github.com/SSSD/sssd/pull/57
-- Commit Summary --
* sysdb: add parent_dom to sysdb_get_direct_parents()
* sdap: sdap_get_primary_fqdn() check if name is already fully-qualified
* sdap: make some nested group related calls public
* LDAP/AD: resolve domain local groups for remote users
-- File Changes --
M src/db/sysdb.h (22)
M src/db/sysdb_search.c (7)
M src/providers/ldap/sdap.c (12)
M src/providers/ldap/sdap_async_initgroups.c (162)
M src/providers/ldap/sdap_async_initgroups_ad.c (409)
M src/providers/ldap/sdap_async_private.h (26)
-- Patch Links --
https://github.com/SSSD/sssd/pull/57.patch
https://github.com/SSSD/sssd/pull/57.diff
As first part of the review, I ran Coverity and it found some warnings:
```
rror: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1554:12:
warning: unused variable 'd' [-Wunused-variable]
# size_t d;
# ^
# 1552| int ret;
# 1553| size_t c;
# 1554|-> size_t d;
# 1555| char **groupnamelist = NULL;
# 1556| struct sysdb_attrs *groups[1];
Error: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1562:25:
warning: unused variable 'msg' [-Wunused-variable]
# struct ldb_message *msg;
# ^
# 1560| const char *class;
# 1561| struct sss_domain_info *obj_dom;
# 1562|-> struct ldb_message *msg;
# 1563| struct ldb_message_element *el;
# 1564| const char *obj_attrs[] = {SYSDB_NAME, SYSDB_MEMBEROF,
# NULL};
Error: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1563:33:
warning: unused variable 'el' [-Wunused-variable]
# struct ldb_message_element *el;
# ^
# 1561| struct sss_domain_info *obj_dom;
# 1562| struct ldb_message *msg;
# 1563|-> struct ldb_message_element *el;
# 1564| const char *obj_attrs[] = {SYSDB_NAME, SYSDB_MEMBEROF,
# NULL};
# 1565| char *local_groups_base_dn;
Error: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1564:17:
warning: unused variable 'obj_attrs' [-Wunused-variable]
# const char *obj_attrs[] = {SYSDB_NAME, SYSDB_MEMBEROF, NULL};
# ^
# 1562| struct ldb_message *msg;
# 1563| struct ldb_message_element *el;
# 1564|-> const char *obj_attrs[] = {SYSDB_NAME, SYSDB_MEMBEROF,
# NULL};
# 1565| char *local_groups_base_dn;
# 1566| uint8_t *obj_base_dn;
Error: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1566:14:
warning: unused variable 'obj_base_dn' [-Wunused-variable]
# uint8_t *obj_base_dn;
# ^
# 1564| const char *obj_attrs[] = {SYSDB_NAME, SYSDB_MEMBEROF,
# NULL};
# 1565| char *local_groups_base_dn;
# 1566|-> uint8_t *obj_base_dn;
# 1567| char **cached_local_parents = NULL;
# 1568| uint8_t *name_start;
Error: COMPILER_WARNING:
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c: scope_hint:
In function 'sdap_ad_get_domain_local_groups_parse_parents'
sssd-1.14.90/src/providers/ldap/sdap_async_initgroups_ad.c:1568:14:
warning: unused variable 'name_start' [-Wunused-variable]
# uint8_t *name_start;
# ^
# 1566| uint8_t *obj_base_dn;
# 1567| char **cached_local_parents = NULL;
# 1568|-> uint8_t *name_start;
# 1569| char **add_list = NULL;
# 1570| char **del_list = NULL;
```
Next, I'm running downstream tests to see if any functionality
regressed..(so I haven't started looking at the code yet..)
"""
See the full comment at
https://github.com/SSSD/sssd/pull/57#issuecomment-256139468