[PATCH] Allow GetDCNameEx to be called for arbitrary sites and trusted domains
Garming Sam
garming at catalyst.net.nz
Tue Apr 3 02:18:47 UTC 2018
Hi metze,
I've changed the patches to follow the calling format you've suggested
(and squashed some patches in the process). Hopefully it's much closer
to what you expect.
Cheers,
Garming
On 29/03/18 20:43, Stefan Metzmacher wrote:
> Hi Garming,
>
> can you please have a look at
> https://git.samba.org/?p=samba.git;a=commitdiff;h=373da95b0b72ec7db8463a
> https://git.samba.org/?p=samba.git;a=commitdiff;h=023bd2d15d5e9599c59281
> https://git.samba.org/?p=samba.git;a=commitdiff;h=8022b63f6cfadc58d6711e
>
> We use that pattern for a few async calls in already.
> dcesrv_netr_LogonSamLogon_base_call(), dcesrv_netr_LogonControl_base_call(),
> dcesrv_lsa_LookupSids_base_call(),
> dcesrv_lsa_LookupNames_base_call()
>
> I think we should use that pattern also for GetDCName* too.
>
> Also wb_irpc_GetDCName_done() should be moved after wb_irpc_GetDCName(),
> that makes it much easier to read and matches the pattern we always use
> for async programming, e.g.
>
> subreq = wb_lookupsids_send(msg,
> server_event_context(),
> sids, req->in.sids->num_sids);
> if (subreq == NULL) {
> return NT_STATUS_NO_MEMORY;
> }
> tevent_req_set_callback(subreq, wb_irpc_lsa_LookupSids3_done,
> state);
> msg->defer_reply = true;
>
> return NT_STATUS_OK;
> }
>
> static void wb_irpc_lsa_LookupSids3_done(struct tevent_req *subreq)
> {
> struct wb_irpc_lsa_LookupSids3_state *state =
> tevent_req_callback_data(subreq,
> struct wb_irpc_lsa_LookupSids3_state);
> struct lsa_RefDomainList *domains = NULL;
> struct lsa_TransNameArray *names = NULL;
> NTSTATUS status;
> uint32_t i;
>
> status = wb_lookupsids_recv(subreq, state->msg,
> &domains, &names);
>
>
> I also think we should have a bug report for that and backport it to
> 4.8.
>
> It seems that some of the patches fix bugs in a former patch,
> these should be squashed to make it easier to understand.
>
> Thanks!
> metze
>
> Am 29.03.2018 um 05:04 schrieb Garming Sam via samba-technical:
>> New version. Fixed some error handling.
>>
>>
>> On 29/03/18 15:29, Garming Sam via samba-technical wrote:
>>> 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/10] 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 63024b5e37d1fce4f8fdeeb362b49bc5e5852dea 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/10] 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.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
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 a998a8f6f22d0fc6ee6b29eb65b2bd295631ebe8 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/10] 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.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
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 87864a4f55d3a5ae395b08d90ada8897b3c94fe8 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/10] netlogon: Allow return of error code in future
asynchronous winbind forwards
We change the naming conventions to match dcesrv_netr_*_base_call used elsewhere.
This is important when we make the underlying Ex2 call asynchronous.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source4/rpc_server/netlogon/dcerpc_netlogon.c | 108 ++++++++++++++++++--------
1 file changed, 76 insertions(+), 32 deletions(-)
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 6420d57..f25a3aa 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2749,14 +2749,25 @@ static NTSTATUS dcesrv_netr_NetrLogonSendToSam(struct dcesrv_call_state *dce_cal
return NT_STATUS_OK;
}
+struct dcesrv_netr_DsRGetDCName_base_state {
+ struct dcesrv_call_state *dce_call;
+ TALLOC_CTX *mem_ctx;
-/*
- netr_DsRGetDCNameEx2
-*/
-static WERROR dcesrv_netr_DsRGetDCNameEx2(struct dcesrv_call_state *dce_call,
- TALLOC_CTX *mem_ctx,
- struct netr_DsRGetDCNameEx2 *r)
+ struct netr_DsRGetDCNameEx2 r;
+
+ struct {
+ struct netr_DsRGetDCName *dc;
+ struct netr_DsRGetDCNameEx *dcex;
+ struct netr_DsRGetDCNameEx2 *dcex2;
+ } _r;
+};
+
+static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName_base_state *state)
{
+ struct dcesrv_call_state *dce_call = state->dce_call;
+ TALLOC_CTX *mem_ctx = state->mem_ctx;
+ struct netr_DsRGetDCNameEx2 *r = &state->r;
+
struct ldb_context *sam_ctx;
struct netr_DsRGetDCNameInfo *info;
struct loadparm_context *lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
@@ -2928,28 +2939,56 @@ 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)
+{
+ struct dcesrv_netr_DsRGetDCName_base_state *state;
+
+ state = talloc_zero(mem_ctx, struct dcesrv_netr_DsRGetDCName_base_state);
+ if (state == NULL) {
+ return WERR_NOT_ENOUGH_MEMORY;
+ }
+
+ state->dce_call = dce_call;
+ state->mem_ctx = mem_ctx;
+
+ state->r = *r;
+ state->_r.dcex2 = r;
+
+ return dcesrv_netr_DsRGetDCName_base_call(state);
+}
+
+/*
netr_DsRGetDCNameEx
*/
static WERROR dcesrv_netr_DsRGetDCNameEx(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
struct netr_DsRGetDCNameEx *r)
{
- struct netr_DsRGetDCNameEx2 r2;
- WERROR werr;
+ struct dcesrv_netr_DsRGetDCName_base_state *state;
- ZERO_STRUCT(r2);
+ state = talloc_zero(mem_ctx, struct dcesrv_netr_DsRGetDCName_base_state);
+ if (state == NULL) {
+ return WERR_NOT_ENOUGH_MEMORY;
+ }
- 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;
+ state->dce_call = dce_call;
+ state->mem_ctx = mem_ctx;
- werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, &r2);
+ state->r.in.server_unc = r->in.server_unc;
+ state->r.in.client_account = NULL;
+ state->r.in.mask = 0;
+ state->r.in.domain_guid = r->in.domain_guid;
+ state->r.in.domain_name = r->in.domain_name;
+ state->r.in.site_name = r->in.site_name;
+ state->r.in.flags = r->in.flags;
+ state->r.out.info = r->out.info;
- return werr;
+ state->_r.dcex = r;
+
+ return dcesrv_netr_DsRGetDCName_base_call(state);
}
/*
@@ -2962,24 +3001,29 @@ 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;
- WERROR werr;
+ struct dcesrv_netr_DsRGetDCName_base_state *state;
- ZERO_STRUCT(r2);
+ state = talloc_zero(mem_ctx, struct dcesrv_netr_DsRGetDCName_base_state);
+ if (state == NULL) {
+ return WERR_NOT_ENOUGH_MEMORY;
+ }
+
+ state->dce_call = dce_call;
+ state->mem_ctx = mem_ctx;
- 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;
+ state->r.in.server_unc = r->in.server_unc;
+ state->r.in.client_account = NULL;
+ state->r.in.mask = 0;
+ state->r.in.domain_name = r->in.domain_name;
+ state->r.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;
+ state->r.in.site_name = NULL; /* this is correct, we should ignore site GUID */
+ state->r.in.flags = r->in.flags;
+ state->r.out.info = r->out.info;
- werr = dcesrv_netr_DsRGetDCNameEx2(dce_call, mem_ctx, &r2);
+ state->_r.dc = r;
- return werr;
+ return dcesrv_netr_DsRGetDCName_base_call(state);
}
/*
netr_NETRLOGONGETTIMESERVICEPARENTDOMAIN
--
1.9.1
From 008a560125bb55d4a7bc704335781c90657f815b 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/10] 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.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
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 c45d1c7be09428f695fd3483cde5e484d4bc456a 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/10] 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).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source3/winbindd/winbindd_irpc.c | 67 ++++++++++++++++++++++++++++++++++++++++
source4/librpc/idl/irpc.idl | 11 +++++++
2 files changed, 78 insertions(+)
diff --git a/source3/winbindd/winbindd_irpc.c b/source3/winbindd/winbindd_irpc.c
index e03312e..211fe13 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,65 @@ static void wb_irpc_lsa_LookupNames4_domains_done(struct tevent_req *subreq)
return;
}
+static void wb_irpc_GetDCName_done(struct tevent_req *subreq);
+
+struct irpc_GetDCName_state {
+ struct irpc_message *msg;
+ struct netr_DsRGetDCNameInfo **dcinfo;
+};
+
+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);
+ if (state == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ 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;
+}
+
+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);
+}
+
NTSTATUS wb_irpc_register(void)
{
NTSTATUS status;
@@ -769,6 +830,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 e98b8beb40973f0e6ce3150eae66bb7915f3f0bc 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/10] 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.
In calling out to winbind, we now also notice if you provide a site
which exists in multiple domains and provide the correct domain (instead
of accidentally returning ourselves).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/flapping.d/getdcname | 2 +
selftest/knownfail.d/getdcname | 8 --
source4/rpc_server/netlogon/dcerpc_netlogon.c | 152 +++++++++++++++++++++++++-
3 files changed, 151 insertions(+), 11 deletions(-)
create mode 100644 selftest/flapping.d/getdcname
delete mode 100644 selftest/knownfail.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
deleted file mode 100644
index 1c086a3..0000000
--- a/selftest/knownfail.d/getdcname
+++ /dev/null
@@ -1,8 +0,0 @@
-^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/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index f25a3aa..fff9303 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2762,6 +2762,33 @@ struct dcesrv_netr_DsRGetDCName_base_state {
} _r;
};
+static void dcesrv_netr_DsRGetDCName_base_done(struct tevent_req *subreq);
+
+static WERROR winbind_forward_GetDCName(struct dcesrv_netr_DsRGetDCName_base_state *state,
+ struct tevent_context *ev,
+ struct dcerpc_binding_handle *h,
+ const char *domain_name,
+ uint32_t flags)
+{
+ struct tevent_req *subreq = NULL;
+
+ subreq = dcerpc_winbind_DsGetDcName_send(state->mem_ctx,
+ ev,
+ h,
+ domain_name,
+ state->r.in.domain_guid,
+ state->r.in.site_name,
+ flags,
+ state->r.out.info);
+ if (subreq == NULL) {
+ return WERR_NOT_ENOUGH_MEMORY;
+ }
+
+ tevent_req_set_callback(subreq, dcesrv_netr_DsRGetDCName_base_done, state);
+
+ return WERR_OK;
+}
+
static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName_base_state *state)
{
struct dcesrv_call_state *dce_call = state->dce_call;
@@ -2782,6 +2809,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
const char *dc_name = NULL;
const char *domain_name = NULL;
const char *pdc_ip;
+ bool different_domain = true;
ZERO_STRUCTP(r->out.info);
@@ -2842,12 +2870,61 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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)) {
- return WERR_NO_SUCH_DOMAIN;
+ 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;
+
+ 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;
+
+ return winbind_forward_GetDCName(state, dce_call->event_ctx,
+ irpc_handle, r->in.domain_name,
+ r->in.flags);
}
guid_str = r->in.domain_guid != NULL ?
@@ -2938,6 +3015,75 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
return WERR_OK;
}
+static void dcesrv_netr_DsRGetDCName_base_done(struct tevent_req *subreq)
+{
+ struct dcesrv_netr_DsRGetDCName_base_state *state =
+ tevent_req_callback_data(subreq,
+ struct dcesrv_netr_DsRGetDCName_base_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)) {
+ 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
+ * 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 (state->r.in.site_name == NULL ||
+ strcasecmp_m("", state->r.in.site_name) == 0 ||
+ strcasecmp_m(state->r.out.info[0]->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->r.out.info[0]->dc_unc) > 2 &&
+ strncmp("\\\\", state->r.out.info[0]->dc_unc, 2) != 0) {
+ const char *dc_unc = NULL;
+
+ dc_unc = talloc_asprintf(state->dce_call,
+ "\\\\%s",
+ state->r.out.info[0]->dc_unc);
+ state->r.out.info[0]->dc_unc = dc_unc;
+ }
+
+ state->r.out.result = ntstatus_to_werror(result);
+ } else {
+ state->r.out.result = WERR_NO_SUCH_DOMAIN;
+ }
+ }
+
+ if (state->_r.dcex2 != NULL) {
+ struct netr_DsRGetDCNameEx2 *r = state->_r.dcex2;
+ r->out.result = state->r.out.result;
+ } else if (state->_r.dcex != NULL) {
+ struct netr_DsRGetDCNameEx *r = state->_r.dcex;
+ r->out.result = state->r.out.result;
+ } else if (state->_r.dc != NULL) {
+ struct netr_DsRGetDCName *r = state->_r.dc;
+ r->out.result = state->r.out.result;
+ }
+
+ status = dcesrv_reply(state->dce_call);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0,(__location__ ": dcesrv_reply() failed - %s\n",
+ nt_errstr(status)));
+ }
+}
+
/*
netr_DsRGetDCNameEx2
*/
--
1.9.1
From 0c32746d40e23ff855eb429cc1a974886fa72e81 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 08/10] 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.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source4/rpc_server/netlogon/dcerpc_netlogon.c | 30 +++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index fff9303..d6d48cb 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2810,6 +2810,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
const char *domain_name = NULL;
const char *pdc_ip;
bool different_domain = true;
+ bool forward_with_dns_name = false;
ZERO_STRUCTP(r->out.info);
@@ -2876,6 +2877,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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,
@@ -2888,6 +2890,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
strcasecmp_m(r->in.domain_name,
lpcfg_dnsdomain(lp_ctx)) == 0) {
different_domain = false;
+ forward_with_dns_name = true;
}
}
} else {
@@ -2896,6 +2899,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
* revert to our domain by default.
*/
different_domain = false;
+ forward_with_dns_name = true;
}
/* Proof server site parameter "site_name" if it was specified */
@@ -2906,6 +2910,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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,
@@ -2922,9 +2927,18 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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);
+ }
+
return winbind_forward_GetDCName(state, dce_call->event_ctx,
- irpc_handle, r->in.domain_name,
- r->in.flags);
+ irpc_handle, domain_name,
+ forward_flags);
}
guid_str = r->in.domain_guid != NULL ?
@@ -3015,6 +3029,12 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
return WERR_OK;
}
+/*
+ * 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 dcesrv_netr_DsRGetDCName_base_done(struct tevent_req *subreq)
{
struct dcesrv_netr_DsRGetDCName_base_state *state =
@@ -3040,6 +3060,12 @@ static void dcesrv_netr_DsRGetDCName_base_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 ||
--
1.9.1
From c897a7d0f827a5aec49df0df0efae71ecf985924 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 09/10] netlogon: Store the client site to clobber any
plausibly returned via winbind
So far, I have never observed the case where the winbind call ever
bothered to return a proper site, but in case it ever does so, we
clobber it here. This has implications for returning a non-local domain
site name, but for now, we ignore them.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source4/rpc_server/netlogon/dcerpc_netlogon.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index d6d48cb..af28f26 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2754,6 +2754,7 @@ struct dcesrv_netr_DsRGetDCName_base_state {
TALLOC_CTX *mem_ctx;
struct netr_DsRGetDCNameEx2 r;
+ const char *client_site;
struct {
struct netr_DsRGetDCName *dc;
@@ -2911,6 +2912,7 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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,
@@ -2936,6 +2938,23 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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.
+ *
+ * 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,
+ remote_addr,
+ NULL,
+ false);
+ state->client_site = client_site;
+
return winbind_forward_GetDCName(state, dce_call->event_ctx,
irpc_handle, domain_name,
forward_flags);
@@ -3086,6 +3105,8 @@ static void dcesrv_netr_DsRGetDCName_base_done(struct tevent_req *subreq)
state->r.out.info[0]->dc_unc = dc_unc;
}
+ state->r.out.info[0]->client_site_name = state->client_site;
+
state->r.out.result = ntstatus_to_werror(result);
} else {
state->r.out.result = WERR_NO_SUCH_DOMAIN;
--
1.9.1
From b10e005f7f8495fe5ecd3fd555e6c3d1aa940630 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 10/10] 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. Errors related to this
don't appear to occur within selftest.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13365
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 af28f26..4b2ec64 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -2872,6 +2872,14 @@ static WERROR dcesrv_netr_DsRGetDCName_base_call(struct dcesrv_netr_DsRGetDCName
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180403/f5d10be0/signature.sig>
More information about the samba-technical
mailing list