From 2f712c8fe0ecaa07f7b15ebeae5213978d033278 Mon Sep 17 00:00:00 2001

From: amitkuma <amitkuma@redhat.com>

Date: Thu, 30 Nov 2017 22:18:39 +0530

Subject: [PATCH 87/87] cache: Check for max_id/min_id in cache_req

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

The cache_req code doesn't check the min_id/max_id

boundaries for requests by ID.

Extending the .lookup_fn function in each plugin

that searches by ID for a check that returns non-zero

if the entry is out of the range and 0 if not.

Resolves: https://pagure.io/SSSD/sssd/issue/3569

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

(cherry picked from commit 2af80640f18966d65cf82106059ce3c060df93bf)

---

 src/responder/common/cache_req/cache_req.c         |   1 +

 src/responder/common/cache_req/cache_req_private.h |   3 +

 src/responder/common/cache_req/cache_req_search.c  |   5 +

 .../common/cache_req/plugins/cache_req_common.c    |  11 ++

 .../cache_req/plugins/cache_req_group_by_id.c      |   6 +

 .../cache_req/plugins/cache_req_object_by_id.c     |   6 +

 .../cache_req/plugins/cache_req_user_by_id.c       |   5 +

 src/tests/cmocka/test_responder_cache_req.c        | 127 +++++++++++++++++----

 src/util/util_errors.c                             |   1 +

 src/util/util_errors.h                             |   1 +

 10 files changed, 141 insertions(+), 25 deletions(-)

diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c

index ad9bc040dd999a205713141e6a1512e47b69c45e..134688b0f62c6546763d91468af3f54b73b6073a 100644

--- a/src/responder/common/cache_req/cache_req.c

+++ b/src/responder/common/cache_req/cache_req.c

@@ -953,6 +953,7 @@ static void cache_req_search_domains_done(struct tevent_req *subreq)

             goto done;

         }

         break;

+    case ERR_ID_OUTSIDE_RANGE:

     case ENOENT:

         if (state->check_next == false) {

             /* Not found. */

diff --git a/src/responder/common/cache_req/cache_req_private.h b/src/responder/common/cache_req/cache_req_private.h

index 95f24c0e5b9ab1150591d308c7288c57fe478c5d..9538b9568ca7f77e377cfee67235c8a52ebbe454 100644

--- a/src/responder/common/cache_req/cache_req_private.h

+++ b/src/responder/common/cache_req/cache_req_private.h

@@ -192,4 +192,7 @@ cache_reg_common_get_acct_domain_recv(TALLOC_CTX *mem_ctx,

                                       struct tevent_req *subreq,

                                       struct cache_req *cr,

                                       char **_domain);

+

+errno_t cache_req_idminmax_check(struct cache_req_data *data,

+                                 struct sss_domain_info *domain);

 #endif /* _CACHE_REQ_PRIVATE_H_ */

diff --git a/src/responder/common/cache_req/cache_req_search.c b/src/responder/common/cache_req/cache_req_search.c

index 3365962d473b0982945de2541e44ba86b43a0db5..7423feb6305df87d368bcc10ba28b9b29d57ecf0 100644

--- a/src/responder/common/cache_req/cache_req_search.c

+++ b/src/responder/common/cache_req/cache_req_search.c

@@ -203,6 +203,11 @@ static errno_t cache_req_search_cache(TALLOC_CTX *mem_ctx,

         *_result = result;

         break;

+    case ERR_ID_OUTSIDE_RANGE:

+        CACHE_REQ_DEBUG(SSSDBG_TRACE_FUNC, cr,

+                        "ID [%s] was filtered out\n",

+                        cr->debugobj);

+        break;

     case ENOENT:

         CACHE_REQ_DEBUG(SSSDBG_TRACE_FUNC, cr,

                         "Object [%s] was not found in cache\n",

diff --git a/src/responder/common/cache_req/plugins/cache_req_common.c b/src/responder/common/cache_req/plugins/cache_req_common.c

index 408c91949ceb3ecaf743f270f58f4e3fcfc3ccb1..bb11eaa86a8bca3f9d15afe48dab9921319d184e 100644

--- a/src/responder/common/cache_req/plugins/cache_req_common.c

+++ b/src/responder/common/cache_req/plugins/cache_req_common.c

@@ -26,6 +26,17 @@

 #include "providers/data_provider.h"

 #include "responder/common/cache_req/cache_req_plugin.h"

+errno_t cache_req_idminmax_check(struct cache_req_data *data,

+	                         struct sss_domain_info *domain)

+{

+   if (((domain->id_min != 0) && (data->id < domain->id_min)) ||

+       ((domain->id_max != 0) && (data->id > domain->id_max))) {

+        DEBUG(SSSDBG_FUNC_DATA, "id exceeds min/max boundaries\n");

+        return ERR_ID_OUTSIDE_RANGE;

+   }

+   return EOK;

+}

+

 static struct ldb_message *

 cache_req_well_known_sid_msg(TALLOC_CTX *mem_ctx,

                              const char *sid,

diff --git a/src/responder/common/cache_req/plugins/cache_req_group_by_id.c b/src/responder/common/cache_req/plugins/cache_req_group_by_id.c

index ce84b1b4458b447ff6b4b036c6e8fe8f4d7758c8..d178283c33c84e277b83772d04973aa6069af967 100644

--- a/src/responder/common/cache_req/plugins/cache_req_group_by_id.c

+++ b/src/responder/common/cache_req/plugins/cache_req_group_by_id.c

@@ -81,6 +81,12 @@ cache_req_group_by_id_lookup(TALLOC_CTX *mem_ctx,

                              struct sss_domain_info *domain,

                              struct ldb_result **_result)

 {

+    errno_t ret;

+

+    ret = cache_req_idminmax_check(data, domain);

+    if (ret != EOK) {

+	return ret;

+    }

     return sysdb_getgrgid_with_views(mem_ctx, domain, data->id, _result);

 }

diff --git a/src/responder/common/cache_req/plugins/cache_req_object_by_id.c b/src/responder/common/cache_req/plugins/cache_req_object_by_id.c

index 1327b480c1b1b68f9826fa229c9b001f2d92b79b..be9488d298885320139ccfcd3c59a83ff088e77d 100644

--- a/src/responder/common/cache_req/plugins/cache_req_object_by_id.c

+++ b/src/responder/common/cache_req/plugins/cache_req_object_by_id.c

@@ -110,6 +110,12 @@ cache_req_object_by_id_lookup(TALLOC_CTX *mem_ctx,

                               struct sss_domain_info *domain,

                               struct ldb_result **_result)

 {

+    errno_t ret;

+

+    ret = cache_req_idminmax_check(data, domain);

+    if (ret != EOK) {

+        return ret;

+    }

     return sysdb_search_object_by_id(mem_ctx, domain, data->id,

                                      data->attrs, _result);

 }

diff --git a/src/responder/common/cache_req/plugins/cache_req_user_by_id.c b/src/responder/common/cache_req/plugins/cache_req_user_by_id.c

index 656fa41af5f39f68c64e241aa97c4eaf3ec57395..151c3e17acf6ef0d958d5a73a36e1c93b9e7a9a9 100644

--- a/src/responder/common/cache_req/plugins/cache_req_user_by_id.c

+++ b/src/responder/common/cache_req/plugins/cache_req_user_by_id.c

@@ -81,6 +81,11 @@ cache_req_user_by_id_lookup(TALLOC_CTX *mem_ctx,

                             struct sss_domain_info *domain,

                             struct ldb_result **_result)

 {

+    errno_t ret;

+    ret = cache_req_idminmax_check(data, domain);

+    if (ret != EOK) {

+        return ret;

+    }

     return sysdb_getpwuid_with_views(mem_ctx, domain, data->id, _result);

 }

diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c

index 0ee0070d0c9fbb89020f522b2f7613f1076a8cbb..5f50b27a5ee846c9ccf71e1e661359a07c2e02e8 100644

--- a/src/tests/cmocka/test_responder_cache_req.c

+++ b/src/tests/cmocka/test_responder_cache_req.c

@@ -59,6 +59,11 @@ struct test_group {

                                     test_single_domain_setup, \

                                     test_single_domain_teardown)

+#define new_single_domain_id_limit_test(test) \

+    cmocka_unit_test_setup_teardown(test_ ## test, \

+                                    test_single_domain_id_limits_setup, \

+                                    test_single_domain_teardown)

+

 #define new_multi_domain_test(test) \

     cmocka_unit_test_setup_teardown(test_ ## test, \

                                     test_multi_domain_setup, \

@@ -521,33 +526,39 @@ __wrap_sss_dp_get_account_send(TALLOC_CTX *mem_ctx,

     return test_req_succeed_send(mem_ctx, rctx->ev);

 }

+static int test_single_domain_setup_common(void **state,

+                                           struct sss_test_conf_param *params)

+{

+    struct cache_req_test_ctx *test_ctx = NULL;

+    errno_t ret;

+

+    assert_true(leak_check_setup());

+

+    test_dom_suite_setup(TESTS_PATH);

+

+    test_ctx = talloc_zero(global_talloc_context, struct cache_req_test_ctx);

+    assert_non_null(test_ctx);

+    *state = test_ctx;

+

+    test_ctx->tctx = create_dom_test_ctx(test_ctx, TESTS_PATH, TEST_CONF_DB,

+                                         TEST_DOM_NAME, TEST_ID_PROVIDER, params);

+    assert_non_null(test_ctx->tctx);

+

+    test_ctx->rctx = mock_rctx(test_ctx, test_ctx->tctx->ev,

+                               test_ctx->tctx->dom, NULL);

+    assert_non_null(test_ctx->rctx);

+

+    ret = sss_ncache_init(test_ctx, 10, 0, &test_ctx->ncache);

+    assert_int_equal(ret, EOK);

+

+    check_leaks_push(test_ctx);

+

+    return 0;

+}

+

 static int test_single_domain_setup(void **state)

 {

-    struct cache_req_test_ctx *test_ctx = NULL;

-    errno_t ret;

-

-    assert_true(leak_check_setup());

-

-    test_dom_suite_setup(TESTS_PATH);

-

-    test_ctx = talloc_zero(global_talloc_context, struct cache_req_test_ctx);

-    assert_non_null(test_ctx);

-    *state = test_ctx;

-

-    test_ctx->tctx = create_dom_test_ctx(test_ctx, TESTS_PATH, TEST_CONF_DB,

-                                         TEST_DOM_NAME, TEST_ID_PROVIDER, NULL);

-    assert_non_null(test_ctx->tctx);

-

-    test_ctx->rctx = mock_rctx(test_ctx, test_ctx->tctx->ev,

-                               test_ctx->tctx->dom, NULL);

-    assert_non_null(test_ctx->rctx);

-

-    ret = sss_ncache_init(test_ctx, 10, 0, &test_ctx->ncache);

-    assert_int_equal(ret, EOK);

-

-    check_leaks_push(test_ctx);

-

-    return 0;

+    return test_single_domain_setup_common(state, NULL);

 }

 static int test_single_domain_teardown(void **state)

@@ -565,6 +576,16 @@ static int test_single_domain_teardown(void **state)

     return 0;

 }

+static int test_single_domain_id_limits_setup(void **state)

+{

+    struct sss_test_conf_param params[] = {

+        { "min_id", "100" },

+        { "max_id", "10000" },

+        { NULL, NULL },             /* Sentinel */

+    };

+    return test_single_domain_setup_common(state, params);

+}

+

 static int test_multi_domain_setup(void **state)

 {

     struct cache_req_test_ctx *test_ctx = NULL;

@@ -596,6 +617,32 @@ static int test_multi_domain_setup(void **state)

     return 0;

 }

+void test_user_by_id_below_id_range(void **state)

+{

+    struct cache_req_test_ctx *test_ctx = NULL;

+

+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);

+

+    /* Test. */

+    run_cache_req(test_ctx, cache_req_user_by_id_send,

+                  cache_req_user_by_id_test_done, test_ctx->tctx->dom,

+                  0, 10, ENOENT);

+    assert_false(test_ctx->dp_called);

+}

+

+void test_user_by_id_above_id_range(void **state)

+{

+    struct cache_req_test_ctx *test_ctx = NULL;

+

+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);

+

+    /* Test. */

+    run_cache_req(test_ctx, cache_req_user_by_id_send,

+                  cache_req_user_by_id_test_done, test_ctx->tctx->dom,

+                  0, 100000, ENOENT);

+    assert_false(test_ctx->dp_called);

+}

+

 static int test_multi_domain_teardown(void **state)

 {

     struct cache_req_test_ctx *test_ctx;

@@ -1332,6 +1379,32 @@ void test_user_by_id_sub_domains_locator_missing_found(void **state)

     talloc_free(tmp_ctx);

 }

+void test_group_by_id_below_id_range(void **state)

+{

+    struct cache_req_test_ctx *test_ctx = NULL;

+

+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);

+

+    /* Test. */

+    run_cache_req(test_ctx, cache_req_group_by_id_send,

+                  cache_req_group_by_id_test_done, test_ctx->tctx->dom,

+                  0, 10, ENOENT);

+    assert_false(test_ctx->dp_called);

+}

+

+void test_group_by_id_above_id_range(void **state)

+{

+    struct cache_req_test_ctx *test_ctx = NULL;

+

+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);

+

+    /* Test. */

+    run_cache_req(test_ctx, cache_req_group_by_id_send,

+                  cache_req_group_by_id_test_done, test_ctx->tctx->dom,

+                  0, 100000, ENOENT);

+    assert_false(test_ctx->dp_called);

+}

+

 void test_user_by_id_sub_domains_locator_missing_notfound(void **state)

 {

     struct cache_req_test_ctx *test_ctx = NULL;

@@ -3874,6 +3947,8 @@ int main(int argc, const char *argv[])

         new_single_domain_test(user_by_id_missing_notfound),

         new_multi_domain_test(user_by_id_multiple_domains_found),

         new_multi_domain_test(user_by_id_multiple_domains_notfound),

+        new_single_domain_id_limit_test(user_by_id_below_id_range),

+        new_single_domain_id_limit_test(user_by_id_above_id_range),

         new_single_domain_test(group_by_name_cache_valid),

         new_single_domain_test(group_by_name_cache_expired),

@@ -3884,6 +3959,8 @@ int main(int argc, const char *argv[])

         new_multi_domain_test(group_by_name_multiple_domains_found),

         new_multi_domain_test(group_by_name_multiple_domains_notfound),

         new_multi_domain_test(group_by_name_multiple_domains_parse),

+        new_single_domain_id_limit_test(group_by_id_below_id_range),

+        new_single_domain_id_limit_test(group_by_id_above_id_range),

         new_single_domain_test(group_by_id_cache_valid),

         new_single_domain_test(group_by_id_cache_expired),

diff --git a/src/util/util_errors.c b/src/util/util_errors.c

index 06c620b40aaa00d6ce58ace3a28449ffbdf8da88..39ce3d7dcf4af4c489a0a9b7768668497cb84ba5 100644

--- a/src/util/util_errors.c

+++ b/src/util/util_errors.c

@@ -117,6 +117,7 @@ struct err_string error_to_str[] = {

     { "Unable to resolve host" }, /* ERR_UNABLE_TO_RESOLVE_HOST */

     { "GetAccountDomain() not supported" }, /* ERR_GET_ACCT_DOM_NOT_SUPPORTED */

     { "The last GetAccountDomain() result is still valid" }, /* ERR_GET_ACCT_DOM_CACHED */

+    { "ID is outside the allowed range" }, /* ERR_ID_OUTSIDE_RANGE */

     { "ERR_LAST" } /* ERR_LAST */

 };

diff --git a/src/util/util_errors.h b/src/util/util_errors.h

index bebd6e198fc0077891a602f80182a993ce3f789b..621a3b116edac45960190684055bcd0692135957 100644

--- a/src/util/util_errors.h

+++ b/src/util/util_errors.h

@@ -139,6 +139,7 @@ enum sssd_errors {

     ERR_UNABLE_TO_RESOLVE_HOST,

     ERR_GET_ACCT_DOM_NOT_SUPPORTED,

     ERR_GET_ACCT_DOM_CACHED,

+    ERR_ID_OUTSIDE_RANGE,

     ERR_LAST            /* ALWAYS LAST */

 };

-- 

2.14.3