Attachment 16567 Details for Bug 14027 – patch for 4.14

[patch] patch for 4.14

offline-backup-dns-for-4.14.patch (text/plain), 19.50 KB, created by Jo Sutton on 2021-03-25 00:01:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jo Sutton

Created: 2021-03-25 00:01:56 UTC

Size: 19.50 KB

patch

obsolete

>From 79b0460b6dcfcc6dd42b0f57ef54a2a61b470411 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 16 Mar 2021 16:13:05 +1300
>Subject: [PATCH 1/4] netcmd: Add test for an offline backup of a directory
> containing hardlinks
>
>This test verifies that when performing an offline backup of a domain
>where the directories to be backed up contain hardlinks, only one
>instance of each file is backed up, and that files in the private
>directory take precedence.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> python/samba/tests/domain_backup_offline.py | 74 +++++++++++++++------
> selftest/knownfail.d/bug-14027              |  1 +
> 2 files changed, 56 insertions(+), 19 deletions(-)
> create mode 100644 selftest/knownfail.d/bug-14027
>
>diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
>index 8b7209ec24d..80ccf3bb93b 100644
>--- a/python/samba/tests/domain_backup_offline.py
>+++ b/python/samba/tests/domain_backup_offline.py
>@@ -31,6 +31,34 @@ from samba.netcmd import CommandError
> # so that we never inadvertently use .runcmd() by accident.
> class DomainBackupOfflineCmp(BlackboxTestCase):
> 
>+    def test_domain_backup_offline_hard_link_tdb(self):
>+        self.hard_link_testcase('tdb')
>+
>+    def test_domain_backup_offline_hard_link_mdb(self):
>+        self.hard_link_testcase('mdb')
>+
>+    def hard_link_testcase(self, backend):
>+        self.prov_dir = self.provision(backend)
>+        self.extract_dir = None
>+
>+        # Create hard links in the private and state directories
>+        os.link(os.path.join(self.prov_dir, "private", "krb5.conf"),
>+                os.path.join(self.prov_dir, "state", "krb5.conf"))
>+
>+        backup_file = self.backup(self.prov_dir)
>+
>+        # Extract the backup
>+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
>+        tf = tarfile.open(backup_file)
>+        tf.extractall(self.extract_dir)
>+
>+        # Ensure that the hard link in the private directory was backed up,
>+        # while the one in the state directory was not.
>+        self.assertTrue(os.path.exists(os.path.join(self.extract_dir,
>+                                                    "private", "krb5.conf")))
>+        self.assertFalse(os.path.exists(os.path.join(self.extract_dir,
>+                                                    "statedir", "krb5.conf")))
>+
>     def test_domain_backup_offline_untar_tdb(self):
>         self.untar_testcase('tdb')
> 
>@@ -44,12 +72,15 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>         self.restore_testcase('mdb')
> 
>     def restore_testcase(self, backend):
>-        prov_dir, backup_file = self.provision_and_backup(backend)
>+        self.prov_dir = self.provision(backend)
>+        self.extract_dir = None
>+        backup_file = self.backup(self.prov_dir)
> 
>-        extract_dir = tempfile.mkdtemp(dir=self.tempdir)
>+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
>         cmd = ("samba-tool domain backup restore --backup-file={f}"
>                " --targetdir={d} "
>-               "--newservername=NEWSERVER").format(f=backup_file, d=extract_dir)
>+               "--newservername=NEWSERVER").format(f=backup_file,
>+                                                   d=self.extract_dir)
>         self.check_output(cmd)
> 
>         # attrs that are altered by the restore process
>@@ -61,22 +92,18 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>                         "interSiteTopologyGenerator"]
>         filter_arg = "--filter=" + ",".join(ignore_attrs)
>         args = ["--two", filter_arg]
>-        self.ldapcmp(prov_dir, extract_dir, args)
>-
>-        shutil.rmtree(prov_dir)
>-        shutil.rmtree(extract_dir)
>+        self.ldapcmp(self.prov_dir, self.extract_dir, args)
> 
>     def untar_testcase(self, backend):
>-        prov_dir, backup_file = self.provision_and_backup(backend)
>+        self.prov_dir = self.provision(backend)
>+        self.extract_dir = None
>+        backup_file = self.backup(self.prov_dir)
> 
>-        extract_dir = tempfile.mkdtemp(dir=self.tempdir)
>+        self.extract_dir = tempfile.mkdtemp(dir=self.tempdir)
>         tf = tarfile.open(backup_file)
>-        tf.extractall(extract_dir)
>+        tf.extractall(self.extract_dir)
> 
>-        self.ldapcmp(prov_dir, extract_dir)
>-
>-        shutil.rmtree(prov_dir)
>-        shutil.rmtree(extract_dir)
>+        self.ldapcmp(self.prov_dir, self.extract_dir)
> 
>     def ldapcmp(self, prov_dir, ex_dir, args=[]):
>         sam_fn = os.path.join("private", "sam.ldb")
>@@ -90,8 +117,8 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>             self.check_output(cmd)
> 
>     # Test the "samba-tool domain backup" command with ldapcmp
>-    def provision_and_backup(self, backend):
>-        prov_dir = tempfile.mkdtemp(dir=self.tempdir)
>+    def provision(self, backend):
>+        target = tempfile.mkdtemp(dir=self.tempdir)
> 
>         # Provision domain.  Use fake ACLs and store xattrs in tdbs so that
>         # NTACL backup will work inside the testenv.
>@@ -100,13 +127,16 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>         # circumstances, causing the ldapcmp to fail.
>         prov_cmd = "samba-tool domain provision " +\
>                    "--domain FOO --realm foo.example.com " +\
>-                   "--targetdir {prov_dir} " +\
>+                   "--targetdir {target} " +\
>                    "--backend-store {backend} " +\
>                    "--host-name OLDSERVER "+\
>                    "--option=\"vfs objects=fake_acls xattr_tdb\""
>-        prov_cmd = prov_cmd.format(prov_dir=prov_dir, backend=backend)
>+        prov_cmd = prov_cmd.format(target=target, backend=backend)
>         self.check_output(prov_cmd)
> 
>+        return target
>+
>+    def backup(self, prov_dir):
>         # Run the backup and check we got one backup tar file
>         cmd = ("samba-tool domain backup offline --targetdir={prov_dir} "
>                "-s {prov_dir}/etc/smb.conf").format(prov_dir=prov_dir)
>@@ -120,4 +150,10 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>                                " file but got {0}".format(len(tar_files)))
> 
>         backup_file = os.path.join(prov_dir, tar_files[0])
>-        return prov_dir, backup_file
>+        return backup_file
>+
>+    def tearDown(self):
>+        # Remove temporary directories
>+        shutil.rmtree(self.prov_dir)
>+        if self.extract_dir:
>+            shutil.rmtree(self.extract_dir)
>diff --git a/selftest/knownfail.d/bug-14027 b/selftest/knownfail.d/bug-14027
>new file mode 100644
>index 00000000000..b1bb5270b3e
>--- /dev/null
>+++ b/selftest/knownfail.d/bug-14027
>@@ -0,0 +1 @@
>+^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_hard_link
>-- 
>2.25.1
>
>
>From 4c886ec08cce7322fdd378ba35874236d0222eb9 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 18 Mar 2021 10:52:52 +1300
>Subject: [PATCH 2/4] netcmd: Add test for an offline backup of nested
> directories
>
>This test verifies that when performing an offline backup of a domain
>where one of the directories to be backed up is nested inside another,
>the contained files are only included once in the backup.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> python/samba/tests/domain_backup_offline.py | 31 +++++++++++++++++++++
> selftest/knownfail.d/bug-14027              |  1 +
> 2 files changed, 32 insertions(+)
>
>diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
>index 80ccf3bb93b..16d3e7c36f4 100644
>--- a/python/samba/tests/domain_backup_offline.py
>+++ b/python/samba/tests/domain_backup_offline.py
>@@ -21,6 +21,7 @@ import shutil
> import tempfile
> from samba.tests import BlackboxTestCase
> from samba.netcmd import CommandError
>+from samba.param import LoadParm
> 
> # The backup tests require that a completely clean LoadParm object gets used
> # for the restore. Otherwise the same global LP gets re-used, and the LP
>@@ -31,6 +32,36 @@ from samba.netcmd import CommandError
> # so that we never inadvertently use .runcmd() by accident.
> class DomainBackupOfflineCmp(BlackboxTestCase):
> 
>+    def test_domain_backup_offline_nested_tdb(self):
>+        self.nested_testcase('tdb')
>+
>+    def test_domain_backup_offline_nested_mdb(self):
>+        self.nested_testcase('mdb')
>+
>+    def nested_testcase(self, backend):
>+        self.prov_dir = self.provision(backend)
>+        self.extract_dir = None
>+
>+        src = os.path.join(self.prov_dir, "private")
>+        dst = os.path.join(self.prov_dir, "state", "private")
>+
>+        # Move private directory inside state directory
>+        shutil.move(src, dst)
>+
>+        smbconf = os.path.join(self.prov_dir, "etc", "smb.conf")
>+
>+        # Update the conf file
>+        lp = LoadParm(filename_for_non_global_lp=smbconf)
>+        lp.set("private dir", dst)
>+        lp.dump(False, smbconf)
>+
>+        backup_file = self.backup(self.prov_dir)
>+
>+        # Ensure each file is only present once in the tar file
>+        tf = tarfile.open(backup_file)
>+        names = tf.getnames()
>+        self.assertEqual(len(names), len(set(names)))
>+
>     def test_domain_backup_offline_hard_link_tdb(self):
>         self.hard_link_testcase('tdb')
> 
>diff --git a/selftest/knownfail.d/bug-14027 b/selftest/knownfail.d/bug-14027
>index b1bb5270b3e..f0746474be2 100644
>--- a/selftest/knownfail.d/bug-14027
>+++ b/selftest/knownfail.d/bug-14027
>@@ -1 +1,2 @@
> ^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_hard_link
>+^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_nested
>-- 
>2.25.1
>
>
>From ec4f2c785e225baf3d6cf8dce13ce601939dfa01 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 16 Mar 2021 16:22:40 +1300
>Subject: [PATCH 3/4] netcmd: Determine which files are to be copied for an
> offline domain backup
>
>The old behaviour attempted to check for and remove files with duplicate
>names, but did not do so due to a bug, and would have left undetermined
>which files were given priority when duplicate filenames were present.
>Now when hardlinks are present, only one instance of each file is
>chosen, with files in the private directory having priority. If one
>backup dir is nested inside another, the files contained in the nested
>directory are only added once. Additionally, the BIND DNS database is
>omitted from the backup.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> python/samba/netcmd/domain_backup.py | 19 ++++++++++++++++---
> selftest/knownfail.d/bug-14027       |  2 --
> 2 files changed, 16 insertions(+), 5 deletions(-)
> delete mode 100644 selftest/knownfail.d/bug-14027
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index 799fd0593e5..c38b69e2b23 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -1105,6 +1105,10 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>         samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
>         sid = get_sid_for_restore(samdb, logger)
> 
>+        # Iterating over the directories in this specific order ensures that
>+        # when the private directory contains hardlinks that are also contained
>+        # in other directories to be backed up (such as in paths.binddns_dir),
>+        # the hardlinks in the private directory take precedence.
>         backup_dirs = [paths.private_dir, paths.state_dir,
>                        os.path.dirname(paths.smbconf)]  # etc dir
>         logger.info('running backup on dirs: {0}'.format(' '.join(backup_dirs)))
>@@ -1117,22 +1121,31 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>                     continue
>                 if working_dir.endswith('.sock') or '.sock/' in working_dir:
>                     continue
>+                # The BIND DNS database can be regenerated, so it doesn't need
>+                # to be backed up.
>+                if working_dir.startswith(os.path.join(paths.binddns_dir, 'dns')):
>+                    continue
> 
>                 for filename in filenames:
>-                    if filename in all_files:
>+                    full_path = os.path.join(working_dir, filename)
>+
>+                    # Ignore files that have already been added. This prevents
>+                    # duplicates if one backup dir is a subdirectory of another,
>+                    # or if backup dirs contain hardlinks.
>+                    if any(os.path.samefile(full_path, file) for file in all_files):
>                         continue
> 
>                     # Assume existing backup files are from a previous backup.
>                     # Delete and ignore.
>                     if filename.endswith(self.backup_ext):
>-                        os.remove(os.path.join(working_dir, filename))
>+                        os.remove(full_path)
>                         continue
> 
>                     # Sock files are autogenerated at runtime, ignore.
>                     if filename.endswith('.sock'):
>                         continue
> 
>-                    all_files.append(os.path.join(working_dir, filename))
>+                    all_files.append(full_path)
> 
>         # Backup secrets, sam.ldb and their downstream files
>         self.backup_secrets(paths.private_dir, lp, logger)
>diff --git a/selftest/knownfail.d/bug-14027 b/selftest/knownfail.d/bug-14027
>deleted file mode 100644
>index f0746474be2..00000000000
>--- a/selftest/knownfail.d/bug-14027
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_hard_link
>-^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_nested
>-- 
>2.25.1
>
>
>From 86a75690ab73ecf07d0be6ee75d55bcbbf14af58 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 16 Mar 2021 22:20:21 +1300
>Subject: [PATCH 4/4] netcmd: Avoid database corruption by opting not to create
> database files during an offline domain backup
>
>If backup dirs contain hardlinks, the backup process could previously
>attempt to open an LMDB database already opened during the backup,
>causing it to be recreated as a new TDB database. This commit ensures
>that new database files are not created during this operation, and that
>the main SamDB database is not modified.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14027
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> python/samba/netcmd/domain_backup.py | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index c38b69e2b23..9eae6d3c3cf 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -313,7 +313,8 @@ class cmd_domain_backup_online(samba.netcmd.Command):
>             shutil.rmtree(paths.sysvol)
> 
>             # Edit the downloaded sam.ldb to mark it as a backup
>-            samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
>+            samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>+                          flags=ldb.FLG_DONT_CREATE_DB)
>             time_str = get_timestamp()
>             add_backup_marker(samdb, "backupDate", time_str)
>             add_backup_marker(samdb, "sidForRestore", new_sid)
>@@ -537,7 +538,8 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
>         # open a DB connection to the restored DB
>         private_dir = os.path.join(targetdir, 'private')
>         samdb_path = os.path.join(private_dir, 'sam.ldb')
>-        samdb = SamDB(url=samdb_path, session_info=system_session(), lp=lp)
>+        samdb = SamDB(url=samdb_path, session_info=system_session(), lp=lp,
>+                      flags=ldb.FLG_DONT_CREATE_DB)
>         backup_type = self.get_backup_type(samdb)
> 
>         if site is None:
>@@ -645,7 +647,8 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
>                                    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)
>+        secrets_ldb = Ldb(secrets_path, session_info=system_session(), lp=lp,
>+                          flags=ldb.FLG_DONT_CREATE_DB)
>         secretsdb_self_join(secrets_ldb, domain=ctx.domain_name,
>                             realm=ctx.realm, dnsdomain=ctx.dnsdomain,
>                             netbiosname=ctx.myname, domainsid=ctx.domsid,
>@@ -937,7 +940,8 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
> 
>         # connect to the local DB (making sure we use the new/renamed config)
>         lp.load(paths.smbconf)
>-        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
>+        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>+                      flags=ldb.FLG_DONT_CREATE_DB)
> 
>         # Edit the cloned sam.ldb to mark it as a backup
>         time_str = get_timestamp()
>@@ -1025,7 +1029,8 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>     # on the secrets.ldb file before backing up that file and secrets.tdb
>     def backup_secrets(self, private_dir, lp, logger):
>         secrets_path = os.path.join(private_dir, 'secrets')
>-        secrets_obj = Ldb(secrets_path + '.ldb', lp=lp)
>+        secrets_obj = Ldb(secrets_path + '.ldb', lp=lp,
>+                          flags=ldb.FLG_DONT_CREATE_DB)
>         logger.info('Starting transaction on ' + secrets_path)
>         secrets_obj.transaction_start()
>         self.offline_tdb_copy(secrets_path + '.ldb')
>@@ -1050,7 +1055,7 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>         else:
>             logger.info('Starting transaction on ' + sam_ldb_path)
>             copy_function = self.offline_tdb_copy
>-            sam_obj = Ldb(sam_ldb_path, lp=lp)
>+            sam_obj = Ldb(sam_ldb_path, lp=lp, flags=ldb.FLG_DONT_CREATE_DB)
>             sam_obj.transaction_start()
> 
>         logger.info('   backing up ' + sam_ldb_path)
>@@ -1102,7 +1107,8 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
> 
>         check_targetdir(logger, targetdir)
> 
>-        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp)
>+        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>+                      flags=ldb.FLG_RDONLY)
>         sid = get_sid_for_restore(samdb, logger)
> 
>         # Iterating over the directories in this specific order ensures that
>@@ -1157,7 +1163,8 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>         #          Writing to a .bak file only works because the DN being
>         #          written to happens to be top level.
>         samdb = SamDB(url=paths.samdb + self.backup_ext,
>-                      session_info=system_session(), lp=lp)
>+                      session_info=system_session(), lp=lp,
>+                      flags=ldb.FLG_DONT_CREATE_DB)
>         time_str = get_timestamp()
>         add_backup_marker(samdb, "backupDate", time_str)
>         add_backup_marker(samdb, "sidForRestore", sid)
>@@ -1169,7 +1176,7 @@ class cmd_domain_backup_offline(samba.netcmd.Command):
>             if not os.path.exists(path + self.backup_ext):
>                 if path.endswith('.ldb'):
>                     logger.info('Starting transaction on solo db: ' + path)
>-                    ldb_obj = Ldb(path, lp=lp)
>+                    ldb_obj = Ldb(path, lp=lp, flags=ldb.FLG_DONT_CREATE_DB)
>                     ldb_obj.transaction_start()
>                     logger.info('   running tdbbackup on the same file')
>                     self.offline_tdb_copy(path)
>-- 
>2.25.1
>

Flags:

abartlet: review+
slow: review-

Actions: View

Attachments on bug 14027: 16566 | 16567 | 16634 | 16635