On Tue, Dec 17, 2013 at 05:02:07PM +0100, Jakub Hrozek wrote:
> On Mon, Dec 16, 2013 at 10:09:58PM +0100, Sumit Bose wrote:
> > On Mon, Dec 16, 2013 at 07:05:16PM +0100, Jakub Hrozek wrote:
> > > On Mon, Dec 16, 2013 at 07:03:11PM +0100, Jakub Hrozek wrote:
> > > > On Mon, Dec 16, 2013 at 12:04:44PM +0100, Sumit Bose wrote:
> > > > > On Sat, Dec 14, 2013 at 10:18:32PM +0100, Jakub Hrozek wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I found this bug when testing the GC patches. Previously, when SSSD
> > > > > > was
> > > > > > started, but subdomains list was up-to-date, the ad_ctx was not
> > > > > > initialized for the subdomain.
> > > > > >
> > > > > > I was also thinking whether we should re-initialize the domain-realm
> > > > > > mappings after sssd startup, the way we re-initialize kdcinfo
> > > > > > files. I
> > > > > > don't think it's strictly necessary because if someone deletes a
> > > > > > file in
> > > > > > /var/lib/sss/pubconf, he should keep the broken pieces, but perhaps
> > > > > > we
> > > > > > should be at least aware..
> > > > >
> > > > > Maybe we can do this during the init phase of the responder? We
> >
> > ah, sorry, I had the typo here ^^^^^^^^ first.
> > I meant to call it during the initialization of the subdomain part of
> > the AD and IPA providers.
> >
> > bye,
> > Sumit
>
> That makes perfect sense :-)
>
> See the attached patch, I moved the code to a separate function.
Actually, I did one more change in the original patch and also did a
similar change in the IPA provider. The result is a second patch. I
mostly split the patches into two because the IPA patch is a
convenience, so from my point of view it's fine to fix it in master
only. The first one is something I'd like in sssd-1-11, too.
>From d7f2e8185c991f95426e199564fdb1b77222c160 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Fri, 13 Dec 2013 19:11:47 +0100
Subject: [PATCH 1/2] AD: Refresh subdomain data structures on startup
Previously, if no changes were done to the list of subdomains, the SSSD
didn't update its list of sdap_domain mappings for the new subdomain.
This resulted in errors as no id_ctx was present for the subdomain
during lookup.
This patch moves the block of code performed during update to a function
of its own and calls it during provider initialization as well.
---
src/providers/ad/ad_subdomains.c | 49 ++++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index
100fb13e99f7bf4b3946b1f5c5f9c626674bfb46..e438a688c364084a3f2bbca338a39d61aa86b5d6
100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -414,6 +414,31 @@ done:
return ret;
}
+static errno_t ad_subdom_reinit(struct ad_subdomains_ctx *ctx)
+{
+ errno_t ret;
+
+ ret = sysdb_update_subdomains(ctx->be_ctx->domain);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+ return ret;
+ }
+
+ ret = sss_write_domain_mappings(ctx->be_ctx->domain, false);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("sss_krb5_write_mappings failed.\n"));
+ /* Just continue */
+ }
+
+ ret = ads_store_sdap_subdom(ctx, ctx->be_ctx->domain);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("ads_store_sdap_subdom failed.\n"));
+ return ret;
+ }
+
+ return EOK;
+}
+
static void ad_subdomains_get_conn_done(struct tevent_req *req);
static void ad_subdomains_master_dom_done(struct tevent_req *req);
static errno_t ad_subdomains_get_slave(struct ad_subdomains_req_ctx *ctx);
@@ -619,25 +644,15 @@ static void ad_subdomains_get_slave_domain_done(struct
tevent_req *req)
goto done;
}
+ DEBUG(SSSDBG_TRACE_LIBS, ("There are %schanges\n",
+ refresh_has_changes ? "" : "no "));
+
if (refresh_has_changes) {
- ret = sysdb_update_subdomains(ctx->sd_ctx->be_ctx->domain);
+ ret = ad_subdom_reinit(ctx->sd_ctx);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not reinitialize subdomains\n"));
goto done;
}
-
- ret = ads_store_sdap_subdom(ctx->sd_ctx, ctx->sd_ctx->be_ctx->domain);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("ads_store_sdap_subdom failed.\n"));
- goto done;
- }
-
- ret = sss_write_domain_mappings(ctx->sd_ctx->be_ctx->domain, false);
- if (ret != EOK) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("sss_krb5_write_mappings failed.\n"));
- /* Just continue */
- }
}
ret = EOK;
@@ -783,9 +798,9 @@ int ad_subdom_init(struct be_ctx *be_ctx,
return EFAULT;
}
- ret = sysdb_update_subdomains(be_ctx->domain);
+ ret = ad_subdom_reinit(ctx);
if (ret != EOK) {
- DEBUG(SSSDBG_MINOR_FAILURE, ("Could not load the list of subdomains. "
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Could not reinitialize subdomains. "
"Users from trusted domains might not be resolved correctly\n"));
/* Ignore this error and try to discover the subdomains later */
}
--
1.8.4.2
>From 6ebe01399450421ace821fb0be9cc3eb316c8c18 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 17 Dec 2013 17:22:45 +0100
Subject: [PATCH 2/2] IPA: Refresh subdomain data structures on startup
Write domain-mappings at startup and initialize internal data structures
on provider startup, not only during updates.
---
src/providers/ipa/ipa_subdomains.c | 51 ++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/src/providers/ipa/ipa_subdomains.c
b/src/providers/ipa/ipa_subdomains.c
index
416e21913be8e991c9f496ff2b54f238b602f304..56fd4f99654aa07f822c49d6d39526765785f0de
100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -267,6 +267,35 @@ ipa_ad_subdom_refresh(struct be_ctx *be_ctx,
return EOK;
}
+static errno_t
+ipa_subdom_reinit(struct ipa_subdomains_ctx *ctx)
+{
+ errno_t ret;
+
+ ret = sysdb_update_subdomains(ctx->be_ctx->domain);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+ return ret;
+ }
+
+ ret = ipa_ad_subdom_refresh(ctx->be_ctx, ctx->id_ctx, ctx->be_ctx->domain);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("ipa_ad_subdom_refresh failed.\n"));
+ return ret;
+ }
+
+ ret = sss_write_domain_mappings(ctx->be_ctx->domain,
+ dp_opt_get_bool(ctx->id_ctx->ipa_options->basic,
+ IPA_SERVER_MODE));
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("sss_krb5_write_mappings failed.\n"));
+ /* Just continue */
+ }
+
+ return EOK;
+}
+
static void
ipa_ad_subdom_remove(struct ipa_subdomains_ctx *ctx,
struct sss_domain_info *subdom)
@@ -921,27 +950,11 @@ static void ipa_subdomains_handler_done(struct tevent_req
*req)
}
if (refresh_has_changes) {
- ret = sysdb_update_subdomains(domain);
+ ret = ipa_subdom_reinit(ctx->sd_ctx);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not reinitialize subdomains\n"));
goto done;
}
-
- ret = ipa_ad_subdom_refresh(ctx->sd_ctx->be_ctx, ctx->sd_ctx->id_ctx,
- domain);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("ipa_ad_subdom_refresh failed.\n"));
- goto done;
- }
-
- ret = sss_write_domain_mappings(domain,
-
dp_opt_get_bool(ctx->sd_ctx->id_ctx->ipa_options->basic,
- IPA_SERVER_MODE));
- if (ret != EOK) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- ("sss_krb5_write_mappings failed.\n"));
- /* Just continue */
- }
}
ret = sysdb_master_domain_update(domain);
@@ -1289,7 +1302,7 @@ int ipa_subdom_init(struct be_ctx *be_ctx,
DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to add subdom offline callback"));
}
- ret = sysdb_update_subdomains(be_ctx->domain);
+ ret = ipa_subdom_reinit(ctx);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE, ("Could not load the list of subdomains. "
"Users from trusted domains might not be resolved correctly\n"));
--
1.8.4.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
