>From cbe9f368abca8cda8869632f88cb82b6861aa1b1 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <preichl@redhat.com>
Date: Wed, 29 Oct 2014 13:11:28 +0000
Subject: [PATCH] SYSDB: sysdb_search_object_by_sid returns ENOENT

sysdb_search_object_by_sid returns ENOENT if no results are found.

Part od solution for:
https://fedorahosted.org/sssd/ticket/1991
---
 src/db/sysdb_ops.c             | 31 +++++++++++++++++--------------
 src/responder/nss/nsssrv_cmd.c | 27 ++++++++++++++-------------
 src/responder/pac/pacsrv_cmd.c | 26 ++++++++++++++++----------
 src/tests/sysdb-tests.c        | 26 ++++++++++++++++++++++----
 4 files changed, 69 insertions(+), 41 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index c88ff9b4bf39c649bc7993f3ed56729b7bfde310..e4080d2aeb1e485d9957d3c043468f8bd914c4f9 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2995,8 +2995,15 @@ int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
 
     ret = sysdb_search_object_by_sid(tmp_ctx, domain, sid_str, NULL, &res);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "search by sid failed: %d (%s)\n",
-              ret, strerror(ret));
+        if (ret == ENOENT) {
+            /* No existing entry. Just quit. */
+            DEBUG(SSSDBG_TRACE_FUNC,
+                  "search by sid did not return any results.\n");
+            ret = EOK;
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "search by sid failed: %d (%s)\n",
+                  ret, strerror(ret));
+        }
         goto done;
     }
 
@@ -3007,12 +3014,6 @@ int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    if (res->count == 0) {
-        /* No existing entry. Just quit. */
-        ret = EOK;
-        goto done;
-    }
-
     ret = sysdb_delete_entry(sysdb, res->msgs[0]->dn, false);
     if (ret != EOK) {
         goto done;
@@ -3518,19 +3519,21 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
         return ENOMEM;
     }
 
-    basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb, SYSDB_DOM_BASE, domain->name);
+    basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb, SYSDB_DOM_BASE,
+                            domain->name);
     if (basedn == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new_fmt failed.\n");
         ret = ENOMEM;
         goto done;
     }
 
-    ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res,
-                     basedn, LDB_SCOPE_SUBTREE, attrs?attrs:def_attrs,
-                     SYSDB_SID_FILTER, sid_str);
+    SSS_LDB_SEARCH(ret, domain->sysdb->ldb, tmp_ctx, &res,
+                   basedn, LDB_SCOPE_SUBTREE, attrs?attrs:def_attrs,
+                   SYSDB_SID_FILTER, sid_str);
     if (ret != EOK) {
-        ret = sysdb_error_to_errno(ret);
-        DEBUG(SSSDBG_OP_FAILURE, "ldb_search failed.\n");
+        if (ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "ldb_search failed.\n");
+        }
         goto done;
     }
 
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 9fca644be164e682f787bda61ea39afa8b703874..0c7515fcfe71c189c4c890ca181aae857f69810c 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4449,6 +4449,20 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
     ret = sysdb_search_object_by_sid(cmdctx, dom, cmdctx->secid, NULL,
                                      &dctx->res);
     if (ret != EOK) {
+        if (ret == ENOENT) {
+            if (!dctx->check_provider) {
+                DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n");
+
+                /* set negative cache only if not result of cache check */
+                ret = sss_ncache_set_sid(nctx->ncache, false, cmdctx->secid);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_MINOR_FAILURE,
+                          "Cannot set negative cache for %s\n", cmdctx->secid);
+                }
+            }
+            return ENOENT;
+        }
+
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n");
         return EIO;
     }
@@ -4459,19 +4473,6 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
         return ENOENT;
     }
 
-    if (dctx->res->count == 0 && !dctx->check_provider) {
-        DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n");
-
-        /* set negative cache only if not result of cache check */
-        ret = sss_ncache_set_sid(nctx->ncache, false, cmdctx->secid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  "Cannot set negative cache for %s\n", cmdctx->secid);
-        }
-
-        return ENOENT;
-    }
-
     /* if this is a caching provider (or if we haven't checked the cache
      * yet) then verify that the cache is uptodate */
     if (dctx->check_provider) {
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index cc92592893899b1fa269188facc1a8154f80991d..5cedea14747b578dd7896d4d9a80c2e81fc04c17 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -298,16 +298,17 @@ static void pac_lookup_sids_done(struct tevent_req *req)
             ret = sysdb_search_object_by_sid(pr_ctx, dom, entries[c].key.str,
                                              NULL, &msg);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \
-                                          "failed.\n");
+                if (ret == ENOENT) {
+                    DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
+                          entries[c].key.str);
+                } else {
+                    DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \
+                          "failed.\n");
+                }
                 continue;
             }
 
-            if (msg->count == 0) {
-                DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
-                                          entries[c].key.str);
-                continue;
-            } else if (msg->count > 1) {
+            if (msg->count > 1) {
                 DEBUG(SSSDBG_CRIT_FAILURE, "More then one result returned " \
                                             "for SID [%s].\n",
                                             entries[c].key.str);
@@ -912,9 +913,14 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     ret = sysdb_search_object_by_sid(tmp_ctx, grp_dom, grp_sid_str,
                                      group_attrs, &group);
     if (ret != EOK) {
-        DEBUG(SSSDBG_TRACE_INTERNAL, "sysdb_search_object_by_sid " \
-                                      "for SID [%s] failed [%d][%s].\n",
-                                      grp_sid_str, ret, strerror(ret));
+        if (ret == ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "Unexpected number of groups returned.\n");
+            ret = EINVAL;
+        } else {
+            DEBUG(SSSDBG_TRACE_INTERNAL, "sysdb_search_object_by_sid "  \
+                  "for SID [%s] failed [%d][%s].\n",
+                  grp_sid_str, ret, strerror(ret));
+        }
         goto done;
     }
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index e01ddf4782c0a5a557f39d1adc2efd74b6234461..c9f141ab066946d9fc9baabab32b833a6a2a1187 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4803,13 +4803,10 @@ START_TEST (test_sysdb_search_return_ENOENT)
     talloc_zfree(res);
 
     /* Search object */
-    /* TODO: Should return ENOENT */
     ret = sysdb_search_object_by_sid(test_ctx, test_ctx->domain,
                                      "S-5-4-3-2-1", NULL, &res);
-    fail_unless(ret == EOK, "sysdb_search_object_by_sid_str failed with "
+    fail_unless(ret == ENOENT, "sysdb_search_object_by_sid_str failed with "
                              "[%d][%s].", ret, strerror(ret));
-    fail_unless(res->count == 0, "sysdb_search_object_by_sid_str should not "
-                                 "return anything.");
     talloc_zfree(res);
 
     /* Search can return more results */
@@ -5026,6 +5023,26 @@ START_TEST(test_sysdb_search_sid_str)
 }
 END_TEST
 
+START_TEST(test_sysdb_delete_by_sid)
+{
+    errno_t ret;
+    struct sysdb_test_ctx *test_ctx;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_if(ret != EOK, "Could not set up the test");
+    check_leaks_push(test_ctx);
+
+    /* Delete the group by SID */
+    ret = sysdb_delete_by_sid(test_ctx->sysdb, test_ctx->domain,
+                              "S-1-2-3-4-NON_EXISTING_SID");
+    fail_unless(ret == EOK, "sysdb_delete_by_sid failed with [%d][%s].",
+                ret, strerror(ret));
+
+    fail_unless(check_leaks_pop(test_ctx) == true, "Memory leak");
+    talloc_free(test_ctx);
+}
+END_TEST
 
 START_TEST(test_sysdb_subdomain_create)
 {
@@ -6070,6 +6087,7 @@ Suite *create_sysdb_suite(void)
     tcase_add_test(tc_sysdb, test_sysdb_search_custom_update);
     tcase_add_test(tc_sysdb, test_sysdb_search_custom);
     tcase_add_test(tc_sysdb, test_sysdb_delete_custom);
+    tcase_add_test(tc_sysdb, test_sysdb_delete_by_sid);
 
     /* test recursive delete */
     tcase_add_test(tc_sysdb, test_sysdb_delete_recursive);
-- 
1.9.3

