[PATCH] Backup/restore only worked if Default-First-Site-Name present
Tim Beale
timbeale at catalyst.net.nz
Thu Sep 20 05:42:25 UTC 2018
The backup/restore tool relied on the Default-First-Site-Name site still
being present. If a domain were provisioned with a different --site
option, or the Default-First-Site-Name was later deleted, then the
restore would fail. This patch-set addresses the problem.
CI link: https://gitlab.com/catalyst-samba/samba/pipelines/30667632
Review appreciated.
Thanks.
-------------- next part --------------
From b4d8118b91712266432d4833830da864cc82a3a3 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 18 Sep 2018 15:24:36 +1200
Subject: [PATCH 1/5] netcmd: Tweak backup-offline output to avoid subunit
truncation
Currently a backup-offline test is occasionally flapping in autobuild,
however, the output is truncated so we can't see what the actual problem
is. The output only ever contains the list of backup dirs. I suspect
that the ']' character printed at the end of the python list might be
getting interpretted by subunit as the end of *all* the output.
If so, we should be able to avoid the problem by printing the list items
without the '['/']'s, i.e. join the list into a single string.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/netcmd/domain_backup.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index bff2bdd..e87debd 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -917,7 +917,7 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
backup_dirs = [paths.private_dir, paths.state_dir,
os.path.dirname(paths.smbconf)] # etc dir
- logger.info('running backup on dirs: {}'.format(backup_dirs))
+ logger.info('running backup on dirs: {}'.format(' '.join(backup_dirs)))
# Recursively get all file paths in the backup directories
all_files = []
--
2.7.4
From afaa38a1fdffa4fa5d9c621b777908108b231555 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 17 Sep 2018 15:36:21 +1200
Subject: [PATCH 2/5] netcmd: Add --site option when restoring a domain
Restoring a backup only worked if the Default-First-Site-Name site was
still present. When the new restored DC account is created, it was
trying to add the new server's DN under CN=Default-First-Site-Name.
However, if the original domain was setup using a different site, then
the restore would fail because the DN didn't exist.
When running the restore command, you should be able to specify the
site that you want the new/restored DC to be in (same as during a
DC 'join'). Passing the correct --site argument is one way to avoid
this problem. (A subsequent patch will further improve the tool so it
can work around non-default sites automatically).
Note we also need to pass the site through to where the new DNS entries
get registered (in the rename case).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/netcmd/domain_backup.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index e87debd..89de40d 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -36,7 +36,7 @@ from samba.netcmd import Option, CommandError
from samba.dcerpc import misc, security
from samba import Ldb
from . fsmo import cmd_fsmo_seize
-from samba.provision import make_smbconf
+from samba.provision import make_smbconf, DEFAULTSITE
from samba.upgradehelpers import update_krbtgt_account_password
from samba.remove_dc import remove_dc
from samba.provision import secretsdb_self_join
@@ -295,6 +295,7 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
help="set IPv4 ipaddress"),
Option("--host-ip6", type="string", metavar="IP6ADDRESS",
help="set IPv6 ipaddress"),
+ Option("--site", help="Site to add the new server in", type=str),
]
takes_optiongroups = {
@@ -303,7 +304,7 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
}
def register_dns_zone(self, logger, samdb, lp, ntdsguid, host_ip,
- host_ip6):
+ host_ip6, site):
'''
Registers the new realm's DNS objects when a renamed domain backup
is restored.
@@ -330,7 +331,7 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
# Add the DNS objects for the new realm (note: the backup clone already
# has the root server objects, so don't add them again)
- fill_dns_data_partitions(samdb, domainsid, names.sitename, domaindn,
+ fill_dns_data_partitions(samdb, domainsid, site, domaindn,
forestdn, dnsdomain, dnsforest, hostname,
host_ip, host_ip6, domainguid, ntdsguid,
dnsadmins_sid, add_root=False)
@@ -361,7 +362,8 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
samdb.transaction_commit()
def run(self, sambaopts=None, credopts=None, backup_file=None,
- targetdir=None, newservername=None, host_ip=None, host_ip6=None):
+ targetdir=None, newservername=None, host_ip=None, host_ip6=None,
+ site=DEFAULTSITE):
if not (backup_file and os.path.exists(backup_file)):
raise CommandError('Backup file not found.')
if targetdir is None:
@@ -413,7 +415,7 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
ncs = [str(r) for r in res[0].get('namingContexts')]
creds = credopts.get_credentials(lp)
- ctx = DCJoinContext(logger, creds=creds, lp=lp,
+ ctx = DCJoinContext(logger, creds=creds, lp=lp, site=site,
forced_local_samdb=samdb,
netbios_name=newservername)
ctx.nc_list = ncs
@@ -451,7 +453,7 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
# know the new DC's IP address)
if is_rename:
self.register_dns_zone(logger, samdb, lp, ctx.ntds_guid,
- host_ip, host_ip6)
+ host_ip, host_ip6, site)
secrets_path = os.path.join(private_dir, 'secrets.ldb')
secrets_ldb = Ldb(secrets_path, session_info=system_session(), lp=lp)
--
2.7.4
From 9fef6bfb688840697376856fec2b6dd6164a0ef2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 18 Sep 2018 17:23:48 +1200
Subject: [PATCH 3/5] tests: Add test-case for restore into non-default site
Add a test-case that exercises the new '--site' restore option and
ensures the restored DC gets added to the correct site.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/tests/domain_backup.py | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index 9699ed0..112167e 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -27,6 +27,7 @@ from samba.auth import system_session
from samba import Ldb, dn_from_dns_name
from samba.netcmd.fsmo import get_fsmo_roleowner
import re
+from samba import sites
def get_prim_dom(secrets_path, lp):
@@ -149,6 +150,32 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
# assert that we don't find user secrets in the DB
self.check_restored_database(lp, expect_secrets=False)
+ def _test_backup_restore_into_site(self):
+ """Does a backup and restores into a non-default site"""
+
+ # create a new non-default site
+ sitename = "Test-Site-For-Backups"
+ sites.create_site(self.ldb, self.ldb.get_config_basedn(), sitename)
+ self.addCleanup(sites.delete_site, self.ldb,
+ self.ldb.get_config_basedn(), sitename)
+
+ # restore the backup DC into the site we just created
+ backup_file = self.create_backup()
+ self.restore_backup(backup_file, ["--site=" + sitename])
+
+ lp = self.check_restored_smbconf()
+ restored_ldb = self.check_restored_database(lp)
+
+ # check the restored DC was added to the site we created, i.e. there's
+ # an entry matching the new DC sitting underneath the site DN
+ site_dn = "CN={},CN=Sites,{}".format(sitename,
+ restored_ldb.get_config_basedn())
+ match_server = "(&(objectClass=server)(cn={}))".format(self.new_server)
+ res = restored_ldb.search(site_dn, scope=ldb.SCOPE_SUBTREE,
+ expression=match_server)
+ self.assertTrue(len(res) == 1,
+ "Failed to find new DC under site")
+
def create_smbconf(self, settings):
"""Creates a very basic smb.conf to pass to the restore tool"""
@@ -372,6 +399,9 @@ class DomainBackupOnline(DomainBackupBase):
def test_backup_restore_no_secrets(self):
self._test_backup_restore_no_secrets()
+ def test_backup_restore_into_site(self):
+ self._test_backup_restore_into_site()
+
class DomainBackupRename(DomainBackupBase):
@@ -400,6 +430,9 @@ class DomainBackupRename(DomainBackupBase):
def test_backup_restore_no_secrets(self):
self._test_backup_restore_no_secrets()
+ def test_backup_restore_into_site(self):
+ self._test_backup_restore_into_site()
+
def test_backup_invalid_args(self):
"""Checks that rename commands with invalid args are rejected"""
@@ -524,3 +557,6 @@ class DomainBackupOffline(DomainBackupBase):
def test_backup_restore(self):
self._test_backup_restore()
+
+ def test_backup_restore_into_site(self):
+ self._test_backup_restore_into_site()
--
2.7.4
From 1a175bd52ad5f1a9953b611bd41ea5db6dee4efa Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 18 Sep 2018 14:54:51 +1200
Subject: [PATCH 4/5] netcmd: Re-create default site for backup-restore (if
missing)
Normally when a new DC joins a domain, samba-tool works out the new
DC's site automatically. However, it does this by querying the existing
DC using CLDAP. In the restore case, there is no DC running. We could
still query the DB on disk and work out the correct site based on the
new DC's IP, however:
- comparing between the CN=Subnet DNs and an IP-address string seems
like it'd be non-trivial to write, and
- in the lab-domain rename case, chances are the user will want a
completely different subnet to what's already in the DB.
The restore command now has a --site option so the user can specify an
appropriate site for the restored DC. This patch makes the restore
command work by default (i.e. without a --site option) even if the
default Default-First-Site-Name doesn't exist. Basically the solution is
to just check Default-First-Site-Name exists and create it if it
doesn't. As the recommended workflow is to use the restored DC as a
temporary seed that you'll later throw away, this approach seems
acceptable. Subsequent DCs will then be joined to the running restored
DC, so an appropriate site will be determined using CLDAP. The only
side-effect is potentially an extra Site object.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/netcmd/domain_backup.py | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index 89de40d..c0b3865 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -51,6 +51,7 @@ from samba.mdb_util import mdb_copy
import errno
import tdb
from subprocess import CalledProcessError
+from samba import sites
# work out a SID (based on a free RID) to use when the domain gets restored.
@@ -361,9 +362,23 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
chk.check_database(controls=controls, attrs=attrs)
samdb.transaction_commit()
+ def create_default_site(self, samdb, logger):
+ '''Creates the default site, if it doesn't already exist'''
+
+ sitename = DEFAULTSITE
+ search_expr = "(&(cn={})(objectclass=site))".format(sitename)
+ res = samdb.search(samdb.get_config_basedn(), scope=ldb.SCOPE_SUBTREE,
+ expression=search_expr)
+
+ if len(res) == 0:
+ logger.info("Creating default site '{}'".format(sitename))
+ sites.create_site(samdb, samdb.get_config_basedn(), sitename)
+
+ return sitename
+
def run(self, sambaopts=None, credopts=None, backup_file=None,
targetdir=None, newservername=None, host_ip=None, host_ip6=None,
- site=DEFAULTSITE):
+ site=None):
if not (backup_file and os.path.exists(backup_file)):
raise CommandError('Backup file not found.')
if targetdir is None:
@@ -407,6 +422,13 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
samdb_path = os.path.join(private_dir, 'sam.ldb')
samdb = SamDB(url=samdb_path, session_info=system_session(), lp=lp)
+ if site is None:
+ # There's no great way to work out the correct site to add the
+ # restored DC to. By default, add it to Default-First-Site-Name,
+ # creating the site if it doesn't already exist
+ site = self.create_default_site(samdb, logger)
+ logger.info("Adding new DC to site '{}'".format(site))
+
# Create account using the join_add_objects function in the join object
# We need namingContexts, account control flags, and the sid saved by
# the backup process.
--
2.7.4
From ea063e78313ceb89922cdd7f1543949fc8e851d7 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 18 Sep 2018 16:30:15 +1200
Subject: [PATCH 5/5] selftest: Change backup testenvs to use non-default site
Previously (i.e. up until the last patch) the backup/restore commands
only worked if the Default-First-Site-Name site was present. If this
site didn't exist, then the various restore testenvs would fail to
start. This is now fixed, but this patch changes the backupfrom testenv
so that it uses a non-default site. This will detect the problem if it
is ever re-introduced.
To do this we need to change provision_ad_dc() so the
extra_provision_options can be specified as an argument. (Note that Perl
treats undef the same as an empty array).
By default, the restore will add the new DC into the
Default-First-Site-Name site. This means the backupfromdc and restored
testenvs will now have different sites, so we need to update the ldapcmp
filters to exclude site-specific attributes.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/target/Samba4.pm | 16 ++++++++++------
testprogs/blackbox/ldapcmp_restoredc.sh | 3 +++
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 68038fb..12d7469 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1861,7 +1861,8 @@ sub read_config_h($)
sub provision_ad_dc($$$$$$)
{
- my ($self, $prefix, $hostname, $domain, $realm, $smbconf_args) = @_;
+ my ($self, $prefix, $hostname, $domain, $realm, $smbconf_args,
+ $extra_provision_options) = @_;
my $prefix_abs = abs_path($prefix);
@@ -1972,7 +1973,6 @@ sub provision_ad_dc($$$$$$)
copy = print1
";
- my $extra_provision_options = undef;
push (@{$extra_provision_options}, "--backend-store=mdb");
print "PROVISIONING AD DC...\n";
my $ret = $self->provision($prefix,
@@ -2538,7 +2538,7 @@ sub setup_ad_dc
}
my $env = $self->provision_ad_dc($path, "addc", "ADDOMAIN",
- "addom.samba.example.com", "");
+ "addom.samba.example.com", "", undef);
unless ($env) {
return undef;
}
@@ -2565,7 +2565,7 @@ sub setup_ad_dc_no_nss
}
my $env = $self->provision_ad_dc($path, "addc_no_nss", "ADNONSSDOMAIN",
- "adnonssdom.samba.example.com", "");
+ "adnonssdom.samba.example.com", "", undef);
unless ($env) {
return undef;
}
@@ -2596,7 +2596,7 @@ sub setup_ad_dc_no_ntlm
my $env = $self->provision_ad_dc($path, "addc_no_ntlm", "ADNONTLMDOMAIN",
"adnontlmdom.samba.example.com",
- "ntlm auth = disabled");
+ "ntlm auth = disabled", undef);
unless ($env) {
return undef;
}
@@ -2627,8 +2627,12 @@ sub setup_backupfromdc
return "UNKNOWN";
}
+ my $provision_args = undef;
+ push (@{$provision_args}, "--site=Backup-Site");
+
my $env = $self->provision_ad_dc($path, "backupfromdc", "BACKUPDOMAIN",
- "backupdom.samba.example.com", "");
+ "backupdom.samba.example.com", "",
+ $provision_args);
unless ($env) {
return undef;
}
diff --git a/testprogs/blackbox/ldapcmp_restoredc.sh b/testprogs/blackbox/ldapcmp_restoredc.sh
index 51951ba..d7a51ae 100755
--- a/testprogs/blackbox/ldapcmp_restoredc.sh
+++ b/testprogs/blackbox/ldapcmp_restoredc.sh
@@ -55,6 +55,9 @@ ldapcmp_with_orig() {
# these are just differences between provisioning a domain and joining a DC
IGNORE_ATTRS="$IGNORE_ATTRS,localPolicyFlags,operatingSystem,displayName"
+ # the restored DC may use a different side compared to the original DC
+ IGNORE_ATTRS="$IGNORE_ATTRS,serverReferenceBL,msDS-IsDomainFor"
+
LDAPCMP_CMD="$PYTHON $BINDIR/samba-tool ldapcmp"
$LDAPCMP_CMD $DB1_PATH $DB2_PATH --two --filter=$IGNORE_ATTRS $BASE_DN_OPTS
}
--
2.7.4
More information about the samba-technical
mailing list