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