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
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
