Hi,

On Thursday 09 September 2010 15:14:10 Ralf Haferkamp wrote:
[..]
> 
> I have started working on a patch to let sssd look up the non-cached
> users via LDAP (and save them into the cache). Find it attached. Note:
> That patch is not really complete (e.g. it doesn't handle rfc2307
> groups correctly). But before putting more effort into this I like to
> make sure that I am not trying to fix a "feature" here.

Find a newer version of my patch attached. Actually it's 3 patches now. 
Please review.

Patch1: This just adds a new flag to save_groups() to indicate that the 
   group's member attribute is already populated with the members' sysdb 
   DN (instead on LDAP DNs). As I need to lookup the group members in
   sysdb anyway, when processing the group, this saves some redundant
   sysdb lookups when storing the group.

Patch2: This is a somewhat improved version of my last patch. 
   - better error handling
   - limit the number of LDAP requests that are issued before
     starting to process the results. This is especially needed when 
     dealing with large groups, otherwise the server might choke on us
     (e.g. OpenLDAP has a (configurable) limit of 100 pending operations 
     per anonymous connection and 1000 per authenticated connection).
     OTOH sending multiple LDAP request at once will speed up things a
     bit compared to just sending the next request after processing the
     result of the previous.
   - populate the "member" attribute with the correct sysdb DNs to 
     utilize Patch1.
   - limit the group unrolling to rfc2307bis for now. rfc2307 and IPA
     need to be treated differently as discussed previously in this 
     thread.

Patch3: This adds a new config option to "ldap_unroll_group_members" to
   enable/disable group unrolling


regards,
        Ralf

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)
From 910f5c16ba1dda5f29a43af134683108d3d10ae3 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp <rha...@suse.de>
Date: Thu, 16 Sep 2010 17:24:17 +0200
Subject: [PATCH 1/3] Shortcut for save_group() to accept sysdb DNs as member attributes

Addtional parameter "sysdb_member_dns" for save_group() and save_groups()
to indicate that the "member" attribute of the groups is populated with
sysdb DNs of the members (instead of LDAP DNs).
---
 src/providers/ldap/sdap_async_accounts.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index 8999ba0..d1c6378 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -609,6 +609,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
                            struct sss_domain_info *dom,
                            struct sysdb_attrs *attrs,
                            bool store_members,
+                           bool sysdb_member_dns,
                            char **_timestamp)
 {
     struct ldb_message_element *el;
@@ -697,7 +698,19 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         }
     }
 
-    if (store_members) {
+    if (sysdb_member_dns) {
+        struct ldb_message_element *el1;
+        ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1);
+        if (ret != EOK) {
+            goto fail;
+        }
+        ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el);
+        if (ret != EOK) {
+            goto fail;
+        }
+        el->values = el1->values;
+        el->num_values = el1->num_values;
+    } else if (store_members) {
         ret = sysdb_attrs_get_el(attrs,
                         opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el);
         if (ret != EOK) {
@@ -808,6 +821,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
                             struct sdap_options *opts,
                             struct sysdb_attrs **groups,
                             int num_groups,
+                            bool sysdb_member_dns,
                             char **_timestamp)
 {
     TALLOC_CTX *tmpctx;
@@ -848,7 +862,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
         /* if 2 pass savemembers = false */
         ret = sdap_save_group(tmpctx, sysdb,
                               opts, dom, groups[i],
-                              (!twopass), &timestamp);
+                              (!twopass), sysdb_member_dns, &timestamp);
 
         /* Do not fail completely on errors.
          * Just report the failure to save and go on */
@@ -872,7 +886,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
         }
     }
 
-    if (twopass) {
+    if (twopass && !sysdb_member_dns) {
 
         for (i = 0; i < num_groups; i++) {
 
@@ -988,6 +1002,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     ret = sdap_save_groups(state, state->sysdb,
                            state->dom, state->opts,
                            state->groups, state->count,
+                           false,
                            &state->higher_timestamp);
     if (ret) {
         DEBUG(2, ("Failed to store groups.\n"));
@@ -1354,7 +1369,7 @@ static void sdap_initgr_nested_store(struct tevent_req *req)
     state = tevent_req_data(req, struct sdap_initgr_nested_state);
 
     ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
-                           state->groups, state->groups_cur, NULL);
+                           state->groups, state->groups_cur, false, NULL);
     if (ret) {
         tevent_req_error(req, ret);
         return;
-- 
1.7.1

From 39a3c4bb7e7871b07e782edcc23d2dc9bb740ad6 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp <rha...@suse.de>
Date: Thu, 16 Sep 2010 17:24:17 +0200
Subject: [PATCH 2/3] Return all group members from getgr(nam|gid)

getgrnam()/getgrgid() should return all group members instead of only those
which have already been cached (in sysdb). To achieve this every member
that is currently not in the cache is looked up via LDAP and saved to the
cache.
---
 src/providers/ldap/sdap_async_accounts.c |  339 +++++++++++++++++++++++++++++-
 1 files changed, 330 insertions(+), 9 deletions(-)

diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index d1c6378..90431dd 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -4,6 +4,7 @@
     Async LDAP Helper routines
 
     Copyright (C) Simo Sorce <sso...@redhat.com> - 2009
+    Copyright (C) 2010, Ralf Haferkamp <rha...@suse.de>, Novell Inc.
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -931,6 +932,25 @@ struct sdap_get_groups_state {
     char *higher_timestamp;
     struct sysdb_attrs **groups;
     size_t count;
+    size_t check_count;
+};
+
+struct sdap_process_group_state {
+    struct tevent_context *ev;
+    struct sdap_options *opts;
+    struct sdap_handle *sh;
+    struct sss_domain_info *dom;
+    struct sysdb_ctx *sysdb;
+
+    struct sysdb_attrs *group;
+    struct sysdb_attrs **new_members;
+    struct ldb_message_element* sysdb_dns;
+    char **queued_members;
+    const char **attrs;
+    const char *filter;
+    size_t queue_idx;
+    size_t count;
+    size_t check_count;
 };
 
 static void sdap_get_groups_process(struct tevent_req *subreq);
@@ -976,13 +996,25 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
     return req;
 }
 
+static void sdap_groupmember_process(struct tevent_req *subreq);
+static void sdap_group_process(struct tevent_req *subreq);
+
+#define GROUPMEMBER_REQ_PARALLEL 50
 static void sdap_get_groups_process(struct tevent_req *subreq)
 {
     struct tevent_req *req = tevent_req_callback_data(subreq,
                                                       struct tevent_req);
     struct sdap_get_groups_state *state = tevent_req_data(req,
                                             struct sdap_get_groups_state);
-    int ret;
+    struct tevent_req *process_member_req;
+    struct tevent_req *process_grp_req;
+    struct ldb_message_element *el;
+    struct sdap_process_group_state *grp_state;
+    const char **attrs;
+    char *filter;
+    int ret,i,k;
+    int queue_len;
+    int num_groups;
 
     ret = sdap_get_generic_recv(subreq, state,
                                 &state->count, &state->groups);
@@ -999,20 +1031,309 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
         return;
     }
 
-    ret = sdap_save_groups(state, state->sysdb,
-                           state->dom, state->opts,
-                           state->groups, state->count,
-                           false,
-                           &state->higher_timestamp);
+    state->check_count = state->count;
+
+    ret = build_attrs_from_map(state, state->opts->user_map, SDAP_OPTS_USER,
+                               &attrs);
     if (ret) {
-        DEBUG(2, ("Failed to store groups.\n"));
         tevent_req_error(req, ret);
         return;
     }
 
-    DEBUG(9, ("Saving %d Groups - Done\n", state->count));
+    /* FIXME: we ignore nested rfc2307bis groups for now */
+    filter = talloc_asprintf(state, "(objectclass=%s)",
+                             state->opts->user_map[SDAP_OC_USER].name);
+    if (!filter) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
 
-    tevent_req_done(req);
+    for (i=0, num_groups=state->count; i < num_groups; i++) {
+        ret = sysdb_attrs_get_el(state->groups[i],
+                state->opts->group_map[SDAP_AT_GROUP_NAME].sys_name, &el);
+        if (ret || el->num_values == 0) {
+            tevent_req_error(req, ENOENT);
+            return;
+        }
+
+        DEBUG(2, ("Processing Group %s\n", (const char*)el->values[0].data));
+        process_grp_req = tevent_req_create(state, &grp_state,
+                                            struct sdap_process_group_state);
+        if (!process_grp_req) {
+            tevent_req_error(req, ENOMEM);
+            return;
+        }
+        grp_state->ev = state->ev;
+        grp_state->opts = state->opts;
+        grp_state->dom = state->dom;
+        grp_state->sh = state->sh;
+        grp_state->sysdb = state->sysdb;
+        grp_state->group =  state->groups[i];
+        grp_state->check_count = 0;
+        grp_state->new_members = NULL;
+        grp_state->queue_idx = 0;
+        grp_state->queued_members = NULL;
+        grp_state->filter = filter;
+        grp_state->attrs = attrs;
+        tevent_req_set_callback(process_grp_req, sdap_group_process, req);
+
+        ret = sysdb_attrs_get_el(state->groups[i],
+                state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el);
+        if (ret) {
+            tevent_req_error(req, ENOENT);
+            return;
+        }
+
+        /* Group without members */
+        if ( el->num_values == 0 ) {
+            tevent_req_done(process_grp_req);
+            return;
+        }
+
+        grp_state->sysdb_dns = talloc(state, struct ldb_message_element);
+        if (!grp_state->sysdb_dns) {
+            tevent_req_error(req, ENOMEM);
+            return;
+        }
+        grp_state->sysdb_dns->values = talloc_array(state, struct ldb_val , el->num_values);
+        if (!grp_state->sysdb_dns->values) {
+            tevent_req_error(req, ENOMEM);
+            return;
+        }
+        grp_state->sysdb_dns->num_values = 0;
+        queue_len=0;
+        /*
+         * For each member check if it is already present in sysdb,
+         * if it isn't read it from LDAP
+         */
+        for (k=0; k < el->num_values; k++) {
+            char *sysdb_dn;
+            DEBUG(7, ("checking member: %s\n", (const char*)el->values[k].data));
+            ret = sdap_find_entry_by_origDN(state, state->sysdb,
+                                            state->dom,
+                                            (const char *)el->values[k].data,
+                                            &sysdb_dn);
+            if (ret == ENOENT) {
+                if (state->opts->schema_type != SDAP_SCHEMA_RFC2307BIS)
+                    continue;
+
+                DEBUG(7, ("member #%d (%s): not found in sysdb, searching LDAP\n",
+                        k, (char *)el->values[k].data));
+
+                /*
+                 * Issue at most GROUPMEMBER_REQ_PARALLEL LDAP searches at once.
+                 * The rest is sent while the the results are being processed.
+                 * We limit the number as of request here, as the Server might
+                 * enforce limits on the number of pending operations per connection.
+                 */
+                if (grp_state->check_count > GROUPMEMBER_REQ_PARALLEL ) {
+                    DEBUG(7, (" queueing search for: %s\n",
+                             (char*)el->values[k].data));
+                    if (! grp_state->queued_members ) {
+                        DEBUG(7,("Allocating queue for %d members\n",
+                                el->num_values - grp_state->check_count));
+                        grp_state->queued_members = talloc_array(grp_state, char*,
+                                el->num_values - grp_state->check_count+1);
+                        if (!grp_state->queued_members) {
+                            tevent_req_error(req, ENOMEM);
+                            return;
+                        }
+                    }
+                    grp_state->queued_members[queue_len]=(char*)el->values[k].data;
+                    queue_len++;
+                } else {
+                    process_member_req = sdap_get_generic_send(grp_state,
+                                                grp_state->ev,
+                                                grp_state->opts,
+                                                grp_state->sh,
+                                                (char *)el->values[k].data,
+                                                LDAP_SCOPE_BASE,
+                                                filter,
+                                                attrs,
+                                                grp_state->opts->user_map,
+                                                SDAP_OPTS_USER);
+                    if (!process_member_req) {
+                        tevent_req_error(req, ENOMEM);
+                        return;
+                    }
+                    tevent_req_set_callback(process_member_req, sdap_groupmember_process,
+                                            process_grp_req);
+                }
+                grp_state->check_count++;
+            } else {
+                /*
+                 * User already cached in sysdb. Remember the sysdb DN for later use
+                 * by sdap_save_groups()
+                 */
+                DEBUG(7,("sysdbdn: %s\n", sysdb_dn));
+                grp_state->sysdb_dns->values[grp_state->sysdb_dns->num_values].data =
+                        (uint8_t*)sysdb_dn;
+                grp_state->sysdb_dns->values[grp_state->sysdb_dns->num_values].length =
+                        strlen(sysdb_dn);
+                grp_state->sysdb_dns->num_values++;
+            }
+        }
+        if (queue_len > 0)
+            grp_state->queued_members[queue_len]=NULL;
+
+        if (grp_state->check_count == 0) {
+            /*
+             * All group members are already cached in sysdb, we are done
+             * with this group. To avoid redundant sysdb lookups, populate the
+             * "member" attribute of the group entry with the sysdb DNs of
+             * the members.
+             */
+            el->values = grp_state->sysdb_dns->values;
+            el->num_values = grp_state->sysdb_dns->num_values;
+            tevent_req_done(process_grp_req);
+        } else {
+            grp_state->count = grp_state->check_count;
+            grp_state->new_members = talloc_zero_array(grp_state,
+                                                       struct sysdb_attrs *,
+                                                       grp_state->count + 1);
+            if (!grp_state->new_members) {
+                tevent_req_error(req, ENOMEM);
+                return;
+            }
+        }
+    }
+}
+
+static void sdap_groupmember_process(struct tevent_req *subreq)
+{
+    struct sysdb_attrs **usr_attrs;
+    size_t count;
+    int ret;
+    struct tevent_req *req = tevent_req_callback_data(subreq,
+                                                      struct tevent_req);
+    struct sdap_process_group_state *state = tevent_req_data(req,
+            struct sdap_process_group_state);
+    struct ldb_message_element *el;
+    struct ldb_dn *dn;
+    char* dn_string;
+
+    /* Are there more searches for uncached users to submit ? */
+    if ( state->queued_members && state->queued_members[state->queue_idx] ) {
+        struct tevent_req *process_member_req;
+        process_member_req = sdap_get_generic_send(state,
+                                    state->ev,
+                                    state->opts,
+                                    state->sh,
+                                    state->queued_members[state->queue_idx],
+                                    LDAP_SCOPE_BASE,
+                                    state->filter,
+                                    state->attrs,
+                                    state->opts->user_map,
+                                    SDAP_OPTS_USER);
+        if (!process_member_req) {
+            tevent_req_error(req, ENOMEM);
+            return;
+        }
+
+        tevent_req_set_callback(process_member_req, sdap_groupmember_process,
+                                req);
+        state->queue_idx++;
+    }
+    state->check_count--;
+    DEBUG(9, ("Members remaining: %d\n", state->check_count));
+
+    ret = sdap_get_generic_recv(subreq, state, &count, &usr_attrs);
+    talloc_zfree(subreq);
+    if (ret) {
+        tevent_req_error(req, ret);
+        return;
+    }
+
+    if (count != 1) {
+        DEBUG(2, ("Expected one user entry and got %d\n", count));
+        tevent_req_error(req, ENOENT);
+        return;
+    }
+    ret = sysdb_attrs_get_el(usr_attrs[0],
+            state->opts->user_map[SDAP_AT_USER_NAME].sys_name, &el);
+    if (el->num_values == 0) {
+        ret = EINVAL;
+    }
+    if (ret) {
+        DEBUG(1, ("Failed to get the member's name\n"));
+        tevent_req_error(req, ret);
+        return;
+    }
+
+    /*
+     * Convert the just received DN into the corresponding sysdb DN
+     * for later usage by sdap_save_groups()
+     */
+    dn = sysdb_user_dn(state->sysdb, state, state->dom->name,
+                       (char*)el[0].values[0].data);
+    if (!dn) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+
+    dn_string = ldb_dn_alloc_linearized(state->group, dn);
+    if (!dn_string) {
+        tevent_req_error(req, ENOMEM);
+        return;
+    }
+
+    state->sysdb_dns->values[state->sysdb_dns->num_values].data =
+            (uint8_t*)dn_string;
+    state->sysdb_dns->values[state->sysdb_dns->num_values].length =
+            strlen(dn_string);
+    state->sysdb_dns->num_values++;
+
+    state->new_members[ state->check_count ] = usr_attrs[0];
+
+    if (state->check_count == 0) {
+        ret = sdap_save_users(state, state->sysdb, state->dom, state->opts,
+                              state->new_members, state->count, NULL);
+        if (ret) {
+            DEBUG(2, ("Failed to store users.\n"));
+            tevent_req_error(req, ret);
+            return;
+        }
+
+        /*
+         * To avoid redundant sysdb lookups, populate the "member" attribute
+         * of the group entry with the sysdb DNs of the members.
+         */
+        ret = sysdb_attrs_get_el(state->group,
+            state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el);
+        el->values = state->sysdb_dns->values;
+        el->num_values = state->sysdb_dns->num_values;
+        DEBUG(9, ("Processed Group - Done\n"));
+        tevent_req_done(req);
+    }
+}
+
+static void sdap_group_process(struct tevent_req *subreq)
+{
+    struct tevent_req *req = tevent_req_callback_data(subreq,
+                                                      struct tevent_req);
+    struct sdap_get_groups_state *state = tevent_req_data(req,
+            struct sdap_get_groups_state);
+
+    state->check_count--;
+    DEBUG(9, ("Groups remaining: %d\n", state->check_count));
+
+    talloc_zfree(subreq);
+
+    if (state->check_count == 0) {
+        int ret;
+        DEBUG(9, ("All groups processed\n"));
+
+        ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
+                               state->groups, state->count, true,
+                               &state->higher_timestamp);
+        if (ret) {
+            DEBUG(2, ("Failed to store groups.\n"));
+            tevent_req_error(req, ret);
+            return;
+        }
+        DEBUG(9, ("Saving %d Groups - Done\n", state->count));
+        tevent_req_done(req);
+    }
 }
 
 int sdap_get_groups_recv(struct tevent_req *req,
-- 
1.7.1

From c39a7324cfd1a73e6994bf4fcb737f89940085e5 Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp <rha...@suse.de>
Date: Thu, 16 Sep 2010 17:24:17 +0200
Subject: [PATCH 3/3] New config option to disable group unrolling

Adds the new sssd.conf switch "ldap_unroll_group_members" (defaults to
"true") for enabling/disabling the unrolling of group members during
getgrgid/getgrnam calls.
---
 src/providers/ldap/ldap_common.c         |    1 +
 src/providers/ldap/sdap.h                |    1 +
 src/providers/ldap/sdap_async_accounts.c |    3 ++-
 3 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 14bdd28..23a0ccc 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -48,6 +48,7 @@ struct dp_option default_basic_opts[] = {
     { "ldap_group_search_scope", DP_OPT_STRING, { "sub" }, NULL_STRING },
     { "ldap_group_search_filter", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ldap_schema", DP_OPT_STRING, { "rfc2307" }, NULL_STRING },
+    { "ldap_unroll_group_members", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER },
     { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_enumeration_refresh_timeout", DP_OPT_NUMBER, { .number = 300 }, NULL_NUMBER },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 4426dac..7093b7f 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -155,6 +155,7 @@ enum sdap_basic_opt {
     SDAP_GROUP_SEARCH_SCOPE,
     SDAP_GROUP_SEARCH_FILTER,
     SDAP_SCHEMA,
+    SDAP_UNROLL_GROUP_MEMBERS,
     SDAP_OFFLINE_TIMEOUT,
     SDAP_FORCE_UPPER_CASE_REALM,
     SDAP_ENUM_REFRESH_TIMEOUT,
diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index 90431dd..598bd5a 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -1114,7 +1114,8 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
                                             (const char *)el->values[k].data,
                                             &sysdb_dn);
             if (ret == ENOENT) {
-                if (state->opts->schema_type != SDAP_SCHEMA_RFC2307BIS)
+                if ((state->opts->schema_type != SDAP_SCHEMA_RFC2307BIS) ||
+                    (! dp_opt_get_bool(state->opts->basic, SDAP_UNROLL_GROUP_MEMBERS)))
                     continue;
 
                 DEBUG(7, ("member #%d (%s): not found in sysdb, searching LDAP\n",
-- 
1.7.1

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to