On Tue, Sep 24, 2013 at 04:06:22PM +0200, Lukas Slebodnik wrote: > >@@ -3639,7 +3662,8 @@ static errno_t nss_cmd_getsidby_search(struct > >nss_dom_ctx *dctx) > > dom = get_next_domain(dom, true); > > continue; > > } > >- return ENOENT; > >+ ret = ENOMEM; > ^^^^^^ > Is it aim or mistake? s/ENOMEM/ENOENT/ >
It's a typo and thanks for the catch. New patches are attached.
>From de71e62fe00027b08ad60ed711604bf2fda9dba6 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Wed, 18 Sep 2013 15:42:41 +0200 Subject: [PATCH 1/2] NSS: Set UID and GID to negative cache after searching all domains https://fedorahosted.org/sssd/ticket/2090 Previously, when searching by UID or GID, the negative cache will only work in case the UID was searched for using fully qualified names. --- src/responder/nss/nsssrv_cmd.c | 171 +++++++++++++++++++++++++---------------- 1 file changed, 105 insertions(+), 66 deletions(-) diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 229548976744220b6f10b184a28a8ac8c11559a7..a62bfd8c3dbaf3ad9a0946330d2337c745d01462 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -1236,7 +1236,8 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) dom = get_next_domain(dom, true); continue; } - return ENOENT; + ret = ENOENT; + goto done; } if (dom != dctx->domain) { @@ -1253,18 +1254,21 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) sysdb = dom->sysdb; if (sysdb == NULL) { DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); - return EIO; + ret = EIO; + goto done; } ret = sysdb_getpwuid(cmdctx, sysdb, dom, cmdctx->id, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (dctx->res->count > 1) { DEBUG(0, ("getpwuid call returned more than one result !?!\n")); - return ENOENT; + ret = ENOENT; + goto done; } if (dctx->res->count == 0 && !dctx->check_provider) { @@ -1274,15 +1278,10 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) continue; } - DEBUG(2, ("No results for getpwuid call\n")); - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; - } - - return ENOENT; + DEBUG(SSSDBG_MINOR_FAILURE, ("No results for getpwuid call\n")); + ret = ENOENT; + goto done; } /* if this is a caching provider (or if we haven't checked the cache @@ -1296,18 +1295,30 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) /* Anything but EOK means we should reenter the mainloop * because we may be refreshing the cache */ - return ret; + goto done; } } /* One result found */ DEBUG(6, ("Returning info for uid [%d@%s]\n", cmdctx->id, dom->name)); - return EOK; + ret = EOK; + goto done; } - DEBUG(2, ("No matching domain found for [%d], fail!\n", cmdctx->id)); - return ENOENT; + /* All domains were tried and none had the entry. */ + ret = ENOENT; +done: + if (ret == ENOENT) { + /* The entry was not found, need to set result in negative cache */ + ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); + if (ret != EOK) { + return ret; + } + } + + DEBUG(SSSDBG_MINOR_FAILURE, ("No matching domain found for [%d]\n", cmdctx->id)); + return ret; } static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx); @@ -2668,7 +2679,8 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) dom = get_next_domain(dom, true); continue; } - return ENOENT; + ret = ENOENT; + goto done; } if (dom != dctx->domain) { @@ -2685,18 +2697,21 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) sysdb = dom->sysdb; if (sysdb == NULL) { DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n")); - return EIO; + ret = EIO; + goto done; } ret = sysdb_getgrgid(cmdctx, sysdb, dom, cmdctx->id, &dctx->res); if (ret != EOK) { DEBUG(1, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (dctx->res->count > 1) { DEBUG(0, ("getgrgid call returned more than one result !?!\n")); - return ENOENT; + ret = ENOENT; + goto done; } if (dctx->res->count == 0 && !dctx->check_provider) { @@ -2706,15 +2721,10 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) continue; } - DEBUG(2, ("No results for getgrgid call\n")); - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; - } - - return ENOENT; + DEBUG(SSSDBG_MINOR_FAILURE, ("No results for getgrgid call\n")); + ret = ENOENT; + goto done; } /* if this is a caching provider (or if we haven't checked the cache @@ -2728,18 +2738,31 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) /* Anything but EOK means we should reenter the mainloop * because we may be refreshing the cache */ - return ret; + goto done; } } /* One result found */ DEBUG(6, ("Returning info for gid [%d@%s]\n", cmdctx->id, dom->name)); - return EOK; + /* Success. Break from the loop and return EOK */ + ret = EOK; + goto done; } - DEBUG(2, ("No matching domain found for [%d], fail!\n", cmdctx->id)); - return ENOENT; + /* All domains were tried and none had the entry. */ + ret = ENOENT; +done: + if (ret == ENOENT) { + /* The entry was not found, need to set result in negative cache */ + ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); + if (ret != EOK) { + return ret; + } + } + + DEBUG(SSSDBG_MINOR_FAILURE, ("No matching domain found for [%d]\n", cmdctx->id)); + return ret; } static int nss_cmd_getgrgid(struct cli_ctx *cctx) @@ -3639,7 +3662,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) dom = get_next_domain(dom, true); continue; } - return ENOENT; + ret = ENOENT; + goto done; } } else { /* if it is a domainless search, skip domains that require fully @@ -3670,7 +3694,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive); if (name == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("sss_get_cased_name failed.\n")); - return ENOMEM; + ret = ENOMEM; + goto done; } /* For subdomains a fully qualified name is needed for @@ -3679,7 +3704,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) sysdb_name = sss_tc_fqname(cmdctx, dom->names, dom, name); if (sysdb_name == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n")); - return ENOMEM; + ret = ENOMEM; + goto done; } } @@ -3702,7 +3728,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) /* There are no further domains or this was a * fully-qualified user request. */ - return ENOENT; + ret = ENOENT; + goto done; } DEBUG(SSSDBG_TRACE_FUNC, ("Requesting info for [%s@%s]\n", @@ -3714,7 +3741,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (sysdb == NULL) { DEBUG(SSSDBG_FATAL_FAILURE, ("Fatal: Sysdb CTX not found for this domain!\n")); - return EIO; + ret = EIO; + goto done; } if (cmdctx->cmd == SSS_NSS_GETSIDBYID) { @@ -3723,7 +3751,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (ret == EOK) { @@ -3735,7 +3764,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (ret == EOK) { @@ -3749,7 +3779,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (ret == EOK) { @@ -3762,7 +3793,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to make request to our cache!\n")); - return EIO; + ret = EIO; + goto done; } if (ret == EOK) { @@ -3774,7 +3806,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) dctx->res = talloc_zero(cmdctx, struct ldb_result); if (dctx->res == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero failed.\n")); - return ENOMEM; + ret = ENOMEM; + goto done; } if (user_found || group_found) { @@ -3782,7 +3815,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) dctx->res->msgs = talloc_array(dctx->res, struct ldb_message *, 1); if (dctx->res->msgs == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n")); - return ENOMEM; + ret = ENOMEM; + goto done; } dctx->res->msgs[0] = talloc_steal(dctx->res, msg); } @@ -3806,20 +3840,8 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) } DEBUG(SSSDBG_OP_FAILURE, ("No matching user or group found.\n")); - - if (cmdctx->cmd == SSS_NSS_GETSIDBYID) { - ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; - } - - ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; - } - } - - return ENOENT; + ret = ENOENT; + goto done; } /* if this is a caching provider (or if we haven't checked the cache @@ -3848,7 +3870,7 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) /* Anything but EOK means we should reenter the mainloop * because we may be refreshing the cache */ - return ret; + goto done; } } @@ -3861,17 +3883,34 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) name, dom->name)); } - return EOK; + /* Success. Break from the loop and return EOK */ + ret = EOK; + goto done; } - if (cmdctx->cmd == SSS_NSS_GETSIDBYID) { - DEBUG(SSSDBG_MINOR_FAILURE, - ("No matching domain found for [%d], fail!\n", cmdctx->id)); - } else { - DEBUG(SSSDBG_MINOR_FAILURE, - ("No matching domain found for [%s], fail!\n", cmdctx->name)); + /* All domains were tried and none had the entry. */ + ret = ENOENT; +done: + if (ret == ENOENT) { + /* The entry was not found, need to set result in negative cache */ + if (cmdctx->cmd == SSS_NSS_GETSIDBYID) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("No matching domain found for [%d], fail!\n", cmdctx->id)); + ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); + if (ret != EOK) { + return ret; + } + + ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); + if (ret != EOK) { + return ret; + } + } else { + DEBUG(SSSDBG_MINOR_FAILURE, + ("No matching domain found for [%s], fail!\n", cmdctx->name)); + } } - return ENOENT; + return ret; } static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx) -- 1.8.3.1
>From 2b073b32885bcfa7df480508525d8504a19c7810 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Wed, 18 Sep 2013 15:49:46 +0200 Subject: [PATCH 2/2] NSS: Failure to store entry negative cache should not be fatal The only effect the failure to store a result to negative cache might have would be a slower lookup next time. --- src/responder/nss/nsssrv_cmd.c | 49 ++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index a62bfd8c3dbaf3ad9a0946330d2337c745d01462..7220e3a35a4b97b3425ff88dbd8e08b7afdaa9c8 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -773,7 +773,8 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) /* set negative cache only if not result of cache check */ ret = sss_ncache_set_user(nctx->ncache, false, dom, name); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot set negcache for %s@%s\n", + name, dom->name)); } /* if a multidomain search, try with next */ @@ -1221,6 +1222,7 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; + int err; nctx = talloc_get_type(cctx->rctx->pvt_ctx, struct nss_ctx); @@ -1311,9 +1313,10 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx) done: if (ret == ENOENT) { /* The entry was not found, need to set result in negative cache */ - ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; + err = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); + if (err != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negative cache for UID %d\n", cmdctx->id)); } } @@ -2594,7 +2597,8 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) /* set negative cache only if not result of cache check */ ret = sss_ncache_set_group(nctx->ncache, false, dom, name); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot set negcache for %s@%s\n", + name, dom->name)); } /* if a multidomain search, try with next */ @@ -2664,6 +2668,7 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; + int err; nctx = talloc_get_type(cctx->rctx->pvt_ctx, struct nss_ctx); @@ -2755,9 +2760,10 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx) done: if (ret == ENOENT) { /* The entry was not found, need to set result in negative cache */ - ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; + err = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); + if (err != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negative cache for GID %d\n", cmdctx->id)); } } @@ -3584,7 +3590,8 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx) /* set negative cache only if not result of cache check */ ret = sss_ncache_set_user(nctx->ncache, false, dom, name); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot set negcache for %s@%s\n", + name, dom->name)); } /* if a multidomain search, try with next */ @@ -3636,6 +3643,7 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) struct sysdb_ctx *sysdb; struct nss_ctx *nctx; int ret; + int err; const char *attrs[] = {SYSDB_NAME, SYSDB_OBJECTCLASS, SYSDB_SID_STR, NULL}; bool user_found = false; bool group_found = false; @@ -3825,12 +3833,14 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx) if (cmdctx->cmd == SSS_NSS_GETSIDBYNAME) { ret = sss_ncache_set_user(nctx->ncache, false, dom, name); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negcache for %s@%s\n", name, dom->name)); } ret = sss_ncache_set_group(nctx->ncache, false, dom, name); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negcache for %s@%s\n", name, dom->name)); } } /* if a multidomain search, try with next */ @@ -3896,14 +3906,16 @@ done: if (cmdctx->cmd == SSS_NSS_GETSIDBYID) { DEBUG(SSSDBG_MINOR_FAILURE, ("No matching domain found for [%d], fail!\n", cmdctx->id)); - ret = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; + err = sss_ncache_set_uid(nctx->ncache, false, cmdctx->id); + if (err != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negative cache for UID %d\n", cmdctx->id)); } - ret = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); - if (ret != EOK) { - return ret; + err = sss_ncache_set_gid(nctx->ncache, false, cmdctx->id); + if (err != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negative cache for GID %d\n", cmdctx->id)); } } else { DEBUG(SSSDBG_MINOR_FAILURE, @@ -3953,7 +3965,8 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx) /* set negative cache only if not result of cache check */ ret = sss_ncache_set_sid(nctx->ncache, false, cmdctx->secid); if (ret != EOK) { - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot set negative cache for %s\n", cmdctx->secid)); } return ENOENT; -- 1.8.3.1
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
