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