[SSSD] [PATCH] MEMBEROF: Do not add the ghost attribute to self

Jakub Hrozek jhrozek at redhat.com
Sun Nov 25 22:50:13 UTC 2012


I found this ghost users bug when working on #1652.

When the ghost attribute is added, the memberof plugin attempts to add
it to self, which is wrong. I think it's simply a copy-n-paste error
from the similar memberuid operation.

The result of the bug were LDB failures. The patch also contains unit
test.
-------------- next part --------------
>From 4cda39e7b37c8889ae9e91aee1dd7e9f953db5d2 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 24 Nov 2012 17:11:06 +0100
Subject: [PATCH] MEMBEROF: Do not add the ghost attribute to self

When a nested group with ghost users is added, its ghost attribute should
propagate within the nested group structure much like the memberuid
attribute. Unlike the memberuid attribute, the ghost attribute is only
semi-managed by the memberof plugin and added manually to the original
entry.

This bug caused LDB errors saying that attribute or value already exists
when a group with a ghost user was added to the hierarchy as groups were
updated with an attribute they already had.
---
 src/ldb_modules/memberof.c | 12 +------
 src/tests/sysdb-tests.c    | 88 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
index f0b5b72ed11ce71aec8ccb6e7a7cbd2d02e9566e..b884228d2ce845a5e52b6f500d47c6ac6dcba42e 100644
--- a/src/ldb_modules/memberof.c
+++ b/src/ldb_modules/memberof.c
@@ -813,7 +813,7 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
     el = ldb_msg_find_element(addop->entry, DB_GHOST);
     if (el) {
         for (i = 0; i < el->num_values; i++) {
-            /* add memberuid to all group's parents */
+            /* add ghost to all group's parents */
             for (j = 0; j < parents->num; j++) {
                 ret = mbof_append_muop(add_ctx, &add_ctx->muops,
                                        &add_ctx->num_muops,
@@ -826,16 +826,6 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
                 }
             }
 
-            /* now add memberuid to the group itself */
-            ret = mbof_append_muop(add_ctx, &add_ctx->muops,
-                                   &add_ctx->num_muops,
-                                   LDB_FLAG_MOD_ADD,
-                                   addop->entry_dn,
-                                   (char *)el->values[i].data,
-                                   DB_GHOST);
-            if (ret != LDB_SUCCESS) {
-                return ret;
-            }
         }
     }
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 61047706315d0b9ba94943b10f9d856dfc6fddf8..aa797ca5e96fc017eec8cc7f698e9e25516dc8b6 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -174,6 +174,7 @@ struct test_data {
 
     struct sysdb_attrs *attrs;
     const char **attrlist;
+    char **memberlist;
     struct ldb_message *msg;
 
     size_t msgs_count;
@@ -252,7 +253,7 @@ static int test_add_group(struct test_data *data)
     int ret;
 
     ret = sysdb_add_group(data->ctx->sysdb, data->groupname,
-                          data->gid, NULL, 0, 0);
+                          data->gid, data->attrs, 0, 0);
     return ret;
 }
 
@@ -270,7 +271,7 @@ static int test_store_group(struct test_data *data)
     int ret;
 
     ret = sysdb_store_group(data->ctx->sysdb, data->groupname,
-                            data->gid, NULL, -1, 0);
+                            data->gid, data->attrs, -1, 0);
     return ret;
 }
 
@@ -423,6 +424,43 @@ static int test_memberof_store_group(struct test_data *data)
     return ret;
 }
 
+static int test_memberof_store_group_with_ghosts(struct test_data *data)
+{
+    int ret;
+    struct sysdb_attrs *attrs = NULL;
+    char *member;
+    int i;
+
+    attrs = sysdb_new_attrs(data);
+    if (!attrs) {
+        return ENOMEM;
+    }
+
+    for (i = 0; data->attrlist && data->attrlist[i]; i++) {
+        member = sysdb_group_strdn(data, data->ctx->domain->name,
+                                   data->attrlist[i]);
+        if (!member) {
+            return ENOMEM;
+        }
+        ret = sysdb_attrs_steal_string(attrs, SYSDB_MEMBER, member);
+        if (ret != EOK) {
+            return ret;
+        }
+    }
+
+    for (i = 0; data->memberlist && data->memberlist[i]; i++) {
+        ret = sysdb_attrs_steal_string(attrs, SYSDB_GHOST,
+                                       data->memberlist[i]);
+        if (ret != EOK) {
+            return ret;
+        }
+    }
+
+    ret = sysdb_store_group(data->ctx->sysdb, data->groupname,
+                            data->gid, attrs, -1, 0);
+    return ret;
+}
+
 static int test_add_basic_netgroup(struct test_data *data)
 {
     const char *description;
@@ -1872,6 +1910,46 @@ START_TEST (test_sysdb_memberof_store_group)
 }
 END_TEST
 
+START_TEST (test_sysdb_memberof_store_group_with_ghosts)
+{
+    struct sysdb_test_ctx *test_ctx;
+    struct test_data *data;
+    int ret;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    if (ret != EOK) {
+        fail("Could not set up the test");
+        return;
+    }
+
+    data = talloc_zero(test_ctx, struct test_data);
+    data->ctx = test_ctx;
+    data->ev = test_ctx->ev;
+    data->gid = _i;
+    data->groupname = talloc_asprintf(data, "testgroup%d", data->gid);
+
+    if (_i == 0) {
+        data->attrlist = NULL;
+    } else {
+        data->attrlist = talloc_array(data, const char *, 2);
+        fail_unless(data->attrlist != NULL, "talloc_array failed.");
+        data->attrlist[0] = talloc_asprintf(data, "testgroup%d", data->gid - 1);
+        data->attrlist[1] = NULL;
+    }
+
+    data->memberlist = talloc_array(data, char *, 2);
+    fail_unless(data->memberlist != NULL, "talloc_array failed.");
+    data->memberlist[0] = talloc_asprintf(data, "testuser%d", data->gid);
+    data->memberlist[1] = NULL;
+
+    ret = test_memberof_store_group_with_ghosts(data);
+
+    fail_if(ret != EOK, "Could not store POSIX group #%d", data->gid);
+    talloc_free(test_ctx);
+}
+END_TEST
+
 START_TEST (test_sysdb_memberof_close_loop)
 {
     struct sysdb_test_ctx *test_ctx;
@@ -4025,6 +4103,12 @@ Suite *create_sysdb_suite(void)
     tcase_add_loop_test(tc_memberof, test_sysdb_remove_local_group_by_gid,
                         MBO_GROUP_BASE , MBO_GROUP_BASE + 10);
 
+    /* Ghost users tests */
+    tcase_add_loop_test(tc_memberof, test_sysdb_memberof_store_group_with_ghosts,
+                        MBO_GROUP_BASE , MBO_GROUP_BASE + 10);
+    tcase_add_loop_test(tc_memberof, test_sysdb_remove_local_group_by_gid,
+                        MBO_GROUP_BASE , MBO_GROUP_BASE + 10);
+
     suite_add_tcase(s, tc_memberof);
 
     TCase *tc_subdomain = tcase_create("SYSDB sub-domain Tests");
-- 
1.8.0



More information about the sssd-devel mailing list