[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