[PATCH] Add '--no-secrets' option to backup tool (for creating a lab domain)

Tim Beale timbeale at catalyst.net.nz
Sun Jul 8 22:06:50 UTC 2018


Hi,

Attached patches are the same as earlier, with an extra patch (#4) that
rejects rename commands that aren't actual renames (i.e. erroneously
using the same NetBIOS name or realm as the current domain).
New CI link: https://gitlab.com/catalyst-samba/samba/pipelines/25356700

Review appreciated.

Thanks,
Tim


On 06/07/18 16:55, Tim Beale via samba-technical wrote:
> Hi,
>
> The attached patches extend the backup tool to better support creating a
> lab domain, i.e. a pre-production test-bed where users can try out
> various things without impacting their production domain. Mostly this
> just involves adding a '--no-secrets' option to the backup commands.
>
> CI link: https://gitlab.com/catalyst-samba/samba/pipelines/25238942
>
> Thanks,
> Tim
>

-------------- next part --------------
From 74716f5d1ca44d88d9ab154e32fa3fbbd52b8953 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 5 Jul 2018 14:33:22 +1200
Subject: [PATCH 1/4] netcmd: Add no-secrets option to domain backups

By default we include all the domain's secrets in the backup file. This
patch adds an extra option to exclude these secrets. In particular, this
is for the use case of creating a lab domain (where you might not feel
comfortable with the secrets for all your users being present).

Mostly this just involves passing the correct option to the join/clone.
I've also made sure that a password is also set for the Admin user
(samba does seem to start up without one set, but this behaviour is
closer to what happens during a provision).

The tests have been extended to use the new option, and to assert that
secrets are/aren't included as expected for some of the builtin testenv
users.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/netcmd/domain_backup.py | 42 +++++++++++++++++++++++++++----
 python/samba/tests/domain_backup.py  | 49 +++++++++++++++++++++++++++++++++---
 2 files changed, 82 insertions(+), 9 deletions(-)

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index c156046..a4b2765 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -135,6 +135,21 @@ def check_online_backup_args(logger, credopts, server, targetdir):
         os.makedirs(targetdir)
 
 
+# For '--no-secrets' backups, this sets the Administrator user's password to a
+# randomly-generated value. This is similar to the provision behaviour
+def set_admin_password(logger, samdb, username):
+    """Sets a randomly generated password for the backup DB's admin user"""
+
+    adminpass = samba.generate_random_password(12, 32)
+    logger.info("Setting %s password in backup to: %s" % (username, adminpass))
+    logger.info("Run 'samba-tool user setpassword %s' after restoring DB" %
+                username)
+    samdb.setpassword("(&(objectClass=user)(sAMAccountName=%s))"
+                      % ldb.binary_encode(username), adminpass,
+                      force_change_at_next_login=False,
+                      username=username)
+
+
 class cmd_domain_backup_online(samba.netcmd.Command):
     '''Copy a running DC's current DB into a backup tar file.
 
@@ -161,9 +176,12 @@ class cmd_domain_backup_online(samba.netcmd.Command):
         Option("--server", help="The DC to backup", type=str),
         Option("--targetdir", type=str,
                help="Directory to write the backup file to"),
+        Option("--no-secrets", action="store_true", default=False,
+               help="Exclude secret values from the backup created")
        ]
 
-    def run(self, sambaopts=None, credopts=None, server=None, targetdir=None):
+    def run(self, sambaopts=None, credopts=None, server=None, targetdir=None,
+            no_secrets=False):
         logger = self.get_logger()
         logger.setLevel(logging.DEBUG)
 
@@ -180,9 +198,10 @@ class cmd_domain_backup_online(samba.netcmd.Command):
         tmpdir = tempfile.mkdtemp(dir=targetdir)
 
         # Run a clone join on the remote
+        include_secrets = not no_secrets
         ctx = join_clone(logger=logger, creds=creds, lp=lp,
-                         include_secrets=True, dns_backend='SAMBA_INTERNAL',
-                         server=server, targetdir=tmpdir)
+                         include_secrets=include_secrets, server=server,
+                         dns_backend='SAMBA_INTERNAL', targetdir=tmpdir)
 
         # get the paths used for the clone, then drop the old samdb connection
         paths = ctx.paths
@@ -209,6 +228,10 @@ class cmd_domain_backup_online(samba.netcmd.Command):
         add_backup_marker(samdb, "backupDate", time_str)
         add_backup_marker(samdb, "sidForRestore", new_sid)
 
+        # ensure the admin user always has a password set (same as provision)
+        if no_secrets:
+            set_admin_password(logger, samdb, creds.get_username())
+
         # Add everything in the tmpdir to the backup tar file
         backup_file = backup_filepath(targetdir, realm, time_str)
         create_backup_tar(logger, tmpdir, backup_file)
@@ -530,6 +553,8 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
                type=str),
         Option("--keep-dns-realm", action="store_true", default=False,
                help="Retain the DNS entries for the old realm in the backup"),
+        Option("--no-secrets", action="store_true", default=False,
+               help="Exclude secret values from the backup created")
        ]
 
     takes_args = ["new_domain_name", "new_dns_realm"]
@@ -626,7 +651,8 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
         samdb.transaction_commit()
 
     def run(self, new_domain_name, new_dns_realm, sambaopts=None,
-            credopts=None, server=None, targetdir=None, keep_dns_realm=False):
+            credopts=None, server=None, targetdir=None, keep_dns_realm=False,
+            no_secrets=False):
         logger = self.get_logger()
         logger.setLevel(logging.INFO)
 
@@ -647,9 +673,11 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
         # Clone and rename the remote server
         lp = sambaopts.get_loadparm()
         creds = credopts.get_credentials(lp)
+        include_secrets = not no_secrets
         ctx = DCCloneAndRenameContext(new_base_dn, new_domain_name,
                                       new_dns_realm, logger=logger,
-                                      creds=creds, lp=lp, include_secrets=True,
+                                      creds=creds, lp=lp,
+                                      include_secrets=include_secrets,
                                       dns_backend='SAMBA_INTERNAL',
                                       server=server, targetdir=tmpdir)
         ctx.do_join()
@@ -694,6 +722,10 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
         logger.info("Fixing DN attributes after rename...")
         self.fix_old_dn_attributes(samdb)
 
+        # ensure the admin user always has a password set (same as provision)
+        if no_secrets:
+            set_admin_password(logger, samdb, creds.get_username())
+
         # Add everything in the tmpdir to the backup tar file
         backup_file = backup_filepath(targetdir, new_dns_realm, time_str)
         create_backup_tar(logger, tmpdir, backup_file)
diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index dabe56f..fad2a93 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -136,6 +136,17 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         lp = self.check_restored_smbconf()
         self.check_restored_database(lp)
 
+    def _test_backup_restore_no_secrets(self):
+        """Does a backup/restore with secrets excluded from the resulting DB"""
+
+        # exclude secrets when we create the backup
+        backup_file = self.create_backup(extra_args=["--no-secrets"])
+        self.restore_backup(backup_file)
+        lp = self.check_restored_smbconf()
+
+        # assert that we don't find user secrets in the DB
+        self.check_restored_database(lp, expect_secrets=False)
+
     def create_smbconf(self, settings):
         """Creates a very basic smb.conf to pass to the restore tool"""
 
@@ -200,7 +211,7 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         self.assertEqual(bkp_lp.get('state directory'), state_dir)
         return bkp_lp
 
-    def check_restored_database(self, bkp_lp):
+    def check_restored_database(self, bkp_lp, expect_secrets=True):
         paths = provision.provision_paths_from_lp(bkp_lp, bkp_lp.get("realm"))
 
         bkp_pd = get_prim_dom(paths.secrets, bkp_lp)
@@ -242,8 +253,29 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         self.assert_partitions_present(samdb)
         self.assert_dcs_present(samdb, self.new_server, expected_count=1)
         self.assert_fsmo_roles(samdb, self.new_server, self.server)
+        self.assert_secrets(samdb, expect_secrets=expect_secrets)
         return samdb
 
+    def assert_user_secrets(self, samdb, username, expect_secrets):
+        """Asserts that a user has/doesn't have secrets as expected"""
+        basedn = str(samdb.get_default_basedn())
+        user_dn = "CN=%s,CN=users,%s" % (username, basedn)
+
+        if expect_secrets:
+            self.assertIsNotNone(samdb.searchone("unicodePwd", user_dn))
+        else:
+            # the search should throw an exception because the secrets
+            # attribute isn't actually there
+            self.assertRaises(KeyError, samdb.searchone, "unicodePwd", user_dn)
+
+    def assert_secrets(self, samdb, expect_secrets):
+        """Check the user secrets in the restored DB match what's expected"""
+
+        # check secrets for the built-in testenv users match what's expected
+        test_users = ["alice", "bob", "jane"]
+        for user in test_users:
+            self.assert_user_secrets(samdb, user, expect_secrets)
+
     def assert_fsmo_roles(self, samdb, server, exclude_server):
         """Asserts the expected server is the FSMO role owner"""
         domain_dn = samdb.domain_dn()
@@ -277,11 +309,13 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         out = self.check_output("samba-tool " + cmd)
         print(out)
 
-    def create_backup(self):
+    def create_backup(self, extra_args=None):
         """Runs the backup cmd to produce a backup file for the testenv DC"""
         # Run the backup command and check we got one backup tar file
         args = self.base_cmd + ["--server=" + self.server, self.user_auth,
                                 "--targetdir=" + self.tempdir]
+        if extra_args:
+            args += extra_args
 
         self.run_cmd(args)
 
@@ -334,6 +368,9 @@ class DomainBackupOnline(DomainBackupBase):
     def test_backup_restore_with_conf(self):
         self._test_backup_restore_with_conf()
 
+    def test_backup_restore_no_secrets(self):
+        self._test_backup_restore_no_secrets()
+
 
 class DomainBackupRename(DomainBackupBase):
 
@@ -358,6 +395,9 @@ class DomainBackupRename(DomainBackupBase):
     def test_backup_restore_with_conf(self):
         self._test_backup_restore_with_conf()
 
+    def test_backup_restore_no_secrets(self):
+        self._test_backup_restore_no_secrets()
+
     def add_link(self, attr, source, target):
         m = ldb.Message()
         m.dn = ldb.Dn(self.ldb, source)
@@ -411,9 +451,10 @@ class DomainBackupRename(DomainBackupBase):
         self.assertTrue(new_server_dn in res[0][link_attr])
 
     # extra checks we run on the restored DB in the rename case
-    def check_restored_database(self, lp):
+    def check_restored_database(self, lp, expect_secrets=True):
         # run the common checks over the restored DB
-        samdb = super(DomainBackupRename, self).check_restored_database(lp)
+        common_test = super(DomainBackupRename, self)
+        samdb = common_test.check_restored_database(lp, expect_secrets)
 
         # check we have actually renamed the DNs
         basedn = str(samdb.get_default_basedn())
-- 
2.7.4


From 0c70959f272725c200a8892399b0ad4a24ed29c9 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 6 Jul 2018 10:35:03 +1200
Subject: [PATCH 2/4] netcmd: Add brief log file of what the backup actually
 contains

There are now several different permutations of backup file that can be
created (i.e. online, rename, with/without secrets). Hopefully the admin
users would organize their backup files sensibly, but it can't hurt to
keep track of what the backup-file actually contains in a simple
human-readable file within the backup tar. E.g. We really don't want
backups with secrets-included and secrets-excluded getting mixed up.

Recording the DC used to make the domain backup may be useful in the
event of a catastrophic failure of the domain, e.g. DC replication may
have been broken for some time prior to the failure.

Recording the samba-tool version string may also be useful if there are
ever any backwards-compatibility issues introduced to the backup files.
The intention is to say we only support restoring a backup with the same
version of samba-tool that actually created the backup, however, it'd be
polite to users to actually record that version somewhere.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/netcmd/domain_backup.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index a4b2765..1e8ccfa 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -110,6 +110,26 @@ def create_backup_tar(logger, tmpdir, backup_filepath):
     tf.close()
 
 
+def create_log_file(targetdir, lp, backup_type, server, include_secrets,
+                    extra_info=None):
+    # create a summary file about the backup, which will get included in the
+    # tar file. This makes it easy for users to see what the backup involved,
+    # without having to untar the DB and interrogate it
+    f = open(os.path.join(targetdir, "backup.txt"), 'w')
+    try:
+        time_str = datetime.datetime.now().strftime('%Y-%b-%d %H:%M:%S')
+        f.write("Backup created %s\n" % time_str)
+        f.write("Using samba-tool version: %s\n" % lp.get('server string'))
+        f.write("Domain %s backup, using DC '%s'\n" % (backup_type, server))
+        f.write("Backup for domain %s (NetBIOS), %s (DNS realm)\n" %
+                (lp.get('workgroup'), lp.get('realm').lower()))
+        f.write("Backup contains domain secrets: %s\n" % str(include_secrets))
+        if extra_info:
+            f.write("%s\n" % extra_info)
+    finally:
+        f.close()
+
+
 # Add a backup-specific marker to the DB with info that we'll use during
 # the restore process
 def add_backup_marker(samdb, marker, value):
@@ -234,6 +254,7 @@ class cmd_domain_backup_online(samba.netcmd.Command):
 
         # Add everything in the tmpdir to the backup tar file
         backup_file = backup_filepath(targetdir, realm, time_str)
+        create_log_file(tmpdir, lp, "online", server, include_secrets)
         create_backup_tar(logger, tmpdir, backup_file)
 
         shutil.rmtree(tmpdir)
@@ -672,6 +693,7 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
 
         # Clone and rename the remote server
         lp = sambaopts.get_loadparm()
+        old_domain = lp.get('workgroup')
         creds = credopts.get_credentials(lp)
         include_secrets = not no_secrets
         ctx = DCCloneAndRenameContext(new_base_dn, new_domain_name,
@@ -728,6 +750,9 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
 
         # Add everything in the tmpdir to the backup tar file
         backup_file = backup_filepath(targetdir, new_dns_realm, time_str)
+        create_log_file(tmpdir, lp, "rename", server, include_secrets,
+                        "Original domain %s (NetBIOS), %s (DNS realm)" %
+                        (old_domain, old_realm))
         create_backup_tar(logger, tmpdir, backup_file)
 
         shutil.rmtree(tmpdir)
-- 
2.7.4


From 13699a8637c07be82ef85160479d4efaa80cca87 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 6 Jul 2018 15:59:31 +1200
Subject: [PATCH 3/4] selftest: Add a 'LABDC' testenv to mimic a preproduction
 test-bed

One of the use-cases for the domain rename tool is to produce a lab
domain that can be used for pre-production testing of Samba.
Basically this involves taking a backup rename with --no-secrets (which
scrubs any sensitive info), and then restoring it.

This patch adds a testenv that mimics how a user would go about creating
a lab-domain. We run the same tests that we run against the restore and
rename testenvs.

Note that the rpc.echo tests for the testallowed and testdenied users
fail, because we don't backup the secrets for these users. So these
tests failing proves that the lab-DC testenv is correct.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/knownfail.d/labdc |  5 +++++
 selftest/target/Samba.pm   |  1 +
 selftest/target/Samba4.pm  | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 source4/selftest/tests.py  | 10 ++++-----
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 selftest/knownfail.d/labdc

diff --git a/selftest/knownfail.d/labdc b/selftest/knownfail.d/labdc
new file mode 100644
index 0000000..65eafd5
--- /dev/null
+++ b/selftest/knownfail.d/labdc
@@ -0,0 +1,5 @@
+# Because the lab-DC testenv scrubs all user info (apart from the Admin),
+# we expect tests relying on other users' credentials to fail.
+# These tests fail because they use testallowed and testdenied users.
+^samba4.rpc.echo.testallowed.*labdc.*
+^samba4.rpc.echo.testdenied.*labdc.*
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 3498567..a6390a6 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -410,6 +410,7 @@ sub get_interface($)
     $interfaces{"backupfromdc"} = 40;
     $interfaces{"restoredc"} = 41;
     $interfaces{"renamedc"} = 42;
+    $interfaces{"labdc"} = 43;
 
     # update lib/socket_wrapper/socket_wrapper.c
     #  #define MAX_WRAPPED_INTERFACES 64
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 4c03ad2..0095089 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2162,6 +2162,7 @@ sub check_env($$)
 
 	restoredc            => ["backupfromdc"],
 	renamedc             => ["backupfromdc"],
+	labdc                => ["backupfromdc"],
 
 	none                 => [],
 );
@@ -2835,6 +2836,61 @@ sub setup_renamedc
 	return $env;
 }
 
+# Set up a DC testenv solely by using the samba-tool 'domain backup rename' and
+# restore commands, using the --no-secrets option. This proves that we can
+# create a realistic lab environment from an online DC ('backupfromdc').
+sub setup_labdc
+{
+	# note: dcvars contains the env info for the dependent testenv ('backupfromdc')
+	my ($self, $prefix, $dcvars) = @_;
+	print "Preparing LAB-DOMAIN DC...\n";
+
+	my $env = $self->prepare_dc_testenv($prefix, "labdc", "LABDOMAIN",
+					    "labdom.samba.example.com", $dcvars->{PASSWORD});
+
+	# create a backup of the 'backupfromdc' which renames the domain and uses
+	# the --no-secrets option to scrub any sensitive info
+	my $backupdir = File::Temp->newdir();
+	my $backup_args = "rename $env->{DOMAIN} $env->{REALM} --no-secrets";
+	my $backup_file = $self->create_backup($env, $dcvars, $backupdir,
+					       $backup_args);
+	unless($backup_file) {
+		return undef;
+	}
+
+	# restore the backup file to populate the lab-DC testenv
+	my $restore_dir = abs_path($prefix);
+	my $restore_opts =  "--newservername=$env->{SERVER} --host-ip=$env->{SERVER_IP}";
+	my $ret = $self->restore_backup_file($backup_file, $restore_opts,
+					     $restore_dir, $env->{SERVERCONFFILE});
+	unless ($ret == 0) {
+		return undef;
+	}
+
+	# because we don't include any secrets in the backup, we need to reset the
+	# admin user's password back to what the testenv expects
+	my $samba_tool = Samba::bindir_path($self, "samba-tool");
+	my $cmd = "$samba_tool user setpassword $env->{USERNAME} ";
+	$cmd .= "--newpassword=$env->{PASSWORD} -H $restore_dir/private/sam.ldb";
+
+	unless(system($cmd) == 0) {
+		warn("Failed to reset admin's password: \n$cmd");
+		return -1;
+	}
+
+	# start samba for the restored DC
+	if (not defined($self->check_or_start($env, "standard"))) {
+	    return undef;
+	}
+
+	my $upn_array = ["$env->{REALM}.upn"];
+	my $spn_array = ["$env->{REALM}.spn"];
+
+	$self->setup_namespaces($env, $upn_array, $spn_array);
+
+	return $env;
+}
+
 sub setup_none
 {
 	my ($self, $path) = @_;
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 121d399..df1bebb 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -812,7 +812,7 @@ plantestsuite_loadlist("samba4.ldap.vlv.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [py
 plantestsuite_loadlist("samba4.ldap.linked_attributes.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(samba4srcdir, "dsdb/tests/python/linked_attributes.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
 
 # These should be the first tests run against testenvs created by backup/restore
-for env in ['restoredc', 'renamedc']:
+for env in ['restoredc', 'renamedc', 'labdc']:
     # check that a restored DC matches the original DC (backupfromdc)
     plantestsuite("samba4.blackbox.ldapcmp_restore", env,
         ["PYTHON=%s" % python,
@@ -869,7 +869,7 @@ for env in ["ad_dc_ntvfs"]:
                            )
 
 # this is a basic sanity-check of Kerberos/NTLM user login
-for env in ["restoredc", "renamedc"]:
+for env in ["restoredc", "renamedc", "labdc"]:
     plantestsuite_loadlist("samba4.ldap.login_basics.python(%s)" % env, env,
         [python, os.path.join(samba4srcdir, "dsdb/tests/python/login_basics.py"),
          "$SERVER", '-U"$USERNAME%$PASSWORD"', "-W$DOMAIN", "--realm=$REALM",
@@ -906,7 +906,7 @@ plansmbtorture4testsuite(t, "vampire_dc", ['$SERVER', '-U$USERNAME%$PASSWORD', '
 plansmbtorture4testsuite(t, "vampire_dc", ['$SERVER', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], modname="samba4.%s.two" % t)
 
 # RPC smoke-tests for testenvs of interest (RODC, etc)
-for env in ['rodc', 'restoredc', 'renamedc']:
+for env in ['rodc', 'restoredc', 'renamedc', 'labdc']:
     plansmbtorture4testsuite('rpc.echo', env, ['ncacn_np:$SERVER', "-k", "yes", '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo")
     plansmbtorture4testsuite('rpc.echo', "%s:local" % env, ['ncacn_np:$SERVER', "-k", "yes", '-P', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo")
     plansmbtorture4testsuite('rpc.echo', "%s:local" % env, ['ncacn_np:$SERVER', "-k", "no", '-Utestallowed\ account%$DC_PASSWORD', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo.testallowed")
@@ -1086,7 +1086,7 @@ for env in [
 planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.kcc.kcc_utils")
 
 for env in [ "simpleserver", "fileserver", "nt4_dc", "ad_dc", "ad_dc_ntvfs",
-             "ad_member", "restoredc", "renamedc" ]:
+             "ad_member", "restoredc", "renamedc", "labdc" ]:
     planoldpythontestsuite(env, "netlogonsvc",
                            extra_path=[os.path.join(srcdir(), 'python/samba/tests')],
                            name="samba.tests.netlogonsvc.python(%s)" % env)
@@ -1111,7 +1111,7 @@ for env in ['vampire_dc', 'promoted_dc', 'rodc']:
 # check the databases are all OK. PLEASE LEAVE THIS AS THE LAST TEST
 for env in ["ad_dc_ntvfs", "ad_dc", "fl2000dc", "fl2003dc", "fl2008r2dc",
             'vampire_dc', 'promoted_dc', 'backupfromdc', 'restoredc',
-            'renamedc']:
+            'renamedc', 'labdc']:
     plantestsuite("samba4.blackbox.dbcheck(%s)" % env, env + ":local" , ["PYTHON=%s" % python, os.path.join(bbdir, "dbcheck.sh"), '$PREFIX/provision', configuration])
 
 # cmocka tests not requiring a specific encironment
-- 
2.7.4


From f57a49bf2d69d8be1e180affb4e0b724a0d4eeb2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 9 Jul 2018 09:44:30 +1200
Subject: [PATCH 4/4] netcmd: Add sanity-check for invalid domain rename args

We are suggesting to users that it's safe to run a renamed domain in
parallel with the old backed-up domain. However, this would not be the
case if the user (foolishly) "renames" their domain using the exact same
NetBIOS name or DNS realm.

Using the same DNS realm fails later on (updating the dnsRoot values),
but using the same NetBIOS name actually succeeds. While we can't make
samba tools completely idiot-proof, we can protect users from the most
basic of (potentially unintended) errors with some simple sanity-checks.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/netcmd/domain_backup.py | 17 ++++++++++++++---
 python/samba/tests/domain_backup.py  | 16 +++++++++++++++-
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index 1e8ccfa..cfd9796 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -691,9 +691,8 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
 
         tmpdir = tempfile.mkdtemp(dir=targetdir)
 
-        # Clone and rename the remote server
+        # setup a join-context for cloning the remote server
         lp = sambaopts.get_loadparm()
-        old_domain = lp.get('workgroup')
         creds = credopts.get_credentials(lp)
         include_secrets = not no_secrets
         ctx = DCCloneAndRenameContext(new_base_dn, new_domain_name,
@@ -702,6 +701,19 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
                                       include_secrets=include_secrets,
                                       dns_backend='SAMBA_INTERNAL',
                                       server=server, targetdir=tmpdir)
+
+        # sanity-check we're not "renaming" the domain to the same values
+        old_domain = ctx.domain_name
+        if old_domain == new_domain_name:
+            shutil.rmtree(tmpdir)
+            raise CommandError("Cannot use the current domain NetBIOS name.")
+
+        old_realm = ctx.realm
+        if old_realm == new_dns_realm:
+            shutil.rmtree(tmpdir)
+            raise CommandError("Cannot use the current domain DNS realm.")
+
+        # do the clone/rename
         ctx.do_join()
 
         # get the paths used for the clone, then drop the old samdb connection
@@ -712,7 +724,6 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
         remote_sam = SamDB(url='ldap://' + server, credentials=creds,
                            session_info=system_session(), lp=lp)
         new_sid = get_sid_for_restore(remote_sam)
-        old_realm = remote_sam.domain_dns_name()
 
         # Grab the remote DC's sysvol files and bundle them into a tar file.
         # Note we end up with 2 sysvol dirs - the original domain's files (that
diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index fad2a93..2df360f 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -19,7 +19,8 @@ import tarfile
 import os
 import shutil
 from samba.tests.samba_tool.base import SambaToolCmdTest
-from samba.tests import TestCaseInTempDir, env_loadparm, create_test_ou
+from samba.tests import (TestCaseInTempDir, env_loadparm, create_test_ou,
+                         BlackboxProcessError)
 import ldb
 from samba.samdb import SamDB
 from samba.auth import system_session
@@ -398,6 +399,19 @@ class DomainBackupRename(DomainBackupBase):
     def test_backup_restore_no_secrets(self):
         self._test_backup_restore_no_secrets()
 
+    def test_backup_invalid_args(self):
+        """Checks that rename commands with invalid args are rejected"""
+
+        # try a "rename" using the same realm as the DC currently has
+        self.base_cmd = ["domain", "backup", "rename", self.restore_domain,
+                         os.environ["REALM"]]
+        self.assertRaises(BlackboxProcessError, self.create_backup)
+
+        # try a "rename" using the same domain as the DC currently has
+        self.base_cmd = ["domain", "backup", "rename", os.environ["DOMAIN"],
+                         self.restore_realm]
+        self.assertRaises(BlackboxProcessError, self.create_backup)
+
     def add_link(self, attr, source, target):
         m = ldb.Message()
         m.dn = ldb.Dn(self.ldb, source)
-- 
2.7.4



More information about the samba-technical mailing list