Blob Blame History Raw
From 664ae9d2247b5139d2286975228baa0cea39a8e4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Wed, 20 Oct 2021 13:59:40 +0200
Subject: [PATCH 81/83] ad: make ad_srv_plugin_ctx_switch_site() public
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If the name of the AD DCs are given explicitly with the ad_server option
the forest and site lookups are not done in the discovery phase, which
is skipped, but with a netlogon query on the current connection. This
patch makes sure the results are stored in the same way as during the
discovery step.

Resolves: https://github.com/SSSD/sssd/issues/5820

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
(cherry picked from commit 918abaf37d7f13d72b29863933e133bcbd24d87c)

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
---
 src/providers/ad/ad_common.c      | 48 +++++++++++++++++++++
 src/providers/ad/ad_common.h      |  3 ++
 src/providers/ad/ad_domain_info.h |  1 -
 src/providers/ad/ad_srv.c         | 70 ++++++-------------------------
 src/providers/ad/ad_subdomains.c  | 34 ++++++++++++++-
 5 files changed, 96 insertions(+), 60 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index c99c4d110..4463349e4 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -1554,3 +1554,51 @@ done:
 
     return ret;
 }
+
+errno_t
+ad_options_switch_site(struct ad_options *ad_options, struct be_ctx *be_ctx,
+                       const char *new_site, const char *new_forest)
+{
+    const char *site;
+    const char *forest;
+    errno_t ret;
+
+    /* Switch forest. */
+    if (new_forest != NULL
+        && (ad_options->current_forest == NULL
+            || strcmp(ad_options->current_forest, new_forest) != 0)) {
+        forest = talloc_strdup(ad_options, new_forest);
+        if (forest == NULL) {
+            return ENOMEM;
+        }
+
+        talloc_zfree(ad_options->current_forest);
+        ad_options->current_forest = forest;
+    }
+
+    if (new_site == NULL) {
+        return EOK;
+    }
+
+    if (ad_options->current_site != NULL
+                    && strcmp(ad_options->current_site, new_site) == 0) {
+        return EOK;
+    }
+
+    site = talloc_strdup(ad_options, new_site);
+    if (site == NULL) {
+        return ENOMEM;
+    }
+
+    talloc_zfree(ad_options->current_site);
+    ad_options->current_site = site;
+
+    ret = sysdb_set_site(be_ctx->domain, ad_options->current_site);
+    if (ret != EOK) {
+        /* Not fatal. */
+        DEBUG(SSSDBG_MINOR_FAILURE, "Unable to store site information "
+              "[%d]: %s\n", ret, sss_strerror(ret));
+    }
+
+    return EOK;
+}
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 311b84f4c..08fcc00fd 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -238,4 +238,7 @@ errno_t ad_inherit_opts_if_needed(struct dp_option *parent_opts,
 errno_t ad_refresh_init(struct be_ctx *be_ctx,
                         struct ad_id_ctx *id_ctx);
 
+errno_t
+ad_options_switch_site(struct ad_options *ad_options, struct be_ctx *be_ctx,
+                       const char *new_site, const char *new_forest);
 #endif /* AD_COMMON_H_ */
diff --git a/src/providers/ad/ad_domain_info.h b/src/providers/ad/ad_domain_info.h
index 631e543f5..cf601cff6 100644
--- a/src/providers/ad/ad_domain_info.h
+++ b/src/providers/ad/ad_domain_info.h
@@ -39,5 +39,4 @@ ad_domain_info_recv(struct tevent_req *req,
                       char **_id,
                       char **_site,
                       char **_forest);
-
 #endif /* _AD_DOMAIN_INFO_H_ */
diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c
index a10c6a247..d45f1601e 100644
--- a/src/providers/ad/ad_srv.c
+++ b/src/providers/ad/ad_srv.c
@@ -196,55 +196,6 @@ fail:
     return NULL;
 }
 
-static errno_t
-ad_srv_plugin_ctx_switch_site(struct ad_srv_plugin_ctx *ctx,
-                              const char *new_site,
-                              const char *new_forest)
-{
-    const char *site;
-    const char *forest;
-    errno_t ret;
-
-    /* Switch forest. */
-    if (new_forest != NULL
-        && (ctx->ad_options->current_forest == NULL
-            || strcmp(ctx->ad_options->current_forest, new_forest) != 0)) {
-        forest = talloc_strdup(ctx->ad_options, new_forest);
-        if (forest == NULL) {
-            return ENOMEM;
-        }
-
-        talloc_zfree(ctx->ad_options->current_forest);
-        ctx->ad_options->current_forest = forest;
-    }
-
-    if (new_site == NULL) {
-        return EOK;
-    }
-
-    if (ctx->ad_options->current_site != NULL
-                    && strcmp(ctx->ad_options->current_site, new_site) == 0) {
-        return EOK;
-    }
-
-    site = talloc_strdup(ctx->ad_options, new_site);
-    if (site == NULL) {
-        return ENOMEM;
-    }
-
-    talloc_zfree(ctx->ad_options->current_site);
-    ctx->ad_options->current_site = site;
-
-    ret = sysdb_set_site(ctx->be_ctx->domain, ctx->ad_options->current_site);
-    if (ret != EOK) {
-        /* Not fatal. */
-        DEBUG(SSSDBG_MINOR_FAILURE, "Unable to store site information "
-              "[%d]: %s\n", ret, sss_strerror(ret));
-    }
-
-    return EOK;
-}
-
 struct ad_srv_plugin_state {
     struct tevent_context *ev;
     struct ad_srv_plugin_ctx *ctx;
@@ -382,16 +333,19 @@ static void ad_srv_plugin_ping_done(struct tevent_req *subreq)
         /* Remember current site so it can be used during next lookup so
          * we can contact directory controllers within a known reachable
          * site first. */
-        ret = ad_srv_plugin_ctx_switch_site(state->ctx, state->site,
-                                            state->forest);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set site [%d]: %s\n",
-                  ret, sss_strerror(ret));
-            goto done;
-        }
+        if (state->site != NULL) {
+            ret = ad_options_switch_site(state->ctx->ad_options,
+                                         state->ctx->be_ctx,
+                                         state->site, state->forest);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set site [%d]: %s\n",
+                      ret, sss_strerror(ret));
+                goto done;
+            }
 
-        /* Do not renew the site again unless we go offline. */
-        state->ctx->renew_site = false;
+            /* Do not renew the site again unless we go offline. */
+            state->ctx->renew_site = false;
+        }
 
         if (strcmp(state->service, "gc") == 0) {
             if (state->forest != NULL) {
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 562047a02..e4e248c1d 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -2080,13 +2080,15 @@ static void ad_subdomains_refresh_master_done(struct tevent_req *subreq)
     const char *realm;
     char *master_sid;
     char *flat_name;
+    char *site = NULL;
     errno_t ret;
+    char *ad_site_override = NULL;
 
     req = tevent_req_callback_data(subreq, struct tevent_req);
     state = tevent_req_data(req, struct ad_subdomains_refresh_state);
 
     ret = ad_domain_info_recv(subreq, state, &flat_name, &master_sid,
-                              NULL, &state->forest);
+                              &site, &state->forest);
     talloc_zfree(subreq);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get master domain information "
@@ -2112,6 +2114,36 @@ static void ad_subdomains_refresh_master_done(struct tevent_req *subreq)
         }
     }
 
+    /* If the site was not discovered during the DNS discovery, e.g. because
+     * the server name was given explicitly in sssd.conf, we try to set the
+     * site here. */
+    if (state->ad_options->current_site == NULL) {
+        /* Ignore AD site found in netlogon attribute if specific site is set in
+         * configuration file. */
+        ad_site_override = dp_opt_get_string(state->ad_options->basic, AD_SITE);
+        if (ad_site_override != NULL) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  "Ignoring AD site found by DNS discovery: '%s', "
+                  "using configured value: '%s' instead.\n",
+                  site, ad_site_override);
+            site = ad_site_override;
+        }
+
+        if (site != NULL) {
+            ret = ad_options_switch_site(state->ad_options, state->be_ctx, site,
+                                         state->forest);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "Failed to store forest and site name, "
+                                         "will try again after a new lookup.\n");
+            }
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  "Site name currently not available will try again later. "
+                  "The site name can be added manually my setting 'ad_site' "
+                  "in sssd.conf.\n");
+        }
+    }
+
     realm = dp_opt_get_cstring(state->ad_options->basic, AD_KRB5_REALM);
     if (realm == NULL) {
         DEBUG(SSSDBG_CONF_SETTINGS, "Missing realm.\n");
-- 
2.26.3