[SSSD] [PATCH] LDAP/AD: do not resolve group members during tokenGroups request

Sumit Bose sbose at redhat.com
Tue Mar 10 14:43:41 UTC 2015


Hi,

with this patch I hope to improve the initgroup-request performance
especially is the user is a member of many nested groups. Currently all
groups the user is a member of which are not in the cache are resolved
completely with all members and nested groups. With this patch only the
group object is looked up to determine the type of the group, POSIX GID
(if any), SID, ... Resolving all members is deferred until a request to
resolve this specific groups is received. This should speed up plain
getgrouplist() calls e.g. used by 'id -G'. Please note that this will
not improve the overall speed of a 'id' call without any options because
in this case after calling getgrouplist() 'id' will call getgrgid() for
every GID returned by getgrouplist(). So in this case all groups are
resolved completely as well. But the overall time is now better spread
over many request and chances for timeout should be much lower.

bye,
Sumid
-------------- next part --------------
From 9f1bafddd69e39712bc9900f76f263dd5c85a4f2 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 9 Mar 2015 16:36:29 +0100
Subject: [PATCH] LDAP/AD: do not resolve group members during tokenGroups
 request

During initgroups requests we try to avoid to resolve the complete
member list of groups if possible, e.g. if there are no nested groups.
The tokenGroups LDAP lookup return the complete list of memberships for
a user hence it is not necessary lookup the other group member and
un-roll nested groups. With this patch only the group entry is looked up
and saved as incomplete group to the cache.

This is achieved by adding a new boolean parameter no_members to
groups_get_send() and sdap_get_groups_send(). The difference to config
options like ldap_group_nesting_level = 0 or ignore_group_members is
that if no_members is set to true groups which are missing in the cache
are created a incomplete groups. As a result a request to lookup this
group will trigger a new LDAP request to resolve the group completely.
This way no information is ignored but the time needed to read all data
is better distributed between different requests.
---
 src/providers/ipa/ipa_subdomains_ext_groups.c |  2 +-
 src/providers/ldap/ldap_common.h              |  3 ++-
 src/providers/ldap/ldap_id.c                  | 14 +++++++----
 src/providers/ldap/sdap_async.h               |  3 ++-
 src/providers/ldap/sdap_async_enum.c          |  2 +-
 src/providers/ldap/sdap_async_groups.c        | 36 ++++++++++++++++++++++++++-
 src/providers/ldap/sdap_async_initgroups.c    | 14 +++++------
 src/providers/ldap/sdap_async_initgroups_ad.c |  2 +-
 src/providers/ldap/sdap_async_private.h       |  6 +++++
 9 files changed, 64 insertions(+), 18 deletions(-)

diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c b/src/providers/ipa/ipa_subdomains_ext_groups.c
index 92889c270fc95dc68bafa86f1091302aa833b70d..d487a58b8adffabe09ff50e31cb750b800b1d252 100644
--- a/src/providers/ipa/ipa_subdomains_ext_groups.c
+++ b/src/providers/ipa/ipa_subdomains_ext_groups.c
@@ -873,7 +873,7 @@ static void ipa_add_ad_memberships_get_next(struct tevent_req *req)
                                  state->sdap_id_ctx->conn,
                                  (const char *) val->data,
                                  BE_FILTER_NAME, BE_ATTR_CORE,
-                                 false);
+                                 false, false);
     if (subreq == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "groups_get_send failed.\n");
         ret = ENOMEM;
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index bf69489a79e903a98878edb53d372d2242df2b0f..57ad1b8458988d7e108f019c20f67bcde32539d4 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -212,7 +212,8 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
                                    const char *name,
                                    int filter_type,
                                    int attrs_type,
-                                   bool noexist_delete);
+                                   bool noexist_delete,
+                                   bool no_members);
 int groups_get_recv(struct tevent_req *req, int *dp_error_out, int *sdap_ret);
 
 struct tevent_req *ldap_netgroup_get_send(TALLOC_CTX *memctx,
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 4705714ade959d8373fb670d44b1d665db01973d..e60e3a53aefcc945922b8b175599f0b5ec54bb1d 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -529,6 +529,7 @@ struct groups_get_state {
     int dp_error;
     int sdap_ret;
     bool noexist_delete;
+    bool no_members;
 };
 
 static int groups_get_retry(struct tevent_req *req);
@@ -545,7 +546,8 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
                                    const char *name,
                                    int filter_type,
                                    int attrs_type,
-                                   bool noexist_delete)
+                                   bool noexist_delete,
+                                   bool no_members)
 {
     struct tevent_req *req;
     struct groups_get_state *state;
@@ -568,6 +570,7 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
     state->conn = conn;
     state->dp_error = DP_ERR_FATAL;
     state->noexist_delete = noexist_delete;
+    state->no_members = no_members;
 
     state->op = sdap_id_op_create(state, state->conn->conn_cache);
     if (!state->op) {
@@ -714,7 +717,8 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
 
     /* TODO: handle attrs_type */
     ret = build_attrs_from_map(state, ctx->opts->group_map, SDAP_OPTS_GROUP,
-                               state->domain->ignore_group_members ?
+                               (state->domain->ignore_group_members
+                                    || state->no_members) ?
                                    (const char **)member_filter : NULL,
                                &state->attrs, NULL);
 
@@ -846,7 +850,7 @@ static void groups_get_search(struct tevent_req *req)
                                   state->attrs, state->filter,
                                   dp_opt_get_int(state->ctx->opts->basic,
                                                  SDAP_SEARCH_TIMEOUT),
-                                  false);
+                                  false, state->no_members);
     if (!subreq) {
         tevent_req_error(req, ENOMEM);
         return;
@@ -1384,7 +1388,7 @@ sdap_handle_acct_req_send(TALLOC_CTX *mem_ctx,
                                  ar->filter_value,
                                  ar->filter_type,
                                  ar->attr_type,
-                                 noexist_delete);
+                                 noexist_delete, false);
         break;
 
     case BE_REQ_INITGROUPS: /* init groups for user */
@@ -1719,7 +1723,7 @@ static struct tevent_req *get_user_and_group_send(TALLOC_CTX *memctx,
     subreq = groups_get_send(req, state->ev, state->id_ctx,
                              state->sdom, state->conn,
                              state->filter_val, state->filter_type,
-                             state->attrs_type, state->noexist_delete);
+                             state->attrs_type, state->noexist_delete, false);
     if (subreq == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "users_get_send failed.\n");
         ret = ENOMEM;
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 1239f28c173373aac23c5796d694c7bd5ca24c96..ef9b3bbadba830bcf730b6fa70867c17d51380af 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -96,7 +96,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
                                        const char **attrs,
                                        const char *filter,
                                        int timeout,
-                                       bool enumeration);
+                                       bool enumeration,
+                                       bool no_members);
 int sdap_get_groups_recv(struct tevent_req *req,
                          TALLOC_CTX *mem_ctx, char **timestamp);
 
diff --git a/src/providers/ldap/sdap_async_enum.c b/src/providers/ldap/sdap_async_enum.c
index 242b3172f367b0b35738bd2e86ea927a4409d2d6..1cc09abdf1aa14e3d1690ea1abe32604ae4ff1cd 100644
--- a/src/providers/ldap/sdap_async_enum.c
+++ b/src/providers/ldap/sdap_async_enum.c
@@ -811,7 +811,7 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
                                   state->attrs, state->filter,
                                   dp_opt_get_int(state->ctx->opts->basic,
                                                  SDAP_ENUM_SEARCH_TIMEOUT),
-                                  true);
+                                  true, false);
     if (!subreq) {
         ret = ENOMEM;
         goto fail;
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 3d0dfba29bcd7b77c7351d04047fee1bbe1a5978..499cb438c9306d0daf8ee074bac97c033d72560c 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1740,6 +1740,7 @@ struct sdap_get_groups_state {
     char *filter;
     int timeout;
     bool enumeration;
+    bool no_members;
 
     char *higher_usn;
     struct sysdb_attrs **groups;
@@ -1769,7 +1770,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
                                        const char **attrs,
                                        const char *filter,
                                        int timeout,
-                                       bool enumeration)
+                                       bool enumeration,
+                                       bool no_members)
 {
     errno_t ret;
     struct tevent_req *req;
@@ -1792,6 +1794,7 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx,
     state->count = 0;
     state->timeout = timeout;
     state->enumeration = enumeration;
+    state->no_members = no_members;
     state->base_filter = filter;
     state->base_iter = 0;
     state->search_bases = sdom->group_search_bases;
@@ -1916,6 +1919,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     bool next_base = false;
     size_t count;
     struct sysdb_attrs **groups;
+    char **groupnamelist;
 
     ret = sdap_get_generic_recv(subreq, state,
                                 &count, &groups);
@@ -1982,6 +1986,36 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
         return;
     }
 
+    if (state->no_members) {
+        ret = sysdb_attrs_primary_name_list(state->sysdb, state,
+                                state->groups, state->count,
+                                state->opts->group_map[SDAP_AT_GROUP_NAME].name,
+                                &groupnamelist);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "sysdb_attrs_primary_name_list failed.\n");
+            tevent_req_error(req, ret);
+            return;
+        }
+
+        ret = sdap_add_incomplete_groups(state->sysdb, state->dom, state->opts,
+                                         groupnamelist, state->groups,
+                                         state->count);
+        if (ret == EOK) {
+            DEBUG(SSSDBG_TRACE_LIBS,
+                  "Reading only group data without members successful.\n");
+            tevent_req_done(req);
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "sdap_add_incomplete_groups failed.\n");
+            tevent_req_error(req, ret);
+        }
+        return;
+
+        ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
+                               state->groups, state->count, false,
+                               NULL, true, NULL);
+    }
+
     /* Check whether we need to do nested searches
      * for RFC2307bis/FreeIPA/ActiveDirectory
      * We don't need to do this for enumeration,
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 7a35e3c3e8575c777bfe5aa500653b7f5005976e..6596d9a76f36e5ab293149aad438c365e56a050c 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -29,12 +29,12 @@
 #include "providers/ldap/sdap_users.h"
 
 /* ==Save-fake-group-list=====================================*/
-static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
-                                          struct sss_domain_info *domain,
-                                          struct sdap_options *opts,
-                                          char **groupnames,
-                                          struct sysdb_attrs **ldap_groups,
-                                          int ldap_groups_count)
+errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
+                                   struct sss_domain_info *domain,
+                                   struct sdap_options *opts,
+                                   char **groupnames,
+                                   struct sysdb_attrs **ldap_groups,
+                                   int ldap_groups_count)
 {
     TALLOC_CTX *tmp_ctx;
     struct ldb_message *msg;
@@ -3157,7 +3157,7 @@ static void sdap_get_initgr_done(struct tevent_req *subreq)
 
     subreq = groups_get_send(req, state->ev, state->id_ctx,
                              state->id_ctx->opts->sdom, state->conn,
-                             gid, BE_FILTER_IDNUM, BE_ATTR_ALL, NULL);
+                             gid, BE_FILTER_IDNUM, BE_ATTR_ALL, false, false);
     if (!subreq) {
         ret = ENOMEM;
         goto fail;
diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
index 1b8c8d981ea14ac0fca0903f16296c8a6701c5dd..9915f1863f172d5d3f59afe03abbbfb87fdf3409 100644
--- a/src/providers/ldap/sdap_async_initgroups_ad.c
+++ b/src/providers/ldap/sdap_async_initgroups_ad.c
@@ -630,7 +630,7 @@ static errno_t sdap_ad_resolve_sids_step(struct tevent_req *req)
 
     subreq = groups_get_send(state, state->ev, state->id_ctx, sdap_domain,
                              state->conn, state->current_sid,
-                             BE_FILTER_SECID, BE_ATTR_CORE, false);
+                             BE_FILTER_SECID, BE_ATTR_CORE, false, true);
     if (subreq == NULL) {
         return ENOMEM;
     }
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index e689394c5db8a3385c333e6b98372c6f6d34366c..3995a2ac357c52f546696284d71d2127d0302409 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -132,4 +132,10 @@ errno_t sdap_nested_group_recv(TALLOC_CTX *mem_ctx,
                                unsigned long *_num_groups,
                                struct sysdb_attrs ***_groups);
 
+errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
+                                   struct sss_domain_info *domain,
+                                   struct sdap_options *opts,
+                                   char **groupnames,
+                                   struct sysdb_attrs **ldap_groups,
+                                   int ldap_groups_count);
 #endif /* _SDAP_ASYNC_PRIVATE_H_ */
-- 
2.1.0



More information about the sssd-devel mailing list