From 7618d442a25a16940f0842d2ad4ae34e60079844 Mon Sep 17 00:00:00 2001

From: amitkuma <amitkuma@redhat.com>

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

Subject: [PATCH 201/201] 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 |  2 +

 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        | 81 +++++++++++++++++++++-

 src/util/util_errors.c                             |  1 +

 src/util/util_errors.h                             |  1 +

 10 files changed, 117 insertions(+), 2 deletions(-)

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

index 83eab8ed2de0b5c7d25306d853aadc9b53cb4842..3ebf64af24bb9a2a67165dd17d27fd25fc7915c2 100644

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

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

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

         }

         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 c0ee5f969f2a171b8a6eb396b3d14b593d157b76..5b79d9429bc29c47c28e0b5a6367a6e7035ca1c3 100644

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

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

@@ -165,4 +165,6 @@ bool

 cache_req_common_dp_recv(struct tevent_req *subreq,

                          struct cache_req *cr);

+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 56d0345cd8f98de574961d3c9628ae7a4c24f9be..ba25dcf578ef80aa8597bccb41da839d7bd146b3 100644

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

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

@@ -197,6 +197,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 b80f310feeebbdbc824db441ff5313632585d3fb..d7c6ab2eac412042f90d491eb8428116156f512b 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 121f95abe86d2466aaea69f0fe68dfb33b1fee9e..04e085456f38e154a04ce0a803f2ba22830c1827 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

@@ -64,6 +64,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 4c88e1035b41969703c1c38d740e15516ac0d622..50c149e8795a61307e445d4c063dc0df1166519c 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

@@ -90,6 +90,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 3c25c7631b3da4a829ab577629334a7ee97980da..05add9564f57655bb53520b848971286bba6cab5 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

@@ -64,6 +64,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 80086232fd437876c2b190fb972c2ee3194d9efd..82ba4125265d0bacd44a9f0082d588eef673a6f0 100644

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

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

@@ -58,6 +58,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, \

@@ -451,7 +456,8 @@ __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(void **state)

+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;

@@ -465,7 +471,7 @@ static int test_single_domain_setup(void **state)

     *state = test_ctx;

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

-                                         TEST_DOM_NAME, TEST_ID_PROVIDER, NULL);

+                                         TEST_DOM_NAME, TEST_ID_PROVIDER, params);

     assert_non_null(test_ctx->tctx);

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

@@ -480,6 +486,11 @@ static int test_single_domain_setup(void **state)

     return 0;

 }

+static int test_single_domain_setup(void **state)

+{

+    return test_single_domain_setup_common(state, NULL);

+}

+

 static int test_single_domain_teardown(void **state)

 {

     struct cache_req_test_ctx *test_ctx;

@@ -495,6 +506,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;

@@ -526,6 +547,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;

@@ -1084,6 +1131,32 @@ void test_group_by_name_multiple_domains_found(void **state)

     check_group(test_ctx, &groups[0], domain);

 }

+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_group_by_name_multiple_domains_notfound(void **state)

 {

     struct cache_req_test_ctx *test_ctx = NULL;

@@ -2170,6 +2243,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),

@@ -2180,6 +2255,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 97eaf160f20bcc8cfe52254070a2d182e19addd4..9f6e413f0414f38dee4f0ee6b5f3fbeaac5d6612 100644

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

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

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

     { "Unable to initialize SSL" }, /* ERR_SSL_FAILURE */

     { "Unable to verify peer" }, /* ERR_UNABLE_TO_VERIFY_PEER */

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

+    { "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 4a250bf0339ba689680c155fa8e6d43f42c2467e..051111ea7c249a5d8e4f302022898648ba2ffdd2 100644

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

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

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

     ERR_SSL_FAILURE,

     ERR_UNABLE_TO_VERIFY_PEER,

     ERR_UNABLE_TO_RESOLVE_HOST,

+    ERR_ID_OUTSIDE_RANGE,

     ERR_LAST            /* ALWAYS LAST */

 };

-- 

2.14.3