From b4abe4088ceec0189f97b1a0e3fce37c23066206 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Fri, 15 Apr 2011 09:19:40 -0400 Subject: Fix regressions in the negative cache Do not throw a DP error when failing to delete a nonexistent entity Add debug logging to the negative cache Fix a regression with the negative cache in multi-domain configurations Fix regression where nonexistent entries were never added to the negative cache --- src/providers/ldap/ldap_id.c | 8 +++--- src/responder/common/negcache.c | 5 ++++ src/responder/nss/nsssrv_cmd.c | 63 +++++++++++++++++++++++++---------------- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index 776df1ac2..a6fb05bd9 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -219,7 +219,7 @@ static void users_get_done(struct tevent_req *subreq) case BE_FILTER_NAME: ret = sysdb_delete_user(state, state->sysdb, state->domain, state->name, 0); - if (ret) { + if (ret != EOK && ret != ENOENT) { tevent_req_error(req, ret); return; } @@ -234,7 +234,7 @@ static void users_get_done(struct tevent_req *subreq) ret = sysdb_delete_user(state, state->sysdb, state->domain, NULL, uid); - if (ret) { + if (ret != EOK && ret != ENOENT) { tevent_req_error(req, ret); return; } @@ -453,7 +453,7 @@ static void groups_get_done(struct tevent_req *subreq) case BE_FILTER_NAME: ret = sysdb_delete_group(state, state->sysdb, state->domain, state->name, 0); - if (ret) { + if (ret != EOK && ret != ENOENT) { tevent_req_error(req, ret); return; } @@ -468,7 +468,7 @@ static void groups_get_done(struct tevent_req *subreq) ret = sysdb_delete_group(state, state->sysdb, state->domain, NULL, gid); - if (ret) { + if (ret != EOK && ret != ENOENT) { tevent_req_error(req, ret); return; } diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c index 5f85df25d..d8c4c2698 100644 --- a/src/responder/common/negcache.c +++ b/src/responder/common/negcache.c @@ -71,6 +71,8 @@ static int sss_ncache_check_str(struct sss_nc_ctx *ctx, char *str, int ttl) char *ep; int ret; + DEBUG(8, ("Checking negative cache for [%s]\n", str)); + ret = string_to_tdb_data(str, &key); if (ret != EOK) goto done; @@ -141,6 +143,9 @@ static int sss_ncache_set_str(struct sss_nc_ctx *ctx, ret = string_to_tdb_data(timest, &data); if (ret != EOK) goto done; + DEBUG(6, ("Adding [%s] to negative cache%s\n", + str, permanent?" permanently":"")); + ret = tdb_store(ctx->tdb, key, data, TDB_REPLACE); if (ret != 0) { DEBUG(1, ("Negative cache failed to set entry: [%s]\n", diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 2153d649c..db301b380 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -532,12 +532,17 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) /* if neg cached, return we didn't find it */ if (ret == EEXIST) { - DEBUG(2, ("User [%s] does not exist! (negative cache)\n", name)); + DEBUG(2, ("User [%s] does not exist in [%s]! (negative cache)\n", + name, dom->name)); /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; continue; } + /* There are no further domains or this was a + * fully-qualified user request. + */ + return ENOENT; } DEBUG(4, ("Requesting info for [%s@%s]\n", name, dom->name)); @@ -560,20 +565,20 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx) } if (dctx->res->count == 0 && !dctx->check_provider) { + /* set negative cache only if not result of cache check */ + ret = sss_ncache_set_user(nctx->ncache, false, dom->name, name); + if (ret != EOK) { + return ret; + } + /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; - continue; + if (dom) continue; } DEBUG(2, ("No results for getpwnam call\n")); - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_user(nctx->ncache, false, dom->name, name); - if (ret != EOK) { - return ret; - } - return ENOENT; } @@ -1794,12 +1799,17 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) /* if neg cached, return we didn't find it */ if (ret == EEXIST) { - DEBUG(2, ("Group [%s] does not exist! (negative cache)\n", name)); + DEBUG(2, ("Group [%s] does not exist in [%s]! (negative cache)\n", + name, dom->name)); /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; continue; } + /* There are no further domains or this was a + * fully-qualified user request. + */ + return ENOENT; } DEBUG(4, ("Requesting info for [%s@%s]\n", name, dom->name)); @@ -1822,20 +1832,20 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx) } if (dctx->res->count == 0 && !dctx->check_provider) { + /* set negative cache only if not result of cache check */ + ret = sss_ncache_set_group(nctx->ncache, false, dom->name, name); + if (ret != EOK) { + return ret; + } + /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; - continue; + if (dom) continue; } DEBUG(2, ("No results for getgrnam call\n")); - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_group(nctx->ncache, false, dom->name, name); - if (ret != EOK) { - return ret; - } - return ENOENT; } @@ -2827,12 +2837,17 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx) /* if neg cached, return we didn't find it */ if (ret == EEXIST) { - DEBUG(2, ("User [%s] does not exist! (negative cache)\n", name)); + DEBUG(2, ("User [%s] does not exist in [%s]! (negative cache)\n", + dom->name, name)); /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; continue; } + /* There are no further domains or this was a + * fully-qualified user request. + */ + return ENOENT; } DEBUG(4, ("Requesting info for [%s@%s]\n", name, dom->name)); @@ -2851,20 +2866,20 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx) } if (dctx->res->count == 0 && !dctx->check_provider) { + /* set negative cache only if not result of cache check */ + ret = sss_ncache_set_user(nctx->ncache, false, dom->name, name); + if (ret != EOK) { + return ret; + } + /* if a multidomain search, try with next */ if (cmdctx->check_next) { dom = dom->next; - continue; + if (dom) continue; } DEBUG(2, ("No results for initgroups call\n")); - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_user(nctx->ncache, false, dom->name, name); - if (ret != EOK) { - return ret; - } - return ENOENT; } -- cgit