On Wed, Jul 27, 2016 at 05:56:48PM +0200, Lukas Slebodnik wrote:
> On (27/07/16 15:57), Jakub Hrozek wrote:
> >On Mon, Jul 25, 2016 at 04:11:16PM +0200, Lukas Slebodnik wrote:
> >> On (25/07/16 13:10), Jakub Hrozek wrote:
> >> >On Wed, Jul 20, 2016 at 03:14:17PM +0200, Sumit Bose wrote:
> >> >> Hi,
> >> >> 
> >> >> it is possible that the CLAP/netlogon reply does not contain any site
> >> >> data. In this case we should not fail but just use what we can get.
> >> >> Especially when looking up the Global Catalog the forest name is needed.
> >> >> If the site name is missing we still can use the forest name to lookup
> >> >> the Global Catalog in DNS.
> >> >> 
> >> >> The first patch is not strictly related to the issue but since it fixes
> >> >> a potential memory leak (we currently do not have it because only
> >> >> short-lived memory contexts are used so far) I think it is worth adding
> >> >> it here.
> >> >> 
> >> >> bye,
> >> >> Sumit
> >> >> 
> >> >
> >> >> From a0cf3c4b04069a71a0d610e1164390ec8bab45ab Mon Sep 17 00:00:00 2001
> >> >> From: Sumit Bose <[email protected]>
> >> >> Date: Tue, 12 Jul 2016 13:16:43 +0200
> >> >> Subject: [PATCH 1/4] AD: avoid memory leak in 
> >> >> netlogon_get_domain_info() and
> >> >>  make it public
> >> >
> >> >ACK
> >> >
> >> >> From 56f0b52cc91580f2ee9216f16d809c3da866728d Mon Sep 17 00:00:00 2001
> >> >> From: Sumit Bose <[email protected]>
> >> >> Date: Tue, 12 Jul 2016 13:29:33 +0200
> >> >> Subject: [PATCH 2/4] AD: netlogon_get_domain_info() allow missing 
> >> >> arguments
> >> >>  and empty results
> >> >
> >> >ACK
> >> >
> >> >> From 0e4564b85f632adedd1129d6b8b200db2a7a87ee Mon Sep 17 00:00:00 2001
> >> >> From: Sumit Bose <[email protected]>
> >> >> Date: Mon, 11 Jul 2016 17:05:29 +0200
> >> >> Subject: [PATCH 3/4] tests: add tests for netlogon_get_domain_info
> >> >
> >> >ACK
> >> >
> >> >
> >> >> From 4b6b33eb18a3d9c40c26503a9c7b8ae80d998e15 Mon Sep 17 00:00:00 2001
> >> >> From: Sumit Bose <[email protected]>
> >> >> Date: Mon, 18 Jul 2016 11:25:47 +0200
> >> >> Subject: [PATCH 4/4] AD: replace ad_get_client_site_parse_ndr() with
> >> >>  netlogon_get_domain_info()
> >> >
> >> >ACK
> >> >
> >> >I tested site resolution and basic lookups manually and everything seems
> >> >to be working.
> >> >
> >> >Coverity didn't report anything either. I will push this patch set once
> >> >the CI run finishes.
> >> 
> >> http://sssd-ci.duckdns.org/logs/job/50/12/summary.html
> >> 
> >> master:
> >> * 3000d86bea2bae735175f93b0e013bbcb3979f1e
> >> * 7fe7073ad2c84946a699528a3bb79f7803c96b69
> >> * 44656ce260030556820c4b6be519e66ffdacb408
> >> * 74bef2150c76c8814bf4c1654ecd3660604eb4e6
> >
> >Is anyone against pushing this patch to sssd-1-13 as well?
> Go ahead.
> 
> LS

The backport wasn't completely trivial, so it would be nice if someone
could check the patches before I push them.

The conflicts were in ad_subdomains.c and the tests.
>From 10710ea9dca0f3f454cac3310e307f337143b599 Mon Sep 17 00:00:00 2001
From: Sumit Bose <[email protected]>
Date: Tue, 12 Jul 2016 13:16:43 +0200
Subject: [PATCH 1/4] AD: avoid memory leak in netlogon_get_domain_info() and
 make it public

Reviewed-by: Jakub Hrozek <[email protected]>
---
 src/providers/ad/ad_common.h      |  6 ++++++
 src/providers/ad/ad_domain_info.c | 29 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 
d0ef724c0e8ffa76701fff903d6b074f5879e9c3..b6cda1481b303a54501669fdc6b05a9924ae842a
 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -186,4 +186,10 @@ int ad_autofs_init(struct be_ctx *be_ctx,
 errno_t ad_machine_account_password_renewal_init(struct be_ctx *be_ctx,
                                                  struct ad_options *ad_opts);
 
+errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
+                                 struct sysdb_attrs *reply,
+                                 char **_flat_name,
+                                 char **_site,
+                                 char **_forest);
+
 #endif /* AD_COMMON_H_ */
diff --git a/src/providers/ad/ad_domain_info.c 
b/src/providers/ad/ad_domain_info.c
index 
5f17ae5427b1206af3ad03dccce9452aefc2e6e2..a06379c263878aa95741055636d0a12764f3ad8c
 100644
--- a/src/providers/ad/ad_domain_info.c
+++ b/src/providers/ad/ad_domain_info.c
@@ -35,12 +35,11 @@
 #include "providers/ad/ad_common.h"
 #include "util/util.h"
 
-static errno_t
-netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
-                         struct sysdb_attrs *reply,
-                         char **_flat_name,
-                         char **_site,
-                         char **_forest)
+errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
+                                 struct sysdb_attrs *reply,
+                                 char **_flat_name,
+                                 char **_site,
+                                 char **_forest)
 {
     errno_t ret;
     struct ldb_message_element *el;
@@ -51,6 +50,7 @@ netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
     const char *flat_name;
     const char *site;
     const char *forest;
+    TALLOC_CTX *tmp_ctx;
 
     ret = sysdb_attrs_get_el(reply, AD_AT_NETLOGON, &el);
     if (ret != EOK) {
@@ -66,13 +66,24 @@ netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
+        return ENOMEM;
+    }
+
     blob.data = el->values[0].data;
     blob.length = el->values[0].length;
 
-    ndr_pull = ndr_pull_init_blob(&blob, mem_ctx);
+    /* The ndr_pull_* calls do not use ndr_pull as a talloc context to
+     * allocate memory but the second argument of ndr_pull_init_blob(). To
+     * make sure no memory is leaked here a temporary talloc context is
+     * needed. */
+    ndr_pull = ndr_pull_init_blob(&blob, tmp_ctx);
     if (ndr_pull == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "ndr_pull_init_blob() failed.\n");
-        return ENOMEM;
+        ret = ENOMEM;
+        goto done;
     }
 
     ndr_err = ndr_pull_netlogon_samlogon_response(ndr_pull, NDR_SCALARS,
@@ -146,7 +157,7 @@ netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
 
     ret = EOK;
 done:
-    talloc_free(ndr_pull);
+    talloc_free(tmp_ctx);
     return ret;
 }
 
-- 
2.4.11

>From ef6317c6303efde8090888b10d7c88c39778e70d Mon Sep 17 00:00:00 2001
From: Sumit Bose <[email protected]>
Date: Tue, 12 Jul 2016 13:29:33 +0200
Subject: [PATCH 2/4] AD: netlogon_get_domain_info() allow missing arguments
 and empty results

netlogon_get_domain_info() should not fail if not all parameters can be
retrieved. It should be the responsibility of the caller to see if the
needed data is available and act accordingly.

Resolves:
https://fedorahosted.org/sssd/ticket/3104

Reviewed-by: Jakub Hrozek <[email protected]>
---
 src/providers/ad/ad_common.h      |   1 +
 src/providers/ad/ad_domain_info.c | 108 +++++++++++++++++++++-----------------
 src/providers/ad/ad_gpo.c         |   2 +-
 src/providers/ad/ad_subdomains.c  |   3 +-
 4 files changed, 64 insertions(+), 50 deletions(-)

diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 
b6cda1481b303a54501669fdc6b05a9924ae842a..c795a41d0198b25b492021b7b140cd2720aa9cbd
 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -188,6 +188,7 @@ errno_t ad_machine_account_password_renewal_init(struct 
be_ctx *be_ctx,
 
 errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
                                  struct sysdb_attrs *reply,
+                                 bool check_next_nearest_site_as_well,
                                  char **_flat_name,
                                  char **_site,
                                  char **_forest);
diff --git a/src/providers/ad/ad_domain_info.c 
b/src/providers/ad/ad_domain_info.c
index 
a06379c263878aa95741055636d0a12764f3ad8c..5302c8083bd832d0772829d9f3a4b8e9537d34b9
 100644
--- a/src/providers/ad/ad_domain_info.c
+++ b/src/providers/ad/ad_domain_info.c
@@ -37,6 +37,7 @@
 
 errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
                                  struct sysdb_attrs *reply,
+                                 bool check_next_nearest_site_as_well,
                                  char **_flat_name,
                                  char **_site,
                                  char **_forest)
@@ -47,9 +48,6 @@ errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
     struct ndr_pull *ndr_pull = NULL;
     enum ndr_err_code ndr_err;
     struct netlogon_samlogon_response response;
-    const char *flat_name;
-    const char *site;
-    const char *forest;
     TALLOC_CTX *tmp_ctx;
 
     ret = sysdb_attrs_get_el(reply, AD_AT_NETLOGON, &el);
@@ -102,57 +100,73 @@ errno_t netlogon_get_domain_info(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    /* get flat name */
-    if (response.data.nt5_ex.domain_name != NULL &&
-        *response.data.nt5_ex.domain_name != '\0') {
-        flat_name = response.data.nt5_ex.domain_name;
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              "No netlogon domain name data available\n");
-        ret = ENOENT;
-        goto done;
+    /* get flat domain name */
+    if (_flat_name != NULL) {
+        if (response.data.nt5_ex.domain_name != NULL &&
+            *response.data.nt5_ex.domain_name != '\0') {
+            *_flat_name = talloc_strdup(mem_ctx,
+                                        response.data.nt5_ex.domain_name);
+            if (*_flat_name == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  "No netlogon flat domain name data available.\n");
+            *_flat_name = NULL;
+        }
     }
 
-    *_flat_name = talloc_strdup(mem_ctx, flat_name);
-    if (*_flat_name == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-        ret = ENOMEM;
-        goto done;
-    }
 
     /* get forest */
-    if (response.data.nt5_ex.forest != NULL &&
-        *response.data.nt5_ex.forest != '\0') {
-        forest = response.data.nt5_ex.forest;
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE, "No netlogon forest data available\n");
-        ret = ENOENT;
-        goto done;
-    }
-
-    *_forest = talloc_strdup(mem_ctx, forest);
-    if (*_forest == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-        ret = ENOMEM;
-        goto done;
+    if (_forest != NULL) {
+        if (response.data.nt5_ex.forest != NULL &&
+            *response.data.nt5_ex.forest != '\0') {
+            *_forest = talloc_strdup(mem_ctx, response.data.nt5_ex.forest);
+            if (*_forest == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE, "No netlogon forest data 
available.\n");
+            *_forest = NULL;
+        }
     }
 
     /* get site name */
-    if (response.data.nt5_ex.client_site != NULL
-        && response.data.nt5_ex.client_site[0] != '\0') {
-        site = response.data.nt5_ex.client_site;
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              "No netlogon site name data available\n");
-        ret = ENOENT;
-        goto done;
-    }
+    if (_site != NULL) {
+        if (response.data.nt5_ex.client_site != NULL
+            && response.data.nt5_ex.client_site[0] != '\0') {
+            *_site = talloc_strdup(mem_ctx, response.data.nt5_ex.client_site);
+            if (*_site == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  "No netlogon site name data available.\n");
+            *_site = NULL;
 
-    *_site = talloc_strdup(mem_ctx, site);
-    if (*_site == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-        ret = ENOMEM;
-        goto done;
+            if (check_next_nearest_site_as_well) {
+                if (response.data.nt5_ex.next_closest_site != NULL
+                        && response.data.nt5_ex.next_closest_site[0] != '\0') {
+                    *_site = talloc_strdup(mem_ctx,
+                                        
response.data.nt5_ex.next_closest_site);
+                    if (*_site == NULL) {
+                        DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                        ret = ENOMEM;
+                        goto done;
+                    }
+                } else {
+                    DEBUG(SSSDBG_MINOR_FAILURE,
+                          "No netlogon next closest site name data "
+                          "available.\n");
+                }
+            }
+        }
     }
 
     ret = EOK;
@@ -388,7 +402,7 @@ ad_master_domain_netlogon_done(struct tevent_req *subreq)
 
     /* Exactly one flat name. Carry on */
 
-    ret = netlogon_get_domain_info(state, reply[0], &state->flat,
+    ret = netlogon_get_domain_info(state, reply[0], false, &state->flat,
                                    &state->site, &state->forest);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 
8b44b5aee798751f4bbd661a38cfc03be2764c21..6b4a7f70c87abbabfb58a9ff2ac1b929eecf2f9c
 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -2739,7 +2739,7 @@ ad_gpo_site_name_retrieval_done(struct tevent_req *subreq)
     ret = ad_master_domain_recv(subreq, state, NULL, NULL, &site, NULL);
     talloc_zfree(subreq);
 
-    if (ret != EOK) {
+    if (ret != EOK || site == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "Cannot retrieve master domain info\n");
         tevent_req_error(req, ENOENT);
         return;
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 
5eebcbbfc6b61f7f6337cf8e1d1c05237b4d0d2e..f7e7e6200ebdeaf0c8fee370ee4f8b6d3c973be4
 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -88,7 +88,6 @@ struct ad_subdomains_req_ctx {
 
     char *master_sid;
     char *flat_name;
-    char *site;
     char *forest;
 };
 
@@ -602,7 +601,7 @@ static void ad_subdomains_master_dom_done(struct tevent_req 
*req)
 
     ret = ad_master_domain_recv(req, ctx,
                                 &ctx->flat_name, &ctx->master_sid,
-                                &ctx->site, &ctx->forest);
+                                NULL, &ctx->forest);
     talloc_zfree(req);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Cannot retrieve master domain info\n");
-- 
2.4.11

>From aec99f417c62fd6de4d01f4380c5126f04681701 Mon Sep 17 00:00:00 2001
From: Sumit Bose <[email protected]>
Date: Mon, 11 Jul 2016 17:05:29 +0200
Subject: [PATCH 3/4] tests: add tests for netlogon_get_domain_info

Reviewed-by: Jakub Hrozek <[email protected]>
---
 Makefile.am                       |  6 +++
 src/tests/cmocka/test_ad_common.c | 82 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 
6bc8a54536ab038b1d43301380f32f75bfe2b0ec..2664d25ac285f6e77bd813a272a130464e3be851
 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2233,6 +2233,11 @@ ad_common_tests_SOURCES = \
     src/tests/cmocka/common_mock_krb5.c \
     src/tests/cmocka/test_ad_common.c \
     src/providers/ad/ad_opts.c \
+    src/providers/ad/ad_domain_info.c \
+    $(NULL)
+ad_common_tests_CFLAGS = \
+    $(NDR_NBT_CFLAGS) \
+    $(NDR_KRB5PAC_CFLAGS) \
     $(NULL)
 ad_common_tests_LDFLAGS = \
     -Wl,-wrap,sdap_set_sasl_options \
@@ -2244,6 +2249,7 @@ ad_common_tests_LDADD = \
     $(KEYUTILS_LIBS) \
     $(KRB5_LIBS) \
     $(SSSD_INTERNAL_LTLIBS) \
+    $(NDR_NBT_LIBS) \
     libsss_ldap_common.la \
     libsss_test_common.la \
     libdlopen_test_providers.la \
diff --git a/src/tests/cmocka/test_ad_common.c 
b/src/tests/cmocka/test_ad_common.c
index 
573e2ad6aebf0231b18e40bdbc75f62b39427e57..db23c5918a849e5ea5473d8a7f930da071d4f643
 100644
--- a/src/tests/cmocka/test_ad_common.c
+++ b/src/tests/cmocka/test_ad_common.c
@@ -31,6 +31,7 @@
 
 /* In order to access opaque types */
 #include "providers/ad/ad_common.c"
+#include "util/crypto/sss_crypto.h"
 
 #include "tests/cmocka/common_mock.h"
 #include "tests/cmocka/common_mock_krb5.h"
@@ -479,6 +480,84 @@ void test_user_conn_list(void **state)
     talloc_free(conn_list);
 }
 
+void test_netlogon_get_domain_info(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct ldb_val val = { 0 };
+    char *flat_name;
+    char *site;
+    char *forest;
+
+    struct ad_common_test_ctx *test_ctx = talloc_get_type(*state,
+                                                     struct 
ad_common_test_ctx);
+    assert_non_null(test_ctx);
+
+    attrs = sysdb_new_attrs(test_ctx);
+    assert_non_null(attrs);
+
+    ret = netlogon_get_domain_info(test_ctx, attrs, false, NULL, NULL, NULL);
+    assert_int_equal(ret, ENOENT);
+
+    ret = sysdb_attrs_add_val(attrs, AD_AT_NETLOGON, &val);
+    assert_int_equal(ret, EOK);
+
+    ret = netlogon_get_domain_info(test_ctx, attrs, false, NULL, NULL, NULL);
+    assert_int_equal(ret, EBADMSG);
+
+    talloc_free(attrs);
+    attrs = sysdb_new_attrs(test_ctx);
+    assert_non_null(attrs);
+
+    val.data = sss_base64_decode(test_ctx, 
"FwAAAP0zAABsGcIYI7j2TL97Rd+TvpATAmFkBWRldmVsAMAYCWFkLXNlcnZlcsAYAkFEAAlBRC1TRVJWRVIAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAQAUAAAD/////",
 &val.length);
+    assert_non_null(val.data);
+
+    ret = sysdb_attrs_add_val(attrs, AD_AT_NETLOGON, &val);
+    assert_int_equal(ret, EOK);
+
+    ret = netlogon_get_domain_info(test_ctx, attrs, false, &flat_name, &site, 
&forest);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(flat_name, "AD");
+    assert_string_equal(site, "Default-First-Site-Name");
+    assert_string_equal(forest, "ad.devel");
+
+    /* missing site */
+    talloc_free(flat_name);
+    talloc_free(site);
+    talloc_free(forest);
+    talloc_free(val.data);
+    talloc_free(attrs);
+    attrs = sysdb_new_attrs(test_ctx);
+    assert_non_null(attrs);
+
+    val.data = sss_base64_decode(test_ctx, 
"FwAAAH0zAABsGcIYI7j2TL97Rd+TvpATAmFkBWRldmVsAMAYCWFkLXNlcnZlcsAYAkFEAAlBRC1TRVJWRVIAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQAABQAAAP////8=",
 &val.length);
+    assert_non_null(val.data);
+
+    ret = sysdb_attrs_add_val(attrs, AD_AT_NETLOGON, &val);
+    assert_int_equal(ret, EOK);
+
+    ret = netlogon_get_domain_info(test_ctx, attrs, false, &flat_name, &site, 
&forest);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(flat_name, "AD");
+    assert_null(site);
+    assert_string_equal(forest, "ad.devel");
+
+    talloc_free(flat_name);
+    talloc_free(site);
+    talloc_free(forest);
+    ret = netlogon_get_domain_info(test_ctx, attrs, true, &flat_name, &site, 
&forest);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(flat_name, "AD");
+    assert_null(site);
+    assert_string_equal(forest, "ad.devel");
+
+    talloc_free(flat_name);
+    talloc_free(site);
+    talloc_free(forest);
+    talloc_free(val.data);
+    talloc_free(attrs);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -509,6 +588,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_user_conn_list,
                                         test_ldap_conn_setup,
                                         test_ldap_conn_teardown),
+        cmocka_unit_test_setup_teardown(test_netlogon_get_domain_info,
+                                        test_ad_common_setup,
+                                        test_ad_common_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.4.11

>From 6a0e0930501ab72d32fa1d7e3c2ecffce4f33dad Mon Sep 17 00:00:00 2001
From: Sumit Bose <[email protected]>
Date: Mon, 18 Jul 2016 11:25:47 +0200
Subject: [PATCH 4/4] AD: replace ad_get_client_site_parse_ndr() with
 netlogon_get_domain_info()

netlogon_get_domain_info() does not fail if only the site is missing in
the CLDAP ping respond. If the site is not available a Global Catalog
can still be looked up with the forest name. Only if the forest name is
missing as well we fall back to the configured domain name.

Resolves:
https://fedorahosted.org/sssd/ticket/3104

Reviewed-by: Jakub Hrozek <[email protected]>
---
 src/providers/ad/ad_srv.c | 153 ++++++++++------------------------------------
 1 file changed, 33 insertions(+), 120 deletions(-)

diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c
index 
e719272520cee11739431a686a6cf09aaf76947e..82fbda6dc19a309c4c8ef2e2b824436f625d2e8e
 100644
--- a/src/providers/ad/ad_srv.c
+++ b/src/providers/ad/ad_srv.c
@@ -405,93 +405,10 @@ done:
     return;
 }
 
-static errno_t ad_get_client_site_parse_ndr(TALLOC_CTX *mem_ctx,
-                                            uint8_t *data,
-                                            size_t length,
-                                            char **_site_name,
-                                            char **_forest_name)
-{
-    TALLOC_CTX *tmp_ctx = NULL;
-    struct ndr_pull *ndr_pull = NULL;
-    struct netlogon_samlogon_response response;
-    enum ndr_err_code ndr_err;
-    char *site = NULL;
-    char *forest = NULL;
-    DATA_BLOB blob;
-    errno_t ret;
-
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
-        return ENOMEM;
-    }
-
-    blob.data = data;
-    blob.length = length;
-
-    ndr_pull = ndr_pull_init_blob(&blob, mem_ctx);
-    if (ndr_pull == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "ndr_pull_init_blob() failed.\n");
-        ret = ENOMEM;
-        goto done;
-    }
-
-    ndr_err = ndr_pull_netlogon_samlogon_response(ndr_pull, NDR_SCALARS,
-                                                  &response);
-    if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-        DEBUG(SSSDBG_OP_FAILURE, "ndr_pull_netlogon_samlogon_response() "
-                                  "failed [%d]\n", ndr_err);
-        ret = EBADMSG;
-        goto done;
-    }
-
-    if (!(response.ntver & NETLOGON_NT_VERSION_5EX)) {
-        DEBUG(SSSDBG_OP_FAILURE, "This NT version does not provide site "
-                                  "information [%x]\n", response.ntver);
-        ret = EBADMSG;
-        goto done;
-    }
-
-    if (response.data.nt5_ex.client_site != NULL
-        && response.data.nt5_ex.client_site[0] != '\0') {
-        site = talloc_strdup(tmp_ctx, response.data.nt5_ex.client_site);
-    } else if (response.data.nt5_ex.next_closest_site != NULL
-               && response.data.nt5_ex.next_closest_site[0] != '\0') {
-        site = talloc_strdup(tmp_ctx, response.data.nt5_ex.next_closest_site);
-    } else {
-        ret = ENOENT;
-        goto done;
-    }
-
-    if (response.data.nt5_ex.forest != NULL
-            && response.data.nt5_ex.forest[0] != '\0') {
-        forest = talloc_strdup(tmp_ctx, response.data.nt5_ex.forest);
-    } else {
-        ret = ENOENT;
-        goto done;
-    }
-
-
-    if (site == NULL || forest == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    *_site_name = talloc_steal(mem_ctx, site);
-    *_forest_name = talloc_steal(mem_ctx, forest);
-
-    ret = EOK;
-
-done:
-    talloc_free(tmp_ctx);
-    return ret;
-}
-
 static void ad_get_client_site_done(struct tevent_req *subreq)
 {
     struct ad_get_client_site_state *state = NULL;
     struct tevent_req *req = NULL;
-    struct ldb_message_element *el = NULL;
     struct sysdb_attrs **reply = NULL;
     size_t reply_count;
     errno_t ret;
@@ -520,25 +437,8 @@ static void ad_get_client_site_done(struct tevent_req 
*subreq)
         goto done;
     }
 
-    ret = sysdb_attrs_get_el(reply[0], AD_AT_NETLOGON, &el);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_el() failed\n");
-        goto done;
-    }
-
-    if (el->num_values == 0) {
-        DEBUG(SSSDBG_OP_FAILURE, "netlogon has no value\n");
-        ret = ENOENT;
-        goto done;
-    } else if (el->num_values > 1) {
-        DEBUG(SSSDBG_OP_FAILURE, "More than one netlogon value?\n");
-        ret = EIO;
-        goto done;
-    }
-
-    ret = ad_get_client_site_parse_ndr(state, el->values[0].data,
-                                       el->values[0].length, &state->site,
-                                       &state->forest);
+    ret = netlogon_get_domain_info(state, reply[0], true, NULL, &state->site,
+                                   &state->forest);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Unable to retrieve site name [%d]: %s\n",
                                   ret, strerror(ret));
@@ -547,6 +447,7 @@ static void ad_get_client_site_done(struct tevent_req 
*subreq)
     }
 
     DEBUG(SSSDBG_TRACE_FUNC, "Found site: %s\n", state->site);
+    DEBUG(SSSDBG_TRACE_FUNC, "Found forest: %s\n", state->forest);
 
 done:
     if (ret != EOK) {
@@ -803,30 +704,42 @@ static void ad_srv_plugin_site_done(struct tevent_req 
*subreq)
 
         ret = EOK;
     }
+
+    primary_domain = state->discovery_domain;
+    backup_domain = NULL;
+
     if (ret == EOK) {
         if (strcmp(state->service, "gc") == 0) {
-            primary_domain = talloc_asprintf(state, AD_SITE_DOMAIN_FMT,
-                                             state->site, state->forest);
-            if (primary_domain == NULL) {
-                ret = ENOMEM;
-                goto done;
-            }
+            if (state->forest != NULL) {
+                if (state->site != NULL) {
+                    primary_domain = talloc_asprintf(state, AD_SITE_DOMAIN_FMT,
+                                                     state->site,
+                                                     state->forest);
+                    if (primary_domain == NULL) {
+                        ret = ENOMEM;
+                        goto done;
+                    }
 
-            backup_domain = state->forest;
+                    backup_domain = state->forest;
+                } else {
+                    primary_domain = state->forest;
+                    backup_domain = NULL;
+                }
+            }
         } else {
-            primary_domain = talloc_asprintf(state, AD_SITE_DOMAIN_FMT,
-                                             state->site, 
state->discovery_domain);
-            if (primary_domain == NULL) {
-                ret = ENOMEM;
-                goto done;
-            }
+            if (state->site != NULL) {
+                primary_domain = talloc_asprintf(state, AD_SITE_DOMAIN_FMT,
+                                                 state->site,
+                                                 state->discovery_domain);
+                if (primary_domain == NULL) {
+                    ret = ENOMEM;
+                    goto done;
+                }
 
-            backup_domain = state->discovery_domain;
+                backup_domain = state->discovery_domain;
+            }
         }
-    } else if (ret == ENOENT) {
-        primary_domain = state->discovery_domain;
-        backup_domain = NULL;
-    } else {
+    } else if (ret != ENOENT && ret != EOK) {
         goto done;
     }
 
-- 
2.4.11

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to