On Sun, Oct 11, 2015 at 06:10:50PM +0200, Pavel Březina wrote: > On 10/11/2015 06:03 PM, Pavel Březina wrote: > >https://fedorahosted.org/sssd/ticket/2833 > > > >We can't search of overridden name/id in LDAP when using local > >overrides. These patches fix this for nss and sudo responder, please let > >me know if there is any other place. > > > >Similar fix needs to be done in cache_req interface, but I'll send it as > >another patch since I have some work in progress in that area. > > Now with patches. >
So far I only read the patches.. > From e5cbb8c9c384336e25c656e1aebe35c2c43b0079 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Sun, 11 Oct 2015 16:45:19 +0200 > Subject: [PATCH 1/3] nss: send original name and id with local views if > possible > > Resolves: > https://fedorahosted.org/sssd/ticket/2833 > --- > src/responder/nss/nsssrv_cmd.c | 121 > +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 121 insertions(+) > > diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c > index > 3e95a3f5a96a91515745a17fad5d20cbc9063d28..eb7f1f549eb8231597370bea02f3ca05492a8bd6 > 100644 > --- a/src/responder/nss/nsssrv_cmd.c > +++ b/src/responder/nss/nsssrv_cmd.c > @@ -600,6 +600,111 @@ is_refreshed_on_bg(enum sss_dp_acct_type req_type, > > static void nsssrv_dp_send_acct_req_done(struct tevent_req *req); > > +static void get_dp_name_and_id(TALLOC_CTX *mem_ctx, > + struct sss_domain_info *dom, > + enum sss_dp_acct_type req_type, > + const char *opt_name, > + uint32_t opt_id, > + const char **_name, > + uint32_t *_id) > +{ > + TALLOC_CTX *tmp_ctx; > + struct ldb_result *res = NULL; > + const char *attr; > + errno_t ret; > + > + /* First set the same values to make things easier. */ > + *_name = opt_name; > + *_id = opt_id; > + > + if (!DOM_HAS_VIEWS(dom) || !is_local_view(dom->view_name)) { > + DEBUG(SSSDBG_TRACE_FUNC, "Not a LOCAL view, continuing with " > + "provided values.\n"); > + return; > + } Good, this means the patch is quite safe.. > + > + tmp_ctx = talloc_new(NULL); > + if (tmp_ctx == NULL) { > + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n"); > + return; > + } > + > + if (opt_name != NULL) { > + switch (req_type) { > + case SSS_DP_USER: > + case SSS_DP_INITGROUPS: > + ret = sysdb_getpwnam_with_views(tmp_ctx, dom, opt_name, &res); > + if (ret != EOK) { > + DEBUG(SSSDBG_CONF_SETTINGS, > + "sysdb_getpwnam_with_views() failed [%d]: %s\n", > + ret, sss_strerror(ret)); > + goto done; > + } > + break; > + case SSS_DP_GROUP: > + ret = sysdb_getgrnam_with_views(tmp_ctx, dom, opt_name, &res); > + if (ret != EOK) { > + DEBUG(SSSDBG_CONF_SETTINGS, > + "sysdb_getgrnam_with_views() failed [%d]: %s\n", > + ret, sss_strerror(ret)); > + goto done; > + } > + break; > + default: > + goto done; > + } > + > + if (res == NULL || res->count != 1) { > + /* This should not happen with LOCAL view and overridden value. > */ > + DEBUG(SSSDBG_TRACE_FUNC, "Entry is missing?! Continuing with " > + "provided values.\n"); > + goto done; > + } > + > + *_name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL); I would prefer: name = ldb_msg_find_attr_as_string() if (name == NULL) { wtf?!? } *_name = talloc_steal(mem_ctx, name); > + talloc_steal(mem_ctx, *_name); > + } else if (opt_id != 0) { > + switch (req_type) { > + case SSS_DP_USER: > + ret = sysdb_getpwuid_with_views(tmp_ctx, dom, opt_id, &res); > + if (ret != EOK) { > + DEBUG(SSSDBG_CONF_SETTINGS, > + "sysdb_getpwuid_with_views() failed [%d]: %s\n", > + ret, sss_strerror(ret)); > + goto done; > + } > + > + attr = SYSDB_UIDNUM; > + break; > + case SSS_DP_GROUP: > + ret = sysdb_getgrgid_with_views(tmp_ctx, dom, opt_id, &res); > + if (ret != EOK) { > + DEBUG(SSSDBG_CONF_SETTINGS, > + "sysdb_getgrgid_with_views() failed [%d]: %s\n", > + ret, sss_strerror(ret)); > + goto done; > + } > + > + attr = SYSDB_GIDNUM; > + break; > + default: > + goto done; > + } > + > + if (res == NULL || res->count != 1) { > + /* This should not happen with LOCAL view and overridden value. > */ > + DEBUG(SSSDBG_TRACE_FUNC, "Entry is missing?! Continuing with " > + "provided values.\n"); > + goto done; > + } > + > + *_id = ldb_msg_find_attr_as_uint(res->msgs[0], attr, 0); I think it's safer to use ldb_msg_find_attr_as_uint64, because uid is normally 32bit...so we'd avoid problems with unsigned int on some strange platforms. I would also prefer a sanity check here, because the default not-found value is zero which sounds a bit dangerous :-) > + } > + > +done: > + talloc_free(tmp_ctx); > +} > + > /* FIXME: do not check res->count, but get in a msgs and check in parent */ > errno_t check_cache(struct nss_dom_ctx *dctx, > struct nss_ctx *nctx, > @@ -611,6 +716,7 @@ errno_t check_cache(struct nss_dom_ctx *dctx, > sss_dp_callback_t callback, > void *pvt) > { > + TALLOC_CTX *tmp_ctx; > errno_t ret; > struct nss_cmd_ctx *cmdctx = dctx->cmdctx; > struct cli_ctx *cctx = cmdctx->cctx; > @@ -628,6 +734,17 @@ errno_t check_cache(struct nss_dom_ctx *dctx, > return ENOENT; > } > > + tmp_ctx = talloc_new(NULL); > + if (tmp_ctx == NULL) { > + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n"); > + return ENOMEM; > + } > + > + /* In case of local view we have to always contant DP with the original > + * name or id. */ > + get_dp_name_and_id(tmp_ctx, dctx->domain, req_type, opt_name, opt_id, > + &opt_name, &opt_id); > + > /* if we have any reply let's check cache validity, but ignore netgroups > * if refresh_expired_interval is set (which implies that another method > * is used to refresh netgroups) > @@ -725,12 +842,16 @@ errno_t check_cache(struct nss_dom_ctx *dctx, > > tevent_req_set_callback(req, nsssrv_dp_send_acct_req_done, cb_ctx); > > + talloc_free(tmp_ctx); > return EAGAIN; > } > > + talloc_free(tmp_ctx); > return EOK; > > error: > + talloc_free(tmp_ctx); > + > ret = nss_cmd_send_error(cmdctx, ret); > if (ret != EOK) { > NSS_CMD_FATAL_ERROR_CODE(cctx, ret); > -- > 1.9.3 > > From c8125859c950ace5b621a98949e5ff211028801c Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Sun, 11 Oct 2015 17:38:34 +0200 > Subject: [PATCH 2/3] sudo: search with view even if user is found LGTM (not tested yet) > From bc1cca81a87bd5b78e5d8406b645dffad97addd4 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Sun, 11 Oct 2015 17:53:28 +0200 > Subject: [PATCH 3/3] sudo: send original name and id with local views if > possible > > Resolves: > https://fedorahosted.org/sssd/ticket/2833 LGTM (not tested yet) _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
