From e3319541af46d8b0de7e0b865c37904cf6216b58 Mon Sep 17 00:00:00 2001 From: Jan Zeleny Date: Wed, 13 Jun 2012 08:14:40 -0400 Subject: Fix an issue in ghost users There was an issue with ghost members in nested groups. Consider a scenario with two groups A and B, B being member of A and having some ghost members. In such case SSSD stored both groups, then added membership between them and then added ghost members to the group B. The problem was that adding ghost members to group B didn't propagate these ghost members to group A. This functionality could have been solved by memberof plugin but the logic is far more complicated that changes this patch introduces. The change is simple: add ghost members at the same time as the group is created, even if groups are supposed to be stored in two passes. That way ghost members will be present at the time A -> B membership is created and they will be propagated as expected. --- src/providers/ldap/sdap_async_groups.c | 122 +++++++++++++-------------------- 1 file changed, 47 insertions(+), 75 deletions(-) diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 720c3cc29..2d4e553f4 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -101,8 +101,7 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs, int num_values) { struct ldb_message_element *el; - struct ldb_message_element *ghostel; - int i, j, k; + int i, j; int ret; errno_t hret; hash_key_t key; @@ -121,30 +120,8 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs, goto done; } - ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &ghostel); - if (ret) { - goto done; - } - - if (ghostel->num_values == 0) { - /* Element was probably newly created, look for "member" again */ - ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); - if (ret != EOK) { - goto done; - } - } - - /* Just allocate both big enough to contain all members for now */ - ghostel->values = talloc_realloc(group_attrs, ghostel->values, struct ldb_val, - ghostel->num_values + num_values); - if (!ghostel->values) { - ret = ENOMEM; - goto done; - } - j = el->num_values; - k = ghostel->num_values; for (i = 0; i < num_values; i++) { if (ghosts == NULL) { hret = HASH_ERROR_KEY_NOT_FOUND; @@ -175,21 +152,12 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs, } else if (hret != HASH_SUCCESS) { ret = EFAULT; goto done; - } else { - DEBUG(SSSDBG_TRACE_FUNC, (" member #%d (%s): ghost!\n", - i, (char *)values[i].data)); - ghostel->values[k].data = (uint8_t *)talloc_strdup(ghostel->values, - value.ptr); - if (ghostel->values[k].data == NULL) { - ret = ENOMEM; - goto done; - } - ghostel->values[k].length = strlen((char *)ghostel->values[k].data); - k++; } + + /* If the member is in ghost table, it has + * already been processed - just skip it */ } el->num_values = j; - ghostel->num_values = k; ret = EOK; @@ -243,6 +211,8 @@ static int sdap_save_group(TALLOC_CTX *memctx, time_t now) { struct ldb_message_element *el; + struct ldb_message_element *el1; + struct ldb_message_element *gh; struct sysdb_attrs *group_attrs; const char *name = NULL; gid_t gid; @@ -374,14 +344,12 @@ static int sdap_save_group(TALLOC_CTX *memctx, } } - if (populate_members) { - struct ldb_message_element *el1; - struct ldb_message_element *gh; - ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1); - if (ret != EOK) { - goto fail; - } + ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1); + if (ret != EOK) { + goto fail; + } + if (populate_members) { ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret != EOK) { goto fail; @@ -389,44 +357,48 @@ static int sdap_save_group(TALLOC_CTX *memctx, el->values = el1->values; el->num_values = el1->num_values; - ret = sysdb_attrs_get_el(attrs, SYSDB_GHOST, &gh); - if (ret != EOK) { - goto fail; - } - ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &el); - if (ret != EOK) { + } + + ret = sysdb_attrs_get_el(attrs, SYSDB_GHOST, &gh); + if (ret != EOK) { + goto fail; + } + ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &el); + if (ret != EOK) { + goto fail; + } + el->values = gh->values; + el->num_values = gh->num_values; + + /* Now process RFC2307bis ghost hash table */ + if (ghosts != NULL) { + cnt = el->num_values + el1->num_values; + el->values = talloc_realloc(attrs, el->values, struct ldb_val, + cnt); + if (el->values == NULL) { + ret = ENOMEM; goto fail; } - el->values = gh->values; - el->num_values = gh->num_values; + for (i = 0; i < el1->num_values; i++) { + key.type = HASH_KEY_STRING; + key.str = (char *)el1->values[i].data; + ret = hash_lookup(ghosts, &key, &value); + if (ret == HASH_ERROR_KEY_NOT_FOUND) { + continue; + } else if (ret != HASH_SUCCESS) { + ret = EFAULT; + goto fail; + } - /* Now process RFC2307bis ghost hash table */ - if (ghosts != NULL) { - cnt = el->num_values + el1->num_values; - el->values = talloc_realloc(attrs, el->values, struct ldb_val, - cnt); - if (el->values == NULL) { + DEBUG(SSSDBG_TRACE_FUNC, ("Adding ghost member [%s] for group [%]s\n", + (char *)value.ptr, name)); + el->values[el->num_values].data = (uint8_t *)talloc_strdup(el->values, value.ptr); + if (el->values[el->num_values].data == NULL) { ret = ENOMEM; goto fail; } - for (i = 0; i < el1->num_values; i++) { - key.type = HASH_KEY_STRING; - key.str = (char *)el1->values[i].data; - ret = hash_lookup(ghosts, &key, &value); - if (ret == HASH_ERROR_KEY_NOT_FOUND) { - continue; - } else if (ret != HASH_SUCCESS) { - ret = EFAULT; - goto fail; - } - el->values[el->num_values].data = (uint8_t *)talloc_strdup(el->values, value.ptr); - if (el->values[el->num_values].data == NULL) { - ret = ENOMEM; - goto fail; - } - el->values[el->num_values].length = strlen(value.ptr)+1; - el->num_values++; - } + el->values[el->num_values].length = strlen(value.ptr)+1; + el->num_values++; } } -- cgit