[PATCH] Domain join with '--server' option didn't determine site correctly

Tim Beale timbeale at catalyst.net.nz
Thu Sep 20 05:58:18 UTC 2018

This patch-set is dependent on the previous patches I sent out, although
it addresses a slightly different problem. The domain join code hits a
similar problem to backup/restore if the Default-First-Site-Name site is

If the '--server' option is specified in the join command, then the
appropriate site for the new DC isn't determined automatically. It just
always adds the DC to Default-First-Site-Name. If
Default-First-Site-Name doesn't exist, then the join fails with an
exception. The work-around would be to run the domain join without the
--server option, which works fine.

CI link: https://gitlab.com/catalyst-samba/samba/pipelines/30668509

Review appreciated.


-------------- next part --------------
From e31ad85797b8c32c478e7ffa5a21354a99dc3dcc Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 20 Sep 2018 13:08:50 +1200
Subject: [PATCH 1/3] selftest: Test join against DC with non-default site

Recent changes around restoring a domain that lacked
Default-First-Site-Name highlighted a problem. Normally when you join a
DC to a domain, samba-tool works out the correct site to use
automatically. However, if the join uses '--server' to select a DC, then
this doesn't work. It defaults back to Default-First-Site-Name, and the
join command fails if this site doesn't exist.

All the testenvs had Default-First-Site-Name present, so this was never
tested. Now the backupfromdc no longer has a Default-First-Site-Name
site, so running a simple join against that DC fails, highlighting the

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
 selftest/knownfail.d/join_ldapcmp | 5 +++++
 source4/selftest/tests.py         | 5 +++++
 2 files changed, 10 insertions(+)
 create mode 100644 selftest/knownfail.d/join_ldapcmp

diff --git a/selftest/knownfail.d/join_ldapcmp b/selftest/knownfail.d/join_ldapcmp
new file mode 100644
index 0000000..e8404cf
--- /dev/null
+++ b/selftest/knownfail.d/join_ldapcmp
@@ -0,0 +1,5 @@
+# 'samba-tool domain join --server' fails if the domain does not contain the
+# Default-First-Site-Name site
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 1803752..54f1d25 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -849,6 +849,11 @@ for env in ['offlinebackupdc', 'restoredc', 'renamedc', 'labdc']:
                   ["PYTHON=%s" % python,
                    os.path.join(bbdir, "ldapcmp_restoredc.sh"),
                    '$PREFIX_ABS/backupfromdc', '$PREFIX_ABS/%s' % env])
+# we also test joining backupfromdc here, as it's a bit special in that it
+# doesn't have Default-First-Site-Name
+for env in ['backupfromdc', 'offlinebackupdc', 'restoredc', 'renamedc',
+	    'labdc']:
     # basic test that we can join the testenv DC
     plantestsuite("samba4.blackbox.join_ldapcmp", env,
                   ["PYTHON=%s" % python, os.path.join(bbdir, "join_ldapcmp.sh")])

From d0536cadac9b623ab787b98b9947f67d49aa0f96 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 19 Sep 2018 10:21:12 +1200
Subject: [PATCH 2/3] join: Avoid duplicating "Default-First-Site-Name" string

The provision code already defines "Default-First-Site-Name" so we might
as well reuse it.

The join.py already uses a suitable default, so assigning the default in
the domain netcmd code is unnecessary.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
 python/samba/join.py          | 5 +++--
 python/samba/netcmd/domain.py | 3 ---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/python/samba/join.py b/python/samba/join.py
index 38a1545..3114113 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -30,7 +30,8 @@ from samba.ndr import ndr_pack, ndr_unpack
 from samba.dcerpc import security, drsuapi, misc, nbt, lsa, drsblobs, dnsserver, dnsp
 from samba.dsdb import DS_DOMAIN_FUNCTION_2003
 from samba.credentials import Credentials, DONT_USE_KERBEROS
-from samba.provision import secretsdb_self_join, provision, provision_fill, FILL_DRS, FILL_SUBDOMAIN
+from samba.provision import (secretsdb_self_join, provision, provision_fill,
+                             FILL_DRS, FILL_SUBDOMAIN, DEFAULTSITE)
 from samba.provision.common import setup_path
 from samba.schema import Schema
 from samba import descriptor
@@ -68,7 +69,7 @@ class DCJoinContext(object):
                  promote_existing=False, plaintext_secrets=False,
                  backend_store=None, forced_local_samdb=None):
         if site is None:
-            site = "Default-First-Site-Name"
+            site = DEFAULTSITE
         ctx.logger = logger
         ctx.creds = creds
diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
index e90266d..ce4f36a 100644
--- a/python/samba/netcmd/domain.py
+++ b/python/samba/netcmd/domain.py
@@ -688,9 +688,6 @@ class cmd_domain_join(Command):
         creds = credopts.get_credentials(lp)
         net = Net(creds, lp, server=credopts.ipaddress)
-        if site is None:
-            site = "Default-First-Site-Name"
         logger = self.get_logger()
         if verbose:

From 4ea2663273961499679dff2199967f81191de760 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 19 Sep 2018 10:44:48 +1200
Subject: [PATCH 3/3] join: Support site detection when --server is specified

When a new DC is joined to the domain, samba-tool would automatically
detect an appropriate site for the new DC. However, it only did this if
the --server option wasn't specified. The new DC's site got
automatically updated as part of the finddc() work, however, this step
gets skipped if we already know the server DC to join to.

In other words, if Default-First-Site-Name doesn't exist and you specify
--server in the join, then you have to also specify --site manually,
otherwise the command fails. This is precisely what's happening in the
join_ldapcmp.sh test, now that the backupfromdc testenv no longer has the
Default-First-Site-Name present.

This patch adds a new find_dc_site() function which uses the same
net.finddc() API (except based on the server-address rather than
domain-name). Assigning DEFAULTSITE has been moved so that it only
gets done if finddc() can't determine the site.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
 python/samba/join.py              | 21 ++++++++++++++++++---
 selftest/knownfail.d/join_ldapcmp |  5 -----
 2 files changed, 18 insertions(+), 8 deletions(-)
 delete mode 100644 selftest/knownfail.d/join_ldapcmp

diff --git a/python/samba/join.py b/python/samba/join.py
index 3114113..3869947 100644
--- a/python/samba/join.py
+++ b/python/samba/join.py
@@ -68,8 +68,6 @@ class DCJoinContext(object):
                  machinepass=None, use_ntvfs=False, dns_backend=None,
                  promote_existing=False, plaintext_secrets=False,
                  backend_store=None, forced_local_samdb=None):
-        if site is None:
-            site = DEFAULTSITE
         ctx.logger = logger
         ctx.creds = creds
@@ -96,7 +94,13 @@ class DCJoinContext(object):
             ctx.samdb = forced_local_samdb
             ctx.server = ctx.samdb.url
-            if not ctx.server:
+            if ctx.server:
+                # work out the DC's site (if not already specified)
+                if site is None:
+                    ctx.site = ctx.find_dc_site(ctx.server)
+            else:
+                # work out the Primary DC for the domain (as well as an
+                # appropriate site for the new DC)
                 ctx.logger.info("Finding a writeable DC for domain '%s'" % domain)
                 ctx.server = ctx.find_dc(domain)
                 ctx.logger.info("Found DC %s" % ctx.server)
@@ -104,6 +108,9 @@ class DCJoinContext(object):
                               credentials=ctx.creds, lp=ctx.lp)
+        if ctx.site is None:
+            ctx.site = DEFAULTSITE
             ctx.samdb.search(scope=ldb.SCOPE_ONELEVEL, attrs=["dn"])
         except ldb.LdbError as e4:
@@ -348,6 +355,14 @@ class DCJoinContext(object):
             ctx.site = ctx.cldap_ret.client_site
         return ctx.cldap_ret.pdc_dns_name
+    def find_dc_site(ctx, server):
+        site = None
+        cldap_ret = ctx.net.finddc(address=server,
+                                   flags=nbt.NBT_SERVER_LDAP | nbt.NBT_SERVER_DS)
+        if cldap_ret.client_site is not None and cldap_ret.client_site != "":
+            site = cldap_ret.client_site
+        return site
     def get_behavior_version(ctx):
         res = ctx.samdb.search(base=ctx.base_dn, scope=ldb.SCOPE_BASE, attrs=["msDS-Behavior-Version"])
         if "msDS-Behavior-Version" in res[0]:
diff --git a/selftest/knownfail.d/join_ldapcmp b/selftest/knownfail.d/join_ldapcmp
deleted file mode 100644
index e8404cf..0000000
--- a/selftest/knownfail.d/join_ldapcmp
+++ /dev/null
@@ -1,5 +0,0 @@
-# 'samba-tool domain join --server' fails if the domain does not contain the
-# Default-First-Site-Name site

More information about the samba-technical mailing list