[PATCH] Allow GetDCNameEx to be called for arbitrary sites and trusted domains

Garming Sam garming at catalyst.net.nz
Thu Mar 29 02:29:25 UTC 2018


Hi,

While looking at automatic site coverage (which has already went
upstream), I noticed that our DC location calls in NETLOGON are
particularly bad. GetDCNameEx only returned WERR_NO_SUCH_DOMAIN when you
asked for a site that the DC did not exist in. Furthermore, it did not
bother checking the domain, which meant that as long as you supplied a
valid site name in both domains, you could get a DC in the wrong domain
returned.

In order to remedy a large amount of the behaviour, I've implemented a
winbind forward call which triggers the dsgetdcname calls (using CLDAP
and DNS) in the winbind DC locator process. This allows arbitrary sites
to be queried for, and by doing so, the trusted domain case also works too.

There are a number of other errors in the RPC call which I have had to
fix, including:

- Failing to forward the error code from GetDCNameEx2 to GetDCNameEx
when it is called internally.
- Returning a more appropriate client site name (and avoid a fallback
that would be wrong in trusted domains)
- Handling of NULL and empty string parameters

There appears to still be issues with querying using a NETBIOS domain
name (including trusted domains), due to winbind sometimes falling back
to NETBIOS queries which have no site-awareness. I've noticed this
against Windows and seen it once in testenv, but the particular test I
expected to fail seems to consistently pass in make test. I've marked
the questionable test in flapping, which may warrant more inspection
later on.

Please review and push.


Cheers,

Garming

-------------- next part --------------
From e05d0eca585920ecab5dc5e95a4f59cfebd00e47 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Tue, 27 Mar 2018 15:24:44 +1300
Subject: [PATCH 01/17] samba_dnsupdate: Put samba.kcc import after path insert
 of bin/python

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/scripting/bin/samba_dnsupdate | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/scripting/bin/samba_dnsupdate b/source4/scripting/bin/samba_dnsupdate
index d6f77d4..508bd53 100755
--- a/source4/scripting/bin/samba_dnsupdate
+++ b/source4/scripting/bin/samba_dnsupdate
@@ -24,7 +24,6 @@ import fcntl
 import sys
 import tempfile
 import subprocess
-from samba.kcc import kcc_utils
 
 # ensure we get messages out immediately, so they get in the samba logs,
 # and don't get swallowed by a timeout
@@ -49,6 +48,7 @@ from samba.samdb import SamDB
 from samba.dcerpc import netlogon, winbind
 from samba.netcmd.dns import cmd_dns
 from samba import gensec
+from samba.kcc import kcc_utils
 import ldb
 
 samba.ensure_third_party_module("dns", "dnspython")
-- 
1.9.1


From 31ea30ecb3908e0434077c2dd5eb42d1dfb6fe37 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 22 Mar 2018 16:54:59 +1300
Subject: [PATCH 02/17] netlogon: Add a comment regarding one of the DC
 location calls

It appears to be basically deprecated, as it was superceded by other
calls. Presumably it is also unused.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 60761b8..cab27d2 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2953,10 +2953,14 @@ static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TAL
 }
 
 /*
-  netr_DsRGetDCName
-*/
+ * netr_DsRGetDCName
+ *
+ * This function is a predecessor to DsrGetDcNameEx2 according to [MS-NRPC].
+ * Although it has a site-guid parameter, the documentation 3.5.4.3.3 DsrGetDcName
+ * insists that it be ignored.
+ */
 static WERROR dcesrv_netr_DsRGetDCName(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
-				struct netr_DsRGetDCName *r)
+				       struct netr_DsRGetDCName *r)
 {
 	struct netr_DsRGetDCNameEx2 r2;
 	WERROR werr;
-- 
1.9.1


From 381bb9668c96946c8ea80a0ad76da645c6f08efe Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 14:25:45 +1300
Subject: [PATCH 03/17] dsdb: Allow the disable of the Windows server site
 fallback

A usage in GetDCNameEx2 could return the wrong result. This may need to
be fixed in other places.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 dfs_server/dfs_server_ad.c                    | 4 ++--
 source4/dsdb/common/util.c                    | 8 ++++++--
 source4/dsdb/samdb/ldb_modules/netlogon.c     | 3 ++-
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 3 ++-
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/dfs_server/dfs_server_ad.c b/dfs_server/dfs_server_ad.c
index 04aa7e0..84a19bd 100644
--- a/dfs_server/dfs_server_ad.c
+++ b/dfs_server/dfs_server_ad.c
@@ -602,7 +602,7 @@ static NTSTATUS dodc_referral(struct loadparm_context *lp_ctx,
 		}
 	}
 
-	site_name = samdb_client_site_name(sam_ctx, r, client_str, NULL);
+	site_name = samdb_client_site_name(sam_ctx, r, client_str, NULL, true);
 
 	status = get_dcs(r, sam_ctx, site_name, need_fqdn, &set, 0);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -713,7 +713,7 @@ static NTSTATUS dosysvol_referral(struct loadparm_context *lp_ctx,
 		}
 	}
 
-	site_name = samdb_client_site_name(sam_ctx, r, client_str, NULL);
+	site_name = samdb_client_site_name(sam_ctx, r, client_str, NULL, true);
 
 	status = get_dcs(r, sam_ctx, site_name, need_fqdn, &set, 0);
 	if (!NT_STATUS_IS_OK(status)) {
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 3b41605..ed91bc7 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -1825,9 +1825,13 @@ const char *samdb_server_site_name(struct ldb_context *ldb, TALLOC_CTX *mem_ctx)
 /*
  * Finds the client site by using the client's IP address.
  * The "subnet_name" returns the name of the subnet if parameter != NULL
+ *
+ * Has a Windows-based fallback to provide the only site available, or an empty
+ * string if there are multiple sites.
  */
 const char *samdb_client_site_name(struct ldb_context *ldb, TALLOC_CTX *mem_ctx,
-				   const char *ip_address, char **subnet_name)
+				   const char *ip_address, char **subnet_name,
+				   bool fallback)
 {
 	const char *attrs[] = { "cn", "siteObject", NULL };
 	struct ldb_dn *sites_container_dn, *subnets_dn, *sites_dn;
@@ -1896,7 +1900,7 @@ const char *samdb_client_site_name(struct ldb_context *ldb, TALLOC_CTX *mem_ctx,
 		}
 	}
 
-	if (site_name == NULL) {
+	if (site_name == NULL && fallback) {
 		/* This is the Windows Server fallback rule: when no subnet
 		 * exists and we have only one site available then use it (it
 		 * is for sure the same as our server site). If more sites do
diff --git a/source4/dsdb/samdb/ldb_modules/netlogon.c b/source4/dsdb/samdb/ldb_modules/netlogon.c
index 80599b8..1e74d1b 100644
--- a/source4/dsdb/samdb/ldb_modules/netlogon.c
+++ b/source4/dsdb/samdb/ldb_modules/netlogon.c
@@ -314,7 +314,8 @@ NTSTATUS fill_netlogon_samlogon_response(struct ldb_context *sam_ctx,
 	server_site      = samdb_server_site_name(sam_ctx, mem_ctx);
 	NT_STATUS_HAVE_NO_MEMORY(server_site);
 	client_site      = samdb_client_site_name(sam_ctx, mem_ctx,
-						  src_address, NULL);
+						  src_address, NULL,
+						  true);
 	NT_STATUS_HAVE_NO_MEMORY(client_site);
 	if (strcasecmp(server_site, client_site) == 0) {
 		server_type |= DS_SERVER_CLOSEST;
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index cab27d2..6420d57 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -3080,7 +3080,8 @@ static WERROR dcesrv_netr_DsRAddressToSitenamesExW(struct dcesrv_call_state *dce
 		ctr->sitename[i].string   = samdb_client_site_name(sam_ctx,
 								   mem_ctx,
 								   addr_str,
-								   &subnet_name);
+								   &subnet_name,
+								   true);
 		W_ERROR_HAVE_NO_MEMORY(ctr->sitename[i].string);
 		ctr->subnetname[i].string = subnet_name;
 	}
-- 
1.9.1


From bb76e41dd9b32c4696a21d0b85947472fca9c44f Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 28 Mar 2018 13:05:11 +1300
Subject: [PATCH 04/17] netlogon: Do not use stack variables for passing down
 DsRGetDCName

This is important when we make the underlying Ex2 call asynchronous.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 44 +++++++++++++--------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 6420d57..0aa1119 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2933,21 +2933,21 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
 				  struct netr_DsRGetDCNameEx *r)
 {
-	struct netr_DsRGetDCNameEx2 r2;
+	struct netr_DsRGetDCNameEx2 *r2 = NULL;
 	WERROR werr;
 
-	ZERO_STRUCT(r2);
+	r2 = talloc_zero(mem_ctx, struct netr_DsRGetDCNameEx2);
 
-	r2.in.server_unc = r->in.server_unc;
-	r2.in.client_account = NULL;
-	r2.in.mask = 0;
-	r2.in.domain_guid = r->in.domain_guid;
-	r2.in.domain_name = r->in.domain_name;
-	r2.in.site_name = r->in.site_name;
-	r2.in.flags = r->in.flags;
-	r2.out.info = r->out.info;
+	r2->in.server_unc = r->in.server_unc;
+	r2->in.client_account = NULL;
+	r2->in.mask = 0;
+	r2->in.domain_guid = r->in.domain_guid;
+	r2->in.domain_name = r->in.domain_name;
+	r2->in.site_name = r->in.site_name;
+	r2->in.flags = r->in.flags;
+	r2->out.info = r->out.info;
 
-	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, &r2);
+	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, r2);
 
 	return werr;
 }
@@ -2962,22 +2962,22 @@ static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TAL
 static WERROR dcesrv_netr_DsRGetDCName(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
 				       struct netr_DsRGetDCName *r)
 {
-	struct netr_DsRGetDCNameEx2 r2;
+	struct netr_DsRGetDCNameEx2 *r2 = NULL;
 	WERROR werr;
 
-	ZERO_STRUCT(r2);
+	r2 = talloc_zero(mem_ctx, struct netr_DsRGetDCNameEx2);
 
-	r2.in.server_unc = r->in.server_unc;
-	r2.in.client_account = NULL;
-	r2.in.mask = 0;
-	r2.in.domain_name = r->in.domain_name;
-	r2.in.domain_guid = r->in.domain_guid;
+	r2->in.server_unc = r->in.server_unc;
+	r2->in.client_account = NULL;
+	r2->in.mask = 0;
+	r2->in.domain_name = r->in.domain_name;
+	r2->in.domain_guid = r->in.domain_guid;
 
-	r2.in.site_name = NULL; /* this is correct, we should ignore site GUID */
-	r2.in.flags = r->in.flags;
-	r2.out.info = r->out.info;
+	r2->in.site_name = NULL; /* this is correct, we should ignore site GUID */
+	r2->in.flags = r->in.flags;
+	r2->out.info = r->out.info;
 
-	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, &r2);
+	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, r2);
 
 	return werr;
 }
-- 
1.9.1


From 8c4b6246707729a1955cc50fba9861eab0d14bb8 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 28 Mar 2018 17:16:25 +1300
Subject: [PATCH 05/17] tests/getdcname: Add a number of tests for GetDCNameEx

This will test the winbind forwarding to deal with sites that the target
DC does not exist in.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/tests/getdcname.py | 459 ++++++++++++++++++++++++++++++++++++++++
 selftest/knownfail.d/getdcname  |   8 +
 source4/selftest/tests.py       |   5 +
 3 files changed, 472 insertions(+)
 create mode 100644 python/samba/tests/getdcname.py
 create mode 100644 selftest/knownfail.d/getdcname

diff --git a/python/samba/tests/getdcname.py b/python/samba/tests/getdcname.py
new file mode 100644
index 0000000..9f8f993
--- /dev/null
+++ b/python/samba/tests/getdcname.py
@@ -0,0 +1,459 @@
+# Unix SMB/CIFS implementation.
+# Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+"""
+    Tests GetDCNameEx calls in NETLOGON
+"""
+
+from samba import auth
+from samba import WERRORError, werror
+import samba.tests
+import time
+import json
+import os
+from samba.credentials import Credentials
+from samba.dcerpc import netlogon
+from samba.tests import delete_force
+from samba.dcerpc.misc import GUID
+
+
+class GetDCNameEx(samba.tests.TestCase):
+
+    def setUp(self):
+        self.lp = samba.tests.env_loadparm()
+        self.creds = Credentials()
+
+        self.netlogon_conn = None
+        self.server = os.environ.get('SERVER')
+        self.realm = os.environ.get('REALM')
+        self.domain = os.environ.get('DOMAIN')
+        self.trust_realm = os.environ.get('TRUST_REALM')
+        self.trust_domain = os.environ.get('TRUST_DOMAIN')
+
+    def _call_get_dc_name(self, domain=None, domain_guid=None,
+                          site_name=None, ex2=False, flags=0):
+        if self.netlogon_conn is None:
+            self.netlogon_conn = netlogon.netlogon("ncalrpc:[schannel]",
+                                                   self.get_loadparm())
+
+        if ex2:
+            return self.netlogon_conn.netr_DsRGetDCNameEx2(self.server,
+                                                           None, 0,
+                                                           domain,
+                                                           domain_guid,
+                                                           site_name,
+                                                           flags)
+        else:
+            return self.netlogon_conn.netr_DsRGetDCNameEx(self.server,
+                                                          domain,
+                                                          domain_guid,
+                                                          site_name,
+                                                          flags)
+
+    def test_get_dc_ex2(self):
+        """Check the most trivial requirements of Ex2 (no domain or site)
+
+        a) The paths are prefixed with two backslashes
+        b) The returned domains conform to the format requested
+        c) The domain matches our own domain
+        """
+        response = self._call_get_dc_name(ex2=True)
+
+        self.assertTrue(response.dc_unc is not None)
+        self.assertTrue(response.dc_unc.startswith('\\\\'))
+        self.assertTrue(response.dc_address is not None)
+        self.assertTrue(response.dc_address.startswith('\\\\'))
+
+        self.assertTrue(response.domain_name.lower() ==
+                        self.realm.lower() or
+                        response.domain_name.lower() ==
+                        self.domain.lower())
+
+        response = self._call_get_dc_name(ex2=True,
+                                          flags=netlogon.DS_RETURN_DNS_NAME)
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+        response = self._call_get_dc_name(ex2=True,
+                                          flags=netlogon.DS_RETURN_FLAT_NAME)
+        self.assertEqual(response.domain_name.lower(),
+                         self.domain.lower())
+
+    def test_get_dc_over_winbind_ex2(self):
+        """Check what happens to Ex2 requests after being forwarded to winbind
+
+        a) The paths must still have the same backslash prefixes
+        b) The returned domain does not match our own domain
+        c) The domain matches the format requested
+        """
+        if self.trust_realm is None:
+            return
+
+        response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                ex2=True)
+        response = self._call_get_dc_name(domain=self.realm,
+                                          ex2=True)
+
+        self.assertTrue(response_trust.dc_unc is not None)
+        self.assertTrue(response_trust.dc_unc.startswith('\\\\'))
+        self.assertTrue(response_trust.dc_address is not None)
+        self.assertTrue(response_trust.dc_address.startswith('\\\\'))
+
+        self.assertNotEqual(response_trust.dc_unc,
+                            response.dc_unc)
+        self.assertNotEqual(response_trust.dc_address,
+                            response.dc_address)
+
+        self.assertTrue(response_trust.domain_name.lower() ==
+                        self.trust_realm.lower() or
+                        response_trust.domain_name.lower() ==
+                        self.trust_domain.lower())
+
+        response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                flags=netlogon.DS_RETURN_DNS_NAME,
+                                                ex2=True)
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+        response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                flags=netlogon.DS_RETURN_FLAT_NAME,
+                                                ex2=True)
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_domain.lower())
+
+    def test_get_dc_over_winbind(self):
+        """Test the standard Ex version (not Ex2)
+
+        Ex calls Ex2 anyways, from now on, just test Ex.
+        """
+        if self.trust_realm is None:
+            return
+
+        response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                flags=netlogon.DS_RETURN_DNS_NAME)
+
+        self.assertTrue(response_trust.dc_unc is not None)
+        self.assertTrue(response_trust.dc_unc.startswith('\\\\'))
+        self.assertTrue(response_trust.dc_address is not None)
+        self.assertTrue(response_trust.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+    def test_get_dc_over_winbind_with_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is a Default-First-Site-Name site.
+        """
+        if self.trust_realm is None:
+            return
+
+        site = 'Default-First-Site-Name'
+        response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                site_name=site,
+                                                flags=netlogon.DS_RETURN_DNS_NAME)
+
+        self.assertTrue(response_trust.dc_unc is not None)
+        self.assertTrue(response_trust.dc_unc.startswith('\\\\'))
+        self.assertTrue(response_trust.dc_address is not None)
+        self.assertTrue(response_trust.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+        self.assertEqual(site.lower(), response_trust.dc_site_name.lower())
+
+    def test_get_dc_over_winbind_invalid_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is no Invalid-First-Site-Name site.
+        """
+        if self.trust_realm is None:
+            return
+
+        site = 'Invalid-First-Site-Name'
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                    site_name=site,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME,
+                                                    ex2=False)
+            self.fail("Failed to give the correct error for incorrect site")
+        except WERRORError as e:
+            enum, estr = e.args
+            if enum != werror.WERR_NO_SUCH_DOMAIN:
+                self.fail("Failed to detect an invalid site name")
+
+    def test_get_dc_over_winbind_invalid_site_ex2(self):
+        """Test the Ex2 version.
+
+        We assume that there is no Invalid-First-Site-Name site.
+        """
+        if self.trust_realm is None:
+            return
+
+        site = 'Invalid-First-Site-Name'
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                    site_name=site,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME,
+                                                    ex2=True)
+            self.fail("Failed to give the correct error for incorrect site")
+        except WERRORError as e:
+            enum, estr = e.args
+            if enum != werror.WERR_NO_SUCH_DOMAIN:
+                self.fail("Failed to detect an invalid site name")
+
+    def test_get_dc_over_winbind_empty_string_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is a Default-First-Site-Name site.
+        """
+        if self.trust_realm is None:
+            return
+
+        site = ''
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                    site_name=site,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME)
+        except WERRORError as e:
+            self.fail("Unable to get empty string site result: " + str(e))
+
+        self.assertTrue(response_trust.dc_unc is not None)
+        self.assertTrue(response_trust.dc_unc.startswith('\\\\'))
+        self.assertTrue(response_trust.dc_address is not None)
+        self.assertTrue(response_trust.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+        self.assertTrue(response_trust.dc_site_name is not None)
+        self.assertNotEqual('', response_trust.dc_site_name)
+
+    def test_get_dc_over_winbind_netbios(self):
+        """Supply a NETBIOS trust domain name."""
+        if self.trust_realm is None:
+            return
+
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_domain,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME,
+                                                    ex2=False)
+        except WERRORError as e:
+            self.fail("Failed to succeed over winbind: " + str(e))
+
+        self.assertTrue(response_trust is not None)
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+    def test_get_dc_over_winbind_with_site_netbios(self):
+        """Supply a NETBIOS trust domain name.
+
+        Sporadically fails because NETBIOS queries do not return site name in
+        winbind. The site check in NETLOGON will trigger and fail the request.
+
+        Currently marked in flapping...
+        """
+        if self.trust_realm is None:
+            return
+
+        site = 'Default-First-Site-Name'
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_domain,
+                                                    site_name=site,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME,
+                                                    ex2=False)
+        except WERRORError as e:
+            self.fail("Failed to succeed over winbind: " + str(e))
+
+        self.assertTrue(response_trust is not None)
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+        self.assertEqual(site.lower(), response_trust.dc_site_name.lower())
+
+    def test_get_dc_over_winbind_domain_guid(self):
+        """Ensure that we do not reject requests supplied with a NULL GUID"""
+
+        if self.trust_realm is None:
+            return
+
+        null_guid = GUID()
+        try:
+            response_trust = self._call_get_dc_name(domain=self.trust_realm,
+                                                    domain_guid=null_guid,
+                                                    flags=netlogon.DS_RETURN_DNS_NAME)
+        except WERRORError as e:
+            self.fail("Unable to get NULL domain GUID result: " + str(e))
+
+        self.assertTrue(response_trust.dc_unc is not None)
+        self.assertTrue(response_trust.dc_unc.startswith('\\\\'))
+        self.assertTrue(response_trust.dc_address is not None)
+        self.assertTrue(response_trust.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response_trust.domain_name.lower(),
+                         self.trust_realm.lower())
+
+    def test_get_dc_with_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is a Default-First-Site-Name site.
+        """
+
+        site = 'Default-First-Site-Name'
+        response = self._call_get_dc_name(domain=self.realm,
+                                          site_name=site,
+                                          flags=netlogon.DS_RETURN_DNS_NAME)
+
+        self.assertTrue(response.dc_unc is not None)
+        self.assertTrue(response.dc_unc.startswith('\\\\'))
+        self.assertTrue(response.dc_address is not None)
+        self.assertTrue(response.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+        self.assertEqual(site.lower(), response.dc_site_name.lower())
+
+    def test_get_dc_invalid_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is no Invalid-First-Site-Name site.
+        """
+        if self.realm is None:
+            return
+
+        site = 'Invalid-First-Site-Name'
+        try:
+            response = self._call_get_dc_name(domain=self.realm,
+                                              site_name=site,
+                                              flags=netlogon.DS_RETURN_DNS_NAME,
+                                              ex2=False)
+            self.fail("Failed to give the correct error for incorrect site")
+        except WERRORError as e:
+            enum, estr = e.args
+            if enum != werror.WERR_NO_SUCH_DOMAIN:
+                self.fail("Failed to detect an invalid site name")
+
+    def test_get_dc_invalid_site_ex2(self):
+        """Test the Ex2 version
+
+        We assume that there is no Invalid-First-Site-Name site.
+        """
+
+        site = 'Invalid-First-Site-Name'
+        try:
+            response = self._call_get_dc_name(domain=self.realm,
+                                              site_name=site,
+                                              flags=netlogon.DS_RETURN_DNS_NAME,
+                                              ex2=True)
+            self.fail("Failed to give the correct error for incorrect site")
+        except WERRORError as e:
+            enum, estr = e.args
+            if enum != werror.WERR_NO_SUCH_DOMAIN:
+                self.fail("Failed to detect an invalid site name")
+
+    def test_get_dc_empty_string_site(self):
+        """Test the standard Ex version (not Ex2)
+
+        We assume that there is a Default-First-Site-Name site.
+        """
+
+        site = ''
+        try:
+            response = self._call_get_dc_name(domain=self.realm,
+                                              site_name=site,
+                                              flags=netlogon.DS_RETURN_DNS_NAME)
+        except WERRORError as e:
+            self.fail("Unable to get empty string site result: " + str(e))
+
+        self.assertTrue(response.dc_unc is not None)
+        self.assertTrue(response.dc_unc.startswith('\\\\'))
+        self.assertTrue(response.dc_address is not None)
+        self.assertTrue(response.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+        self.assertTrue(response.dc_site_name is not None)
+        self.assertNotEqual('', response.dc_site_name)
+
+    def test_get_dc_netbios(self):
+        """Supply a NETBIOS domain name."""
+
+        try:
+            response = self._call_get_dc_name(domain=self.domain,
+                                              flags=netlogon.DS_RETURN_DNS_NAME,
+                                              ex2=False)
+        except WERRORError as e:
+            self.fail("Failed to succeed over winbind: " + str(e))
+
+        self.assertTrue(response is not None)
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+    def test_get_dc_with_site_netbios(self):
+        """Supply a NETBIOS domain name."""
+
+        site = 'Default-First-Site-Name'
+        try:
+            response = self._call_get_dc_name(domain=self.domain,
+                                              site_name=site,
+                                              flags=netlogon.DS_RETURN_DNS_NAME,
+                                              ex2=False)
+        except WERRORError as e:
+            self.fail("Failed to succeed over winbind: " + str(e))
+
+        self.assertTrue(response is not None)
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+        self.assertEqual(site.lower(), response.dc_site_name.lower())
+
+    def test_get_dc_with_domain_guid(self):
+        """Ensure that we do not reject requests supplied with a NULL GUID"""
+
+        null_guid = GUID()
+        response = self._call_get_dc_name(domain=self.realm,
+                                          domain_guid=null_guid,
+                                          flags=netlogon.DS_RETURN_DNS_NAME)
+
+        self.assertTrue(response.dc_unc is not None)
+        self.assertTrue(response.dc_unc.startswith('\\\\'))
+        self.assertTrue(response.dc_address is not None)
+        self.assertTrue(response.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+    def test_get_dc_with_empty_string_domain(self):
+        """Ensure that empty domain resolve to the DC domain"""
+        response = self._call_get_dc_name(domain='',
+                                          flags=netlogon.DS_RETURN_DNS_NAME)
+
+        self.assertTrue(response.dc_unc is not None)
+        self.assertTrue(response.dc_unc.startswith('\\\\'))
+        self.assertTrue(response.dc_address is not None)
+        self.assertTrue(response.dc_address.startswith('\\\\'))
+
+        self.assertEqual(response.domain_name.lower(),
+                         self.realm.lower())
+
+    # TODO Thorough tests of domain GUID
+    #
+    # The domain GUID does not seem to be authoritative, and seems to be a
+    # fallback case for renamed domains.
diff --git a/selftest/knownfail.d/getdcname b/selftest/knownfail.d/getdcname
new file mode 100644
index 0000000..1c086a3
--- /dev/null
+++ b/selftest/knownfail.d/getdcname
@@ -0,0 +1,8 @@
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_empty_string_site.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_domain_guid.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_empty_string_site.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_ex2.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_netbios.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site_netbios.fl2008r2dc:local.*
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 660a75f..138c207 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -669,6 +669,11 @@ if have_heimdal_support:
                            extra_args=['-U"$USERNAME%$PASSWORD"'],
                            environ={'CLIENT_IP': '127.0.0.11',
                                     'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+
+planoldpythontestsuite("fl2008r2dc:local",
+                       "samba.tests.getdcname",
+                       extra_args=['-U"$USERNAME%$PASSWORD"'])
+
 planoldpythontestsuite("ad_dc",
                        "samba.tests.net_join_no_spnego",
                        extra_args=['-U"$USERNAME%$PASSWORD"'])
-- 
1.9.1


From 2b9a0dbceb6067ee7c066b2358b81f09e466c8d9 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 21 Mar 2018 11:25:19 +1300
Subject: [PATCH 06/17] winbindd_irpc: Add an IRPC call to trigger a DC locate

Calling the top level winbindd API would probably be more appropriate,
but we lack certain structures. We introduce this call in order to
return the result to NETLOGON (in order to give site-aware and domain
aware DC location).

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source3/winbindd/winbindd_irpc.c | 62 ++++++++++++++++++++++++++++++++++++++++
 source4/librpc/idl/irpc.idl      | 11 +++++++
 2 files changed, 73 insertions(+)

diff --git a/source3/winbindd/winbindd_irpc.c b/source3/winbindd/winbindd_irpc.c
index e03312e..8463987 100644
--- a/source3/winbindd/winbindd_irpc.c
+++ b/source3/winbindd/winbindd_irpc.c
@@ -29,6 +29,8 @@
 #include "librpc/gen_ndr/ndr_lsa_c.h"
 #include "libcli/security/dom_sid.h"
 #include "passdb/lookup_sid.h" /* only for LOOKUP_NAME_NO_NSS flag */
+#include "librpc/gen_ndr/ndr_irpc.h"
+#include "librpc/gen_ndr/ndr_netlogon.h"
 
 struct wb_irpc_forward_state {
 	struct irpc_message *msg;
@@ -726,6 +728,60 @@ static void wb_irpc_lsa_LookupNames4_domains_done(struct tevent_req *subreq)
 	return;
 }
 
+struct irpc_GetDCName_state {
+	struct irpc_message *msg;
+	struct netr_DsRGetDCNameInfo **dcinfo;
+};
+
+static void wb_irpc_GetDCName_done(struct tevent_req *subreq)
+{
+	struct irpc_GetDCName_state *state = tevent_req_callback_data(
+		subreq, struct irpc_GetDCName_state);
+	NTSTATUS status;
+
+	status = wb_dsgetdcname_recv(subreq, state, state->dcinfo);
+	TALLOC_FREE(subreq);
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0,("RPC callback failed for %s - %s\n", "DSGETDCNAME",
+			 nt_errstr(status)));
+		irpc_send_reply(state->msg, status);
+		return;
+	}
+
+	irpc_send_reply(state->msg, NT_STATUS_OK);
+}
+
+static NTSTATUS wb_irpc_GetDCName(struct irpc_message *msg,
+				  struct winbind_DsGetDcName *req)
+{
+
+	struct tevent_req *subreq = NULL;
+	struct irpc_GetDCName_state *state = NULL;
+
+	state = talloc_zero(msg, struct irpc_GetDCName_state);
+
+	state->msg = msg;
+	state->dcinfo = req->out.dc_info;
+
+	subreq = wb_dsgetdcname_send(msg,
+				     server_event_context(),
+				     req->in.domain_name,
+				     req->in.domain_guid,
+				     req->in.site_name,
+				     req->in.flags);
+	if (subreq == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	tevent_req_set_callback(subreq,
+				wb_irpc_GetDCName_done,
+				state);
+
+	msg->defer_reply = true;
+
+	return NT_STATUS_OK;
+}
+
 NTSTATUS wb_irpc_register(void)
 {
 	NTSTATUS status;
@@ -769,6 +825,12 @@ NTSTATUS wb_irpc_register(void)
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
+	status = IRPC_REGISTER(winbind_imessaging_context(),
+			       irpc, WINBIND_DSGETDCNAME,
+			       wb_irpc_GetDCName, NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
 
 	return NT_STATUS_OK;
 }
diff --git a/source4/librpc/idl/irpc.idl b/source4/librpc/idl/irpc.idl
index 65ae4b6..40751b7 100644
--- a/source4/librpc/idl/irpc.idl
+++ b/source4/librpc/idl/irpc.idl
@@ -218,4 +218,15 @@ import "misc.idl", "security.idl", "nbt.idl", "netlogon.idl", "server_id.idl";
 	 * or replicated by DRS.
 	 */
 	NTSTATUS dnssrv_reload_dns_zones();
+
+	/*
+	 * IRPC call to ask winbindd to return a valid DC
+	 */
+	NTSTATUS winbind_DsGetDcName(
+		[in,string,charset(UTF8)] char *domain_name,
+		[in,unique] GUID *domain_guid,
+		[in,string,unique,charset(UTF8)] char *site_name,
+		[in] uint32 flags,
+		[out] netr_DsRGetDCNameInfo **dc_info
+		);
 }
-- 
1.9.1


From 88c50f7150f660452d00003dfe7f69edf421136d Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 21 Mar 2018 11:25:19 +1300
Subject: [PATCH 07/17] netlogon: Forward GetDCNameEx2 to winbind via IRPC

Here we simply forward everything without alteration (the same struct is
returned). This helps us to fix the case where the DC does not exist in
the target site, furthermore, this is supposed to work for trusted
domains.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/flapping.d/getdcname                 |   2 +
 selftest/knownfail.d/getdcname                |   9 ++-
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 108 +++++++++++++++++++++++++-
 3 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 selftest/flapping.d/getdcname

diff --git a/selftest/flapping.d/getdcname b/selftest/flapping.d/getdcname
new file mode 100644
index 0000000..4c12e75
--- /dev/null
+++ b/selftest/flapping.d/getdcname
@@ -0,0 +1,2 @@
+# winbind appears to return inconsistent answers (depending on whether or not it uses NETBIOS queries or not)
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site_netbios.fl2008r2dc:local.*
diff --git a/selftest/knownfail.d/getdcname b/selftest/knownfail.d/getdcname
index 1c086a3..f7c579a 100644
--- a/selftest/knownfail.d/getdcname
+++ b/selftest/knownfail.d/getdcname
@@ -1,8 +1,11 @@
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_empty_string_site.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_domain_guid.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_empty_string_site.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_ex2.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_netbios.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site_netbios.fl2008r2dc:local.*
+
+# Introduced failures due to new winbind handling
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site_ex2.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site.fl2008r2dc:local.*
+^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site_ex2.fl2008r2dc:local.*
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 0aa1119..5e95389 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2749,6 +2749,89 @@ static NTSTATUS dcesrv_netr_NetrLogonSendToSam(struct dcesrv_call_state *dce_cal
 	return NT_STATUS_OK;
 }
 
+struct winbind_forward_state {
+	struct dcesrv_call_state *dce_call;
+	struct netr_DsRGetDCNameInfo *info;
+	struct netr_DsRGetDCNameEx2 *r;
+};
+
+static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
+{
+	struct winbind_forward_state *state =
+		tevent_req_callback_data(subreq,
+		struct winbind_forward_state);
+	NTSTATUS result, status;
+
+	status = dcerpc_winbind_DsGetDcName_recv(subreq,
+						 state->dce_call,
+						 &result);
+
+	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
+		state->r->out.result = WERR_TIMEOUT;
+	} else if (!NT_STATUS_IS_OK(status)) {
+		state->dce_call->fault_code = DCERPC_FAULT_CANT_PERFORM;
+		DEBUG(0,(__location__ ": IRPC callback failed %s\n",
+			 nt_errstr(status)));
+	} else {
+		/*
+		 * Make sure to return our DC UNC with // prefix.
+		 * Winbind currently doesn't send the leading slashes
+		 * for some reason.
+		 */
+		if (strlen(state->info->dc_unc) > 2 &&
+		    strncmp("\\\\", state->info->dc_unc, 2) != 0) {
+			const char *dc_unc = NULL;
+
+			dc_unc = talloc_asprintf(state->dce_call,
+						 "\\\\%s",
+						 state->info->dc_unc);
+			state->info->dc_unc = dc_unc;
+		}
+
+		*state->r->out.info = state->info;
+		state->r->out.result = ntstatus_to_werror(result);
+	}
+
+	status = dcesrv_reply(state->dce_call);
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0,(__location__ ": dcesrv_reply() failed - %s\n",
+			 nt_errstr(status)));
+	}
+}
+
+static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
+				       struct tevent_context *ev,
+				       struct dcerpc_binding_handle *h,
+				       const char *domain_name,
+				       struct GUID *domain_guid,
+				       const char *site_name,
+				       uint32_t flags,
+				       struct dcesrv_call_state *dce_call,
+				       struct netr_DsRGetDCNameEx2 *r)
+{
+	struct tevent_req *subreq = NULL;
+	struct winbind_forward_state *state = NULL;
+
+	state = talloc_zero(mem_ctx, struct winbind_forward_state);
+	state->dce_call = dce_call;
+	state->r = r;
+
+	subreq = dcerpc_winbind_DsGetDcName_send(mem_ctx,
+						 ev,
+						 h,
+						 domain_name,
+						 domain_guid,
+						 site_name,
+						 flags,
+						 &state->info);
+	if (subreq == NULL) {
+		return WERR_NOT_ENOUGH_MEMORY;
+	}
+
+	tevent_req_set_callback(subreq, winbind_forward_GetDCName_done, state);
+
+	return WERR_OK;
+}
 
 /*
   netr_DsRGetDCNameEx2
@@ -2836,7 +2919,30 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 	W_ERROR_HAVE_NO_MEMORY(server_site_name);
 	if ((r->in.site_name != NULL) && (strcasecmp(r->in.site_name,
 						     server_site_name) != 0)) {
-		return WERR_NO_SUCH_DOMAIN;
+
+		struct dcerpc_binding_handle *irpc_handle = NULL;
+
+		irpc_handle = irpc_binding_handle_by_name(mem_ctx,
+							  dce_call->msg_ctx,
+							  "winbind_server",
+							  &ndr_table_irpc);
+		if (irpc_handle == NULL) {
+			DEBUG(0,("Failed to get binding_handle for "
+				 "winbind_server task\n"));
+			dce_call->fault_code = DCERPC_FAULT_CANT_PERFORM;
+			return WERR_SERVICE_NOT_FOUND;
+		}
+
+		dcerpc_binding_handle_set_timeout(irpc_handle, 60);
+
+		dce_call->state_flags |= DCESRV_CALL_STATE_FLAG_ASYNC;
+
+		winbind_forward_GetDCName(mem_ctx, dce_call->event_ctx,
+					  irpc_handle, r->in.domain_name,
+					  r->in.domain_guid, r->in.site_name,
+					  r->in.flags, dce_call, r);
+
+		return WERR_OK;
 	}
 
 	guid_str = r->in.domain_guid != NULL ?
-- 
1.9.1


From 04caf8ef4fb93f2d39f813c23420e2aadf45e5d7 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 22 Mar 2018 17:10:01 +1300
Subject: [PATCH 08/17] netlogon: Require the GetDCNameEx2 must return the DC
 in the site

MS-NRPC 3.5.4.3.1 'SiteName: A null-terminated string that contains
the name of the site in which the DC MUST be located.'

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/knownfail.d/getdcname                |  2 --
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 41 ++++++++++++++++++---------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/selftest/knownfail.d/getdcname b/selftest/knownfail.d/getdcname
index f7c579a..84fcdd9 100644
--- a/selftest/knownfail.d/getdcname
+++ b/selftest/knownfail.d/getdcname
@@ -6,6 +6,4 @@
 
 # Introduced failures due to new winbind handling
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site_ex2.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site_ex2.fl2008r2dc:local.*
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 5e95389..27e9066 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2774,22 +2774,37 @@ static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 			 nt_errstr(status)));
 	} else {
 		/*
-		 * Make sure to return our DC UNC with // prefix.
-		 * Winbind currently doesn't send the leading slashes
-		 * for some reason.
+		 * Either the supplied site name is NULL (possibly via
+		 * TRY_NEXT_CLOSEST_SITE) or the resulting site name matches
+		 * the input match name.
+		 *
+		 * TODO: Currently this means that requests with NETBIOS domain
+		 * names will fail because they do not return the site name.
 		 */
-		if (strlen(state->info->dc_unc) > 2 &&
-		    strncmp("\\\\", state->info->dc_unc, 2) != 0) {
-			const char *dc_unc = NULL;
+		if (state->r->in.site_name == NULL ||
+		    strcasecmp_m("", state->r->in.site_name) == 0 ||
+		    strcasecmp_m(state->info->dc_site_name,
+				 state->r->in.site_name) == 0) {
+			/*
+			 * Make sure to return our DC UNC with // prefix.
+			 * Winbind currently doesn't send the leading slashes
+			 * for some reason.
+			 */
+			if (strlen(state->info->dc_unc) > 2 &&
+			    strncmp("\\\\", state->info->dc_unc, 2) != 0) {
+				const char *dc_unc = NULL;
+
+				dc_unc = talloc_asprintf(state->dce_call,
+							 "\\\\%s",
+							 state->info->dc_unc);
+				state->info->dc_unc = dc_unc;
+			}
 
-			dc_unc = talloc_asprintf(state->dce_call,
-						 "\\\\%s",
-						 state->info->dc_unc);
-			state->info->dc_unc = dc_unc;
+			*state->r->out.info = state->info;
+			state->r->out.result = ntstatus_to_werror(result);
+		} else {
+			state->r->out.result = WERR_NO_SUCH_DOMAIN;
 		}
-
-		*state->r->out.info = state->info;
-		state->r->out.result = ntstatus_to_werror(result);
 	}
 
 	status = dcesrv_reply(state->dce_call);
-- 
1.9.1


From 0b97c63017ec5ab1556ce6c6c5e999c6ac6a20c9 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 12:05:21 +1300
Subject: [PATCH 09/17] netlogon: Use winbind DC locate when the domain differs

Previously, you could have the same site in different domains and it
would accidentally return ourselves.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/knownfail.d/getdcname                |  6 -----
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 34 +++++++++++++++++++++++++--
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/selftest/knownfail.d/getdcname b/selftest/knownfail.d/getdcname
index 84fcdd9..917ebb6 100644
--- a/selftest/knownfail.d/getdcname
+++ b/selftest/knownfail.d/getdcname
@@ -1,9 +1,3 @@
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_domain_guid.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_ex2.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_netbios.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_with_site.fl2008r2dc:local.*
-
 # Introduced failures due to new winbind handling
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site.fl2008r2dc:local.*
 ^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site.fl2008r2dc:local.*
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 27e9066..87f05ee 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2869,6 +2869,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 	const char *dc_name = NULL;
 	const char *domain_name = NULL;
 	const char *pdc_ip;
+	bool different_domain = true;
 
 	ZERO_STRUCTP(r->out.info);
 
@@ -2929,11 +2930,40 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 		return WERR_INVALID_FLAGS;
 	}
 
+	/* Attempt winbind search only if we suspect the domain is incorrect */
+	if (r->in.domain_name != NULL && strcmp("", r->in.domain_name) != 0) {
+		if (r->in.flags & DS_IS_FLAT_NAME) {
+			if (strcasecmp_m(r->in.domain_name,
+					 lpcfg_sam_name(lp_ctx)) == 0) {
+				different_domain = false;
+			}
+		} else if (r->in.flags & DS_IS_DNS_NAME) {
+			if (strcasecmp_m(r->in.domain_name,
+					 lpcfg_dnsdomain(lp_ctx)) == 0) {
+				different_domain = false;
+			}
+		} else {
+			if (strcasecmp_m(r->in.domain_name,
+					 lpcfg_sam_name(lp_ctx)) == 0 ||
+			    strcasecmp_m(r->in.domain_name,
+					 lpcfg_dnsdomain(lp_ctx)) == 0) {
+				different_domain = false;
+			}
+		}
+	} else {
+		/*
+		 * We need to be able to handle empty domain names, where we
+		 * revert to our domain by default.
+		 */
+		different_domain = false;
+	}
+
 	/* Proof server site parameter "site_name" if it was specified */
 	server_site_name = samdb_server_site_name(sam_ctx, mem_ctx);
 	W_ERROR_HAVE_NO_MEMORY(server_site_name);
-	if ((r->in.site_name != NULL) && (strcasecmp(r->in.site_name,
-						     server_site_name) != 0)) {
+	if (different_domain || (r->in.site_name != NULL &&
+				 (strcasecmp_m(r->in.site_name,
+					     server_site_name) != 0))) {
 
 		struct dcerpc_binding_handle *irpc_handle = NULL;
 
-- 
1.9.1


From c8947637bc333e3f5586c969fa2cb57134271df6 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 12:10:58 +1300
Subject: [PATCH 10/17] netlogon: Add a TODO regarding GetDCNameEx2 results

We already had this issue previously, but now the problem has been
deferred to these winbind calls.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 87f05ee..a13c331 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2755,6 +2755,11 @@ struct winbind_forward_state {
 	struct netr_DsRGetDCNameEx2 *r;
 };
 
+/*
+ * TODO: We can still erroneously return ourselves across winbind when a domain
+ * does not exist. We may wish to pass down a flag to prevent that. Although
+ * this is partly mitigated by the site name check in this function.
+ */
 static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 {
 	struct winbind_forward_state *state =
-- 
1.9.1


From a5710a16c8088be545352b2541c7988f8389b072 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 12:36:25 +1300
Subject: [PATCH 11/17] netlogon: Resolve calls to GetDCNameEx2 within the same
 NETLOGON domain

The return will have the DNS domain, which is not quite as nice, but it
does not seem to violate any assumptions

The reason we do this is because DNS based querying seems to be much
more reliable than the NETBIOS based queries.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 31 ++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index a13c331..27a913d 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2756,9 +2756,10 @@ struct winbind_forward_state {
 };
 
 /*
- * TODO: We can still erroneously return ourselves across winbind when a domain
- * does not exist. We may wish to pass down a flag to prevent that. Although
- * this is partly mitigated by the site name check in this function.
+ * TODO: We can still erroneously return ourselves across winbind when a
+ * (NETBIOS) domain does not exist. We may wish to pass down a flag to prevent
+ * that. This is partly mitigated by the site name check in this function, and
+ * because it preferably sends the DNS domain.
  */
 static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 {
@@ -2785,6 +2786,12 @@ static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 		 *
 		 * TODO: Currently this means that requests with NETBIOS domain
 		 * names will fail because they do not return the site name.
+		 *
+		 * In the standard single domain case, the caller will pass
+		 * down the DNS domain instead (which perhaps should be
+		 * reverted to the NETBIOS name here).
+		 *
+		 * For trusted domains, changes to winbind will need to be made.
 		 */
 		if (state->r->in.site_name == NULL ||
 		    strcasecmp_m("", state->r->in.site_name) == 0 ||
@@ -2875,6 +2882,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 	const char *domain_name = NULL;
 	const char *pdc_ip;
 	bool different_domain = true;
+	bool forward_with_dns_name = false;
 
 	ZERO_STRUCTP(r->out.info);
 
@@ -2941,6 +2949,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 			if (strcasecmp_m(r->in.domain_name,
 					 lpcfg_sam_name(lp_ctx)) == 0) {
 				different_domain = false;
+				forward_with_dns_name = true;
 			}
 		} else if (r->in.flags & DS_IS_DNS_NAME) {
 			if (strcasecmp_m(r->in.domain_name,
@@ -2953,6 +2962,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 			    strcasecmp_m(r->in.domain_name,
 					 lpcfg_dnsdomain(lp_ctx)) == 0) {
 				different_domain = false;
+				forward_with_dns_name = true;
 			}
 		}
 	} else {
@@ -2961,6 +2971,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 		 * revert to our domain by default.
 		 */
 		different_domain = false;
+		forward_with_dns_name = true;
 	}
 
 	/* Proof server site parameter "site_name" if it was specified */
@@ -2971,6 +2982,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 					     server_site_name) != 0))) {
 
 		struct dcerpc_binding_handle *irpc_handle = NULL;
+		uint32_t forward_flags = r->in.flags;
 
 		irpc_handle = irpc_binding_handle_by_name(mem_ctx,
 							  dce_call->msg_ctx,
@@ -2987,10 +2999,19 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 
 		dce_call->state_flags |= DCESRV_CALL_STATE_FLAG_ASYNC;
 
+		domain_name = r->in.domain_name;
+
+		/* This only fixes the single domain case with NETBIOS names. */
+		if (forward_with_dns_name) {
+			forward_flags &= ~DS_IS_FLAT_NAME;
+			forward_flags |= DS_IS_DNS_NAME;
+			domain_name = lpcfg_dnsdomain(lp_ctx);
+		}
+
 		winbind_forward_GetDCName(mem_ctx, dce_call->event_ctx,
-					  irpc_handle, r->in.domain_name,
+					  irpc_handle, domain_name,
 					  r->in.domain_guid, r->in.site_name,
-					  r->in.flags, dce_call, r);
+					  forward_flags, dce_call, r);
 
 		return WERR_OK;
 	}
-- 
1.9.1


From bd85fa66091ad34a1b60ff52afd934e2860a841f Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 14:32:07 +1300
Subject: [PATCH 12/17] netlogon: Store the client site to clobber the one
 returned via winbind

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 27a913d..5b921c4 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2753,6 +2753,7 @@ struct winbind_forward_state {
 	struct dcesrv_call_state *dce_call;
 	struct netr_DsRGetDCNameInfo *info;
 	struct netr_DsRGetDCNameEx2 *r;
+	const char *client_site;
 };
 
 /*
@@ -2812,6 +2813,8 @@ static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 				state->info->dc_unc = dc_unc;
 			}
 
+			state->info->client_site_name = state->client_site;
+
 			*state->r->out.info = state->info;
 			state->r->out.result = ntstatus_to_werror(result);
 		} else {
@@ -2833,6 +2836,7 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 				       struct GUID *domain_guid,
 				       const char *site_name,
 				       uint32_t flags,
+				       const char *client_site,
 				       struct dcesrv_call_state *dce_call,
 				       struct netr_DsRGetDCNameEx2 *r)
 {
@@ -2840,6 +2844,7 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 	struct winbind_forward_state *state = NULL;
 
 	state = talloc_zero(mem_ctx, struct winbind_forward_state);
+	state->client_site = client_site;
 	state->dce_call = dce_call;
 	state->r = r;
 
@@ -2983,6 +2988,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 
 		struct dcerpc_binding_handle *irpc_handle = NULL;
 		uint32_t forward_flags = r->in.flags;
+		const char *client_site = NULL;
 
 		irpc_handle = irpc_binding_handle_by_name(mem_ctx,
 							  dce_call->msg_ctx,
@@ -3008,10 +3014,23 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 			domain_name = lpcfg_dnsdomain(lp_ctx);
 		}
 
+		/*
+		 * Retrieve the client site to override the winbind response.
+		 *
+		 * DO NOT use Windows fallback for client site.
+		 * In the case of multiple domains, this is plainly wrong.
+		 */
+		client_site = samdb_client_site_name(sam_ctx,
+						     mem_ctx,
+						     remote_addr,
+						     NULL,
+						     false);
+
 		winbind_forward_GetDCName(mem_ctx, dce_call->event_ctx,
 					  irpc_handle, domain_name,
 					  r->in.domain_guid, r->in.site_name,
-					  forward_flags, dce_call, r);
+					  forward_flags, client_site,
+					  dce_call, r);
 
 		return WERR_OK;
 	}
-- 
1.9.1


From 2769cc0f5106fc08dbc4c48127aab4a4eb6f53a4 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 14:56:01 +1300
Subject: [PATCH 13/17] netlogon: Do not fault upon IRPC failure in GetDCName

Unlike other forwarded calls, failures are to be expected with
non-existent domains.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 5b921c4..b7c4969 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2776,9 +2776,9 @@ static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT)) {
 		state->r->out.result = WERR_TIMEOUT;
 	} else if (!NT_STATUS_IS_OK(status)) {
-		state->dce_call->fault_code = DCERPC_FAULT_CANT_PERFORM;
-		DEBUG(0,(__location__ ": IRPC callback failed %s\n",
-			 nt_errstr(status)));
+		DBG_NOTICE("DC location via winbind failed - %s\n",
+			   nt_errstr(status));
+		state->r->out.result = WERR_NO_SUCH_DOMAIN;
 	} else {
 		/*
 		 * Either the supplied site name is NULL (possibly via
-- 
1.9.1


From aaddc59b1f2e29bf6e20f15a30382531e060d77b Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 16:45:16 +1300
Subject: [PATCH 14/17] netlogon: Remove variables we can infer in winbind
 forward

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index b7c4969..c6237ce 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2833,8 +2833,6 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 				       struct tevent_context *ev,
 				       struct dcerpc_binding_handle *h,
 				       const char *domain_name,
-				       struct GUID *domain_guid,
-				       const char *site_name,
 				       uint32_t flags,
 				       const char *client_site,
 				       struct dcesrv_call_state *dce_call,
@@ -2852,8 +2850,8 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 						 ev,
 						 h,
 						 domain_name,
-						 domain_guid,
-						 site_name,
+						 r->in.domain_guid,
+						 r->in.site_name,
 						 flags,
 						 &state->info);
 	if (subreq == NULL) {
@@ -3028,7 +3026,6 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 
 		winbind_forward_GetDCName(mem_ctx, dce_call->event_ctx,
 					  irpc_handle, domain_name,
-					  r->in.domain_guid, r->in.site_name,
 					  forward_flags, client_site,
 					  dce_call, r);
 
-- 
1.9.1


From 85927e85c50283ef8bfee24339c60a5f3775eb24 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 26 Mar 2018 16:59:16 +1300
Subject: [PATCH 15/17] netlogon: Add a note regarding clients belonging to
 multiple subnets

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index c6237ce..a55e62a 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -3017,6 +3017,10 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 		 *
 		 * DO NOT use Windows fallback for client site.
 		 * In the case of multiple domains, this is plainly wrong.
+		 *
+		 * Note: It's possible that the client may belong to multiple
+		 * subnets across domains. It's not clear what this would mean,
+		 * but here we only return what this domain knows.
 		 */
 		client_site = samdb_client_site_name(sam_ctx,
 						     mem_ctx,
-- 
1.9.1


From 472f342e1873b01e66b63d39d2384e8fe4460514 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Tue, 27 Mar 2018 12:19:31 +1300
Subject: [PATCH 16/17] netlogon: Allow zero-GUID to act the same as NULL in
 GetDCNameEx2

This matches Windows behaviour and allows rpcclient to work against Samba without knowing the GUID
ahead of time.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index a55e62a..073739c 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2946,6 +2946,14 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 		return WERR_INVALID_FLAGS;
 	}
 
+	/*
+	 * If we send an all-zero GUID, we should ignore it as winbind actually
+	 * checks it with a DNS query. Windows also appears to ignore it.
+	 */
+	if (r->in.domain_guid != NULL && GUID_all_zero(r->in.domain_guid)) {
+		r->in.domain_guid = NULL;
+	}
+
 	/* Attempt winbind search only if we suspect the domain is incorrect */
 	if (r->in.domain_name != NULL && strcmp("", r->in.domain_name) != 0) {
 		if (r->in.flags & DS_IS_FLAT_NAME) {
-- 
1.9.1


From c2d07ceb473e4e912374474a57f726d701e22453 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 29 Mar 2018 13:47:53 +1300
Subject: [PATCH 17/17] netlogon: Allow netr_DsRGetDCName and
 netr_DsRGetDCNameEx to forward errors

Although it seems elegant to plumb it directly to the GetDCNameEx2, we
don't get the right error code returned.

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/knownfail.d/getdcname                |  3 --
 source4/rpc_server/netlogon/dcerpc_netlogon.c | 50 +++++++++++++++++++++------
 2 files changed, 40 insertions(+), 13 deletions(-)
 delete mode 100644 selftest/knownfail.d/getdcname

diff --git a/selftest/knownfail.d/getdcname b/selftest/knownfail.d/getdcname
deleted file mode 100644
index 917ebb6..0000000
--- a/selftest/knownfail.d/getdcname
+++ /dev/null
@@ -1,3 +0,0 @@
-# Introduced failures due to new winbind handling
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_invalid_site.fl2008r2dc:local.*
-^samba.tests.getdcname.samba.tests.getdcname.GetDCNameEx.test_get_dc_over_winbind_invalid_site.fl2008r2dc:local.*
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 073739c..165654c 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2754,6 +2754,7 @@ struct winbind_forward_state {
 	struct netr_DsRGetDCNameInfo *info;
 	struct netr_DsRGetDCNameEx2 *r;
 	const char *client_site;
+	WERROR *return_code;
 };
 
 /*
@@ -2822,6 +2823,10 @@ static void winbind_forward_GetDCName_done(struct tevent_req *subreq)
 		}
 	}
 
+	if (state->return_code != NULL) {
+		*state->return_code = state->r->out.result;
+	}
+
 	status = dcesrv_reply(state->dce_call);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0,(__location__ ": dcesrv_reply() failed - %s\n",
@@ -2836,7 +2841,8 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 				       uint32_t flags,
 				       const char *client_site,
 				       struct dcesrv_call_state *dce_call,
-				       struct netr_DsRGetDCNameEx2 *r)
+				       struct netr_DsRGetDCNameEx2 *r,
+				       WERROR *return_code)
 {
 	struct tevent_req *subreq = NULL;
 	struct winbind_forward_state *state = NULL;
@@ -2845,6 +2851,7 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 	state->client_site = client_site;
 	state->dce_call = dce_call;
 	state->r = r;
+	state->return_code = return_code;
 
 	subreq = dcerpc_winbind_DsGetDcName_send(mem_ctx,
 						 ev,
@@ -2863,12 +2870,10 @@ static WERROR winbind_forward_GetDCName(TALLOC_CTX *mem_ctx,
 	return WERR_OK;
 }
 
-/*
-  netr_DsRGetDCNameEx2
-*/
-static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
-					  TALLOC_CTX *mem_ctx,
-					  struct netr_DsRGetDCNameEx2 *r)
+static WERROR dcesrv_netr_DsRGetDCNameEx2_internal(struct dcesrv_call_state *dce_call,
+						   TALLOC_CTX *mem_ctx,
+						   struct netr_DsRGetDCNameEx2 *r,
+						   WERROR *return_code)
 {
 	struct ldb_context *sam_ctx;
 	struct netr_DsRGetDCNameInfo *info;
@@ -3039,7 +3044,7 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 		winbind_forward_GetDCName(mem_ctx, dce_call->event_ctx,
 					  irpc_handle, domain_name,
 					  forward_flags, client_site,
-					  dce_call, r);
+					  dce_call, r, return_code);
 
 		return WERR_OK;
 	}
@@ -3133,6 +3138,18 @@ static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
 }
 
 /*
+  netr_DsRGetDCNameEx2
+*/
+static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
+					  TALLOC_CTX *mem_ctx,
+					  struct netr_DsRGetDCNameEx2 *r)
+{
+	return dcesrv_netr_DsRGetDCNameEx2_internal(dce_call,
+						    mem_ctx,
+						    r, NULL);
+}
+
+/*
   netr_DsRGetDCNameEx
 */
 static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
@@ -3152,7 +3169,14 @@ static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TAL
 	r2->in.flags = r->in.flags;
 	r2->out.info = r->out.info;
 
-	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, r2);
+	werr = dcesrv_netr_DsRGetDCNameEx2_internal(dce_call, mem_ctx, r2,
+						    &r->out.result);
+
+	/*
+	 * Handle the non-async case.
+	 * In the async case, this will be clobbered later.
+	 */
+	r->out.result = r2->out.result;
 
 	return werr;
 }
@@ -3182,7 +3206,13 @@ static WERROR dcesrv_netr_DsRGetDCName(struct dcesrv_call_state *dce_call, TALLO
 	r2->in.flags = r->in.flags;
 	r2->out.info = r->out.info;
 
-	werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, r2);
+	werr = dcesrv_netr_DsRGetDCNameEx2_internal(dce_call, mem_ctx, r2,
+						    &r->out.result);
+	/*
+	 * Handle the non-async case.
+	 * In the async case, this will be clobbered later.
+	 */
+	r->out.result = r2->out.result;
 
 	return werr;
 }
-- 
1.9.1



More information about the samba-technical mailing list