URL: https://github.com/SSSD/sssd/pull/475 Author: jhrozek Title: #475: LDAP: Only add a sdap_domain instance for the current domain when instantiating a new ad_id_ctx Action: opened
PR body: """ NOTE: This fix doesn't address the segfault, only the condition that led to it. I would prefer to track the segfault and the search base issues separately NOTE2: I didn't have much time to test this PR yet. I'm mostly submitting it to get feedback Please see the full commit message below. I'm really confused about this issue mostly because it seems we've had this bug for quite some time but did not see it. I would be glad if somebody helps me understand if iterating over all domains and adding all domains that are not yet present in the ad_id_ctx->ad_options->sdap_id_ctx->sdom list is correct or not The commit message follows: Resolves: https://pagure.io/SSSD/sssd/issue/3594 Previously, sdap_domain_subdom_add() was called when a new ad_id_ctx was being instantiated in the AD subdomains provider. The sdap_domain_subdom_add() call iterates over all known subdomains and adds a sdap_domain instance for every domain that is not present in an existing sdap_domain list. This is problematic for the AD subdomains provider e.g in this scenario found by downstream ticket #3594: - there is a domain child1.sssdad.com the sssd is joined to - the subdomain provider auto-discovers ssdad_tree.com and sssdad.com, in this order (which is important). The list of sss_domain_info objects is updated in this order, too - for each domain, ad_subdom_ad_ctx_new() is called. This function creates a new ad_id_ctx and calls sdap_domain_subdom_add() to add an sdap_domain object into the sdap_id_ctx. The sdap_domain_subdom_add() call adds both domains to the list -- for the sssdad_tree subdomain is is ok, because subsequent calls only use the first sdap_domain object which is ssdad_tree.com (remember, order is important) -- for the sssdad.com domain, ssdad_tree.com is added first, which then causes all searches in the sssdad.com to have a search base from ssdad_tree.com Because the sdap_domain instance in sdap_id_ctx should not be a list, but a single domain, this patch adds a utility function that creates an sdap_domain instance for a single subdomain instance. """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/475/head:pr475 git checkout pr475
From 360ae8b45b78a930ae8ccb6e93d19c922e211a66 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 13 Dec 2017 18:50:32 +0100 Subject: [PATCH] LDAP: Only add a sdap_domain instance for the current domain when instantiating a new ad_id_ctx Resolves: https://pagure.io/SSSD/sssd/issue/3594 Previously, sdap_domain_subdom_add() was called when a new ad_id_ctx was being instantiated in the AD subdomains provider. The sdap_domain_subdom_add() call iterates over all known subdomains and adds a sdap_domain instance for every domain that is not present in an existing sdap_domain list. This is problematic for the AD subdomains provider e.g in this scenario found by downstream ticket #3594: - there is a domain child1.sssdad.com the sssd is joined to - the subdomain provider auto-discovers ssdad_tree.com and sssdad.com, in this order (which is important). The list of sss_domain_info objects is updated in this order, too - for each domain, ad_subdom_ad_ctx_new() is called. This function creates a new ad_id_ctx and calls sdap_domain_subdom_add() to add an sdap_domain object into the sdap_id_ctx. The sdap_domain_subdom_add() call adds both domains to the list -- for the sssdad_tree subdomain is is ok, because subsequent calls only use the first sdap_domain object which is ssdad_tree.com (remember, order is important) -- for the sssdad.com domain, ssdad_tree.com is added first, which then causes all searches in the sssdad.com to have a search base from ssdad_tree.com Because the sdap_domain instance in sdap_id_ctx should not be a list, but a single domain, this patch adds a utility function that creates an sdap_domain instance for a single subdomain instance. --- src/providers/ad/ad_subdomains.c | 4 +--- src/providers/ldap/ldap_common.h | 4 ++++ src/providers/ldap/sdap_domain.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index 3fb9b950f..32037859f 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -258,9 +258,7 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx, be_fo_set_srv_lookup_plugin(be_ctx, ad_srv_plugin_send, ad_srv_plugin_recv, srv_ctx, "AD"); - ret = sdap_domain_subdom_add(ad_id_ctx->sdap_id_ctx, - ad_id_ctx->sdap_id_ctx->opts->sdom, - subdom->parent); + ret = sdap_domain_subdom_create(ad_id_ctx->sdap_id_ctx, subdom); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize sdap domain\n"); talloc_free(ad_options); diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 44dbc3fb0..43db1941f 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -324,6 +324,10 @@ sdap_domain_subdom_add(struct sdap_id_ctx *sdap_id_ctx, struct sdap_domain *sdom_list, struct sss_domain_info *parent); +errno_t +sdap_domain_subdom_create(struct sdap_id_ctx *sdap_id_ctx, + struct sss_domain_info *subdom); + void sdap_domain_remove(struct sdap_options *opts, struct sss_domain_info *dom); diff --git a/src/providers/ldap/sdap_domain.c b/src/providers/ldap/sdap_domain.c index d384b2e4a..7b25fbd76 100644 --- a/src/providers/ldap/sdap_domain.c +++ b/src/providers/ldap/sdap_domain.c @@ -123,6 +123,46 @@ sdap_domain_add(struct sdap_options *opts, return ret; } +errno_t +sdap_domain_subdom_create(struct sdap_id_ctx *sdap_id_ctx, + struct sss_domain_info *subdom) +{ + errno_t ret; + struct sdap_domain *sdom; + + ret = sdap_domain_add(sdap_id_ctx->opts, subdom, &sdom); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot add new sdap domain for domain %s [%d]: %s\n", + subdom->name, ret, strerror(ret)); + return ret; + } + + /* Update search bases */ + talloc_zfree(sdom->search_bases); + sdom->search_bases = talloc_array(sdom, struct sdap_search_base *, 2); + if (sdom->search_bases == NULL) { + return ENOMEM; + } + sdom->search_bases[1] = NULL; + + ret = sdap_create_search_base(sdom, sdom->basedn, LDAP_SCOPE_SUBTREE, + NULL, &sdom->search_bases[0]); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, "Cannot create new sdap search base\n"); + return ret; + } + + sdom->user_search_bases = sdom->search_bases; + sdom->group_search_bases = sdom->search_bases; + sdom->netgroup_search_bases = sdom->search_bases; + sdom->sudo_search_bases = sdom->search_bases; + sdom->service_search_bases = sdom->search_bases; + sdom->autofs_search_bases = sdom->search_bases; + + return EOK; +} + errno_t sdap_domain_subdom_add(struct sdap_id_ctx *sdap_id_ctx, struct sdap_domain *sdom_list,
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org