Attachment 16665 Details for Bug 14743 – patch for 4.14

[patch] patch for 4.14

samba-tool-domain-backup-issues-for-4.14.patch (text/plain), 53.16 KB, created by Jo Sutton on 2021-06-16 23:39:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jo Sutton

Created: 2021-06-16 23:39:05 UTC

Size: 53.16 KB

patch

obsolete

>From 810693a9853ed41a55d965df7eed43a23e850772 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 01/10] 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>
>(cherry picked from commit 0e5738887524b467bfebcf657bcb00ed71827784)
>---
> 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.31.1.362.g311531c9de
>
>
>From 1f4cb443fdf0adc81d41796bad88a644a36d5958 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 02/10] 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>
>(cherry picked from commit f994783f4279884ec4d2ee3e7db80fb7af267d1c)
>---
> 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.31.1.362.g311531c9de
>
>
>From f510796b902039aac390c55fc070561084526f67 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 03/10] 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>
>(cherry picked from commit 3723148e7aa7e6d4a48a1a38112f121f52b6ee6f)
>---
> 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.31.1.362.g311531c9de
>
>
>From 7b104b6098bb463d1fb44ef0b166168257fb56ed 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 04/10] 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>
>(cherry picked from commit 4cf773591d49166b8c7ef8d637d7edfe755d48aa)
>---
> 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.31.1.362.g311531c9de
>
>
>From a740160d6d0500933b0b54f1f80d78db89e756b0 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 24 May 2021 16:40:55 +1200
>Subject: [PATCH 05/10] netcmd: Fix error-checking condition
>
>This condition probably meant to check the argument of the most recently
>thrown exception, rather than the previous one again.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit e8c242bed19432d96e78dc345ab5f06422c5b104)
>---
> python/samba/netcmd/domain_backup.py | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index 9eae6d3c3cf..8d261fd4b97 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -602,11 +602,12 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
>                                        controls=["show_deleted:0",
>                                                  "show_recycled:0"])
>             except LdbError as dup_e:
>-                if enum != ldb.ERR_NO_SUCH_OBJECT:
>-                    raise e
>+                (dup_enum, _) = dup_e.args
>+                if dup_enum != ldb.ERR_NO_SUCH_OBJECT:
>+                    raise
> 
>             if (len(dup_res) != 1):
>-                raise e
>+                raise
> 
>             objectguid = samdb.schema_format_value("objectGUID",
>                                                        dup_res[0]["objectGUID"][0])
>-- 
>2.31.1.362.g311531c9de
>
>
>From 3ad35c24961b9a1c486920b3c845ae93a59c8b9c Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 26 May 2021 13:40:30 +1200
>Subject: [PATCH 06/10] netcmd: Ignore rIDUsedPool attribute in offline domain
> backup test
>
>The RID Set of the newly created DC account has all its values
>initialised to zero. If the rIDUsedPool attribute was previously
>non-zero, then the restore process will cause its value to change.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 658e5a6cc20b57f48477affd370fe25458178b92)
>---
> python/samba/tests/domain_backup_offline.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
>index 16d3e7c36f4..83cd6cfa57f 100644
>--- a/python/samba/tests/domain_backup_offline.py
>+++ b/python/samba/tests/domain_backup_offline.py
>@@ -116,7 +116,7 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
> 
>         # attrs that are altered by the restore process
>         ignore_attrs = ["servicePrincipalName", "lastLogonTimestamp",
>-                        "rIDAllocationPool", "rIDAvailablePool",
>+                        "rIDAllocationPool", "rIDAvailablePool", "rIDUsedPool",
>                         "localPolicyFlags", "operatingSystem", "displayName",
>                         "dnsRecord", "dNSTombstoned",
>                         "msDS-NC-Replica-Locations", "msDS-HasInstantiatedNCs",
>-- 
>2.31.1.362.g311531c9de
>
>
>From 0cd944e44b43cb808abae2fc48f70b1f97eed276 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 24 May 2021 14:58:40 +1200
>Subject: [PATCH 07/10] netcmd: Add tests for performing an offline backup
> immediately after joining a domain
>
>This currently fails due to the DC not having a rIDNextRID attribute,
>which is required for the restore process.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit b7e6a1c5da7283c49586dc29f85ab19e0e57b0f6)
>---
> python/samba/tests/domain_backup_offline.py | 61 ++++++++++++++++++---
> selftest/knownfail.d/bug-14669              |  1 +
> source4/selftest/tests.py                   |  2 +-
> 3 files changed, 56 insertions(+), 8 deletions(-)
> create mode 100644 selftest/knownfail.d/bug-14669
>
>diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
>index 83cd6cfa57f..21f42c6dab8 100644
>--- a/python/samba/tests/domain_backup_offline.py
>+++ b/python/samba/tests/domain_backup_offline.py
>@@ -19,9 +19,12 @@ import tarfile
> import os
> import shutil
> import tempfile
>-from samba.tests import BlackboxTestCase
>+from samba.tests import BlackboxTestCase, BlackboxProcessError
> from samba.netcmd import CommandError
> from samba.param import LoadParm
>+from samba.join import join_DC
>+from samba.credentials import Credentials
>+from samba.logger import get_samba_logger
> 
> # 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
>@@ -62,6 +65,23 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>         names = tf.getnames()
>         self.assertEqual(len(names), len(set(names)))
> 
>+    def test_domain_backup_offline_join_restore_tdb(self):
>+        self.join_restore_testcase('tdb')
>+
>+    def test_domain_backup_offline_join_restore_mdb(self):
>+        self.join_restore_testcase('mdb')
>+
>+    def join_restore_testcase(self, backend):
>+        self.prov_dir = self.join(backend)
>+        self.extract_dir = None
>+
>+        try:
>+            backup_file = self.backup(self.prov_dir)
>+        except BlackboxProcessError as e:
>+            self.fail(e)
>+
>+        self.extract_dir = self.restore(backup_file)
>+
>     def test_domain_backup_offline_hard_link_tdb(self):
>         self.hard_link_testcase('tdb')
> 
>@@ -107,12 +127,7 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>         self.extract_dir = None
>         backup_file = self.backup(self.prov_dir)
> 
>-        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=self.extract_dir)
>-        self.check_output(cmd)
>+        self.extract_dir = self.restore(backup_file)
> 
>         # attrs that are altered by the restore process
>         ignore_attrs = ["servicePrincipalName", "lastLogonTimestamp",
>@@ -167,6 +182,27 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
> 
>         return target
> 
>+    def join(self, backend):
>+        target = tempfile.mkdtemp(dir=self.tempdir)
>+
>+        join_cmd = "samba-tool domain join {domain} DC " +\
>+                   "--server {server} " +\
>+                   "--realm {realm} " +\
>+                   "--username {username}%{password} " +\
>+                   "--targetdir {target} " +\
>+                   "--backend-store {backend} " +\
>+                   "--option=\"vfs objects=dfs_samba4 acl_xattr fake_acls xattr_tdb\""
>+        join_cmd = join_cmd.format(server=os.environ["DC_SERVER"],
>+                                   domain=os.environ["DOMAIN"],
>+                                   realm=os.environ["REALM"],
>+                                   username=os.environ["USERNAME"],
>+                                   password=os.environ["PASSWORD"],
>+                                   target=target,
>+                                   backend=backend)
>+        self.check_output(join_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} "
>@@ -183,6 +219,17 @@ class DomainBackupOfflineCmp(BlackboxTestCase):
>         backup_file = os.path.join(prov_dir, tar_files[0])
>         return backup_file
> 
>+    def restore(self, backup_file):
>+        # Restore from a backup file
>+        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)
>+        self.check_output(cmd)
>+
>+        return extract_dir
>+
>     def tearDown(self):
>         # Remove temporary directories
>         shutil.rmtree(self.prov_dir)
>diff --git a/selftest/knownfail.d/bug-14669 b/selftest/knownfail.d/bug-14669
>new file mode 100644
>index 00000000000..83f8cbfe456
>--- /dev/null
>+++ b/selftest/knownfail.d/bug-14669
>@@ -0,0 +1 @@
>+^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_join_restore
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index 4c9ddccd01b..0a83bcd6987 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -906,7 +906,7 @@ for env in ["ad_dc_backup", smbv1_disabled_testenv]:
>     planoldpythontestsuite(env + ":local", "samba.tests.domain_backup",
>                            extra_args=['-U"$USERNAME%$PASSWORD"'])
> 
>-planoldpythontestsuite("none",
>+planoldpythontestsuite("ad_dc",
>                        "samba.tests.domain_backup_offline")
> # Encrypted secrets
> # ensure default provision (ad_dc) and join (vampire_dc)
>-- 
>2.31.1.362.g311531c9de
>
>
>From cd92f1bac43dfba147535b8297dd64d0c6782fc2 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 24 May 2021 12:59:59 +1200
>Subject: [PATCH 08/10] dsdb: Add next_free_rid() function to allocate a RID
> without modifying the database
>
>If used to generate SIDs for objects, care should be taken, as the
>possibility for having duplicate objectSIDs can arise.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit cc98e03e7a0f2bf7a1ace2950fe6500f53640c1b)
>---
> python/samba/samdb.py | 105 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
>diff --git a/python/samba/samdb.py b/python/samba/samdb.py
>index f95709ab7c8..d65c6f2e3b1 100644
>--- a/python/samba/samdb.py
>+++ b/python/samba/samdb.py
>@@ -1385,6 +1385,111 @@ schemaUpdateNow: 1
>         '''return a new RID from the RID Pool on this DSA'''
>         return dsdb._dsdb_allocate_rid(self)
> 
>+    def next_free_rid(self):
>+        '''return the next free RID from the RID Pool on this DSA.
>+
>+        :note: This function is not intended for general use, and care must be
>+            taken if it is used to generate objectSIDs. The returned RID is not
>+            formally reserved for use, creating the possibility of duplicate
>+            objectSIDs.
>+        '''
>+        rid, _ = self.free_rid_bounds()
>+        return rid
>+
>+    def free_rid_bounds(self):
>+        '''return the low and high bounds (inclusive) of RIDs that are
>+            available for use in this DSA's current RID pool.
>+
>+        :note: This function is not intended for general use, and care must be
>+            taken if it is used to generate objectSIDs. The returned range of
>+            RIDs is not formally reserved for use, creating the possibility of
>+            duplicate objectSIDs.
>+        '''
>+        # Get DN of this server's RID Set
>+        server_name_dn = ldb.Dn(self, self.get_serverName())
>+        res = self.search(base=server_name_dn,
>+                          scope=ldb.SCOPE_BASE,
>+                          attrs=["serverReference"])
>+        try:
>+            server_ref = res[0]["serverReference"]
>+        except KeyError:
>+            raise ldb.LdbError(
>+                ldb.ERR_NO_SUCH_ATTRIBUTE,
>+                "No RID Set DN - "
>+                "Cannot find attribute serverReference of %s "
>+                "to calculate reference dn" % server_name_dn) from None
>+        server_ref_dn = ldb.Dn(self, server_ref[0].decode("utf-8"))
>+
>+        res = self.search(base=server_ref_dn,
>+                          scope=ldb.SCOPE_BASE,
>+                          attrs=["rIDSetReferences"])
>+        try:
>+            rid_set_refs = res[0]["rIDSetReferences"]
>+        except KeyError:
>+            raise ldb.LdbError(
>+                ldb.ERR_NO_SUCH_ATTRIBUTE,
>+                "No RID Set DN - "
>+                "Cannot find attribute rIDSetReferences of %s "
>+                "to calculate reference dn" % server_ref_dn) from None
>+        rid_set_dn = ldb.Dn(self, rid_set_refs[0].decode("utf-8"))
>+
>+        # Get the alloc pools and next RID of this RID Set
>+        res = self.search(base=rid_set_dn,
>+                          scope=ldb.SCOPE_BASE,
>+                          attrs=["rIDAllocationPool",
>+                                 "rIDPreviousAllocationPool",
>+                                 "rIDNextRID"])
>+
>+        uint32_max = 2**32 - 1
>+        uint64_max = 2**64 - 1
>+
>+        try:
>+            alloc_pool = int(res[0]["rIDAllocationPool"][0])
>+        except KeyError:
>+            alloc_pool = uint64_max
>+        if alloc_pool == uint64_max:
>+            raise ldb.LdbError(ldb.ERR_OPERATIONS_ERROR,
>+                               "Bad RID Set %s" % rid_set_dn)
>+
>+        try:
>+            prev_pool = int(res[0]["rIDPreviousAllocationPool"][0])
>+        except KeyError:
>+            prev_pool = uint64_max
>+        try:
>+            next_rid = int(res[0]["rIDNextRID"][0])
>+        except KeyError:
>+            next_rid = uint32_max
>+
>+        # If we never used a pool, set up our first pool
>+        if prev_pool == uint64_max or next_rid == uint32_max:
>+            prev_pool = alloc_pool
>+            next_rid = prev_pool & uint32_max
>+
>+        next_rid += 1
>+
>+        # Now check if our current pool is still usable
>+        prev_pool_lo = prev_pool & uint32_max
>+        prev_pool_hi = prev_pool >> 32
>+        if next_rid > prev_pool_hi:
>+            # We need a new pool, check if we already have a new one
>+            # Otherwise we return an error code.
>+            if alloc_pool == prev_pool:
>+                raise ldb.LdbError(ldb.ERR_OPERATIONS_ERROR,
>+                                   "RID pools out of RIDs")
>+
>+            # Now use the new pool
>+            prev_pool = alloc_pool
>+            prev_pool_lo = prev_pool & uint32_max
>+            prev_pool_hi = prev_pool >> 32
>+            next_rid = prev_pool_lo
>+
>+        if next_rid < prev_pool_lo or next_rid > prev_pool_hi:
>+            raise ldb.LdbError(ldb.ERR_OPERATIONS_ERROR,
>+                               "Bad RID chosen %d from range %d-%d" %
>+                               (next_rid, prev_pool_lo, prev_pool_hi))
>+
>+        return next_rid, prev_pool_hi
>+
>     def normalize_dn_in_domain(self, dn):
>         '''return a new DN expanded by adding the domain DN
> 
>-- 
>2.31.1.362.g311531c9de
>
>
>From f5f6903a45e6172f6d63744f2646a7b8fc37d8f8 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Mon, 24 May 2021 16:46:28 +1200
>Subject: [PATCH 09/10] python/tests/dsdb: Add tests for RID allocation
> functions
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 7c7cad81844950c3efe9a540a47b9d4e1ce1b2a1)
>---
> python/samba/tests/dsdb.py | 305 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 304 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
>index 33cfcc12271..f1d0557743e 100644
>--- a/python/samba/tests/dsdb.py
>+++ b/python/samba/tests/dsdb.py
>@@ -50,13 +50,316 @@ class DsdbTests(TestCase):
> 
>         base_dn = self.samdb.domain_dn()
> 
>-        self.account_dn = "cn=" + user_name + ",cn=Users," + base_dn
>+        self.account_dn = "CN=" + user_name + ",CN=Users," + base_dn
>         self.samdb.newuser(username=user_name,
>                            password=user_pass,
>                            description=user_description)
>         # Cleanup (teardown)
>         self.addCleanup(delete_force, self.samdb, self.account_dn)
> 
>+        # Get server reference DN
>+        res = self.samdb.search(base=ldb.Dn(self.samdb,
>+                                            self.samdb.get_serverName()),
>+                                scope=ldb.SCOPE_BASE,
>+                                attrs=["serverReference"])
>+        # Get server reference
>+        self.server_ref_dn = ldb.Dn(
>+            self.samdb, res[0]["serverReference"][0].decode("utf-8"))
>+
>+        # Get RID Set DN
>+        res = self.samdb.search(base=self.server_ref_dn,
>+                                scope=ldb.SCOPE_BASE,
>+                                attrs=["rIDSetReferences"])
>+        rid_set_refs = res[0]
>+        self.assertIn("rIDSetReferences", rid_set_refs)
>+        rid_set_str = rid_set_refs["rIDSetReferences"][0].decode("utf-8")
>+        self.rid_set_dn = ldb.Dn(self.samdb, rid_set_str)
>+
>+    def get_rid_set(self, rid_set_dn):
>+        res = self.samdb.search(base=rid_set_dn,
>+                                scope=ldb.SCOPE_BASE,
>+                                attrs=["rIDAllocationPool",
>+                                       "rIDPreviousAllocationPool",
>+                                       "rIDUsedPool",
>+                                       "rIDNextRID"])
>+        return res[0]
>+
>+    def test_ridalloc_next_free_rid(self):
>+        # Test RID allocation. We assume that RID
>+        # pools allocated to us are continguous.
>+        self.samdb.transaction_start()
>+        try:
>+            orig_rid_set = self.get_rid_set(self.rid_set_dn)
>+            self.assertIn("rIDAllocationPool", orig_rid_set)
>+            self.assertIn("rIDPreviousAllocationPool", orig_rid_set)
>+            self.assertIn("rIDUsedPool", orig_rid_set)
>+            self.assertIn("rIDNextRID", orig_rid_set)
>+
>+            # Get rIDNextRID value from RID set.
>+            next_rid = int(orig_rid_set["rIDNextRID"][0])
>+
>+            # Check the result of next_free_rid().
>+            next_free_rid = self.samdb.next_free_rid()
>+            self.assertEqual(next_rid + 1, next_free_rid)
>+
>+            # Check calling it twice in succession gives the same result.
>+            next_free_rid2 = self.samdb.next_free_rid()
>+            self.assertEqual(next_free_rid, next_free_rid2)
>+
>+            # Ensure that the RID set attributes have not changed.
>+            rid_set2 = self.get_rid_set(self.rid_set_dn)
>+            self.assertEqual(orig_rid_set, rid_set2)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_no_ridnextrid(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Delete the rIDNextRID attribute of the RID set,
>+            # and set up previous and next pools.
>+            prev_lo = 1000
>+            prev_hi = 1999
>+            next_lo = 3000
>+            next_hi = 3999
>+            msg = ldb.Message()
>+            msg.dn = self.rid_set_dn
>+            msg["rIDNextRID"] = ldb.MessageElement([],
>+                                                   ldb.FLAG_MOD_DELETE,
>+                                                   "rIDNextRID")
>+            msg["rIDPreviousAllocationPool"] = (
>+                ldb.MessageElement(str((prev_hi << 32) | prev_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDPreviousAllocationPool"))
>+            msg["rIDAllocationPool"] = (
>+                ldb.MessageElement(str((next_hi << 32) | next_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDAllocationPool"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure that next_free_rid() returns the start of the next pool
>+            # plus one.
>+            next_free_rid3 = self.samdb.next_free_rid()
>+            self.assertEqual(next_lo + 1, next_free_rid3)
>+
>+            # Check the result of allocate_rid() matches.
>+            rid = self.samdb.allocate_rid()
>+            self.assertEqual(next_free_rid3, rid)
>+
>+            # Check that the result of next_free_rid() has now changed.
>+            next_free_rid4 = self.samdb.next_free_rid()
>+            self.assertEqual(rid + 1, next_free_rid4)
>+
>+            # Check the range of available RIDs.
>+            free_lo, free_hi = self.samdb.free_rid_bounds()
>+            self.assertEqual(rid + 1, free_lo)
>+            self.assertEqual(next_hi, free_hi)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_no_free_rids(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Exhaust our current pool of RIDs.
>+            pool_lo = 2000
>+            pool_hi = 2999
>+            msg = ldb.Message()
>+            msg.dn = self.rid_set_dn
>+            msg["rIDPreviousAllocationPool"] = (
>+                ldb.MessageElement(str((pool_hi << 32) | pool_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDPreviousAllocationPool"))
>+            msg["rIDAllocationPool"] = (
>+                ldb.MessageElement(str((pool_hi << 32) | pool_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDAllocationPool"))
>+            msg["rIDNextRID"] = (
>+            ldb.MessageElement(str(pool_hi),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDNextRID"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure that calculating the next free RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.next_free_rid()
>+
>+            self.assertEqual("RID pools out of RIDs", err.exception.args[1])
>+
>+            # Ensure we can still allocate a new RID.
>+            self.samdb.allocate_rid()
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_new_ridset(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Test what happens with RID Set values set to zero (similar to
>+            # when a RID Set is first created, except we also set
>+            # rIDAllocationPool to zero).
>+            msg = ldb.Message()
>+            msg.dn = self.rid_set_dn
>+            msg["rIDPreviousAllocationPool"] = (
>+                ldb.MessageElement("0",
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDPreviousAllocationPool"))
>+            msg["rIDAllocationPool"] = (
>+                ldb.MessageElement("0",
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDAllocationPool"))
>+            msg["rIDNextRID"] = (
>+                ldb.MessageElement("0",
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDNextRID"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure that calculating the next free RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.next_free_rid()
>+
>+            self.assertEqual("RID pools out of RIDs", err.exception.args[1])
>+
>+            # Set values for the next pool.
>+            pool_lo = 2000
>+            pool_hi = 2999
>+            msg = ldb.Message()
>+            msg.dn = self.rid_set_dn
>+            msg["rIDAllocationPool"] = (
>+                ldb.MessageElement(str((pool_hi << 32) | pool_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDAllocationPool"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure the next free RID value is equal to the next pool's lower
>+            # bound.
>+            next_free_rid5 = self.samdb.next_free_rid()
>+            self.assertEqual(pool_lo, next_free_rid5)
>+
>+            # Check the range of available RIDs.
>+            free_lo, free_hi = self.samdb.free_rid_bounds()
>+            self.assertEqual(pool_lo, free_lo)
>+            self.assertEqual(pool_hi, free_hi)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_move_to_new_pool(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Test moving to a new pool from the previous pool.
>+            pool_lo = 2000
>+            pool_hi = 2999
>+            new_pool_lo = 4500
>+            new_pool_hi = 4599
>+            msg = ldb.Message()
>+            msg.dn = self.rid_set_dn
>+            msg["rIDPreviousAllocationPool"] = (
>+                ldb.MessageElement(str((pool_hi << 32) | pool_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDPreviousAllocationPool"))
>+            msg["rIDAllocationPool"] = (
>+                ldb.MessageElement(str((new_pool_hi << 32) | new_pool_lo),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDAllocationPool"))
>+            msg["rIDNextRID"] = (
>+                ldb.MessageElement(str(pool_hi - 1),
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDNextRID"))
>+            self.samdb.modify(msg)
>+
>+            # We should have remained in the previous pool.
>+            next_free_rid6 = self.samdb.next_free_rid()
>+            self.assertEqual(pool_hi, next_free_rid6)
>+
>+            # Check the range of available RIDs.
>+            free_lo, free_hi = self.samdb.free_rid_bounds()
>+            self.assertEqual(pool_hi, free_lo)
>+            self.assertEqual(pool_hi, free_hi)
>+
>+            # Allocate a new RID.
>+            rid2 = self.samdb.allocate_rid()
>+            self.assertEqual(next_free_rid6, rid2)
>+
>+            # We should now move to the next pool.
>+            next_free_rid7 = self.samdb.next_free_rid()
>+            self.assertEqual(new_pool_lo, next_free_rid7)
>+
>+            # Check the new range of available RIDs.
>+            free_lo2, free_hi2 = self.samdb.free_rid_bounds()
>+            self.assertEqual(new_pool_lo, free_lo2)
>+            self.assertEqual(new_pool_hi, free_hi2)
>+
>+            # Ensure that allocate_rid() matches.
>+            rid3 = self.samdb.allocate_rid()
>+            self.assertEqual(next_free_rid7, rid3)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_no_ridsetreferences(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Delete the rIDSetReferences attribute.
>+            msg = ldb.Message()
>+            msg.dn = self.server_ref_dn
>+            msg["rIDSetReferences"] = (
>+                ldb.MessageElement([],
>+                                   ldb.FLAG_MOD_DELETE,
>+                                   "rIDSetReferences"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure calculating the next free RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.next_free_rid()
>+
>+            enum, estr = err.exception.args
>+            self.assertEqual(ldb.ERR_NO_SUCH_ATTRIBUTE, enum)
>+            self.assertIn("No RID Set DN - "
>+                          "Cannot find attribute rIDSetReferences of %s "
>+                          "to calculate reference dn" % self.server_ref_dn,
>+                          estr)
>+
>+            # Ensure allocating a new RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.allocate_rid()
>+
>+            enum, estr = err.exception.args
>+            self.assertEqual(ldb.ERR_ENTRY_ALREADY_EXISTS, enum)
>+            self.assertIn("No RID Set DN - "
>+                          "Failed to add RID Set %s - "
>+                          "Entry %s already exists" %
>+                          (self.rid_set_dn, self.rid_set_dn),
>+                          estr)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>+    def test_ridalloc_no_rid_set(self):
>+        self.samdb.transaction_start()
>+        try:
>+            # Set the rIDSetReferences attribute to not point to a RID Set.
>+            fake_rid_set_str = self.account_dn
>+            msg = ldb.Message()
>+            msg.dn = self.server_ref_dn
>+            msg["rIDSetReferences"] = (
>+                ldb.MessageElement(fake_rid_set_str,
>+                                   ldb.FLAG_MOD_REPLACE,
>+                                   "rIDSetReferences"))
>+            self.samdb.modify(msg)
>+
>+            # Ensure calculating the next free RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.next_free_rid()
>+
>+            enum, estr = err.exception.args
>+            self.assertEqual(ldb.ERR_OPERATIONS_ERROR, enum)
>+            self.assertIn("Bad RID Set " + fake_rid_set_str, estr)
>+
>+            # Ensure allocating a new RID fails.
>+            with self.assertRaises(ldb.LdbError) as err:
>+                self.samdb.allocate_rid()
>+
>+            enum, estr = err.exception.args
>+            self.assertEqual(ldb.ERR_OPERATIONS_ERROR, enum)
>+            self.assertIn("Bad RID Set " + fake_rid_set_str,  estr)
>+        finally:
>+            self.samdb.transaction_cancel()
>+
>     def test_get_oid_from_attrid(self):
>         oid = self.samdb.get_oid_from_attid(591614)
>         self.assertEqual(oid, "1.2.840.113556.1.4.1790")
>-- 
>2.31.1.362.g311531c9de
>
>
>From fbe985cb5edd9420742937eb6523bee6ce989b79 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 27 May 2021 15:35:35 +1200
>Subject: [PATCH 10/10] netcmd: Use next_free_rid() function to calculate a SID
> for restoring a backup
>
>This means we won't get errors if the DC doesn't have a rIDNextRID
>attribute, but we will still error if there is no RID Set or if all its
>pools are exhausted.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14669
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 59d293b60608172ae61551c642d13d3b215924e4)
>---
> python/samba/netcmd/domain_backup.py | 57 +++++++---------------------
> selftest/knownfail.d/bug-14669       |  1 -
> 2 files changed, 14 insertions(+), 44 deletions(-)
> delete mode 100644 selftest/knownfail.d/bug-14669
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index 8d261fd4b97..f441e7407ee 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -62,50 +62,21 @@ from samba.credentials import SMB_SIGNING_REQUIRED
> # This ensures that the restored DC's SID won't clash with any other RIDs
> # already in use in the domain
> def get_sid_for_restore(samdb, logger):
>-    # Find the DN of the RID set of the server
>-    res = samdb.search(base=ldb.Dn(samdb, samdb.get_serverName()),
>-                       scope=ldb.SCOPE_BASE, attrs=["serverReference"])
>-    server_ref_dn = ldb.Dn(samdb, str(res[0]['serverReference'][0]))
>-    res = samdb.search(base=server_ref_dn,
>-                       scope=ldb.SCOPE_BASE,
>-                       attrs=['rIDSetReferences'])
>-    rid_set_dn = ldb.Dn(samdb, str(res[0]['rIDSetReferences'][0]))
>-
>-    # Get the alloc pools and next RID of the RID set
>-    res = samdb.search(base=rid_set_dn,
>-                       scope=ldb.SCOPE_SUBTREE,
>-                       expression="(rIDNextRID=*)",
>-                       attrs=['rIDAllocationPool',
>-                              'rIDPreviousAllocationPool',
>-                              'rIDNextRID'])
>-
>-    # Decode the bounds of the RID allocation pools
>+    # Allocate a new RID without modifying the database. This should be safe,
>+    # because we acquire the RID master role after creating an account using
>+    # this RID during the restore process. Acquiring the RID master role
>+    # creates a new RID pool which we will fetch RIDs from, so we shouldn't get
>+    # duplicates.
>     try:
>-        rid = int(res[0].get('rIDNextRID')[0])
>-    except IndexError:
>-        logger.info("The RID pool for this DC is not initalized "
>-                    "(e.g. it may be a fairly new DC).")
>-        logger.info("To initialize it, create a temporary user on this DC "
>-                    "(you can delete it later).")
>-        raise CommandError("Cannot create backup - "
>-                           "please initialize this DC's RID pool first.")
>-
>-    def split_val(num):
>-        high = (0xFFFFFFFF00000000 & int(num)) >> 32
>-        low = 0x00000000FFFFFFFF & int(num)
>-        return low, high
>-    pool_l, pool_h = split_val(res[0].get('rIDPreviousAllocationPool')[0])
>-    npool_l, npool_h = split_val(res[0].get('rIDAllocationPool')[0])
>-
>-    # Calculate next RID based on pool bounds
>-    if rid == npool_h:
>-        raise CommandError('Out of RIDs, finished AllocPool')
>-    if rid == pool_h:
>-        if pool_h == npool_h:
>-            raise CommandError('Out of RIDs, finished PrevAllocPool.')
>-        rid = npool_l
>-    else:
>-        rid += 1
>+        rid = samdb.next_free_rid()
>+    except LdbError as err:
>+        logger.info("A SID could not be allocated for restoring the domain. "
>+                    "Either no RID Set was found on this DC, "
>+                    "or the RID Set was not usable.")
>+        logger.info("To initialise this DC's RID pools, obtain a RID Set from "
>+                    "this domain's RID master, or run samba-tool dbcheck "
>+                    "to fix the existing RID Set.")
>+        raise CommandError("Cannot create backup", err)
> 
>     # Construct full SID
>     sid = dom_sid(samdb.get_domain_sid())
>diff --git a/selftest/knownfail.d/bug-14669 b/selftest/knownfail.d/bug-14669
>deleted file mode 100644
>index 83f8cbfe456..00000000000
>--- a/selftest/knownfail.d/bug-14669
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba.tests.domain_backup_offline.samba.tests.domain_backup_offline.DomainBackupOfflineCmp.test_domain_backup_offline_join_restore
>-- 
>2.31.1.362.g311531c9de
>

Flags:

abartlet: review+
dbagnall: review+

Actions: View

Attachments on bug 14743: 16664 | 16665