From b22f24ead56e401c37750ecd34a5e99506d17058 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Nov 20 2012 17:04:22 +0000 Subject: LDAP: Only convert direct parents' ghost attribute to member https://fedorahosted.org/sssd/ticket/1612 This patch changes the handling of ghost attributes when saving the actual user entry. Instead of always linking all groups that contained the ghost attribute with the new user entry, the original member attributes are now saved in the group object and the user entry is only linked with its direct parents. As the member attribute is compared against the originalDN of the user, if either the originalDN or the originalMember attributes are missing, the user object is linked with all the groups as a fallback. The original member attributes are only saved if the LDAP schema supports nesting. --- diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 5541d3d..51b070d 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -124,6 +124,7 @@ #define SYSDB_ORIG_DN "originalDN" #define SYSDB_ORIG_MODSTAMP "originalModifyTimestamp" #define SYSDB_ORIG_MEMBEROF "originalMemberOf" +#define SYSDB_ORIG_MEMBER "orig_member" #define SYSDB_ORIG_MEMBER_USER "originalMemberUser" #define SYSDB_ORIG_MEMBER_HOST "originalMemberHost" @@ -662,6 +663,7 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, const char *gecos, const char *homedir, const char *shell, + const char *orig_dn, struct sysdb_attrs *attrs, int cache_timeout, time_t now); @@ -708,6 +710,7 @@ int sysdb_store_user(struct sysdb_ctx *sysdb, const char *gecos, const char *homedir, const char *shell, + const char *orig_dn, struct sysdb_attrs *attrs, char **remove_attrs, uint64_t cache_timeout, diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 8b624a3..0eef6d0 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -860,6 +860,7 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, const char *gecos, const char *homedir, const char *shell, + const char *orig_dn, struct sysdb_attrs *attrs, int cache_timeout, time_t now) @@ -868,14 +869,16 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, struct ldb_message *msg; struct ldb_message **groups; struct ldb_message_element *alias_el; + struct ldb_message_element *orig_members; size_t group_count = 0; struct sysdb_attrs *id_attrs; - const char *group_attrs[] = {SYSDB_NAME, SYSDB_GHOST, NULL}; + const char *group_attrs[] = {SYSDB_NAME, SYSDB_GHOST, SYSDB_ORIG_MEMBER, NULL}; struct ldb_dn *tmpdn; const char *userdn; char *filter; uint32_t id; int ret, i, j; + bool add_member = false; struct sss_domain_info *domain = sysdb->domain; @@ -1029,7 +1032,7 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, } /* We need to find all groups that contain this object as a ghost user - * and replace the ghost user there by actual member record + * and replace the ghost user by actual member record in direct parents. * Note that this object can be referred to either by its name or any * of its aliases */ @@ -1047,8 +1050,33 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, msg->dn = groups[i]->dn; - ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_MEMBER, userdn); - if (ret) goto done; + if (orig_dn == NULL) { + /* We have no way of telling which groups this user belongs to. + * Add it to all that reference it in the ghost attribute */ + add_member = true; + } else { + add_member = false; + orig_members = ldb_msg_find_element(groups[i], SYSDB_ORIG_MEMBER); + if (orig_members) { + for (j = 0; j < orig_members->num_values; j++) { + if (strcmp((const char *) orig_members->values[j].data, + orig_dn) == 0) { + /* This is a direct member. Add the member attribute */ + add_member = true; + } + } + } else { + /* Nothing to compare the originalDN with. Let's rely on the + * memberof plugin to do the right thing during initgroups.. + */ + add_member = true; + } + } + + if (add_member) { + ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_MEMBER, userdn); + if (ret) goto done; + } ret = add_string(msg, LDB_FLAG_MOD_DELETE, SYSDB_GHOST, name); if (ret) goto done; @@ -1485,6 +1513,7 @@ int sysdb_store_user(struct sysdb_ctx *sysdb, const char *gecos, const char *homedir, const char *shell, + const char *orig_dn, struct sysdb_attrs *attrs, char **remove_attrs, uint64_t cache_timeout, @@ -1535,8 +1564,8 @@ int sysdb_store_user(struct sysdb_ctx *sysdb, if (ret == ENOENT) { /* users doesn't exist, turn into adding a user */ - ret = sysdb_add_user(sysdb, name, uid, gid, - gecos, homedir, shell, attrs, cache_timeout, now); + ret = sysdb_add_user(sysdb, name, uid, gid, gecos, homedir, + shell, orig_dn, attrs, cache_timeout, now); if (ret == EEXIST) { /* This may be a user rename. If there is a user with the * same UID, remove it and try to add the basic user again @@ -1554,8 +1583,8 @@ int sysdb_store_user(struct sysdb_ctx *sysdb, DEBUG(SSSDBG_MINOR_FAILURE, ("A user with the same UID [%llu] was removed from the " "cache\n", (unsigned long long) uid)); - ret = sysdb_add_user(sysdb, name, uid, gid, gecos, - homedir, shell, attrs, cache_timeout, now); + ret = sysdb_add_user(sysdb, name, uid, gid, gecos, homedir, + shell, orig_dn, attrs, cache_timeout, now); } /* Handle the result of sysdb_add_user */ diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 231d481..ea1ce99 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -614,7 +614,7 @@ errno_t sysdb_store_domuser(struct sss_domain_info *domain, CHECK_DOMAIN_INFO(domain); return sysdb_store_user(domain->sysdb, name, pwd, uid, gid, gecos, homedir, - shell, attrs, remove_attrs, cache_timeout, now); + shell, NULL, attrs, remove_attrs, cache_timeout, now); } errno_t sysdb_delete_domuser(struct sss_domain_info *domain, diff --git a/src/providers/ipa/ipa_hbac_private.h b/src/providers/ipa/ipa_hbac_private.h index bb1ea4e..f313ca1 100644 --- a/src/providers/ipa/ipa_hbac_private.h +++ b/src/providers/ipa/ipa_hbac_private.h @@ -34,7 +34,6 @@ #define IPA_UNIQUE_ID "ipauniqueid" #define IPA_MEMBER "member" -#define SYSDB_ORIG_MEMBER "orig_member" #define HBAC_HOSTS_SUBDIR "hbac_hosts" #define HBAC_HOSTGROUPS_SUBDIR "hbac_hostgroups" diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index aea22ab..d7d262d 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -205,6 +205,7 @@ sdap_process_ghost_members(struct sysdb_attrs *attrs, struct sdap_options *opts, hash_table_t *ghosts, bool populate_members, + bool store_original_member, struct sysdb_attrs *sysdb_attrs) { errno_t ret; @@ -235,6 +236,19 @@ sdap_process_ghost_members(struct sysdb_attrs *attrs, return ret; } + if (store_original_member) { + DEBUG(SSSDBG_TRACE_FUNC, ("The group has %d members\n", memberel->num_values)); + for (i = 0; i < memberel->num_values; i++) { + ret = sysdb_attrs_add_string(sysdb_attrs, SYSDB_ORIG_MEMBER, + (const char *) memberel->values[i].data); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, ("Could not add member [%s]\n", + (const char *) memberel->values[i].data)); + return ret; + } + } + } + if (populate_members) { ret = sysdb_attrs_get_el(sysdb_attrs, SYSDB_MEMBER, &sysdb_memberel); if (ret != EOK) { @@ -301,6 +315,7 @@ static int sdap_save_group(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct sysdb_attrs *attrs, bool populate_members, + bool store_original_member, hash_table_t *ghosts, char **_usn_value, time_t now) @@ -475,7 +490,8 @@ static int sdap_save_group(TALLOC_CTX *memctx, } ret = sdap_process_ghost_members(attrs, opts, ghosts, - populate_members, group_attrs); + populate_members, store_original_member, + group_attrs); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Failed to save ghost members\n")); goto fail; @@ -598,6 +614,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx, char *higher_usn = NULL; char *usn_value; bool twopass; + bool has_nesting = false; int ret; errno_t sret; int i; @@ -615,6 +632,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx, case SDAP_SCHEMA_IPA_V1: case SDAP_SCHEMA_AD: twopass = true; + has_nesting = true; break; default: @@ -649,8 +667,8 @@ static int sdap_save_groups(TALLOC_CTX *memctx, /* if 2 pass savemembers = false */ ret = sdap_save_group(tmpctx, sysdb, opts, dom, groups[i], - populate_members, ghosts, - &usn_value, now); + populate_members, has_nesting, + ghosts, &usn_value, now); /* Do not fail completely on errors. * Just report the failure to save and go on */ diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index e2e7b72..5304c62 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -47,6 +47,7 @@ int sdap_save_user(TALLOC_CTX *memctx, const char *gecos; const char *homedir; const char *shell; + const char *orig_dn; uid_t uid; gid_t gid, primary_gid; struct sysdb_attrs *user_attrs; @@ -241,12 +242,23 @@ int sdap_save_user(TALLOC_CTX *memctx, goto fail; } - ret = sdap_attrs_add_string(attrs, SYSDB_ORIG_DN, - "original DN", - name, user_attrs); - if (ret != EOK) { + ret = sysdb_attrs_get_el(attrs, SYSDB_ORIG_DN, &el); + if (ret) { goto fail; } + if (!el || el->num_values == 0) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("originalDN is not available for [%s].\n", name)); + } else { + orig_dn = (const char *) el->values[0].data; + DEBUG(SSSDBG_TRACE_INTERNAL, ("Adding originalDN [%s] to attributes " + "of [%s].\n", orig_dn, name)); + + ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_DN, orig_dn); + if (ret) { + goto fail; + } + } ret = sysdb_attrs_get_el(attrs, SYSDB_MEMBEROF, &el); if (ret) { @@ -358,7 +370,7 @@ int sdap_save_user(TALLOC_CTX *memctx, DEBUG(6, ("Storing info for user %s\n", name)); ret = sysdb_store_user(ctx, name, pwd, uid, gid, gecos, homedir, shell, - user_attrs, missing, cache_timeout, now); + orig_dn, user_attrs, missing, cache_timeout, now); if (ret) goto fail; if (_usn_value) { diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c index 451bdff..ce66fa1 100644 --- a/src/providers/proxy/proxy_id.c +++ b/src/providers/proxy/proxy_id.c @@ -260,6 +260,7 @@ static int save_user(struct sysdb_ctx *sysdb, bool lowercase, pwd->pw_gecos, pwd->pw_dir, shell, + NULL, attrs, NULL, cache_timeout, diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index 202765a..6b6a7a9 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -391,7 +391,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx) ret = sysdb_store_user(sysdb, pwd->pw_name, NULL, pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos, pwd->pw_dir, - pwd->pw_shell, user_attrs, NULL, + pwd->pw_shell, NULL, user_attrs, NULL, pr_ctx->dom->user_timeout, 0); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n", diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index d9afe7b..2b8b92d 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -191,7 +191,7 @@ static int test_add_user(struct test_data *data) ret = sysdb_add_user(data->ctx->sysdb, data->username, data->uid, 0, gecos, homedir, "/bin/bash", - NULL, 0, 0); + NULL, NULL, 0, 0); return ret; } @@ -207,7 +207,7 @@ static int test_store_user(struct test_data *data) ret = sysdb_store_user(data->ctx->sysdb, data->username, "x", data->uid, 0, gecos, homedir, data->shell ? data->shell : "/bin/bash", - NULL, NULL, -1, 0); + NULL, NULL, NULL, -1, 0); return ret; } @@ -2260,7 +2260,8 @@ START_TEST(test_user_rename) /* Store and verify the first user */ ret = sysdb_store_user(test_ctx->sysdb, fromname, NULL, userid, 0, - fromname, "/", "/bin/sh", NULL, NULL, 0, 0); + fromname, "/", "/bin/sh", + NULL, NULL, NULL, 0, 0); fail_unless(ret == EOK, "Could not add first user"); ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, fromname, &res); @@ -2281,11 +2282,11 @@ START_TEST(test_user_rename) /* Perform rename and check that GID is the same, but name changed */ ret = sysdb_add_user(test_ctx->sysdb, toname, userid, 0, - fromname, "/", "/bin/sh", NULL, 0, 0); + fromname, "/", "/bin/sh", NULL, NULL, 0, 0); fail_unless(ret == EEXIST, "A second user added with low level call?"); ret = sysdb_store_user(test_ctx->sysdb, toname, NULL, userid, 0, - fromname, "/", "/bin/sh", NULL, NULL, 0, 0); + fromname, "/", "/bin/sh", NULL, NULL, NULL, 0, 0); fail_unless(ret == EOK, "Could not add second user"); ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, toname, &res); @@ -3484,7 +3485,7 @@ START_TEST(test_sysdb_subdomain_store_user) ret = sysdb_store_user(subdomain->sysdb, "subdomuser", NULL, 12345, 0, "Sub Domain User", "/home/subdomuser", "/bin/bash", - NULL, NULL, -1, 0); + NULL, NULL, NULL, -1, 0); fail_unless(ret == EOK, "sysdb_store_user failed."); base_dn =ldb_dn_new(test_ctx, test_ctx->sysdb->ldb, "cn=sysdb"); diff --git a/src/tools/sss_seed.c b/src/tools/sss_seed.c index 8eaf959..5aff1ed 100644 --- a/src/tools/sss_seed.c +++ b/src/tools/sss_seed.c @@ -740,7 +740,7 @@ static int seed_cache_user(struct seed_ctx *sctx) ret = sysdb_add_user(sctx->sysdb, sctx->uctx->name, sctx->uctx->uid, sctx->uctx->gid, sctx->uctx->gecos, sctx->uctx->home, - sctx->uctx->shell, NULL, 0, 0); + sctx->uctx->shell, NULL, NULL, 0, 0); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Failed to add user to the cache. (%d)[%s]\n", diff --git a/src/tools/sss_sync_ops.c b/src/tools/sss_sync_ops.c index 7183eb3..10a1a12 100644 --- a/src/tools/sss_sync_ops.c +++ b/src/tools/sss_sync_ops.c @@ -467,7 +467,8 @@ int useradd(TALLOC_CTX *mem_ctx, int ret; ret = sysdb_add_user(sysdb, data->name, data->uid, data->gid, - data->gecos, data->home, data->shell, NULL, 0, 0); + data->gecos, data->home, data->shell, + NULL, NULL, 0, 0); if (ret) { goto done; }