[SSSD] [PATCH] local overrides: don't concact server with overriden name/id
Jakub Hrozek
jhrozek at redhat.com
Mon Oct 12 13:46:38 UTC 2015
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?= <pbrezina at redhat.com>
> 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?= <pbrezina at redhat.com>
> 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?= <pbrezina at redhat.com>
> 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)
More information about the sssd-devel
mailing list