URL: https://github.com/SSSD/sssd/pull/820 Author: pbrezina Title: #820: ad: delete domains disabled through ad_enabled_domains from cache Action: opened
PR body: """ Steps to reproduce: 1. Have at least one subdomain in ad domain (e.g. child.ad.vm is subdomain of ad.vm). 2. Enable all domains, set ad_enabled_domains = [ad.vm] ... ad_enabled_domains = 3. Look up 'administra...@child.ad.vm' $ id administra...@child.ad.vm uid=1678800500(administra...@child.ad.vm) ... 4. Disable the subdomain by setting 'ad_enabled_domains = ad.vm' 5. Restart sssd without clearing the cache 6. Request for *@child.ad.vm will go to data provider and try to lookup the user in child.ad.vm domain which will yield 'domain not found'. However if the user is cached it will return the user. $ id administra...@child.ad.vm uid=1678800500(administra...@child.ad.vm) ... Subdomains that are not root domains are removed from cache. Root domains are disabled in sysdb with new `enabled` attribute. Resolves: https://pagure.io/SSSD/sssd/issue/4009 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/820/head:pr820 git checkout pr820
From 5893608b6a2b85c3e0474ccc86c86434d67f03d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Thu, 30 May 2019 10:48:07 +0200 Subject: [PATCH 1/4] ad: remove subdomain that has been disabled through ad_enabled_domains from sysdb If previously enabled subdomain was disabled by removing it from ad_enabled_domains option in sssd.conf, its cached content (including the domain object itself) was kept in sysdb. Therefore eventhough the domain was effectively disabled in backed its cached data was still available in responders. Subdomains that are disabled on server side are correctly removed from sysdb in `ad_subdomains_refresh()` so this issue is related only to the configuration option. Resolves: https://pagure.io/SSSD/sssd/issue/4009 --- src/providers/ad/ad_subdomains.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index c4ac230653..5a96decbfc 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -825,6 +825,15 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx, if (is_domain_enabled(sd_name, enabled_domains_list) == false) { DEBUG(SSSDBG_TRACE_FUNC, "Disabling subdomain %s\n", sd_name); + + /* The subdomain is now disabled in configuraiton file, we + * need to delete its cached content so it is not returned + * by responders. The subdomain shares sysdb with its parent + * domain so it is OK to use domain->sysdb. */ + ret = sysdb_subdomain_delete(domain->sysdb, sd_name); + if (ret != EOK) { + goto fail; + } continue; } else { DEBUG(SSSDBG_TRACE_FUNC, "Enabling subdomain %s\n", sd_name); From 276a5612aaf4f7086a5569c46eca4d80a5830880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Thu, 30 May 2019 12:14:58 +0200 Subject: [PATCH 2/4] sysdb: add sysdb_domain_set_enabled() This will be used in subsequent patches to disable subdomains. Resolves: https://pagure.io/SSSD/sssd/issue/4009 --- src/db/sysdb.c | 7 ++++++- src/db/sysdb.h | 6 ++++++ src/db/sysdb_subdomains.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 51acb86056..6bbc6abb9b 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1110,7 +1110,7 @@ errno_t sysdb_set_bool(struct sysdb_ctx *sysdb, errno_t ret; int lret; - if (dn == NULL || cn_value == NULL || attr_name == NULL) { + if (dn == NULL || attr_name == NULL) { return EINVAL; } @@ -1134,6 +1134,11 @@ errno_t sysdb_set_bool(struct sysdb_ctx *sysdb, msg->dn = dn; if (res->count == 0) { + if (cn_value == NULL) { + ret = ENOENT; + goto done; + } + lret = ldb_msg_add_string(msg, "cn", cn_value); if (lret != LDB_SUCCESS) { ret = sysdb_error_to_errno(lret); diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 89b0d95715..ab65067380 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -155,6 +155,7 @@ #define SYSDB_SUBDOMAIN_TRUST_DIRECTION "trustDirection" #define SYSDB_UPN_SUFFIXES "upnSuffixes" #define SYSDB_SITE "site" +#define SYSDB_ENABLED "enabled" #define SYSDB_BASE_ID "baseID" #define SYSDB_ID_RANGE_SIZE "idRangeSize" @@ -520,6 +521,11 @@ errno_t sysdb_set_site(struct sss_domain_info *dom, const char *site); +errno_t +sysdb_domain_set_enabled(struct sysdb_ctx *sysdb, + const char *name, + bool enabled); + errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, const char *name, const char *realm, const char *flat_name, const char *domain_id, diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 34d052fdda..d467dfce53 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -1200,6 +1200,18 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, } } + ret = ldb_msg_add_empty(msg, SYSDB_ENABLED, LDB_FLAG_MOD_REPLACE, NULL); + if (ret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(ret); + goto done; + } + + ret = ldb_msg_add_string(msg, SYSDB_ENABLED, "TRUE"); + if (ret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(ret); + goto done; + } + ret = ldb_modify(sysdb->ldb, msg); if (ret != LDB_SUCCESS) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to add subdomain attributes to " @@ -1420,3 +1432,22 @@ sysdb_set_site(struct sss_domain_info *dom, talloc_free(tmp_ctx); return ret; } + +errno_t +sysdb_domain_set_enabled(struct sysdb_ctx *sysdb, + const char *name, + bool enabled) +{ + struct ldb_dn *dn; + errno_t ret; + + dn = ldb_dn_new_fmt(NULL, sysdb->ldb, SYSDB_DOM_BASE, name); + if (dn == NULL) { + return ENOMEM; + } + + ret = sysdb_set_bool(sysdb, dn, NULL, SYSDB_ENABLED, enabled); + talloc_free(dn); + + return ret; +} From a09bffc496021d731c784e491f7ba88c36ac9bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Thu, 30 May 2019 12:51:47 +0200 Subject: [PATCH 3/4] ad: set enabled=false attribute for subdomains that no longer exists Only forest root domain needs to be disabled because it has to be available for other tasks. All other non-root domains are removed from cache completely so it does not make sense for them. Resolves: https://pagure.io/SSSD/sssd/issue/4009 --- src/providers/ad/ad_subdomains.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index 5a96decbfc..220815562e 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -696,6 +696,13 @@ static errno_t ad_subdomains_refresh(struct be_ctx *be_ctx, if (sss_domain_is_forest_root(dom)) { DEBUG(SSSDBG_TRACE_ALL, "Skipping removal of forest root sdap data.\n"); + + ret = sysdb_domain_set_enabled(dom->sysdb, dom->name, false); + if (ret != EOK && ret != ENOENT) { + DEBUG(SSSDBG_OP_FAILURE, "Unable to disable domain %s " + "[%d]: %s\n", dom->name, ret, sss_strerror(ret)); + goto done; + } continue; } @@ -864,6 +871,12 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx, } else { DEBUG(SSSDBG_TRACE_FUNC, "Disabling forest root domain %s\n", root_name); + ret = sysdb_domain_set_enabled(domain->sysdb, root_name, false); + if (ret != EOK && ret != ENOENT) { + DEBUG(SSSDBG_OP_FAILURE, "Unable to disable domain %s " + "[%d]: %s\n", root_name, ret, sss_strerror(ret)); + goto fail; + } } } From 9b0061442e55e8ead8775ccaa93ee4afa5e7479c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Thu, 30 May 2019 12:52:33 +0200 Subject: [PATCH 4/4] sysdb: read and interpret domain's enabled attribute Disable domain if its sysdb object has enabled=false. Resolves: https://pagure.io/SSSD/sssd/issue/4009 --- src/db/sysdb_private.h | 3 ++- src/db/sysdb_subdomains.c | 29 ++++++++++++++++++--- src/tests/cmocka/test_fqnames.c | 2 +- src/tests/cmocka/test_negcache.c | 2 +- src/tests/cmocka/test_nss_srv.c | 2 +- src/tests/cmocka/test_responder_cache_req.c | 2 +- src/tests/sysdb-tests.c | 8 +++--- 7 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index 58544d8260..f3d34dd6f9 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -206,7 +206,8 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, const char *forest, const char **upn_suffixes, uint32_t trust_direction, - struct confdb_ctx *confdb); + struct confdb_ctx *confdb, + bool enabled); /* Helper functions to deal with the timestamp cache should not be used * outside the sysdb itself. The timestamp cache should be completely diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index d467dfce53..cf09b424e0 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -39,7 +39,8 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, const char *forest, const char **upn_suffixes, uint32_t trust_direction, - struct confdb_ctx *confdb) + struct confdb_ctx *confdb, + bool enabled) { struct sss_domain_info *dom; bool inherit_option; @@ -127,7 +128,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, dom->enumerate = enumerate; dom->fqnames = true; dom->mpg_mode = mpg_mode; - dom->state = DOM_ACTIVE; + dom->state = enabled ? DOM_ACTIVE : DOM_DISABLED; /* use fully qualified names as output in order to avoid causing * conflicts with users who have the same name and either the @@ -313,6 +314,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, SYSDB_SUBDOMAIN_FOREST, SYSDB_SUBDOMAIN_TRUST_DIRECTION, SYSDB_UPN_SUFFIXES, + SYSDB_ENABLED, NULL}; struct sss_domain_info *dom; struct ldb_dn *basedn; @@ -322,6 +324,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, const char *id; const char *forest; const char *str_mpg_mode; + bool enabled; enum sss_domain_mpg_mode mpg_mode; bool enumerate; uint32_t trust_direction; @@ -406,10 +409,14 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, SYSDB_SUBDOMAIN_TRUST_DIRECTION, 0); + enabled = ldb_msg_find_attr_as_bool(res->msgs[i], SYSDB_ENABLED, true); + for (dom = domain->subdomains; dom; dom = get_next_domain(dom, SSS_GND_INCLUDE_DISABLED)) { if (strcasecmp(dom->name, name) == 0) { - sss_domain_set_state(dom, DOM_ACTIVE); + if (enabled) { + sss_domain_set_state(dom, DOM_ACTIVE); + } /* in theory these may change, but it should never happen */ if (strcasecmp(dom->realm, realm) != 0) { @@ -522,7 +529,8 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, if (dom == NULL) { dom = new_subdomain(domain, domain, name, realm, flat, id, mpg_mode, enumerate, forest, - upn_suffixes, trust_direction, confdb); + upn_suffixes, trust_direction, confdb, + enabled); if (dom == NULL) { ret = ENOMEM; goto done; @@ -548,12 +556,15 @@ errno_t sysdb_master_domain_update(struct sss_domain_info *domain) struct ldb_message_element *tmp_el; struct ldb_dn *basedn; struct ldb_result *res; + enum sss_domain_state state; + bool enabled; const char *attrs[] = {"cn", SYSDB_SUBDOMAIN_REALM, SYSDB_SUBDOMAIN_FLAT, SYSDB_SUBDOMAIN_ID, SYSDB_SUBDOMAIN_FOREST, SYSDB_UPN_SUFFIXES, + SYSDB_ENABLED, NULL}; char *view_name = NULL; @@ -650,6 +661,16 @@ errno_t sysdb_master_domain_update(struct sss_domain_info *domain) talloc_zfree(domain->upn_suffixes); } + state = sss_domain_get_state(domain); + enabled = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_ENABLED, true); + if (!enabled) { + sss_domain_set_state(domain, DOM_DISABLED); + } else if (state == DOM_DISABLED) { + /* We do not want to enable INACTIVE or INCONSISTENT domain. This + * is managed by data provider. */ + sss_domain_set_state(domain, DOM_ACTIVE); + } + ret = sysdb_get_view_name(tmp_ctx, domain->sysdb, &view_name); if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_get_view_name failed.\n"); diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c index b374be2108..406ef55a98 100644 --- a/src/tests/cmocka/test_fqnames.c +++ b/src/tests/cmocka/test_fqnames.c @@ -310,7 +310,7 @@ static int parse_name_test_setup(void **state) */ test_ctx->subdom = new_subdomain(dom, dom, SUBDOMNAME, NULL, SUBFLATNAME, NULL, MPG_DISABLED, false, - NULL, NULL, 0, NULL); + NULL, NULL, 0, NULL, true); assert_non_null(test_ctx->subdom); check_leaks_push(test_ctx); diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c index 7ab8a0981f..1476b9cc24 100644 --- a/src/tests/cmocka/test_negcache.c +++ b/src/tests/cmocka/test_negcache.c @@ -664,7 +664,7 @@ static void test_sss_ncache_prepopulate(void **state) subdomain = new_subdomain(tc, tc->dom, testdom[0], testdom[1], testdom[2], testdom[3], false, false, NULL, NULL, 0, - tc->confdb); + tc->confdb, true); assert_non_null(subdomain); ret = sysdb_subdomain_store(tc->sysdb, diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 0ae1775719..95c080caf5 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -3475,7 +3475,7 @@ static int nss_subdom_test_setup_common(void **state, bool nonfqnames) subdomain = new_subdomain(nss_test_ctx, nss_test_ctx->tctx->dom, testdom[0], testdom[1], testdom[2], testdom[3], false, false, NULL, NULL, 0, - nss_test_ctx->tctx->confdb); + nss_test_ctx->tctx->confdb, true); assert_non_null(subdomain); ret = sysdb_subdomain_store(nss_test_ctx->tctx->sysdb, diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c index b4cf7f9fe8..2611c589b5 100644 --- a/src/tests/cmocka/test_responder_cache_req.c +++ b/src/tests/cmocka/test_responder_cache_req.c @@ -687,7 +687,7 @@ int test_subdomain_setup(void **state) test_ctx->subdomain = new_subdomain(test_ctx, test_ctx->tctx->dom, testdom[0], testdom[1], testdom[2], testdom[3], MPG_DISABLED, false, NULL, NULL, 0, - test_ctx->tctx->confdb); + test_ctx->tctx->confdb, true); assert_non_null(test_ctx->subdomain); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 6ce787a441..18fe783064 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -1541,7 +1541,7 @@ START_TEST (test_sysdb_get_user_attr_subdomain) /* Create subdomain */ subdomain = new_subdomain(test_ctx, test_ctx->domain, "test.sub", "TEST.SUB", "test", "S-3", - MPG_DISABLED, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL, true); fail_if(subdomain == NULL, "Failed to create new subdomain."); ret = sss_names_init_from_args(test_ctx, @@ -6271,7 +6271,7 @@ START_TEST(test_sysdb_subdomain_store_user) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - MPG_DISABLED, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL, true); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], @@ -6350,7 +6350,7 @@ START_TEST(test_sysdb_subdomain_user_ops) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - MPG_DISABLED, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL, true); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], @@ -6423,7 +6423,7 @@ START_TEST(test_sysdb_subdomain_group_ops) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - MPG_DISABLED, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL, true); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3],
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
