URL: https://github.com/SSSD/sssd/pull/884 Author: sumit-bose Title: #884: ipa: support disabled domains Action: opened
PR body: """ IPA does not disable domains with the help of a flag in the domain objects but more general with the help of the SID blacklist. With this patch the blacklist is read with other data about trusted domains and if the domain SID of a trusted domain is found the domain is disabled. As a result uses and groups from this domain cannot be looked up anymore. Related to https://pagure.io/SSSD/sssd/issue/4078 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/884/head:pr884 git checkout pr884
From 2e68ac52f74c792f0ac6743cb301d74f09172734 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 12 Sep 2019 14:49:30 +0200 Subject: [PATCH 1/3] utils: extend some find_domain_* calls to search disabled domain This extension is needed to support disabled domains since it is now important to know if a domain is really unknown or only disabled. While an unknown domain might typically lead to an error, a caller might just ignore requests for disabled domains or objects from disabled domains. Related to https://pagure.io/SSSD/sssd/issue/4078 --- src/providers/ipa/ipa_id.c | 3 +- src/responder/sudo/sudosrv_get_sudorules.c | 3 +- src/tests/cmocka/test_utils.c | 90 ++++++++++++++++++++++ src/util/domain_info_utils.c | 31 +++++--- src/util/util.h | 7 +- 5 files changed, 122 insertions(+), 12 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c index 9abee34cb4..f34692aa24 100644 --- a/src/providers/ipa/ipa_id.c +++ b/src/providers/ipa/ipa_id.c @@ -138,7 +138,8 @@ static errno_t ipa_resolve_user_list_get_user_step(struct tevent_req *req) state->user_domain = find_domain_by_object_name_ex( state->ipa_ctx->sdap_id_ctx->be->domain, - ar->filter_value, true); + ar->filter_value, true, + SSS_GND_DESCEND); /* Use provided domain as fallback because no known domain was found in the * user name. */ if (state->user_domain == NULL) { diff --git a/src/responder/sudo/sudosrv_get_sudorules.c b/src/responder/sudo/sudosrv_get_sudorules.c index a0edb5b7e2..73720e3512 100644 --- a/src/responder/sudo/sudosrv_get_sudorules.c +++ b/src/responder/sudo/sudosrv_get_sudorules.c @@ -147,7 +147,8 @@ static errno_t sudosrv_format_runas(struct resp_ctx *rctx, continue; } - dom = find_domain_by_object_name_ex(rctx->domains, value, true); + dom = find_domain_by_object_name_ex(rctx->domains, value, true, + SSS_GND_DESCEND); if (dom == NULL) { continue; } diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c index 4bff56bb97..666f329030 100644 --- a/src/tests/cmocka/test_utils.c +++ b/src/tests/cmocka/test_utils.c @@ -401,6 +401,92 @@ void test_find_domain_by_name_disabled(void **state) } } +void test_find_domain_by_name_ex_disabled(void **state) +{ + struct dom_list_test_ctx *test_ctx = talloc_get_type(*state, + struct dom_list_test_ctx); + struct sss_domain_info *dom; + struct sss_domain_info *disabled_dom; + size_t c; + size_t mis; + + mis = test_ctx->dom_count/2; + assert_true((mis >= 1 && mis < test_ctx->dom_count)); + + dom = test_ctx->dom_list; + for (c = 0; c < mis; c++) { + assert_non_null(dom); + dom = dom->next; + } + assert_non_null(dom); + sss_domain_set_state(dom, DOM_DISABLED); + disabled_dom = dom; + + dom = find_domain_by_name(test_ctx->dom_list, disabled_dom->name, true); + assert_null(dom); + + dom = find_domain_by_name_ex(test_ctx->dom_list, disabled_dom->name, true, + SSS_GND_DESCEND); + assert_null(dom); + + dom = find_domain_by_name_ex(test_ctx->dom_list, disabled_dom->name, true, + SSS_GND_DESCEND | SSS_GND_INCLUDE_DISABLED); + assert_non_null(dom); + assert_ptr_equal(disabled_dom, dom); + + dom = find_domain_by_name_ex(test_ctx->dom_list, disabled_dom->name, true, + SSS_GND_ALL_DOMAINS); + assert_non_null(dom); + assert_ptr_equal(disabled_dom, dom); +} + +void test_find_domain_by_object_name_ex(void **state) +{ + struct dom_list_test_ctx *test_ctx = talloc_get_type(*state, + struct dom_list_test_ctx); + struct sss_domain_info *dom; + struct sss_domain_info *disabled_dom; + size_t c; + size_t mis; + char *obj_name; + + mis = test_ctx->dom_count/2; + assert_true((mis >= 1 && mis < test_ctx->dom_count)); + + dom = test_ctx->dom_list; + for (c = 0; c < mis; c++) { + assert_non_null(dom); + dom = dom->next; + } + assert_non_null(dom); + sss_domain_set_state(dom, DOM_DISABLED); + disabled_dom = dom; + + obj_name = talloc_asprintf(global_talloc_context, "myname@%s", + disabled_dom->name); + assert_non_null(obj_name); + + + dom = find_domain_by_object_name(test_ctx->dom_list, obj_name); + assert_null(dom); + + dom = find_domain_by_object_name_ex(test_ctx->dom_list, obj_name, true, + SSS_GND_DESCEND); + assert_null(dom); + + dom = find_domain_by_object_name_ex(test_ctx->dom_list, obj_name, true, + SSS_GND_DESCEND | SSS_GND_INCLUDE_DISABLED); + assert_non_null(dom); + assert_ptr_equal(disabled_dom, dom); + + dom = find_domain_by_object_name_ex(test_ctx->dom_list, obj_name, true, + SSS_GND_ALL_DOMAINS); + assert_non_null(dom); + assert_ptr_equal(disabled_dom, dom); + + talloc_free(obj_name); +} + void test_find_domain_by_sid_null(void **state) { struct dom_list_test_ctx *test_ctx = talloc_get_type(*state, @@ -1897,6 +1983,10 @@ int main(int argc, const char *argv[]) setup_dom_list, teardown_dom_list), cmocka_unit_test_setup_teardown(test_find_domain_by_name_disabled, setup_dom_list, teardown_dom_list), + cmocka_unit_test_setup_teardown(test_find_domain_by_name_ex_disabled, + setup_dom_list, teardown_dom_list), + cmocka_unit_test_setup_teardown(test_find_domain_by_object_name_ex, + setup_dom_list, teardown_dom_list), cmocka_unit_test_setup_teardown(test_sss_names_init, confdb_test_setup, diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index b95c89ed90..aa3582f038 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -93,9 +93,10 @@ bool subdomain_enumerates(struct sss_domain_info *parent, return false; } -struct sss_domain_info *find_domain_by_name(struct sss_domain_info *domain, - const char *name, - bool match_any) +struct sss_domain_info *find_domain_by_name_ex(struct sss_domain_info *domain, + const char *name, + bool match_any, + uint32_t gnd_flags) { struct sss_domain_info *dom = domain; @@ -103,21 +104,31 @@ struct sss_domain_info *find_domain_by_name(struct sss_domain_info *domain, return NULL; } - while (dom && sss_domain_get_state(dom) == DOM_DISABLED) { - dom = get_next_domain(dom, SSS_GND_DESCEND); + if (!(gnd_flags & SSS_GND_INCLUDE_DISABLED)) { + while (dom && sss_domain_get_state(dom) == DOM_DISABLED) { + dom = get_next_domain(dom, gnd_flags); + } } + while (dom) { if (strcasecmp(dom->name, name) == 0 || ((match_any == true) && (dom->flat_name != NULL) && (strcasecmp(dom->flat_name, name) == 0))) { return dom; } - dom = get_next_domain(dom, SSS_GND_DESCEND); + dom = get_next_domain(dom, gnd_flags); } return NULL; } +struct sss_domain_info *find_domain_by_name(struct sss_domain_info *domain, + const char *name, + bool match_any) +{ + return find_domain_by_name_ex(domain, name, match_any, SSS_GND_DESCEND); +} + struct sss_domain_info *find_domain_by_sid(struct sss_domain_info *domain, const char *sid) { @@ -175,7 +186,8 @@ sss_get_domain_by_sid_ldap_fallback(struct sss_domain_info *domain, struct sss_domain_info * find_domain_by_object_name_ex(struct sss_domain_info *domain, - const char *object_name, bool strict) + const char *object_name, bool strict, + uint32_t gnd_flags) { TALLOC_CTX *tmp_ctx; struct sss_domain_info *dom = NULL; @@ -203,7 +215,7 @@ find_domain_by_object_name_ex(struct sss_domain_info *domain, dom = domain; } } else { - dom = find_domain_by_name(domain, domainname, true); + dom = find_domain_by_name_ex(domain, domainname, true, gnd_flags); } done: @@ -215,7 +227,8 @@ struct sss_domain_info * find_domain_by_object_name(struct sss_domain_info *domain, const char *object_name) { - return find_domain_by_object_name_ex(domain, object_name, false); + return find_domain_by_object_name_ex(domain, object_name, false, + SSS_GND_DESCEND); } errno_t sssd_domain_init(TALLOC_CTX *mem_ctx, diff --git a/src/util/util.h b/src/util/util.h index c7edbbbab0..f205c20888 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -534,6 +534,10 @@ struct sss_domain_info *get_next_domain(struct sss_domain_info *domain, struct sss_domain_info *find_domain_by_name(struct sss_domain_info *domain, const char *name, bool match_any); +struct sss_domain_info *find_domain_by_name_ex(struct sss_domain_info *domain, + const char *name, + bool match_any, + uint32_t gnd_flags); struct sss_domain_info *find_domain_by_sid(struct sss_domain_info *domain, const char *sid); enum sss_domain_state sss_domain_get_state(struct sss_domain_info *dom); @@ -552,7 +556,8 @@ find_domain_by_object_name(struct sss_domain_info *domain, struct sss_domain_info * find_domain_by_object_name_ex(struct sss_domain_info *domain, - const char *object_name, bool strict); + const char *object_name, bool strict, + uint32_t gnd_flags); bool subdomain_enumerates(struct sss_domain_info *parent, const char *sd_name); From 6f7773d1b00a918ab70e734586270ff8a0a34bd1 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Wed, 7 Aug 2019 18:14:15 +0200 Subject: [PATCH 2/3] ipa: support disabled domains IPA does not disable domains with the help of a flag in the domain objects but more general with the help of the SID blacklist. With this patch the blacklist is read with other data about trusted domains and if the domain SID of a trusted domain is found the domain is disabled. As a result uses and groups from this domain cannot be looked up anymore. Related to https://pagure.io/SSSD/sssd/issue/4078 --- src/providers/ipa/ipa_subdomains.c | 149 +++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 10 deletions(-) diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index d000f12301..afbf364904 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -43,6 +43,7 @@ #define IPA_TRUSTED_DOMAIN_SID "ipaNTTrustedDomainSID" #define IPA_RANGE_TYPE "ipaRangeType" #define IPA_ADDITIONAL_SUFFIXES "ipaNTAdditionalSuffixes" +#define IPA_SID_BLACKLIST_INCOMING "ipaNTSIDBlacklistIncoming" #define IPA_BASE_ID "ipaBaseID" #define IPA_ID_RANGE_SIZE "ipaIDRangeSize" @@ -745,6 +746,129 @@ static void ipa_subdom_store_step(struct sss_domain_info *parent, } } +static errno_t add_dom_sids_to_list(TALLOC_CTX *mem_ctx, const char **sids, + char ***list) +{ + size_t c; + errno_t ret; + + for (c = 0; sids != NULL && sids[c] != NULL; c++) { + if (is_domain_sid(sids[c])) { + ret = add_string_to_list(mem_ctx, sids[c], list); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "add_string_to_list failed.\n"); + return ret; + } + } + } + + return EOK; +} + +static errno_t ipa_get_disabled_domain_sids(TALLOC_CTX *mem_ctx, size_t count, + struct sysdb_attrs **reply, + char ***disabled_domain_sids) +{ + size_t c; + char **dom_sid_list = NULL; + const char **tmp_list; + int ret; + + for (c = 0; c < count; c++) { + ret = sysdb_attrs_get_string_array(reply[c], IPA_SID_BLACKLIST_INCOMING, + mem_ctx, &tmp_list); + if (ret != EOK) { + if (ret != ENOENT) { + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_attrs_get_string_array failed, list of disabled " + "domains might be incomplete.\n"); + } + continue; + } + + ret = add_dom_sids_to_list(mem_ctx, tmp_list, &dom_sid_list); + talloc_free(tmp_list); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "add_dom_sids_to_list failed.\n"); + talloc_free(dom_sid_list); + return ret; + } + } + + *disabled_domain_sids = dom_sid_list; + + return EOK; +} + +static errno_t ipa_subdomains_check_domain_state(struct sss_domain_info *dom, + char **disabled_domain_sids) +{ + int ret; + + if (dom->domain_id == NULL) { + return EINVAL; + } + + if (disabled_domain_sids != NULL + && string_in_list(dom->domain_id, disabled_domain_sids, true)) { + DEBUG(SSSDBG_TRACE_ALL, "Domain [%s] is disabled on the server.\n", + dom->name); + /* disable domain if not already disabled */ + if (sss_domain_get_state(dom) != DOM_DISABLED) { + sss_domain_set_state(dom, DOM_DISABLED); + ret = sysdb_domain_set_enabled(dom->sysdb, dom->name, false); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_domain_set_enabled failed.\n"); + return ret; + } + } + } else { + /* enabled domain if it was disabled */ + DEBUG(SSSDBG_TRACE_ALL, "Domain [%s] is enabled on the server.\n", + dom->name); + if (sss_domain_get_state(dom) == DOM_DISABLED) { + sss_domain_set_state(dom, DOM_ACTIVE); + ret = sysdb_domain_set_enabled(dom->sysdb, dom->name, true); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_domain_set_enabled failed.\n"); + return ret; + } + } + } + + return EOK; +} + + +static void ipa_subdomains_update_dom_state(struct sss_domain_info *parent, + int count, + struct sysdb_attrs **reply) +{ + int ret; + struct sss_domain_info *dom; + char **disabled_domain_sids = NULL; + + ret = ipa_get_disabled_domain_sids(reply, count, reply, + &disabled_domain_sids); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "ipa_get_disabled_domain_sids failed, " + "assuming no domain is disabled.\n"); + disabled_domain_sids = NULL; + } + + for (dom = get_next_domain(parent, SSS_GND_DESCEND|SSS_GND_INCLUDE_DISABLED); + dom && IS_SUBDOMAIN(dom); /* if we get back to a parent, stop */ + dom = get_next_domain(dom, SSS_GND_INCLUDE_DISABLED)) { + + /* check if domain should be disabled/enabled */ + ret = ipa_subdomains_check_domain_state(dom, disabled_domain_sids); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to check domain state, " + "state of domain [%s] might be wrong.\n", dom->name); + } + } +} + static errno_t ipa_subdomains_refresh(struct ipa_subdomains_ctx *ctx, int count, struct sysdb_attrs **reply, bool *changes) @@ -1376,7 +1500,7 @@ ipa_subdomains_slave_send(TALLOC_CTX *mem_ctx, errno_t ret; const char *attrs[] = { IPA_CN, IPA_FLATNAME, IPA_TRUSTED_DOMAIN_SID, IPA_TRUST_DIRECTION, IPA_ADDITIONAL_SUFFIXES, - NULL }; + IPA_SID_BLACKLIST_INCOMING, NULL }; req = tevent_req_create(mem_ctx, &state, struct ipa_subdomains_slave_state); @@ -1530,18 +1654,23 @@ static void ipa_subdomains_slave_search_done(struct tevent_req *subreq) "expected.\n"); } - if (!has_changes) { - ret = EOK; - goto done; + /* If there are no changes this step can be skipped, but + * ipa_subdomains_update_dom_state() must be called after that in all case + * to cover existing an newly added domains. Since the domain state is not + * handled by a domain flag but by the blacklist has_changes does not + * cover the state. */ + if (has_changes) { + ret = ipa_subdom_reinit(state->sd_ctx); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Could not reinitialize subdomains\n"); + goto done; + } } - ret = ipa_subdom_reinit(state->sd_ctx); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, "Could not reinitialize subdomains\n"); - goto done; - } + ipa_subdomains_update_dom_state(state->sd_ctx->be_ctx->domain, + reply_count, reply); - if (state->sd_ctx->ipa_id_ctx->server_mode == NULL) { + if (!has_changes || state->sd_ctx->ipa_id_ctx->server_mode == NULL) { ret = EOK; goto done; } From c5cbe81a56422b15da47052643d990a2e9057b72 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 12 Sep 2019 14:45:08 +0200 Subject: [PATCH 3/3] ipa: ignore objects from disabled domains on the client It is possible that a domain is already disabled on an IPA client but still active on the server. This might happen e.g. if the version of SSSD running on the IPA server does not support disabled domains or if SSSD on the IPA client updates the domain data before the IPA server and sees a freshly disabled domain more early. As a result the server is still sending objects from disabled domains in the lists of group members or group memberships of a user. The client should just ignore those objects. Related to https://pagure.io/SSSD/sssd/issue/4078 --- src/providers/ipa/ipa_s2n_exop.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index c635da6707..4d813bcb1e 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -637,10 +637,16 @@ static errno_t add_v1_user_data(struct sss_domain_info *dom, } if (domain != NULL) { - obj_domain = find_domain_by_name(parent_domain, domain, true); + obj_domain = find_domain_by_name_ex(parent_domain, domain, true, SSS_GND_ALL_DOMAINS); if (obj_domain == NULL) { DEBUG(SSSDBG_OP_FAILURE, "find_domain_by_name failed.\n"); return ENOMEM; + } else if (sss_domain_get_state(obj_domain) == DOM_DISABLED) { + /* skipping objects from disabled domains */ + DEBUG(SSSDBG_TRACE_ALL, + "Skipping object [%s] from disabled domain.\n", + list[c]); + continue; } } else { obj_domain = parent_domain; @@ -656,6 +662,7 @@ static errno_t add_v1_user_data(struct sss_domain_info *dom, gc++; } } + attrs->ngroups = gc; tag = ber_peek_tag(ber, &ber_len); DEBUG(SSSDBG_TRACE_ALL, "BER tag is [%d]\n", (int) tag); @@ -1567,11 +1574,15 @@ static errno_t process_members(struct sss_domain_info *domain, parent_domain = get_domains_head(domain); for (c = 0; members[c] != NULL; c++) { - obj_domain = find_domain_by_object_name(parent_domain, members[c]); + obj_domain = find_domain_by_object_name_ex(parent_domain, members[c], + false, SSS_GND_ALL_DOMAINS); if (obj_domain == NULL) { DEBUG(SSSDBG_OP_FAILURE, "find_domain_by_object_name failed.\n"); ret = ENOMEM; goto done; + } else if (sss_domain_get_state(obj_domain) == DOM_DISABLED) { + /* skip members from disabled domains */ + continue; } ret = sysdb_search_user_by_name(tmp_ctx, obj_domain, members[c], attrs,
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/[email protected]
