Hi,
I'm sending the first version of the patch solving issues with nested groups 
without GID. The patch is definitely not final, let's say it's the first draft 
of final patch. It depends on Jakub's recent series of rfc2307bis patches and 
it won't be final until they are pushed.

At this moment I'd like to hear your opinion on the concept of the patch.

Thanks in advance
Jan
From 9266a27184cb74f1a427f795b917546a7a8a4f89 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Wed, 20 Apr 2011 06:43:31 -0400
Subject: [PATCH 2/3] AD group processing

---
 src/db/sysdb.h                           |    3 +-
 src/db/sysdb_ops.c                       |   41 +++++----
 src/providers/ldap/ldap_id_cleanup.c     |   21 ++---
 src/providers/ldap/sdap_async_accounts.c |  132 +++++++++++++++++++++--------
 src/responder/nss/nsssrv_cmd.c           |   14 +++-
 5 files changed, 141 insertions(+), 70 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 190e8120e04748dd4d8c73074df68f15ab51b546..434181dc5dc3ce5454dbcbcbe2019751d3d26be0 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -62,6 +62,7 @@
 
 #define SYSDB_MEMBER "member"
 #define SYSDB_MEMBERUID "memberUid"
+#define SYSDB_POSIX "posixGroup"
 
 #define SYSDB_DEFAULTGROUP "defaultGroup"
 #define SYSDB_GECOS "gecos"
@@ -516,7 +517,7 @@ int sysdb_add_incomplete_group(struct sysdb_ctx *ctx,
                                struct sss_domain_info *domain,
                                const char *name,
                                gid_t gid,
-                               const char *original_dn);
+                               const char *original_dn, bool posix);
 
 /* Add netgroup (only basic attrs and w/o checks) */
 int sysdb_add_basic_netgroup(struct sysdb_ctx *ctx,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 1fb389da9db50020098d696200acac26905f3095..a6bdc1c7e64b98b7d68972625bdb010e760ed603 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1062,6 +1062,8 @@ int sysdb_add_group(TALLOC_CTX *mem_ctx,
     uint32_t id;
     time_t now;
     int ret;
+    int tmp_ret;
+    const char *posix = NULL;
 
     if (domain->id_max != 0 && gid != 0 &&
         (gid < domain->id_min || gid > domain->id_max)) {
@@ -1110,30 +1112,29 @@ int sysdb_add_group(TALLOC_CTX *mem_ctx,
     ret = sysdb_add_basic_group(tmpctx, ctx, domain, name, gid);
     if (ret) goto done;
 
-    if (gid == 0) {
-        ret = sysdb_get_new_id(tmpctx, ctx, domain, &id);
-        if (ret) goto done;
-
+    if (!attrs) {
+        attrs = sysdb_new_attrs(tmpctx);
         if (!attrs) {
-            attrs = sysdb_new_attrs(tmpctx);
-            if (!attrs) {
-                ret = ENOMEM;
-                goto done;
-            }
+            ret = ENOMEM;
+            goto done;
         }
+    }
+
+    ret = sysdb_attrs_get_string(attrs, SYSDB_POSIX, &posix);
+    if (ret == ENOENT) {
+        tmp_ret = sysdb_attrs_add_bool(attrs, SYSDB_POSIX, true);
+        if (tmp_ret) goto done;
+    }
+
+    if (((ret == EOK && strcmp(posix, "TRUE") == 0) || ret == ENOENT)
+        && gid == 0) {
+        ret = sysdb_get_new_id(tmpctx, ctx, domain, &id);
+        if (ret) goto done;
 
         ret = sysdb_attrs_add_uint32(attrs, SYSDB_GIDNUM, id);
         if (ret) goto done;
     }
 
-    if (!attrs) {
-        attrs = sysdb_new_attrs(tmpctx);
-        if (!attrs) {
-            ret = ENOMEM;
-            goto done;
-        }
-    }
-
     now = time(NULL);
 
     ret = sysdb_attrs_add_time_t(attrs, SYSDB_LAST_UPDATE, now);
@@ -1163,7 +1164,8 @@ int sysdb_add_incomplete_group(struct sysdb_ctx *ctx,
                                struct sss_domain_info *domain,
                                const char *name,
                                gid_t gid,
-                               const char *original_dn)
+                               const char *original_dn,
+                               bool posix)
 {
     TALLOC_CTX *tmpctx;
     time_t now;
@@ -1194,6 +1196,9 @@ int sysdb_add_incomplete_group(struct sysdb_ctx *ctx,
                                  now-1);
     if (ret) goto done;
 
+    ret = sysdb_attrs_add_bool(attrs, SYSDB_POSIX, posix);
+    if (ret) goto done;
+
     if (original_dn) {
         ret = sysdb_attrs_add_string(attrs, SYSDB_ORIG_DN, original_dn);
         if (ret) goto done;
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c
index 7f7a02c64bb54314363da459acc8e7e34d8850c4..f424d2a7f932565f1bf8b9cc6be137ff192e74ec 100644
--- a/src/providers/ldap/ldap_id_cleanup.c
+++ b/src/providers/ldap/ldap_id_cleanup.c
@@ -434,18 +434,17 @@ static int cleanup_groups(TALLOC_CTX *memctx,
         }
 
         gid = (gid_t) ldb_msg_find_attr_as_uint(msgs[i], SYSDB_GIDNUM, 0);
-        if (!gid) {
-            DEBUG(2, ("Entry has no GID\n"));
-            ret = EIO;
-            goto done;
+        if (gid) {
+            /* Search for users that are members of this group, or
+             * that have this group as their primary GID
+             */
+            subfilter = talloc_asprintf(tmpctx, "(|(%s=%s)(%s=%lu))",
+                                        SYSDB_MEMBEROF, dn,
+                                        SYSDB_GIDNUM, (long unsigned) gid);
+        } else {
+            subfilter = talloc_asprintf(tmpctx, "(%s=%s)",
+                                        SYSDB_MEMBEROF, dn);
         }
-
-        /* Search for users that are members of this group, or
-         * that have this group as their primary GID
-         */
-        subfilter = talloc_asprintf(tmpctx, "(|(%s=%s)(%s=%lu))",
-                                    SYSDB_MEMBEROF, dn,
-                                    SYSDB_GIDNUM, (long unsigned) gid);
         if (!subfilter) {
             DEBUG(2, ("Failed to build filter\n"));
             ret = ENOMEM;
diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index c14dab9a8ffe421e0588e1e699eb536b1e540c3a..6f843744ec7a1b201e9bfdbd9d8aa3795666832e 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -679,6 +679,8 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     int ret;
     char *usn_value = NULL;
     TALLOC_CTX *tmpctx = NULL;
+    const char *posix_group_str;
+    bool posix_group;
 
     tmpctx = talloc_new(memctx);
     if (!tmpctx) {
@@ -700,6 +702,22 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         goto fail;
     }
 
+    ret = sysdb_attrs_get_string(attrs, SYSDB_POSIX, &posix_group_str);
+    if (ret != EOK && ret != ENOENT) {
+        DEBUG(1, ("Failed to save the group: couldn't retrieve posixGroup\n"));
+        goto fail;
+    } else if (ret == ENOENT) {
+        posix_group = true;
+    } else {
+        posix_group = (strcmp(posix_group_str,"TRUE") == 0);
+    }
+
+    DEBUG(8, ("This is%s a posix group", (posix_group)?"":" not"));
+    ret = sysdb_attrs_add_bool(group_attrs, SYSDB_POSIX, posix_group);
+    if (ret != EOK) {
+        goto fail;
+    }
+
     ret = sysdb_attrs_get_uint32_t(attrs,
                                    opts->group_map[SDAP_AT_GROUP_GID].sys_name,
                                    &gid);
@@ -711,7 +729,8 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     }
 
     /* check that the gid is valid for this domain */
-    if (OUT_OF_ID_RANGE(gid, dom->id_min, dom->id_max)) {
+    if ((posix_group || gid != 0) &&
+        OUT_OF_ID_RANGE(gid, dom->id_min, dom->id_max)) {
             DEBUG(2, ("Group [%s] filtered out! (id out of range)\n",
                       name));
         ret = EINVAL;
@@ -1642,7 +1661,7 @@ static struct tevent_req *sdap_nested_group_process_send(
         struct sysdb_ctx *sysdb, struct sysdb_attrs *group,
         hash_table_t *users, hash_table_t *groups,
         struct sdap_options *opts, struct sdap_handle *sh,
-        uint32_t nesting);
+        uint32_t nesting, bool posix);
 static void sdap_nested_done(struct tevent_req *req);
 static errno_t sdap_nested_group_process_recv(struct tevent_req *req);
 static void sdap_get_groups_process(struct tevent_req *subreq)
@@ -1703,7 +1722,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
                                                 state->group_hash,
                                                 state->opts,
                                                 state->sh,
-                                                0);
+                                                0, true);
         if (!subreq) {
             tevent_req_error(req, EIO);
             return;
@@ -2046,6 +2065,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
     gid_t gid;
     int ret;
     bool in_transaction = false;
+    bool posix;
 
     /* There are no groups in LDAP but we should add user to groups ?? */
     if (ldap_groups_count == 0) return EOK;
@@ -2105,11 +2125,15 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
             }
 
             if (strcmp(name, missing[i]) == 0) {
+                posix = true;
                 ret = sysdb_attrs_get_uint32_t(ldap_groups[ai],
                                                SYSDB_GIDNUM,
                                                &gid);
-                if (ret) {
-                    DEBUG(1, ("The GID attribute is missing or malformed\n"));
+                if (ret == ENOENT) {
+                    gid = 0;
+                    posix = false;
+                } else if (ret) {
+                    DEBUG(1, ("The GID attribute is malformed\n"));
                     goto fail;
                 }
 
@@ -2123,7 +2147,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
 
                 DEBUG(8, ("Adding fake group %s to sysdb\n", name));
                 ret = sysdb_add_incomplete_group(sysdb, dom, name,
-                                                 gid, original_dn);
+                                                 gid, original_dn, posix);
                 if (ret != EOK) {
                     goto fail;
                 }
@@ -2459,10 +2483,9 @@ static struct tevent_req *sdap_initgr_nested_send(TALLOC_CTX *memctx,
         return NULL;
     }
 
-    state->filter = talloc_asprintf(state, "(&(objectclass=%s)(%s=*)(%s=*))",
+    state->filter = talloc_asprintf(state, "(&(objectclass=%s)(%s=*))",
                                     opts->group_map[SDAP_OC_GROUP].name,
-                                    opts->group_map[SDAP_AT_GROUP_NAME].name,
-                                    opts->group_map[SDAP_AT_GROUP_GID].name);
+                                    opts->group_map[SDAP_AT_GROUP_NAME].name);
     if (!state->filter) {
         talloc_zfree(req);
         return NULL;
@@ -3309,6 +3332,7 @@ struct sdap_nested_group_ctx {
     char *member_dn;
 
     struct sdap_deref_ctx *derefctx;
+    bool posix_member_group;
 };
 
 static errno_t sdap_nested_group_process_deref_step(struct tevent_req *req);
@@ -3320,7 +3344,7 @@ static struct tevent_req *sdap_nested_group_process_send(
         struct sysdb_ctx *sysdb, struct sysdb_attrs *group,
         hash_table_t *users, hash_table_t *groups,
         struct sdap_options *opts, struct sdap_handle *sh,
-        uint32_t nesting)
+        uint32_t nesting, bool posix)
 {
     errno_t ret;
     int hret;
@@ -3329,6 +3353,7 @@ static struct tevent_req *sdap_nested_group_process_send(
     const char *groupname;
     hash_key_t key;
     hash_value_t value;
+    gid_t gid;
 
     req = tevent_req_create(mem_ctx, &state, struct sdap_nested_group_ctx);
     if (!req) {
@@ -3343,6 +3368,8 @@ static struct tevent_req *sdap_nested_group_process_send(
     state->opts = opts;
     state->sh = sh;
     state->nesting_level = nesting;
+    /* When inspecting group members, first assume that they are posix-compatible */
+    state->posix_member_group = true;
 
     /* If this is too many levels deep, just return success */
     if (nesting > dp_opt_get_int(opts->basic, SDAP_NESTING_LEVEL)) {
@@ -3377,6 +3404,31 @@ static struct tevent_req *sdap_nested_group_process_send(
         goto immediate;
     }
 
+    if (!posix) {
+        ret = sysdb_attrs_get_uint32_t(group,
+                                       opts->user_map[SDAP_AT_GROUP_GID].name,
+                                       &gid);
+        if (ret != EOK) {
+            DEBUG(8, ("Marking group as non-posix and setting GID=0!\n"));
+            ret = sysdb_attrs_add_uint32(group,
+                                         opts->user_map[SDAP_AT_GROUP_GID].name,
+                                         0);
+            if (ret != EOK) {
+                DEBUG(1, ("Failed to add a GID to non-posix group!\n"));
+                goto immediate;
+            }
+
+            ret = sysdb_attrs_add_bool(group, SYSDB_POSIX, false);
+            if (ret != EOK) {
+                DEBUG(2, ("Error: Failed to mark group as non-posix!\n"));
+            }
+        } else {
+            /* This should never happen, but let's do the check to be sure */
+            DEBUG(1, ("Non-posix group has a GID!\n"));
+            goto immediate;
+        }
+    }
+
     value.type = HASH_VALUE_PTR;
     value.ptr = talloc_steal(groups, group);
 
@@ -3979,10 +4031,9 @@ static errno_t sdap_nested_group_lookup_group(struct tevent_req *req)
     }
 
     filter = talloc_asprintf(
-            sdap_attrs, "(&(objectclass=%s)(%s=*)(%s=*))",
+            sdap_attrs, "(&(objectclass=%s)(%s=*))",
             state->opts->group_map[SDAP_OC_GROUP].name,
-            state->opts->group_map[SDAP_AT_GROUP_NAME].name,
-            state->opts->group_map[SDAP_AT_GROUP_GID].name);
+            state->opts->group_map[SDAP_AT_GROUP_NAME].name);
     if (!filter) {
         talloc_free(sdap_attrs);
         return ENOMEM;
@@ -4131,7 +4182,8 @@ static void sdap_nested_group_process_group(struct tevent_req *subreq)
                                             state->sysdb, replies[0],
                                             state->users, state->groups,
                                             state->opts, state->sh,
-                                            state->nesting_level + 1);
+                                            state->nesting_level + 1,
+                                            state->posix_member_group);
     if (!subreq) {
         tevent_req_error(req, EIO);
         goto done;
@@ -4142,26 +4194,30 @@ static void sdap_nested_group_process_group(struct tevent_req *subreq)
     return;
 
 skip:
-    if (state->derefctx) {
-        state->derefctx->expired_groups_index++;
-        ret = sdap_nested_group_process_noderef(req);
+    if (state->posix_member_group) {
+        state->posix_member_group = false;
+        ret = sdap_nested_group_lookup_group(req);
+        if (ret != EOK) {
+            tevent_req_error(req, ret);
+        }
     } else {
-        state->member_index++;
-        talloc_zfree(state->member_dn);
-        ret = sdap_nested_group_process_step(req);
-    }
+        if (state->derefctx) {
+            state->derefctx->expired_groups_index++;
+            ret = sdap_nested_group_process_noderef(req);
+        } else {
+            state->member_index++;
+            talloc_zfree(state->member_dn);
+            ret = sdap_nested_group_process_step(req);
+        }
 
-    if (ret == EOK) {
-        /* EOK means it's complete */
-        tevent_req_done(req);
-    } else if (ret != EAGAIN) {
-        tevent_req_error(req, ret);
+        if (ret == EOK) {
+            /* EOK means it's complete */
+            tevent_req_done(req);
+        } else if (ret != EAGAIN) {
+            tevent_req_error(req, ret);
+        }
     }
 
-    /* EAGAIN means that we should re-enter
-     * the mainloop
-     */
-
 done:
     talloc_free(tmp_ctx);
 }
@@ -4391,7 +4447,8 @@ sdap_nested_group_process_deref_result(struct tevent_req *req)
                                             dctx->deref_result[dctx->result_index],
                                             state->users, state->groups,
                                             state->opts, state->sh,
-                                            state->nesting_level + 1);
+                                            state->nesting_level + 1,
+                                            state->posix_member_group);
                 if (!subreq) return EIO;
 
                 tevent_req_set_callback(subreq,
@@ -4485,12 +4542,11 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send(
         return NULL;
     }
 
-    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
+    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s)(%s=*))",
                              opts->group_map[SDAP_AT_GROUP_MEMBER].name,
                              clean_orig_dn,
                              opts->group_map[SDAP_OC_GROUP].name,
-                             opts->group_map[SDAP_AT_GROUP_NAME].name,
-                             opts->group_map[SDAP_AT_GROUP_GID].name);
+                             opts->group_map[SDAP_AT_GROUP_NAME].name);
     if (!filter) {
         talloc_zfree(req);
         return NULL;
@@ -4682,10 +4738,12 @@ errno_t save_rfc2307bis_user_memberships(
     ret = diff_string_lists(tmp_ctx,
                             ldap_grouplist, sysdb_grouplist,
                             &add_groups, &del_groups, NULL);
+
     if (ret != EOK) {
         goto error;
     }
 
+    DEBUG(8, ("Updating memberships for %s\n", state->name));
     ret = sysdb_update_members(state->sysdb, state->dom, state->name,
                                SYSDB_MEMBER_USER,
                                (const char *const *)add_groups,
@@ -4896,12 +4954,11 @@ static errno_t rfc2307bis_nested_groups_step(struct tevent_req *req)
     }
 
     filter = talloc_asprintf(
-            tmp_ctx, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
+            tmp_ctx, "(&(%s=%s)(objectclass=%s)(%s=*))",
             state->opts->group_map[SDAP_AT_GROUP_MEMBER].name,
             clean_orig_dn,
             state->opts->group_map[SDAP_OC_GROUP].name,
-            state->opts->group_map[SDAP_AT_GROUP_NAME].name,
-            state->opts->group_map[SDAP_AT_GROUP_GID].name);
+            state->opts->group_map[SDAP_AT_GROUP_NAME].name);
     if (!filter) {
         ret = ENOMEM;
         goto error;
@@ -5180,6 +5237,7 @@ static errno_t rfc2307bis_nested_groups_update_sysdb(
     talloc_free(ldap_grouplist);
     talloc_free(sysdb_grouplist);
 
+    DEBUG(8, ("Updating memberships for %s\n", name));
     ret = sysdb_update_members(state->sysdb, state->dom, name,
                                SYSDB_MEMBER_GROUP,
                                (const char *const *)add_groups,
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index db301b380892bc0dde1bf27aa47a308d09ac1f38..b0dd4fcc784762ec7712d75f3906cc79def4fd9f 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -2745,6 +2745,8 @@ static int fill_initgr(struct sss_packet *packet, struct ldb_result *res)
     size_t blen;
     gid_t gid;
     int ret, i, num;
+    int skipped = 0;
+    bool posix;
 
     if (res->count == 0) {
         return ENOENT;
@@ -2762,14 +2764,20 @@ static int fill_initgr(struct sss_packet *packet, struct ldb_result *res)
     /* skip first entry, it's the user entry */
     for (i = 0; i < num; i++) {
         gid = ldb_msg_find_attr_as_uint64(res->msgs[i + 1], SYSDB_GIDNUM, 0);
+        posix = ldb_msg_find_attr_as_bool(res->msgs[i + 1], SYSDB_POSIX, false);
         if (!gid) {
-            DEBUG(1, ("Incomplete group object for initgroups! Aborting\n"));
-            return EFAULT;
+            if (!posix) {
+                skipped++;
+                continue;
+            } else {
+                DEBUG(1, ("Incomplete group object for initgroups! Aborting\n"));
+                return EFAULT;
+            }
         }
         ((uint32_t *)body)[2 + i] = gid;
     }
 
-    ((uint32_t *)body)[0] = num; /* num results */
+    ((uint32_t *)body)[0] = num-skipped; /* num results */
     ((uint32_t *)body)[1] = 0; /* reserved */
 
     return EOK;
-- 
1.7.4.1

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

Reply via email to