On Fri, Sep 06, 2013 at 02:30:54PM +0200, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/2066
Please note, that these patches has to be applied atop
0004-util-add-find_subdomain_by_sid.patch from my simple provider
patches. You can also pull it from branch ad-groups at my fedorapeople repo.
I still don't know why AD returns Domain Users from root domain in
tokenGroups, even though subaduser is not member of DU(a)ad.pb (AD bug
perhaps?), but with these patches we store it correctly.
Did you check if the SID of root domain's Domain Users is also returned
in the PAC?
This should be also a step forward to #2064.
From 3b07596669cac3ec57e7a20e210b6ccd1fc5fa3b Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Fri, 6 Sep 2013 11:08:56 +0200
Subject: [PATCH 1/4] util: add get_domains_head()
This function will return head of the domain list.
Resolves:
https://fedorahosted.org/sssd/ticket/2066
---
src/util/domain_info_utils.c | 13 +++++++++++++
src/util/util.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c
index f9d9057a811f3e08c451d7f6b44bc14202559962..65f3c21f0ad3de2d560347260d317fa21b3e69b3
100644
--- a/src/util/domain_info_utils.c
+++ b/src/util/domain_info_utils.c
@@ -27,6 +27,19 @@
/* the directory domain - realm mappings are written to */
#define KRB5_MAPPING_DIR PUBCONF_PATH"/krb5.include.d"
+struct sss_domain_info *get_domains_head(struct sss_domain_info *domain)
+{
+ struct sss_domain_info *dom = NULL;
+
+ /* get to the top level domain */
+ for (dom = domain; dom->parent != NULL; dom = dom->parent);
+
+ /* proceed to the list head */
+ for (; dom->prev != NULL; dom = dom->prev);
Can we currently have a setup with multiple top level domains?
+
+ return dom;
+}
+
From caf817e5b64cd09c2f953d48a5761f33735ea95e Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Fri, 6 Sep 2013 13:13:04 +0200
Subject: [PATCH 2/4] sysdb: get_sysdb_grouplist() can return either names or
dn
We need to work with distinguish names when processing
cross-domain membership, because groups and users may
be stored in different sysdb tree.
Resolves:
https://fedorahosted.org/sssd/ticket/2066
---
The code is fine but I would prefer if you created a think wrapper like
you did in patch #3 for sysdb_update_members_dn(). The true/false
boolean values in the code look unreadable to me.
From 786b665d4906283a70ddf2e7c040a97f43a57aad Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Fri, 6 Sep 2013 13:44:46 +0200
Subject: [PATCH 3/4] sysdb: sysdb_update_members can take either name or dn
We need to work with distinguish names when processing
cross-domain membership, because groups and users may
be stored in different sysdb tree.
Resolves:
https://fedorahosted.org/sssd/ticket/2066
ACK
From 7cdd86441a937f9a12a9a5a1a5d2decff927809c Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Fri, 6 Sep 2013 10:43:35 +0200
Subject: [PATCH 4/4] ad: store group in correct tree on initgroups via
tokenGroups
If tokenGroups contains group from different domain than user's,
we stored it under the user's domain tree in sysdb. This patch
changes it so we store it under group's domain tree.
Resolves:
https://fedorahosted.org/sssd/ticket/2066
---
src/providers/ldap/sdap_async_initgroups_ad.c | 50 +++++++++++++++++++++------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c
b/src/providers/ldap/sdap_async_initgroups_ad.c
index 08d0e7dad8e870480b72fbd4179576d445df396b..05f2336ad01391a6427e332bcb9367ee4e745f9b
100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -364,9 +364,12 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req
*subreq)
char *sid_str;
gid_t gid;
time_t now;
+ struct sss_domain_info *group_domain;
struct sysdb_attrs **users;
struct ldb_message_element *el;
struct ldb_message *msg;
+ struct ldb_dn *group_ldb_dn;
+ const char *group_str_dn;
char **ldap_grouplist;
char **sysdb_grouplist;
char **add_groups;
@@ -471,13 +474,21 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req
*subreq)
continue;
}
+ group_domain = find_subdomain_by_sid(get_domains_head(state->domain),
+ sid_str);
+ if (group_domain == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Domain not found for SID %s\n",
+ sid_str));
+ continue;
+ }
+
DEBUG(SSSDBG_TRACE_LIBS,
("Processing membership GID [%lu]\n",
gid));
/* Check whether this GID already exists in the sysdb */
- ret = sysdb_search_group_by_gid(tmp_ctx, state->sysdb, state->domain,
- gid, attrs, &msg);
+ ret = sysdb_search_group_by_gid(tmp_ctx, group_domain->sysdb,
+ group_domain, gid, attrs, &msg);
if (ret == EOK) {
group_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
if (!group_name) {
@@ -492,9 +503,10 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req
*subreq)
* the group or its GID occurs, it will replace this
* temporary entry.
*/
+
group_name = sid_str;
- ret = sysdb_add_incomplete_group(state->sysdb,
- state->domain,
+ ret = sysdb_add_incomplete_group(group_domain->sysdb,
+ group_domain,
group_name, gid,
NULL, sid_str, false, now);
if (ret != EOK) {
@@ -511,13 +523,31 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req
*subreq)
goto done;
}
+ group_ldb_dn = sysdb_group_dn(group_domain->sysdb, tmp_ctx,
+ group_domain, group_name);
+ if (group_ldb_dn == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("sysdb_group_dn() failed\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ group_str_dn = ldb_dn_get_linearized(group_ldb_dn);
+ if (group_str_dn == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("ldb_dn_get_linearized() failed\n"));
+ ret = EINVAL;
+ goto done;
+ }
+
ldap_grouplist[group_count] =
- talloc_strdup(ldap_grouplist, group_name);
+ talloc_strdup(ldap_grouplist, group_str_dn);
if (!ldap_grouplist[group_count]) {
ret = ENOMEM;
goto done;
}
+ talloc_zfree(group_ldb_dn);
+ group_str_dn = NULL;
Why do you NULL the group_str_dn pointer here?
+
group_count++;
}
ldap_grouplist[group_count] = NULL;
@@ -526,7 +556,7 @@ sdap_get_ad_tokengroups_initgroups_lookup_done(struct tevent_req
*subreq)
* so we can update it.
*/
ret = get_sysdb_grouplist(state, state->sysdb, state->domain,
- state->username, &sysdb_grouplist, false);
+ state->username, &sysdb_grouplist, true);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
("Could not get the list of groups for [%s] in the sysdb: "