Attachment 16664 Details for Bug 14743 – patch for 4.13

[patch] patch for 4.13

samba-tool-domain-backup-issues-for-4.13.patch (text/plain), 59.24 KB, created by Joseph Sutton on 2021-06-16 23:38:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Sutton

Created: 2021-06-16 23:38:02 UTC

Size: 59.24 KB

patch

obsolete

>From d09d5df963116b3ce50bcc6022ef0ac2a1c07ae1 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 18 Nov 2020 12:11:10 +1300
>Subject: [PATCH 01/12] samba-tool domain backup: Confirm the sidForRestore we
> will put into the backup is free
>
>Otherwise the administrator might only find there is a problem once they
>attempt to restore the domain!
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14575
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>(cherry picked from commit 15609cb91986b3e29c5b1a3b6c69c04829f43eb4)
>---
> python/samba/netcmd/domain_backup.py | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index a3dc7fb454f..25dba78fa03 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -107,6 +107,32 @@ def get_sid_for_restore(samdb, logger):
> 
>     # Construct full SID
>     sid = dom_sid(samdb.get_domain_sid())
>+    sid_for_restore = str(sid) + '-' + str(rid)
>+
>+    # Confirm the SID is not already in use
>+    try:
>+        res = samdb.search(scope=ldb.SCOPE_BASE,
>+                           base='<SID=%s>' % sid_for_restore,
>+                           attrs=[],
>+                           controls=['show_deleted:1',
>+                                     'show_recycled:1'])
>+        if len(res) != 1:
>+            # This case makes no sense, but neither does a corrupt RID set
>+            raise CommandError("Cannot create backup - "
>+                               "this DC's RID pool is corrupt, "
>+                               "the next SID (%s) appears to be in use." %
>+                               sid_for_restore)
>+        raise CommandError("Cannot create backup - "
>+                           "this DC's RID pool is corrupt, "
>+                           "the next SID %s points to existing object %s. "
>+                           "Please run samba-tool dbcheck on the source DC." %
>+                           (sid_for_restore, res[0].dn))
>+    except ldb.LdbError as e:
>+        (enum, emsg) = e.args
>+        if enum != ldb.ERR_NO_SUCH_OBJECT:
>+            # We want NO_SUCH_OBJECT, anything else is a serious issue
>+            raise
>+
>     return str(sid) + '-' + str(rid)
> 
> 
>-- 
>2.31.1.362.g311531c9de
>
>
>From ba3fd299d03d734a24585fd9dba679ec2994397d Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 13 Nov 2020 15:26:07 +1300
>Subject: [PATCH 02/12] samba-tool: Give better error information when the
> 'domain backup restore' fails with a duplicate SID
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14575
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
>
>Autobuild-User(master): Gary Lockyer <gary@samba.org>
>Autobuild-Date(master): Thu Nov 26 21:15:40 UTC 2020 on sn-devel-184
>
>(cherry picked from commit 8ad82ae66157c893a2b84d353ec4d9feb4815ede)
>---
> python/samba/netcmd/domain_backup.py | 45 +++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
>diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
>index 25dba78fa03..fff39fcbc2a 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -27,6 +27,7 @@ import tdb
> import samba.getopt as options
> from samba.samdb import SamDB, get_default_backend_store
> import ldb
>+from ldb import LdbError
> from samba.samba3 import libsmb_samba_internal as libsmb
> from samba.samba3 import param as s3param
> from samba.ntacls import backup_online, backup_restore, backup_offline
>@@ -576,7 +577,49 @@ class cmd_domain_backup_restore(cmd_fsmo_seize):
>                            attrs=['sidForRestore'])
>         sid = res[0].get('sidForRestore')[0]
>         logger.info('Creating account with SID: ' + str(sid))
>-        ctx.join_add_objects(specified_sid=dom_sid(str(sid)))
>+        try:
>+            ctx.join_add_objects(specified_sid=dom_sid(str(sid)))
>+        except LdbError as e:
>+            (enum, emsg) = e.args
>+            if enum != ldb.ERR_CONSTRAINT_VIOLATION:
>+                raise
>+
>+            dup_res = []
>+            try:
>+                dup_res = samdb.search(base=ldb.Dn(samdb, "<SID=%s>" % sid),
>+                                       scope=ldb.SCOPE_BASE,
>+                                       attrs=['objectGUID'],
>+                                       controls=["show_deleted:0",
>+                                                 "show_recycled:0"])
>+            except LdbError as dup_e:
>+                if enum != ldb.ERR_NO_SUCH_OBJECT:
>+                    raise e
>+
>+            if (len(dup_res) != 1):
>+                raise e
>+
>+            objectguid = samdb.schema_format_value("objectGUID",
>+                                                       dup_res[0]["objectGUID"][0])
>+            objectguid = objectguid.decode('utf-8')
>+            logger.error("The RID Pool on the source DC for the backup in %s "
>+                         "may be corrupt "
>+                         "or in conflict with SIDs already allocated "
>+                         "in the domain. " % backup_file)
>+            logger.error("Running 'samba-tool dbcheck' on the source "
>+                         "DC (and obtaining a new backup) may correct the issue.")
>+            logger.error("Alternatively please obtain a new backup "
>+                         "against a different DC.")
>+            logger.error("The SID we wish to use (%s) is recorded in "
>+                         "@SAMBA_DSDB as the sidForRestore attribute."
>+                         % sid)
>+
>+            raise CommandError("Domain restore failed because there "
>+                               "is already an existing object (%s) "
>+                               "with SID %s and objectGUID %s.  "
>+                               "This conflicts with "
>+                               "the new DC account we want to add "
>+                               "for the restored domain.   " % (
>+                                dup_res[0].dn, sid, objectguid))
> 
>         m = ldb.Message()
>         m.dn = ldb.Dn(samdb, '@ROOTDSE')
>-- 
>2.31.1.362.g311531c9de
>
>
>From f263c8b3c07cac803cea8a721442946c9679a112 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 03/12] 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 b3f14c9e8a95436458e69510bb53ba436121c71e 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 04/12] 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 645b629350a15e26d661bf95e7f608d17de06219 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 05/12] 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 fff39fcbc2a..c5749a5c8e2 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -1097,6 +1097,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)))
>@@ -1109,22 +1113,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 9febaea96ee7553c1bb83d9322ee062e1c4e0b78 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 06/12] 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 c5749a5c8e2..c5a7083f536 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -305,7 +305,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)
>@@ -529,7 +530,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:
>@@ -637,7 +639,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,
>@@ -929,7 +932,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()
>@@ -1017,7 +1021,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')
>@@ -1042,7 +1047,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)
>@@ -1094,7 +1099,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
>@@ -1149,7 +1155,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)
>@@ -1161,7 +1168,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 81aabe8461e33ded0073c93d9cdfb1dd856e8f70 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 07/12] 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 c5a7083f536..3a1bb924a9a 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -594,11 +594,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 320f2affb7996f24d9ef3cf086eb157d24d20b43 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 08/12] 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 3cc3bdaaed05d1c092d3c860afa2fe8f897e577b 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 09/12] 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 258c9122edc..e16e64fb2f0 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -884,7 +884,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 a20bee8fe5ff295705849d6e64bdfa763e8123c1 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 10/12] 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 d903babb406..d13c5e3b7a2 100644
>--- a/python/samba/samdb.py
>+++ b/python/samba/samdb.py
>@@ -1285,6 +1285,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 5b24f5065708157a2474261eec9a5bc1e7014fa7 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 11/12] 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 fb9bde39c3578a6d71dbb4e2e7064c906bb173d0 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 12/12] 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 3a1bb924a9a..a629b31d70f 100644
>--- a/python/samba/netcmd/domain_backup.py
>+++ b/python/samba/netcmd/domain_backup.py
>@@ -61,50 +61,21 @@ from samba.ndr import ndr_pack
> # 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