From 2f712c8fe0ecaa07f7b15ebeae5213978d033278 Mon Sep 17 00:00:00 2001 From: amitkuma 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 Reviewed-by: Fabiano Fidêncio (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