From dda0258705de7255e6ec54b7f9adbde83a220996 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Aug 05 2015 09:28:37 +0000 Subject: NSS: Initgr memory cache should work with fq names We need to stored two versions of name to the initgroups memory cache. Otherwise it could be stored many times if sssd is configured with case_sensitive = false. It would be impossible to invalidate all version of names after user login. As a result of this wrong user groups could be returned from initgroups memory cache. Therefore we store raw name provided by glibc function and internal sanitized fully qualified name, which is unique for particular user. This patch also increase average space for initgroups because there are also stored two quite long names in case of fq names. Resolves: https://fedorahosted.org/sssd/ticket/2712 Reviewed-by: Michal Židek --- diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 778618e..e754245 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -1387,6 +1387,7 @@ static int nss_cmd_getbynam(enum sss_cli_command cmd, struct cli_ctx *cctx) } rawname = (const char *)body; + dctx->mc_name = rawname; DEBUG(SSSDBG_TRACE_FUNC, "Running command [%d] with input [%s].\n", dctx->cmdctx->cmd, rawname); @@ -3976,6 +3977,13 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx, } if (changed) { + char *fq_name = sss_tc_fqname(tmp_ctx, dom->names, dom, name); + if (!fq_name) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Could not create fq name\n"); + goto done; + } + for (i = 0; i < gnum; i++) { id = groups[i]; @@ -3987,7 +3995,7 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx, } } - to_sized_string(&delete_name, name); + to_sized_string(&delete_name, fq_name); ret = sss_mmap_cache_initgr_invalidate(nctx->initgr_mc_ctx, &delete_name); if (ret != EOK && ret != ENOENT) { @@ -4007,6 +4015,7 @@ static int fill_initgr(struct sss_packet *packet, struct sss_domain_info *dom, struct ldb_result *res, struct nss_ctx *nctx, + const char *mc_name, const char *name) { uint8_t *body; @@ -4095,9 +4104,18 @@ static int fill_initgr(struct sss_packet *packet, } if (nctx->initgr_mc_ctx) { - to_sized_string(&rawname, name); + struct sized_string unique_name; + char *fq_name = sss_tc_fqname(packet, dom->names, dom, name); + if (!fq_name) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Could not create fq name\n"); + return ENOMEM; + } + + to_sized_string(&rawname, mc_name); + to_sized_string(&unique_name, fq_name); ret = sss_mmap_cache_initgr_store(&nctx->initgr_mc_ctx, &rawname, - num - skipped, gids); + &unique_name, num - skipped, gids); if (ret != EOK && ret != ENOMEM) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to store user %s(%s) in mmap cache!\n", @@ -4125,7 +4143,7 @@ static int nss_cmd_initgr_send_reply(struct nss_dom_ctx *dctx) } ret = fill_initgr(cctx->creq->out, dctx->domain, dctx->res, nctx, - dctx->rawname); + dctx->mc_name, cmdctx->name); if (ret) { return ret; } @@ -4173,8 +4191,10 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx) name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive); if (!name) return ENOMEM; - name = sss_reverse_replace_space(dctx, name, + name = sss_reverse_replace_space(cmdctx, name, nctx->rctx->override_space); + /* save name so it can be used in initgr reply */ + cmdctx->name = name; if (name == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "sss_reverse_replace_space failed\n"); diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index ab9e769..62f4c54 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -31,8 +31,8 @@ #define SSS_AVG_PASSWD_PAYLOAD (MC_SLOT_SIZE * 4) /* short group name and no gids (private user group */ #define SSS_AVG_GROUP_PAYLOAD (MC_SLOT_SIZE * 3) -/* average place for 40 supplementary groups */ -#define SSS_AVG_INITGROUP_PAYLOAD (MC_SLOT_SIZE * 4) +/* average place for 40 supplementary groups + 2 names */ +#define SSS_AVG_INITGROUP_PAYLOAD (MC_SLOT_SIZE * 5) #define MC_NEXT_BARRIER(val) ((((val) + 1) & 0x00ffffff) | 0xf0000000) @@ -965,6 +965,7 @@ done: errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc, struct sized_string *name, + struct sized_string *unique_name, uint32_t num_groups, uint8_t *gids_buf) { @@ -973,6 +974,7 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc, struct sss_mc_initgr_data *data; size_t data_len; size_t rec_len; + size_t pos; int ret; if (mcc == NULL) { @@ -980,20 +982,22 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc, return EINVAL; } - /* array of gids + name */ - data_len = num_groups * sizeof(uint32_t) + name->len; + /* array of gids + name + unique_name */ + data_len = num_groups * sizeof(uint32_t) + name->len + unique_name->len; rec_len = sizeof(struct sss_mc_rec) + sizeof(struct sss_mc_initgr_data) + data_len; if (rec_len > mcc->dt_size) { return ENOMEM; } - ret = sss_mc_get_record(_mcc, rec_len, name, &rec); + /* use unique name for searching potential old records */ + ret = sss_mc_get_record(_mcc, rec_len, unique_name, &rec); if (ret != EOK) { return ret; } data = (struct sss_mc_initgr_data *)rec->data; + pos = 0; MC_RAISE_BARRIER(rec); @@ -1001,16 +1005,22 @@ errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc, * Use the first key twice. */ sss_mmap_set_rec_header(mcc, rec, rec_len, mcc->valid_time_slot, - name->str, name->len, name->str, name->len); + name->str, name->len, + unique_name->str, unique_name->len); /* initgroups struct */ - data->strs_len = name->len; + data->strs_len = name->len + unique_name->len; data->data_len = data_len; - data->reserved = MC_INVALID_VAL32; data->num_groups = num_groups; - memcpy(data->gids, gids_buf, num_groups * sizeof(uint32_t)); - memcpy(&data->gids[num_groups], name->str, name->len); - data->strs = data->name = MC_PTR_DIFF(&data->gids[num_groups], data); + memcpy((char *)data->gids + pos, gids_buf, num_groups * sizeof(uint32_t)); + pos += num_groups * sizeof(uint32_t); + + memcpy((char *)data->gids + pos, unique_name->str, unique_name->len); + data->strs = data->unique_name = MC_PTR_DIFF((char *)data->gids + pos, data); + pos += unique_name->len; + + memcpy((char *)data->gids + pos, name->str, name->len); + data->name = MC_PTR_DIFF((char *)data->gids + pos, data); MC_LOWER_BARRIER(rec); diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h index b09e4a6..b84fbc8 100644 --- a/src/responder/nss/nsssrv_mmap_cache.h +++ b/src/responder/nss/nsssrv_mmap_cache.h @@ -53,6 +53,7 @@ errno_t sss_mmap_cache_gr_store(struct sss_mc_ctx **_mcc, errno_t sss_mmap_cache_initgr_store(struct sss_mc_ctx **_mcc, struct sized_string *name, + struct sized_string *unique_name, uint32_t num_groups, uint8_t *gids_buf); diff --git a/src/responder/nss/nsssrv_private.h b/src/responder/nss/nsssrv_private.h index e9f00b1..e5a2486 100644 --- a/src/responder/nss/nsssrv_private.h +++ b/src/responder/nss/nsssrv_private.h @@ -79,6 +79,8 @@ struct nss_dom_ctx { /* Service-specific */ const char *protocol; + + const char *mc_name; }; struct setent_step_ctx { diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h index b5917b3..22c1ae6 100644 --- a/src/util/mmap_cache.h +++ b/src/util/mmap_cache.h @@ -138,14 +138,15 @@ struct sss_mc_grp_data { }; struct sss_mc_initgr_data { - rel_ptr_t name; /* ptr to name string, rel. to struct base addr */ + rel_ptr_t unique_name; /* ptr to unique name string, rel. to struct base addr */ + rel_ptr_t name; /* ptr to raw name string, rel. to struct base addr */ rel_ptr_t strs; /* ptr to concatenation of all strings */ - uint32_t reserved; uint32_t strs_len; /* length of strs */ uint32_t data_len; /* all initgroups data len */ uint32_t num_groups; /* number of groups */ uint32_t gids[0]; /* array of all groups - * string with name is stored after gids */ + * string with name and unique_name is stored + * after gids */ }; #pragma pack()