cache_req: do not set ncache if dp request fails

We will only remember entry in the negative cache if the data provider
requests succeeded because only then we can be sure that the entry
does not exist.

Resolves:
https://fedorahosted.org/sssd/ticket/3151

Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

3 files changed, 36 insertions, 10 deletions

diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c
index 3a37d652b..f546e6130 100644
--- a/src/responder/common/cache_req/cache_req.c
+++ b/src/responder/common/cache_req/cache_req.c
@@ -317,6 +317,7 @@ struct cache_req_state {
     struct cache_req_result **results;
     size_t num_results;
     bool check_next;
+    bool dp_success;
 };
 
 static errno_t cache_req_add_result(struct cache_req_state *state,
@@ -357,6 +358,7 @@ struct tevent_req *cache_req_send(TALLOC_CTX *mem_ctx,
     }
 
     state->ev = ev;
+    state->dp_success = true;
     state->cr = cr = cache_req_create(state, rctx, data, ncache, midpoint);
     if (state->cr == NULL) {
         ret = ENOMEM;
@@ -563,9 +565,16 @@ static errno_t cache_req_next_domain(struct tevent_req *req)
     /* We have searched all available domains and no result was found.
      *
      * If the plug-in uses a negative cache which is shared among all domains
-     * (e.g. unique identifires such as user or group id or sid), we add it
-     * here and return object not found error. */
-    cache_req_global_ncache_add(cr);
+     * (e.g. unique identifiers such as user or group id or sid), we add it
+     * here and return object not found error.
+     *
+     * However, we can only set the negative cache if all data provider
+     * requests succeeded because only then we can be sure that it does
+     * not exist-
+     */
+    if (state->dp_success) {
+        cache_req_global_ncache_add(cr);
+    }
 
     return ENOENT;
 }
@@ -626,14 +635,18 @@ static void cache_req_done(struct tevent_req *subreq)
     struct cache_req_state *state;
     struct ldb_result *result;
     struct tevent_req *req;
+    bool dp_success;
     errno_t ret;
 
     req = tevent_req_callback_data(subreq, struct tevent_req);
     state = tevent_req_data(req, struct cache_req_state);
 
-    ret = cache_req_search_recv(state, subreq, &result);
+    ret = cache_req_search_recv(state, subreq, &result, &dp_success);
     talloc_zfree(subreq);
 
+    /* Remember if any DP request fails. */
+    state->dp_success = !dp_success ? false : state->dp_success;
+
     switch (ret) {
     case EOK:
         /* We got some data from this search. Save it. */
diff --git a/src/responder/common/cache_req/cache_req_private.h b/src/responder/common/cache_req/cache_req_private.h
index 978389a93..e79fe869b 100644
--- a/src/responder/common/cache_req/cache_req_private.h
+++ b/src/responder/common/cache_req/cache_req_private.h
@@ -97,7 +97,8 @@ cache_req_search_send(TALLOC_CTX *mem_ctx,
 
 errno_t cache_req_search_recv(TALLOC_CTX *mem_ctx,
                               struct tevent_req *req,
-                              struct ldb_result **_result);
+                              struct ldb_result **_result,
+                              bool *_dp_success);
 
 struct tevent_req *
 cache_req_steal_data_and_send(TALLOC_CTX *mem_ctx,
diff --git a/src/responder/common/cache_req/cache_req_search.c b/src/responder/common/cache_req/cache_req_search.c
index 2f8234029..aabff389a 100644
--- a/src/responder/common/cache_req/cache_req_search.c
+++ b/src/responder/common/cache_req/cache_req_search.c
@@ -168,7 +168,7 @@ static errno_t cache_req_dpreq_params(TALLOC_CTX *mem_ctx,
     return EOK;
 }
 
-static void cache_req_search_process_dp(TALLOC_CTX *mem_ctx,
+static bool cache_req_search_process_dp(TALLOC_CTX *mem_ctx,
                                         struct tevent_req *subreq,
                                         struct cache_req *cr)
 {
@@ -185,6 +185,8 @@ static void cache_req_search_process_dp(TALLOC_CTX *mem_ctx,
                         ret, sss_strerror(ret));
         CACHE_REQ_DEBUG(SSSDBG_TRACE_FUNC, cr,
                         "Due to an error we will return cached data\n");
+
+        return false;
     }
 
     if (err_maj) {
@@ -193,9 +195,11 @@ static void cache_req_search_process_dp(TALLOC_CTX *mem_ctx,
                         (unsigned int)err_maj, (unsigned int)err_min, err_msg);
         CACHE_REQ_DEBUG(SSSDBG_TRACE_FUNC, cr,
                         "Due to an error we will return cached data\n");
+
+        return false;
     }
 
-    return;
+    return true;
 }
 
 static enum cache_object_status
@@ -230,6 +234,7 @@ struct cache_req_search_state {
 
     /* output data */
     struct ldb_result *result;
+    bool dp_success;
 };
 
 static errno_t cache_req_search_dp(struct tevent_req *req,
@@ -390,12 +395,16 @@ static void cache_req_search_done(struct tevent_req *subreq)
     req = tevent_req_callback_data(subreq, struct tevent_req);
     state = tevent_req_data(req, struct cache_req_search_state);
 
-    cache_req_search_process_dp(state, subreq, state->cr);
+    state->dp_success = cache_req_search_process_dp(state, subreq, state->cr);
 
     /* Get result from cache again. */
     ret = cache_req_search_cache(state, state->cr, &state->result);
     if (ret == ENOENT) {
-        cache_req_search_ncache_add(state->cr);
+        /* Only store entry in negative cache if DP request succeeded
+         * because only then we know that the entry does not exist. */
+        if (state->dp_success) {
+            cache_req_search_ncache_add(state->cr);
+        }
         tevent_req_error(req, ENOENT);
         return;
     } else if (ret != EOK) {
@@ -412,11 +421,14 @@ static void cache_req_search_done(struct tevent_req *subreq)
 
 errno_t cache_req_search_recv(TALLOC_CTX *mem_ctx,
                               struct tevent_req *req,
-                              struct ldb_result **_result)
+                              struct ldb_result **_result,
+                              bool *_dp_success)
 {
     struct cache_req_search_state *state = NULL;
     state = tevent_req_data(req, struct cache_req_search_state);
 
+    *_dp_success = state->dp_success;
+
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
     *_result = talloc_steal(mem_ctx, state->result);