From 7618d442a25a16940f0842d2ad4ae34e60079844 Mon Sep 17 00:00:00 2001 From: amitkuma 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 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 | 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