[SSSD] [PATCH] ad: support cross domain membership

Jakub Hrozek jhrozek at redhat.com
Fri Oct 25 12:22:48 UTC 2013


On Fri, Oct 25, 2013 at 12:58:23PM +0200, Pavel Březina wrote:
> On 10/25/2013 10:55 AM, Pavel Březina wrote:
> >On 10/24/2013 08:40 PM, Jakub Hrozek wrote:
> >>On Wed, Oct 02, 2013 at 04:13:32PM +0200, Pavel Březina wrote:
> >>>On 10/01/2013 09:54 PM, Jakub Hrozek wrote:
> >>>>On Tue, Sep 24, 2013 at 03:17:47PM +0200, Pavel Březina wrote:
> >>>>>On 09/24/2013 01:32 PM, Jakub Hrozek wrote:
> >>>>>>On Wed, Sep 11, 2013 at 02:40:14PM +0200, Pavel Březina wrote:
> >>>>>>>https://fedorahosted.org/sssd/ticket/2064
> >>>>>>>
> >>>>>>>These patch set depends on: [PATCH] ad: store group in correct
> >>>>>>>tree on initgroups via tokenGroups
> >>>>>>>
> >>>>>>>You can also pull it with all dependencies from my repository:
> >>>>>>>fedorapeople.org:public_git/sssd.git #ad-groups
> >>>>>>>
> >>>>>>>The fundamental changes in this patch set are: - lookup groups
> >>>>>>>in global catalog - pick up member domain from its originalDN
> >>>>>>
> >>>>>>>From 0273d17f24eac7b60dfc0515a9e3b97ad16d1199 Mon Sep 17
> >>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> >>>>>>><pbrezina at redhat.com> Date: Mon, 9 Sep 2013 15:52:03 +0200
> >>>>>>>Subject: [PATCH 1/9] ad: shortcut if possible during get object
> >>>>>>>by ID or SID
> >>>>>>>
> >>>>>>>When getByID or getBySID comes from responder, the request
> >>>>>>>doesn't necessarily have to contain correct domain, since
> >>>>>>>responder iterates over all domains until it finds a match.
> >>>>>>>
> >>>>>>>Every domain has its own ID range, so we can simply shortcut
> >>>>>>>if domain does not match and avoid LDAP round trip. Responder
> >>>>>>>will continue with next domain until it finds the correct one.
> >>>>>>
> >>>>>>This patch seems OK to me, but I'd like a second look from
> >>>>>>someone who understands the ranges better (which is probably
> >>>>>>Sumit)
> >>>>>>
> >>>>>>>From f74d4637980438032649dfbf079fa6c839862586 Mon Sep 17
> >>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> >>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 10:40:06 +0200
> >>>>>>>Subject: [PATCH 2/9] ad: simplify get_conn_list()
> >>>>>>>
> >>>>>>>It was originally design to return list of connection objects,
> >>>>>>>it really always work with only one connection.
> >>>>>>
> >>>>>>I'd like to review this patch and the following along with my
> >>>>>>patches to look up POSIX IDs in GC, they touch the same code.
> >>>>>>
> >>>>>>>From ad5dc9e7557ef605fc5d7fc759e5cb6c2f9a148c Mon Sep 17
> >>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> >>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:50 +0200
> >>>>>>>Subject: [PATCH 4/9] sdap_domain_add(): fix possible memory
> >>>>>>>leak
> >>>>>>
> >>>>>>ACK.
> >>>>>>
> >>>>>>>From 9f2c212e01700289d70002c8c39b732ca6c11cee Mon Sep 17
> >>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
> >>>>>>><pbrezina at redhat.com> Date: Tue, 10 Sep 2013 14:45:52 +0200
> >>>>>>>Subject: [PATCH 5/9] sdap: store base dn in sdap_domain
> >>>>>>>
> >>>>>>>Groups may contain members from different domains. Remembering
> >>>>>>>base dn in domain object gives us the ability to simply lookup
> >>>>>>>correct domain by comparing object dn with domain base dn.
> >>>>>>>
> >>>>>>>Resolves: https://fedorahosted.org/sssd/ticket/2064
> >>>>>>
> >>>>>>I haven't tested these patches yet.
> >>>>>
> >>>>>I'm sending rebased version of my patches.
> >>>>>
> >>>>>[PATCH 4/9] sdap_domain_add(): fix possible memory leak was removed
> >>>>>from the patch set since recent Sumit's patch removed the code I
> >>>>>fixed :-)
> >>>>
> >>>>Hi,
> >>>>
> >>>>can you check if patches #6 and #7 still apply after the recent
> >>>>changes in 1.11 ? We actually do use the LDAP fallback now..
> >>>
> >>>They won't apply, but I think it is quite all right to just skip them.
> >>>The purpose of these patches was to always contact GC for get_group and
> >>>initgroups.
> >>>
> >>>We always contact GC first at the moment and having LDAP as fallback is
> >>>fine for groups. If there will be a member from different domain, we
> >>>will just fail - but if there won't be foreign member it will work.
> >>
> >>Hi,
> >>
> >>In general the patches look good to me but I think the first two can be
> >>simplified.
> >>
> >>Instead of magically trying to find the base DN from the entry's
> >>original DN, I think we can simplify it to:
> >>
> >>struct sdap_domain *
> >>sdap_domain_get_by_dn(struct sdap_options *opts,
> >>                       const char *dn)
> >>{
> >>     struct sdap_domain *sditer = NULL;
> >>
> >>     DLIST_FOR_EACH(sditer, opts->sdom) {
> >>         if (sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> >>sditer->search_bases, NULL) ||
> >>             sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> >>sditer->user_search_bases, NULL) ||
> >>             sss_ldap_dn_in_search_bases(tmp_ctx, dn,
> >>sditer->group_search_bases, NULL)) {
> >>             return sditer;
> >>         }
> >>     }
> >>
> >>     return NULL;
> >>}
> >>
> >>Then you won't need to store the base DN, no need for the magic with
> >>strstr
> >>and the code will work even for custom search bases.
> >
> >Hi,
> >new patches are attached. I just added all of the search bases.
> 
> And here's one more iteration. While working on token groups patches
> I found a bug in the first patch. I tried to shortcut using id
> mapping even if id mapping is disabled. This made the posix groups
> unresolvable since id mapping returned an error when trying to map
> GID to SID.
> 

Thanks. See also one more attached patch I applied on top of yours to
make cross-domain group resolution work. I'm still chasing one bug in
fill_members that might cause some members to be not qualified, though.
-------------- next part --------------
>From 20aa51439c5a598a9bbf51890642fc025cef92d4 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 25 Oct 2013 14:15:02 +0200
Subject: [PATCH] LDAP: Check all search bases during nested group processing

---
 src/providers/ldap/sdap_async_nested_groups.c | 55 ++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index fafbb36ba2f918787c3a862ef2098e11da5552da..01b3a002c374a32a48e749995ad58dc03196c6e6 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -392,6 +392,42 @@ done:
     return ret;
 }
 
+static bool
+sdap_nested_member_is_ent(struct sdap_nested_group_ctx *group_ctx,
+                          const char *dn, char **filter, bool is_user)
+{
+    struct sdap_domain *sditer = NULL;
+    bool ret;
+    struct sdap_search_base **search_bases;
+
+    DLIST_FOR_EACH(sditer, group_ctx->opts->sdom) {
+        search_bases = is_user ? sditer->user_search_bases : \
+                                 sditer->group_search_bases;
+
+        ret = sss_ldap_dn_in_search_bases(group_ctx, dn, search_bases,
+                                          filter);
+        if (ret == true) {
+            break;
+        }
+    }
+
+    return ret;
+}
+
+static inline bool
+sdap_nested_member_is_user(struct sdap_nested_group_ctx *group_ctx,
+                           const char *dn, char **filter)
+{
+    return sdap_nested_member_is_ent(group_ctx, dn, filter, true);
+}
+
+static inline bool
+sdap_nested_member_is_group(struct sdap_nested_group_ctx *group_ctx,
+                            const char *dn, char **filter)
+{
+    return sdap_nested_member_is_ent(group_ctx, dn, filter, false);
+}
+
 static errno_t
 sdap_nested_group_split_members(TALLOC_CTX *mem_ctx,
                                 struct sdap_nested_group_ctx *group_ctx,
@@ -475,13 +511,11 @@ sdap_nested_group_split_members(TALLOC_CTX *mem_ctx,
         /* try to determine type by dn */
         if (type == SDAP_NESTED_GROUP_DN_UNKNOWN) {
             /* user */
-            is_user = sss_ldap_dn_in_search_bases(tmp_ctx, dn,
-                           group_ctx->user_search_bases,
-                           &user_filter);
+            is_user = sdap_nested_member_is_user(group_ctx, dn,
+                                                 &user_filter);
 
-            is_group = sss_ldap_dn_in_search_bases(tmp_ctx, dn,
-                           group_ctx->group_search_bases,
-                           &group_filter);
+            is_group = sdap_nested_member_is_group(group_ctx, dn,
+                                                   &group_filter);
 
             if (is_user && is_group) {
                 /* search bases overlap */
@@ -2031,7 +2065,6 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
     size_t num_entries = 0;
     size_t i, j;
     bool member_found;
-    bool bret;
     errno_t ret;
 
     req = tevent_req_callback_data(subreq, struct tevent_req);
@@ -2110,9 +2143,7 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
             /* we found a user */
 
             /* skip the user if it is not amongst configured search bases */
-            bret = sss_ldap_dn_in_search_bases(state, orig_dn,
-                                               opts->sdom->user_search_bases, NULL);
-            if (!bret) {
+            if (!sdap_nested_member_is_user(state->group_ctx, orig_dn, NULL)) {
                 continue;
             }
 
@@ -2137,9 +2168,7 @@ sdap_nested_group_deref_direct_process(struct tevent_req *subreq)
             }
 
             /* skip the group if it is not amongst configured search bases */
-            bret = sss_ldap_dn_in_search_bases(state, orig_dn,
-                                               opts->sdom->group_search_bases, NULL);
-            if (!bret) {
+            if (!sdap_nested_member_is_group(state->group_ctx, orig_dn, NULL)) {
                 continue;
             }
 
-- 
1.8.3.1



More information about the sssd-devel mailing list