[SSSD] [PATCH] LDAP: Don't add a user member twice when adding a primary group

Jakub Hrozek jhrozek at redhat.com
Sun Aug 17 18:47:33 UTC 2014


Hi,

The attached patch fixes https://fedorahosted.org/sssd/ticket/2406

I tried to use a permissive modification initially, but realized ldb
still checks for attribute duplicated when modifiying an entry. So I
added a check to the sssd code directly. It's a nested loop which is not
great, but it should be invoked in quite rare situations only.
-------------- next part --------------
>From f56facb917927669d1d9230753440656a714de29 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 16 Aug 2014 19:54:45 +0200
Subject: [PATCH] LDAP: Don't add a user member twice when adding a primary
 group

https://fedorahosted.org/sssd/ticket/2406

In the AD case, deployments sometimes add groups as parents of the
primary GID group. These groups are then returned during initgroups
in the tokenGroups attribute and member/memberof links are established
between the user and the group. However, any update of these groups
would remove the links, so a sequence of calls: id -G user; id user; id
-G user would return different group memberships.

Our code errored out in the rare case when the user was *also* an LDAP
member of his primary group.
---
 src/providers/ldap/sdap_async_groups.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index a21b0332e0acc111d387cc39fe8009794a4baa43..92506daf9cc438d4e9cd6247d62db93b29d994c8 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -202,7 +202,7 @@ static int sdap_fill_memberships(struct sdap_options *opts,
                                  size_t nuserdns)
 {
     struct ldb_message_element *el;
-    int i, j;
+    int i, ii, j;
     int ret;
     errno_t hret;
     hash_key_t key;
@@ -285,12 +285,30 @@ static int sdap_fill_memberships(struct sdap_options *opts,
     }
     el->num_values = j;
 
+    j = 0;
     for (i=0; i < nuserdns; i++) {
-        el->values[el->num_values + i].data = (uint8_t *) \
+        /* This is bad complexity, but the this loop should only be invoked in
+         * the very rare scenario of AD POSIX group that is primary group of
+         * some users but has user member attributes at the same time
+         */
+        for (ii = 0; ii < el->num_values; ii++) {
+            if (strcmp(userdns[i], (const char *) el->values[ii].data) == 0) {
+                DEBUG(SSSDBG_TRACE_INTERNAL,
+                      "Member %s already included, skipping\n", userdns[i]);
+                break;
+            }
+        }
+
+        if (ii < el->num_values) {
+            continue;
+        }
+
+        el->values[el->num_values + j].data = (uint8_t *) \
                                           talloc_steal(group_attrs, userdns[i]);
-        el->values[el->num_values + i].length = strlen(userdns[i]);
+        el->values[el->num_values + j].length = strlen(userdns[i]);
+        j++;
     }
-    el->num_values += nuserdns;
+    el->num_values += j;
 
     ret = EOK;
 
-- 
1.9.3



More information about the sssd-devel mailing list