netcmd: Add no-secrets option to domain backups
authorTim Beale <timbeale@catalyst.net.nz>
Thu, 5 Jul 2018 02:33:22 +0000 (14:33 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Tue, 10 Jul 2018 02:42:10 +0000 (04:42 +0200)
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@catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
python/samba/netcmd/domain_backup.py
python/samba/tests/domain_backup.py

index c156046b26ea98037be5d7f27dd19a0c89e62b34..a4b276579052712153aaade8ea79ddb919f4962c 100644 (file)
@@ -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)
index dabe56f1cc545485e8d595f365734557153d91ff..fad2a93dfb2569444d8a3db8369fd3aefe984320 100644 (file)
@@ -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())