Attachment 18085 Details for Bug 15424 – patch in master backported to Samba 4.19 v5

[patch] patch in master backported to Samba 4.19 v5

CVE-2023-4154-dirsync-4.19-v5.patch (text/plain), 75.34 KB, created by Andrew Bartlett on 2023-09-08 03:26:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2023-09-08 03:26:49 UTC

Size: 75.34 KB

patch

obsolete

>From e43a164556b9e1d118b3c982677a32ddd9da963f Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Wed, 2 Aug 2023 10:44:32 +0200
>Subject: [PATCH 1/9] CVE-2023-4154 s4:dsdb:tests: Fix code spelling
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>(cherry picked from commit b29793ffdee5d9b9c1c05830622e80f7faec7670)
>---
> source4/dsdb/tests/python/acl.py                   | 12 ++++++------
> .../dsdb/tests/python/ad_dc_search_performance.py  |  2 +-
> source4/dsdb/tests/python/confidential_attr.py     |  2 +-
> source4/dsdb/tests/python/dirsync.py               |  8 ++++----
> source4/dsdb/tests/python/ldap.py                  | 14 +++++++-------
> source4/dsdb/tests/python/ldap_modify_order.py     |  4 ++--
> source4/dsdb/tests/python/ldap_syntaxes.py         |  4 ++--
> source4/dsdb/tests/python/login_basics.py          |  2 +-
> source4/dsdb/tests/python/password_settings.py     |  4 ++--
> source4/dsdb/tests/python/passwords.py             |  4 ++--
> source4/dsdb/tests/python/sam.py                   |  2 +-
> source4/dsdb/tests/python/sec_descriptor.py        | 14 +++++++-------
> source4/dsdb/tests/python/token_group.py           |  4 ++--
> source4/dsdb/tests/python/user_account_control.py  |  2 +-
> 14 files changed, 39 insertions(+), 39 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
>index 13785316abb..49d9ca6383d 100755
>--- a/source4/dsdb/tests/python/acl.py
>+++ b/source4/dsdb/tests/python/acl.py
>@@ -228,7 +228,7 @@ class AclAddTests(AclTests):
>         self.assertEqual(len(res), 0)
> 
>     def test_add_u1(self):
>-        """Testing OU with the rights of Doman Admin not creator of the OU """
>+        """Testing OU with the rights of Domain Admin not creator of the OU """
>         self.assert_top_ou_deleted()
>         # Change descriptor for top level OU
>         self.ldb_owner.create_ou("OU=test_add_ou1," + self.base_dn)
>@@ -241,7 +241,7 @@ class AclAddTests(AclTests):
>         self.ldb_notowner.newgroup("test_add_group1", groupou="OU=test_add_ou2,OU=test_add_ou1",
>                                    grouptype=samba.dsdb.GTYPE_DISTRIBUTION_DOMAIN_LOCAL_GROUP)
>         # Make sure we HAVE created the two objects -- user and group
>-        # !!! We should not be able to do that, but however beacuse of ACE ordering our inherited Deny ACE
>+        # !!! We should not be able to do that, but however because of ACE ordering our inherited Deny ACE
>         # !!! comes after explicit (A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA) that comes from somewhere
>         res = self.ldb_admin.search(self.base_dn, expression="(distinguishedName=%s,%s)" % ("CN=test_add_user1,OU=test_add_ou2,OU=test_add_ou1", self.base_dn))
>         self.assertGreater(len(res), 0)
>@@ -302,7 +302,7 @@ class AclAddTests(AclTests):
>         self.assertEqual(len(res), 0)
> 
>     def test_add_u4(self):
>-        """ 4 Testing OU with the rights of Doman Admin creator of the OU"""
>+        """ 4 Testing OU with the rights of Domain Admin creator of the OU"""
>         self.assert_top_ou_deleted()
>         self.ldb_owner.create_ou("OU=test_add_ou1," + self.base_dn)
>         self.ldb_owner.create_ou("OU=test_add_ou2,OU=test_add_ou1," + self.base_dn)
>@@ -4298,7 +4298,7 @@ class AclDeleteTests(AclTests):
>         self.assertEqual(len(res), 0)
> 
>     def test_delete_u3(self):
>-        """User indentified by SID has RIGHT_DELETE to another User object"""
>+        """User identified by SID has RIGHT_DELETE to another User object"""
>         user_dn = self.get_user_dn("test_delete_user1")
>         # Create user that we try to delete
>         self.ldb_admin.newuser("test_delete_user1", self.user_pass)
>@@ -4589,7 +4589,7 @@ class AclCARTests(AclTests):
>         minPwdAge = self.ldb_admin.get_minPwdAge()
>         # Reset the "minPwdAge" as it was before
>         self.addCleanup(self.ldb_admin.set_minPwdAge, minPwdAge)
>-        # Set it temporarely to "0"
>+        # Set it temporarily to "0"
>         self.ldb_admin.set_minPwdAge("0")
> 
>         self.user_with_wp = "acl_car_user1"
>@@ -5041,7 +5041,7 @@ class AclUndeleteTests(AclTests):
>         self.sd_utils.dacl_add_ace(self.deleted_dn2, mod)
>         self.undelete_deleted(self.deleted_dn2, self.testuser2_dn)
> 
>-        # attempt undelete with simultanious addition of url, WP to which is denied
>+        # attempt undelete with simultaneous addition of url, WP to which is denied
>         mod = "(OD;;WP;9a9a0221-4a5b-11d1-a9c3-0000f80367c1;;%s)" % str(self.sid)
>         self.sd_utils.dacl_add_ace(self.deleted_dn3, mod)
>         try:
>diff --git a/source4/dsdb/tests/python/ad_dc_search_performance.py b/source4/dsdb/tests/python/ad_dc_search_performance.py
>index 0afd7a2582e..44e468097d8 100644
>--- a/source4/dsdb/tests/python/ad_dc_search_performance.py
>+++ b/source4/dsdb/tests/python/ad_dc_search_performance.py
>@@ -180,7 +180,7 @@ class UserTests(samba.tests.TestCase):
>         maybe_not = ['!(', '']
>         joiners = ['&', '|']
> 
>-        # The number of permuations is 18432, which is not huge but
>+        # The number of permutations is 18432, which is not huge but
>         # would take hours to search. So we take a sample.
>         all_permutations = list(itertools.product(joiners,
>                                                   classes, classes,
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index eb75da6374f..8ca56bd1023 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -722,7 +722,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
> 
>         self.attr_filters = [None, ["*"], ["name"]]
> 
>-        # Note dirsync behaviour is slighty different for the attribute under
>+        # Note dirsync behaviour is slightly different for the attribute under
>         # test - when you have full access rights, it only returns the objects
>         # that actually have this attribute (i.e. it doesn't return an empty
>         # message with just the DN). So we add the 'name' attribute into the
>diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
>index 1ac719e4332..ca0947e2d21 100755
>--- a/source4/dsdb/tests/python/dirsync.py
>+++ b/source4/dsdb/tests/python/dirsync.py
>@@ -338,7 +338,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertEqual(len(res.msgs[0]), 3)
> 
>     def test_dirsync_othernc(self):
>-        """Check that dirsync return information for entries that are normaly referrals (ie. other NCs)"""
>+        """Check that dirsync return information for entries that are normally referrals (ie. other NCs)"""
>         res = self.ldb_admin.search(self.base_dn,
>                                     expression="(objectclass=configuration)",
>                                     attrs=["name"],
>@@ -459,7 +459,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         delete_force(self.ldb_admin, ouname)
> 
>     def test_dirsync_linkedattributes(self):
>-        """Check that dirsync returnd deleted objects too"""
>+        """Check that dirsync returned deleted objects too"""
>         # Let's search for members
>         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>         res = self.ldb_simple.search(self.base_dn,
>@@ -541,7 +541,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertEqual(len(res[0].get("member")), 0)
> 
>     def test_dirsync_deleted_items(self):
>-        """Check that dirsync returnd deleted objects too"""
>+        """Check that dirsync returned deleted objects too"""
>         # Let's create an OU
>         ouname = "OU=testou3,%s" % self.base_dn
>         self.ouname = ouname
>@@ -712,7 +712,7 @@ class ExtendedDirsyncTests(SimpleDirsyncTests):
>         self.assertIn(b">;<SID=010500000000000515", resEX0[0]["member"][0])
> 
>     def test_dirsync_deleted_items(self):
>-        """Check that dirsync returnd deleted objects too"""
>+        """Check that dirsync returned deleted objects too"""
>         # Let's create an OU
>         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>         ouname = "OU=testou3,%s" % self.base_dn
>diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py
>index a50a5f7b8d6..a8995782db5 100755
>--- a/source4/dsdb/tests/python/ldap.py
>+++ b/source4/dsdb/tests/python/ldap.py
>@@ -155,7 +155,7 @@ class BasicTests(samba.tests.TestCase):
>             (num, _) = e.args
>             self.assertEqual(num, ERR_CONSTRAINT_VIOLATION)
> 
>-        # We cannot instanciate from an abstract object class ("connectionPoint"
>+        # We cannot instantiate from an abstract object class ("connectionPoint"
>         # or "leaf"). In the first case we use "connectionPoint" (subclass of
>         # "leaf") to prevent a naming violation - this returns us a
>         # "ERR_UNWILLING_TO_PERFORM" since it is not structural. In the second
>@@ -178,7 +178,7 @@ class BasicTests(samba.tests.TestCase):
>             (num, _) = e.args
>             self.assertEqual(num, ERR_OBJECT_CLASS_VIOLATION)
> 
>-        # Objects instanciated using "satisfied" abstract classes (concrete
>+        # Objects instantiated using "satisfied" abstract classes (concrete
>         # subclasses) are allowed
>         self.ldb.add({
>             "dn": "cn=ldaptestuser,cn=users," + self.base_dn,
>@@ -214,7 +214,7 @@ class BasicTests(samba.tests.TestCase):
>             "objectClass": "person"})
> 
>         # We can remove derivation classes of the structural objectclass
>-        # but they're going to be readded afterwards
>+        # but they're going to be re-added afterwards
>         m = Message()
>         m.dn = Dn(ldb, "cn=ldaptestuser,cn=users," + self.base_dn)
>         m["objectClass"] = MessageElement("top", FLAG_MOD_DELETE,
>@@ -1824,7 +1824,7 @@ delete: description
>         delete_force(self.ldb, "cn=ldaptestcontainer," + self.base_dn)
> 
>     def test_groupType_int32(self):
>-        """Test groupType (int32) behaviour (should appear to be casted to a 32 bit signed integer before comparsion)"""
>+        """Test groupType (int32) behaviour (should appear to be casted to a 32 bit signed integer before comparison)"""
> 
>         res1 = ldb.search(base=self.base_dn, scope=SCOPE_SUBTREE,
>                           attrs=["groupType"], expression="groupType=2147483653")
>@@ -2158,7 +2158,7 @@ servicePrincipalName: host/ldaptest2computer29
>             "givenname": "testy",
>             "sn": "ldap user2"})
> 
>-        # Testing Ambigious Name Resolution
>+        # Testing Ambiguous Name Resolution
>         # Testing ldb.search for (&(anr=ldap testy)(objectClass=user))
>         res = ldb.search(expression="(&(anr=ldap testy)(objectClass=user))")
>         self.assertEqual(len(res), 3, "Found only %d of 3 for (&(anr=ldap testy)(objectClass=user))" % len(res))
>@@ -2402,7 +2402,7 @@ member: cn=ldaptestuser2,cn=users,""" + self.base_dn + """
> 
>         # Testing ldb.search for (&(member=CN=ldaptestuser4,CN=ldaptestcontainer2," + self.base_dn + ")(objectclass=group)) to check subtree renames and linked attributes"
>         res = ldb.search(self.base_dn, expression="(&(member=CN=ldaptestuser4,CN=ldaptestcontainer2," + self.base_dn + ")(objectclass=group))", scope=SCOPE_SUBTREE)
>-        self.assertEqual(len(res), 1, "Could not find (&(member=CN=ldaptestuser4,CN=ldaptestcontainer2," + self.base_dn + ")(objectclass=group)), perhaps linked attributes are not consistant with subtree renames?")
>+        self.assertEqual(len(res), 1, "Could not find (&(member=CN=ldaptestuser4,CN=ldaptestcontainer2," + self.base_dn + ")(objectclass=group)), perhaps linked attributes are not consistent with subtree renames?")
> 
>         # Testing ldb.rename (into itself) of cn=ldaptestcontainer2," + self.base_dn + " to cn=ldaptestcontainer,cn=ldaptestcontainer2," + self.base_dn
>         try:
>@@ -3002,7 +3002,7 @@ nTSecurityDescriptor: """ + sddl
>             delete_force(self.ldb, user_dn)
>         #
>         # Test modify_ldif() with SDDL security descriptor input
>-        # New desctiptor test
>+        # New descriptor test
>         #
>         try:
>             self.ldb.add_ldif("""
>diff --git a/source4/dsdb/tests/python/ldap_modify_order.py b/source4/dsdb/tests/python/ldap_modify_order.py
>index 54f89597eab..80c4a3a7fe6 100644
>--- a/source4/dsdb/tests/python/ldap_modify_order.py
>+++ b/source4/dsdb/tests/python/ldap_modify_order.py
>@@ -101,7 +101,7 @@ class ModifyOrderTests(samba.tests.TestCase):
> 
>         clusters = {}
>         for i, attrs in enumerate(permutations(mod_attrs)):
>-            # for each permuation we construct a string describing the
>+            # for each permutation we construct a string describing the
>             # requested operations, and a string describing the result
>             # (which may be an exception). The we cluster the
>             # attribute strings by their results.
>@@ -249,7 +249,7 @@ class ModifyOrderTests(samba.tests.TestCase):
>         self._test_modify_order(start_attrs, mod_attrs)
> 
>     def test_modify_order_inapplicable(self):
>-        #attrbutes that don't go on a user
>+        #attributes that don't go on a user
>         start_attrs = [("objectclass", "user"),
>                        ("givenName", "a")]
> 
>diff --git a/source4/dsdb/tests/python/ldap_syntaxes.py b/source4/dsdb/tests/python/ldap_syntaxes.py
>index d8efb3a105e..081c28073f1 100755
>--- a/source4/dsdb/tests/python/ldap_syntaxes.py
>+++ b/source4/dsdb/tests/python/ldap_syntaxes.py
>@@ -210,7 +210,7 @@ name: """ + object_name + """
>                               expression="(%s=S:5:ABCDE)" % self.dn_string_attribute)
>         self.assertEqual(len(res), 0)
> 
>-        # search by DN+Stirng
>+        # search by DN+String
>         res = self.ldb.search(base=self.base_dn,
>                               scope=SCOPE_SUBTREE,
>                               expression="(%s=S:5:ABCDE:%s)" % (self.dn_string_attribute, self.base_dn))
>@@ -293,7 +293,7 @@ name: """ + object_name + """
>             self.assertEqual(num, ERR_CONSTRAINT_VIOLATION)
> 
>     def test_dn_binary(self):
>-        # add obeject with correct value
>+        # add object with correct value
>         object_name1 = "obj-DN-Binary1" + time.strftime("%s", time.gmtime())
>         ldif = self._get_object_ldif(object_name1, self.dn_binary_class_name, self.dn_binary_class_ldap_display_name,
>                                      self.dn_binary_attribute, ": B:4:1234:" + self.base_dn)
>diff --git a/source4/dsdb/tests/python/login_basics.py b/source4/dsdb/tests/python/login_basics.py
>index babe04879b1..46983e85a72 100755
>--- a/source4/dsdb/tests/python/login_basics.py
>+++ b/source4/dsdb/tests/python/login_basics.py
>@@ -75,7 +75,7 @@ class BasicUserAuthTests(BasePasswordTestCase):
>             ldap_url = self.host_url
>             print("Performs a lockout attempt against LDAP using NTLM")
> 
>-        # get the intial logon values for this user
>+        # get the initial logon values for this user
>         res = self._check_account(userdn,
>                                   badPwdCount=0,
>                                   badPasswordTime=("greater", 0),
>diff --git a/source4/dsdb/tests/python/password_settings.py b/source4/dsdb/tests/python/password_settings.py
>index bac89f3e3c8..1fc0c05cdd4 100644
>--- a/source4/dsdb/tests/python/password_settings.py
>+++ b/source4/dsdb/tests/python/password_settings.py
>@@ -300,7 +300,7 @@ class PasswordSettingsTestCase(PasswordTestCase):
>         self.set_attribute(group2, "member", group1)
>         self.assert_PSO_applied(user, group2_pso)
> 
>-        # add another level to the group heirachy & check this PSO takes effect
>+        # add another level to the group hierarchy & check this PSO takes effect
>         self.set_attribute(group3, "member", group2)
>         self.assert_PSO_applied(user, group3_pso)
> 
>@@ -727,7 +727,7 @@ class PasswordSettingsTestCase(PasswordTestCase):
>     # the msDS-ResultantPSO attribute on a user that doesn't exist yet (it
>     # won't have any group membership or PSOs applied directly against it yet).
>     # In theory it's possible to still get an applicable PSO via the user's
>-    # primaryGroupID (i.e. 'Domain Users' by default). However, testing aginst
>+    # primaryGroupID (i.e. 'Domain Users' by default). However, testing against
>     # Windows shows that the PSO doesn't take effect during the user add
>     # operation. (However, the Windows GUI tools presumably adds the user in 2
>     # steps, because it does enforce the PSO for users added via the GUI).
>diff --git a/source4/dsdb/tests/python/passwords.py b/source4/dsdb/tests/python/passwords.py
>index cd78be4e8b1..de69293da76 100755
>--- a/source4/dsdb/tests/python/passwords.py
>+++ b/source4/dsdb/tests/python/passwords.py
>@@ -1365,12 +1365,12 @@ userPassword: thatsAcomplPASS4
>     def test_zero_length(self):
>         # Get the old "minPwdLength"
>         minPwdLength = self.ldb.get_minPwdLength()
>-        # Set it temporarely to "0"
>+        # Set it temporarily to "0"
>         self.ldb.set_minPwdLength("0")
> 
>         # Get the old "pwdProperties"
>         pwdProperties = self.ldb.get_pwdProperties()
>-        # Set them temporarely to "0" (to deactivate eventually the complexity)
>+        # Set them temporarily to "0" (to deactivate eventually the complexity)
>         self.ldb.set_pwdProperties("0")
> 
>         self.ldb.setpassword("(sAMAccountName=testuser)", "")
>diff --git a/source4/dsdb/tests/python/sam.py b/source4/dsdb/tests/python/sam.py
>index 08365beb2c6..c413b0813e0 100755
>--- a/source4/dsdb/tests/python/sam.py
>+++ b/source4/dsdb/tests/python/sam.py
>@@ -3775,7 +3775,7 @@ class SamTests(samba.tests.TestCase):
>     def test_protected_sid_objects(self):
>         """Test deletion of objects with RID < 1000"""
>         # a list of some well-known sids
>-        # objects in Builtin are aready covered by objectclass
>+        # objects in Builtin are already covered by objectclass
>         protected_list = [
>             ["CN=Domain Admins", "CN=Users,"],
>             ["CN=Schema Admins", "CN=Users,"],
>diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py
>index 86c6e4f8799..4e3230f3548 100755
>--- a/source4/dsdb/tests/python/sec_descriptor.py
>+++ b/source4/dsdb/tests/python/sec_descriptor.py
>@@ -689,7 +689,7 @@ class OwnerGroupDescriptorTests(DescriptorTests):
> 
>     # Tests for SCHEMA
> 
>-    # Defalt descriptor tests ##################################################################
>+    # Default descriptor tests ##################################################################
> 
>     def test_130(self):
>         user_name = "testuser1"
>@@ -949,7 +949,7 @@ class OwnerGroupDescriptorTests(DescriptorTests):
> 
>     # Tests for CONFIGURATION
> 
>-    # Defalt descriptor tests ##################################################################
>+    # Default descriptor tests ##################################################################
> 
>     def test_160(self):
>         user_name = "testuser1"
>@@ -2531,7 +2531,7 @@ class RightsAttributesTests(DescriptorTests):
>         _ldb = self.get_ldb_connection("testuser_attr", "samba123@")
>         res = _ldb.search(base=object_dn, expression="", scope=SCOPE_BASE,
>                           attrs=["sDRightsEffective"])
>-        # user whould have no rights at all
>+        # user should have no rights at all
>         self.assertEqual(len(res), 1)
>         self.assertEqual(str(res[0]["sDRightsEffective"][0]), "0")
>         # give the user Write DACL and see what happens
>@@ -2539,7 +2539,7 @@ class RightsAttributesTests(DescriptorTests):
>         self.sd_utils.dacl_add_ace(object_dn, mod)
>         res = _ldb.search(base=object_dn, expression="", scope=SCOPE_BASE,
>                           attrs=["sDRightsEffective"])
>-        # user whould have DACL_SECURITY_INFORMATION
>+        # user should have DACL_SECURITY_INFORMATION
>         self.assertEqual(len(res), 1)
>         self.assertEqual(str(res[0]["sDRightsEffective"][0]), ("%d") % SECINFO_DACL)
>         # give the user Write Owners and see what happens
>@@ -2547,14 +2547,14 @@ class RightsAttributesTests(DescriptorTests):
>         self.sd_utils.dacl_add_ace(object_dn, mod)
>         res = _ldb.search(base=object_dn, expression="", scope=SCOPE_BASE,
>                           attrs=["sDRightsEffective"])
>-        # user whould have DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION
>+        # user should have DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION
>         self.assertEqual(len(res), 1)
>         self.assertEqual(str(res[0]["sDRightsEffective"][0]), ("%d") % (SECINFO_DACL | SECINFO_GROUP | SECINFO_OWNER))
>-        # no way to grant security privilege bu adding ACE's so we use a memeber of Domain Admins
>+        # no way to grant security privilege bu adding ACE's so we use a member of Domain Admins
>         _ldb = self.get_ldb_connection("testuser_attr2", "samba123@")
>         res = _ldb.search(base=object_dn, expression="", scope=SCOPE_BASE,
>                           attrs=["sDRightsEffective"])
>-        # user whould have DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION
>+        # user should have DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION
>         self.assertEqual(len(res), 1)
>         self.assertEqual(str(res[0]["sDRightsEffective"][0]),
>                           ("%d") % (SECINFO_DACL | SECINFO_GROUP | SECINFO_OWNER | SECINFO_SACL))
>diff --git a/source4/dsdb/tests/python/token_group.py b/source4/dsdb/tests/python/token_group.py
>index 632002a34d5..df45ee0f331 100755
>--- a/source4/dsdb/tests/python/token_group.py
>+++ b/source4/dsdb/tests/python/token_group.py
>@@ -153,7 +153,7 @@ class StaticTokenTest(samba.tests.TestCase):
>         extra_sidset = set(extra_sids)
> 
>         if len(missing_sidset.symmetric_difference(extra_sidset)):
>-            print("dn token sids unexpeted")
>+            print("dn token sids unexpected")
>             print("tokengroups: %s" % dn_tokengroups)
>             print("user sids: %s" % self.user_sids)
>             print("actual difference: %s" % missing_sidset)
>@@ -427,7 +427,7 @@ class DynamicTokenTest(samba.tests.TestCase):
>         extra_sidset = set(extra_sids)
> 
>         if len(missing_sidset.symmetric_difference(extra_sidset)):
>-            print("dn token sids unexpeted")
>+            print("dn token sids unexpected")
>             print("tokengroups: %s" % dn_tokengroups)
>             print("user sids: %s" % self.user_sids)
>             print("actual difference: %s" % missing_sidset)
>diff --git a/source4/dsdb/tests/python/user_account_control.py b/source4/dsdb/tests/python/user_account_control.py
>index ca99ce3fba1..04e61e9e791 100755
>--- a/source4/dsdb/tests/python/user_account_control.py
>+++ b/source4/dsdb/tests/python/user_account_control.py
>@@ -852,7 +852,7 @@ class UserAccountControlTests(samba.tests.TestCase):
>             self.add_computer_ldap(computername, others={"userAccountControl": [str(bit_add)]})
>             delete_force(self.admin_samdb, "CN=%s,%s" % (computername, self.OU))
>             if bit in priv_bits:
>-                self.fail("Unexpectdly able to set userAccountControl bit 0x%08X (%s) on %s"
>+                self.fail("Unexpectedly able to set userAccountControl bit 0x%08X (%s) on %s"
>                           % (bit, bit_str, computername))
> 
>         except LdbError as e4:
>-- 
>2.25.1
>
>
>From 51a2165f4dd704141c090e12a78c9cf0d0827933 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 11:55:55 +1200
>Subject: [PATCH 2/9] CVE-2023-4154 dsdb/tests: Do not run SimpleDirsyncTests
> twice
>
>To re-use setup code, the super-class must have no test_*() methods
>otherwise these will be run as well as the class-local tests.
>
>We rename tests that would otherwise have duplicate names
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail                   | 2 +-
> source4/dsdb/tests/python/dirsync.py | 7 ++-----
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 37c75d7ca33..4e34effbbd1 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -151,7 +151,7 @@
> ^samba4.smb2.acls.*.inheritflags
> ^samba4.smb2.acls.*.owner
> ^samba4.smb2.acls.*.ACCESSBASED
>-^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.test_dirsync_deleted_items
>+^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.SimpleDirsyncTests.test_dirsync_deleted_items_OBJECT_SECURITY
> #^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.*
> ^samba4.libsmbclient.opendir.(NT1|SMB3).opendir # This requires netbios browsing
> ^samba4.rpc.drsuapi.*.drsuapi.DsGetDomainControllerInfo$.*$$
>diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
>index ca0947e2d21..ad136b7efee 100755
>--- a/source4/dsdb/tests/python/dirsync.py
>+++ b/source4/dsdb/tests/python/dirsync.py
>@@ -458,7 +458,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertTrue(res[0].get("name") is not None)
>         delete_force(self.ldb_admin, ouname)
> 
>-    def test_dirsync_linkedattributes(self):
>+    def test_dirsync_linkedattributes_OBJECT_SECURITY(self):
>         """Check that dirsync returned deleted objects too"""
>         # Let's search for members
>         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>@@ -585,9 +585,6 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>                                     expression="(&(objectClass=organizationalUnit)(!(isDeleted=*)))",
>                                     controls=controls)
> 
>-
>-class ExtendedDirsyncTests(SimpleDirsyncTests):
>-
>     def test_dirsync_linkedattributes_range(self):
>         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>         res = self.ldb_admin.search(self.base_dn,
>@@ -711,7 +708,7 @@ class ExtendedDirsyncTests(SimpleDirsyncTests):
>         self.assertIn(b"<GUID=", resEX0[0]["member"][0])
>         self.assertIn(b">;<SID=010500000000000515", resEX0[0]["member"][0])
> 
>-    def test_dirsync_deleted_items(self):
>+    def test_dirsync_deleted_items_OBJECT_SECURITY(self):
>         """Check that dirsync returned deleted objects too"""
>         # Let's create an OU
>         self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>-- 
>2.25.1
>
>
>From 69932b8d1fd3ada85ffc285ebf961769fd094a6e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 13:15:40 +1200
>Subject: [PATCH 3/9] CVE-2023-4154 dsdb/tests: Use self.addCleanup() and
> delete_force()
>
>Thie helps ensure this test is reliable even in spite of errors while
>running.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/tests/python/confidential_attr.py | 6 ++----
> source4/dsdb/tests/python/dirsync.py           | 6 +-----
> 2 files changed, 3 insertions(+), 9 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index 8ca56bd1023..3997848f8f9 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -98,7 +98,9 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
> 
>         userou = "OU=conf-attr-test"
>         self.ou = "{0},{1}".format(userou, self.base_dn)
>+        samba.tests.delete_force(self.ldb_admin, self.ou, controls=['tree_delete:1'])
>         self.ldb_admin.create_ou(self.ou)
>+        self.addCleanup(samba.tests.delete_force, self.ldb_admin, self.ou, controls=['tree_delete:1'])
> 
>         # use a common username prefix, so we can use sAMAccountName=CATC-* as
>         # a search filter to only return the users we're interested in
>@@ -139,10 +141,6 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>                          "{0} searchFlags already {1}".format(self.conf_attr,
>                                                               search_flags))
> 
>-    def tearDown(self):
>-        super(ConfidentialAttrCommon, self).tearDown()
>-        self.ldb_admin.delete(self.ou, ["tree_delete:1"])
>-
>     def add_attr(self, dn, attr, value):
>         m = Message()
>         m.dn = Dn(self.ldb_admin, dn)
>diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
>index ad136b7efee..e06b85bc749 100755
>--- a/source4/dsdb/tests/python/dirsync.py
>+++ b/source4/dsdb/tests/python/dirsync.py
>@@ -137,10 +137,6 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         if self.ouname:
>             delete_force(self.ldb_admin, self.ouname)
>         self.sd_utils.modify_sd_on_dn(self.base_dn, self.desc_sddl)
>-        try:
>-            self.ldb_admin.deletegroup("testgroup")
>-        except Exception:
>-            pass
> 
>     # def test_dirsync_errors(self):
> 
>@@ -499,6 +495,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertEqual(len(res[0].get("member")), size)
> 
>         self.ldb_admin.newgroup("testgroup")
>+        self.addCleanup(self.ldb_admin.deletegroup, "testgroup")
>         self.ldb_admin.add_remove_group_members("testgroup", [self.simple_user],
>                                                 add_members_operation=True)
> 
>@@ -537,7 +534,6 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>                                     attrs=["member"],
>                                     controls=[control1])
> 
>-        self.ldb_admin.deletegroup("testgroup")
>         self.assertEqual(len(res[0].get("member")), 0)
> 
>     def test_dirsync_deleted_items(self):
>-- 
>2.25.1
>
>
>From 5e5b6bd1e143818ff75d6fcd06a62b219b79e62d Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 14:44:28 +1200
>Subject: [PATCH 4/9] CVE-2023-4154 dsdb/tests: Force the test attribute to be
> not-confidential at the start
>
>Rather than fail, if the last run failed to reset things, just force
>the DC into the required state.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/tests/python/confidential_attr.py | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index 3997848f8f9..ee7f554a008 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -136,10 +136,12 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
> 
>         # sanity-check the flag is not already set (this'll cause problems if
>         # previous test run didn't clean up properly)
>-        search_flags = self.get_attr_search_flags(self.attr_dn)
>-        self.assertEqual(0, int(search_flags) & SEARCH_FLAG_CONFIDENTIAL,
>-                         "{0} searchFlags already {1}".format(self.conf_attr,
>-                                                              search_flags))
>+        search_flags = int(self.get_attr_search_flags(self.attr_dn))
>+        if search_flags & SEARCH_FLAG_CONFIDENTIAL:
>+            self.set_attr_search_flags(self.attr_dn, str(search_flags &~ SEARCH_FLAG_CONFIDENTIAL))
>+        search_flags = int(self.get_attr_search_flags(self.attr_dn))
>+        self.assertEqual(0, search_flags & SEARCH_FLAG_CONFIDENTIAL,
>+                         f"{self.conf_attr} searchFlags did not reset to omit SEARCH_FLAG_CONFIDENTIAL ({search_flags})")
> 
>     def add_attr(self, dn, attr, value):
>         m = Message()
>-- 
>2.25.1
>
>
>From 06208e165651a5d90aafb10ed40ccc6785d1ef94 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 11:56:56 +1200
>Subject: [PATCH 5/9] CVE-2023-4154 dsdb/tests: Check that secret attributes
> are not visible with DirSync ever.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/dirsync         |  1 +
> source4/dsdb/tests/python/dirsync.py | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
> create mode 100644 selftest/knownfail.d/dirsync
>
>diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
>new file mode 100644
>index 00000000000..9367f92e109
>--- /dev/null
>+++ b/selftest/knownfail.d/dirsync
>@@ -0,0 +1 @@
>+^samba4.ldap.dirsync.python$.*$.__main__.SimpleDirsyncTests.test_dirsync_unicodePwd
>\ No newline at end of file
>diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
>index e06b85bc749..2cacaf01251 100755
>--- a/source4/dsdb/tests/python/dirsync.py
>+++ b/source4/dsdb/tests/python/dirsync.py
>@@ -742,6 +742,18 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertEqual(guid2, guid)
>         self.assertEqual(str(res[0].dn), "")
> 
>+    def test_dirsync_unicodePwd(self):
>+        res = self.ldb_admin.search(self.base_dn,
>+                                    attrs=["unicodePwd", "supplementalCredentials", "samAccountName"],
>+                                    expression="(samAccountName=krbtgt)",
>+                                    controls=["dirsync:1:0:0"])
>+
>+        self.assertTrue(len(res) == 1)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue("samAccountName" in res[0])
>+        self.assertTrue(res[0].get("samAccountName"))
>+        self.assertTrue(res[0].get("unicodePwd") is None)
>+        self.assertTrue(res[0].get("supplementalCredentials") is None)
> 
> if not getattr(opts, "listtests", False):
>     lp = sambaopts.get_loadparm()
>-- 
>2.25.1
>
>
>From f1c02852315502c5eadf790a27ed3abb912f143d Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 11:18:46 +1200
>Subject: [PATCH 6/9] CVE-2023-4154 dsdb/tests: Speed up DirSync test by only
> checking positive matches once
>
>When we (expect to) get back a result, do not waste time against a potentially
>slow server confirming we also get back results for all the other attribute
>combinations.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/tests/python/confidential_attr.py | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index ee7f554a008..678a5a82948 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -742,7 +742,13 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         #   want to weed out results from any previous test runs
>         search = "(&{0}{1})".format(expr, self.extra_filter)
> 
>-        for attr in self.attr_filters:
>+        # If we expect to return multiple results, only check the first
>+        if expected_num > 0:
>+            attr_filters = [self.attr_filters[0]]
>+        else:
>+            attr_filters = self.attr_filters
>+
>+        for attr in attr_filters:
>             res = samdb.search(base_dn, expression=search, scope=SCOPE_SUBTREE,
>                                attrs=attr, controls=self.dirsync)
>             self.assertEqual(len(res), expected_num,
>-- 
>2.25.1
>
>
>From b5a9d803c46dc39eecd55cf046a25973ca5306e1 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 14:30:19 +1200
>Subject: [PATCH 7/9] CVE-2023-4154 dsdb/tests: Add test for
> SEARCH_FLAG_RODC_ATTRIBUTE behaviour
>
>SEARCH_FLAG_RODC_ATTRIBUTE should be like SEARCH_FLAG_CONFIDENTIAL,
>but for DirSync and DRS replication.  Accounts with
>GUID_DRS_GET_CHANGES rights should not be able to read this
>attribute.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> .../dsdb/tests/python/confidential_attr.py    | 45 ++++++++++++++++---
> 1 file changed, 40 insertions(+), 5 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index 678a5a82948..edbc9593204 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -30,13 +30,15 @@ import time
> from samba.tests.subunitrun import SubunitOptions, TestProgram
> import samba.getopt as options
> from ldb import SCOPE_BASE, SCOPE_SUBTREE
>-from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_PRESERVEONDELETE
>+from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_RODC_ATTRIBUTE, SEARCH_FLAG_PRESERVEONDELETE
> from ldb import Message, MessageElement, Dn
> from ldb import FLAG_MOD_REPLACE, FLAG_MOD_ADD
> from samba.auth import system_session
> from samba import gensec, sd_utils
> from samba.samdb import SamDB
> from samba.credentials import Credentials, DONT_USE_KERBEROS
>+from samba.dcerpc import security
>+
> import samba.tests
> import samba.dsdb
> 
>@@ -137,11 +139,11 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>         # sanity-check the flag is not already set (this'll cause problems if
>         # previous test run didn't clean up properly)
>         search_flags = int(self.get_attr_search_flags(self.attr_dn))
>-        if search_flags & SEARCH_FLAG_CONFIDENTIAL:
>-            self.set_attr_search_flags(self.attr_dn, str(search_flags &~ SEARCH_FLAG_CONFIDENTIAL))
>+        if search_flags & SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE:
>+            self.set_attr_search_flags(self.attr_dn, str(search_flags &~ (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE)))
>         search_flags = int(self.get_attr_search_flags(self.attr_dn))
>-        self.assertEqual(0, search_flags & SEARCH_FLAG_CONFIDENTIAL,
>-                         f"{self.conf_attr} searchFlags did not reset to omit SEARCH_FLAG_CONFIDENTIAL ({search_flags})")
>+        self.assertEqual(0, search_flags & (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE),
>+                         f"{self.conf_attr} searchFlags did not reset to omit SEARCH_FLAG_CONFIDENTIAL and SEARCH_FLAG_RODC_ATTRIBUTE ({search_flags})")
> 
>     def add_attr(self, dn, attr, value):
>         m = Message()
>@@ -1098,5 +1100,38 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         user_matching, user_non_matching = time_searches(self.ldb_user)
>         assertRangesOverlap(user_matching, user_non_matching)
> 
>+# Check that using the dirsync controls doesn't reveal confidential
>+# "RODC filtered attribute" values to users with only
>+# GUID_DRS_GET_CHANGES.  The tests is so similar to the Confidential
>+# attribute test we base it on that.
>+class RodcFilteredAttrDirsync(ConfidentialAttrTestDirsync):
>+
>+    def setUp(self):
>+        super().setUp()
>+        self.dirsync = ["dirsync:1:0:1000"]
>+
>+        user_sid = self.sd_utils.get_object_sid(self.get_user_dn(self.user))
>+        mod = "(OA;;CR;%s;;%s)" % (security.GUID_DRS_GET_CHANGES,
>+                                   str(user_sid))
>+        self.sd_utils.dacl_add_ace(self.base_dn, mod)
>+
>+        self.ldb_user = self.get_ldb_connection(self.user, self.user_pass)
>+
>+        self.addCleanup(self.sd_utils.dacl_delete_aces, self.base_dn, mod)
>+
>+    def make_attr_confidential(self):
>+        """Marks the attribute under test as being confidential AND RODC
>+           filtered (which should mean it is not visible with only
>+           GUID_DRS_GET_CHANGES)
>+        """
>+
>+        # work out the original 'searchFlags' value before we overwrite it
>+        old_value = self.get_attr_search_flags(self.attr_dn)
>+
>+        self.set_attr_search_flags(self.attr_dn, str(SEARCH_FLAG_RODC_ATTRIBUTE|SEARCH_FLAG_CONFIDENTIAL))
>+
>+        # reset the value after the test completes
>+        self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value)
>+
> 
> TestProgram(module=__name__, opts=subunitopts)
>-- 
>2.25.1
>
>
>From 49564ce145334c512b5ae5b8fea4bba48a4826ed Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 22 Aug 2023 15:08:17 +1200
>Subject: [PATCH 8/9] CVE-2023-4154 dsdb/tests: Extend attribute read DirSync
> tests
>
>The aim here is to document the expected (even if not implemented)
>SEARCH_FLAG_RODC_ATTRIBUTE vs SEARCH_FLAG_CONFIDENTIAL, behaviour, so
>that any change once CVE-2023-4154 is fixed can be noted.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/dirsync         |  15 +-
> source4/dsdb/tests/python/dirsync.py | 456 +++++++++++++++++++++++----
> 2 files changed, 414 insertions(+), 57 deletions(-)
>
>diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
>index 9367f92e109..db098549a08 100644
>--- a/selftest/knownfail.d/dirsync
>+++ b/selftest/knownfail.d/dirsync
>@@ -1 +1,14 @@
>-^samba4.ldap.dirsync.python$.*$.__main__.SimpleDirsyncTests.test_dirsync_unicodePwd
>\ No newline at end of file
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_OBJ_SEC_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES_OBJECT_SECURITY_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_with_GET_CHANGES_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element$.*$
>diff --git a/source4/dsdb/tests/python/dirsync.py b/source4/dsdb/tests/python/dirsync.py
>index 2cacaf01251..a0691f0afe0 100755
>--- a/source4/dsdb/tests/python/dirsync.py
>+++ b/source4/dsdb/tests/python/dirsync.py
>@@ -3,6 +3,7 @@
> # Unit tests for dirsync control
> # Copyright (C) Matthieu Patou <mat@matws.net> 2011
> # Copyright (C) Jelmer Vernooij <jelmer@samba.org> 2014
>+# Copyright (C) Catalyst.Net Ltd
> #
> # This program is free software; you can redistribute it and/or modify
> # it under the terms of the GNU General Public License as published by
>@@ -30,7 +31,8 @@ import base64
> import ldb
> from ldb import LdbError, SCOPE_BASE
> from ldb import Message, MessageElement, Dn
>-from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE
>+from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE, FLAG_MOD_REPLACE
>+from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_RODC_ATTRIBUTE
> from samba.dcerpc import security, misc, drsblobs
> from samba.ndr import ndr_unpack, ndr_pack
> 
>@@ -60,7 +62,6 @@ if len(args) < 1:
> host = args.pop()
> if "://" not in host:
>     ldaphost = "ldap://%s" % host
>-    ldapshost = "ldaps://%s" % host
> else:
>     ldaphost = host
>     start = host.rindex("://")
>@@ -77,8 +78,8 @@ creds = credopts.get_credentials(lp)
> class DirsyncBaseTests(samba.tests.TestCase):
> 
>     def setUp(self):
>-        super(DirsyncBaseTests, self).setUp()
>-        self.ldb_admin = SamDB(ldapshost, credentials=creds, session_info=system_session(lp), lp=lp)
>+        super().setUp()
>+        self.ldb_admin = SamDB(ldaphost, credentials=creds, session_info=system_session(lp), lp=lp)
>         self.base_dn = self.ldb_admin.domain_dn()
>         self.domain_sid = security.dom_sid(self.ldb_admin.get_domain_sid())
>         self.user_pass = samba.generate_random_password(12, 16)
>@@ -87,63 +88,60 @@ class DirsyncBaseTests(samba.tests.TestCase):
>         # used for anonymous login
>         print("baseDN: %s" % self.base_dn)
> 
>-    def get_user_dn(self, name):
>-        return "CN=%s,CN=Users,%s" % (name, self.base_dn)
>+        userou = "OU=dirsync-test"
>+        self.ou = f"{userou},{self.base_dn}"
>+        samba.tests.delete_force(self.ldb_admin, self.ou, controls=['tree_delete:1'])
>+        self.ldb_admin.create_ou(self.ou)
>+        self.addCleanup(samba.tests.delete_force, self.ldb_admin, self.ou, controls=['tree_delete:1'])
> 
>-    def get_ldb_connection(self, target_username, target_password):
>-        creds_tmp = Credentials()
>-        creds_tmp.set_username(target_username)
>-        creds_tmp.set_password(target_password)
>-        creds_tmp.set_domain(creds.get_domain())
>-        creds_tmp.set_realm(creds.get_realm())
>-        creds_tmp.set_workstation(creds.get_workstation())
>-        creds_tmp.set_gensec_features(creds_tmp.get_gensec_features()
>-                                      | gensec.FEATURE_SEAL)
>-        creds_tmp.set_kerberos_state(DONT_USE_KERBEROS)  # kinit is too expensive to use in a tight loop
>-        ldb_target = SamDB(url=ldaphost, credentials=creds_tmp, lp=lp)
>-        return ldb_target
>-
>-
>-# tests on ldap add operations
>-class SimpleDirsyncTests(DirsyncBaseTests):
>-
>-    def setUp(self):
>-        super(SimpleDirsyncTests, self).setUp()
>         # Regular user
>         self.dirsync_user = "test_dirsync_user"
>         self.simple_user = "test_simple_user"
>         self.admin_user = "test_admin_user"
>-        self.ouname = None
>+        self.dirsync_pass = self.user_pass
>+        self.simple_pass = self.user_pass
>+        self.admin_pass = self.user_pass
> 
>-        self.ldb_admin.newuser(self.dirsync_user, self.user_pass)
>-        self.ldb_admin.newuser(self.simple_user, self.user_pass)
>-        self.ldb_admin.newuser(self.admin_user, self.user_pass)
>+        self.ldb_admin.newuser(self.dirsync_user, self.dirsync_pass, userou=userou)
>+        self.ldb_admin.newuser(self.simple_user, self.simple_pass, userou=userou)
>+        self.ldb_admin.newuser(self.admin_user, self.admin_pass, userou=userou)
>         self.desc_sddl = self.sd_utils.get_sd_as_sddl(self.base_dn)
> 
>         user_sid = self.sd_utils.get_object_sid(self.get_user_dn(self.dirsync_user))
>         mod = "(OA;;CR;%s;;%s)" % (security.GUID_DRS_GET_CHANGES,
>                                    str(user_sid))
>         self.sd_utils.dacl_add_ace(self.base_dn, mod)
>+        self.addCleanup(self.sd_utils.dacl_delete_aces, self.base_dn, mod)
> 
>         # add admins to the Domain Admins group
>         self.ldb_admin.add_remove_group_members("Domain Admins", [self.admin_user],
>                                                 add_members_operation=True)
> 
>-    def tearDown(self):
>-        super(SimpleDirsyncTests, self).tearDown()
>-        delete_force(self.ldb_admin, self.get_user_dn(self.dirsync_user))
>-        delete_force(self.ldb_admin, self.get_user_dn(self.simple_user))
>-        delete_force(self.ldb_admin, self.get_user_dn(self.admin_user))
>-        if self.ouname:
>-            delete_force(self.ldb_admin, self.ouname)
>-        self.sd_utils.modify_sd_on_dn(self.base_dn, self.desc_sddl)
>+    def get_user_dn(self, name):
>+        return ldb.Dn(self.ldb_admin, "CN={0},{1}".format(name, self.ou))
>+
>+    def get_ldb_connection(self, target_username, target_password):
>+        creds_tmp = Credentials()
>+        creds_tmp.set_username(target_username)
>+        creds_tmp.set_password(target_password)
>+        creds_tmp.set_domain(creds.get_domain())
>+        creds_tmp.set_realm(creds.get_realm())
>+        creds_tmp.set_workstation(creds.get_workstation())
>+        creds_tmp.set_gensec_features(creds_tmp.get_gensec_features()
>+                                      | gensec.FEATURE_SEAL)
>+        creds_tmp.set_kerberos_state(DONT_USE_KERBEROS)  # kinit is too expensive to use in a tight loop
>+        ldb_target = SamDB(url=ldaphost, credentials=creds_tmp, lp=lp)
>+        return ldb_target
>+
>+# tests on ldap add operations
>+class SimpleDirsyncTests(DirsyncBaseTests):
> 
>     # def test_dirsync_errors(self):
> 
>     def test_dirsync_supported(self):
>         """Test the basic of the dirsync is supported"""
>         self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.user_pass)
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         res = self.ldb_admin.search(self.base_dn, expression="samaccountname=*", controls=["dirsync:1:0:1"])
>         res = self.ldb_dirsync.search(self.base_dn, expression="samaccountname=*", controls=["dirsync:1:0:1"])
>         try:
>@@ -169,8 +167,8 @@ class SimpleDirsyncTests(DirsyncBaseTests):
> 
>     def test_dirsync_errors(self):
>         """Test if dirsync returns the correct LDAP errors in case of pb"""
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>-        self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>         try:
>             self.ldb_simple.search(self.base_dn,
>                                    expression="samaccountname=*",
>@@ -292,11 +290,11 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>                                     attrs=["parentGUID"],
>                                     controls=["dirsync:1:0:1"])
>         self.assertEqual(len(res.msgs), 0)
>-        ouname = "OU=testou,%s" % self.base_dn
>+        ouname = "OU=testou,%s" % self.ou
>         self.ouname = ouname
>         self.ldb_admin.create_ou(ouname)
>         delta = Message()
>-        delta.dn = Dn(self.ldb_admin, str(ouname))
>+        delta.dn = Dn(self.ldb_admin, ouname)
>         delta["cn"] = MessageElement("test ou",
>                                      FLAG_MOD_ADD,
>                                      "cn")
>@@ -457,7 +455,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>     def test_dirsync_linkedattributes_OBJECT_SECURITY(self):
>         """Check that dirsync returned deleted objects too"""
>         # Let's search for members
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         res = self.ldb_simple.search(self.base_dn,
>                                      expression="(name=Administrators)",
>                                      controls=["dirsync:1:1:1"])
>@@ -582,7 +580,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>                                     controls=controls)
> 
>     def test_dirsync_linkedattributes_range(self):
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         res = self.ldb_admin.search(self.base_dn,
>                                     attrs=["member;range=1-1"],
>                                     expression="(name=Administrators)",
>@@ -594,7 +592,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertTrue(len(res[0].get("member")) > 0)
> 
>     def test_dirsync_linkedattributes_range_user(self):
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         try:
>             res = self.ldb_simple.search(self.base_dn,
>                                          attrs=["member;range=1-1"],
>@@ -608,7 +606,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
> 
>     def test_dirsync_linkedattributes(self):
>         flag_incr_linked = 2147483648
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         res = self.ldb_admin.search(self.base_dn,
>                                     attrs=["member"],
>                                     expression="(name=Administrators)",
>@@ -676,7 +674,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>     def test_dirsync_extended_dn(self):
>         """Check that dirsync works together with the extended_dn control"""
>         # Let's search for members
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         res = self.ldb_simple.search(self.base_dn,
>                                      expression="(name=Administrators)",
>                                      controls=["dirsync:1:1:1"])
>@@ -707,7 +705,7 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>     def test_dirsync_deleted_items_OBJECT_SECURITY(self):
>         """Check that dirsync returned deleted objects too"""
>         # Let's create an OU
>-        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass)
>+        self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass)
>         ouname = "OU=testou3,%s" % self.base_dn
>         self.ouname = ouname
>         self.ldb_admin.create_ou(ouname)
>@@ -742,18 +740,364 @@ class SimpleDirsyncTests(DirsyncBaseTests):
>         self.assertEqual(guid2, guid)
>         self.assertEqual(str(res[0].dn), "")
> 
>-    def test_dirsync_unicodePwd(self):
>+class SpecialDirsyncTests(DirsyncBaseTests):
>+
>+    def setUp(self):
>+        super().setUp()
>+
>+        self.schema_dn = self.ldb_admin.get_schema_basedn()
>+
>+        # the tests work by setting the 'Confidential' or 'RODC Filtered' bit in the searchFlags
>+        # for an existing schema attribute. This only works against Windows if
>+        # the systemFlags does not have FLAG_SCHEMA_BASE_OBJECT set for the
>+        # schema attribute being modified. There are only a few attributes that
>+        # meet this criteria (most of which only apply to 'user' objects)
>+        self.conf_attr = "homePostalAddress"
>+        attr_cn = "CN=Address-Home"
>+        # schemaIdGuid for homePostalAddress (used for ACE tests)
>+        self.attr_dn = f"{attr_cn},{self.schema_dn}"
>+
>+        userou = "OU=conf-attr-test"
>+        self.ou = "{0},{1}".format(userou, self.base_dn)
>+        samba.tests.delete_force(self.ldb_admin, self.ou, controls=['tree_delete:1'])
>+        self.ldb_admin.create_ou(self.ou)
>+        self.addCleanup(samba.tests.delete_force, self.ldb_admin, self.ou, controls=['tree_delete:1'])
>+
>+        # add a test object with this attribute set
>+        self.conf_value = "abcdef"
>+        self.conf_user = "conf-user"
>+        self.ldb_admin.newuser(self.conf_user, self.user_pass, userou=userou)
>+        self.conf_dn = self.get_user_dn(self.conf_user)
>+        self.add_attr(self.conf_dn, self.conf_attr, self.conf_value)
>+
>+        # sanity-check the flag is not already set (this'll cause problems if
>+        # previous test run didn't clean up properly)
>+
>+        search_flags = int(self.get_attr_search_flags(self.attr_dn))
>+        if search_flags & SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE:
>+            self.set_attr_search_flags(self.attr_dn, str(search_flags &~ (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE)))
>+        search_flags = int(self.get_attr_search_flags(self.attr_dn))
>+        self.assertEqual(0, search_flags & (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE),
>+                         f"{self.conf_attr} searchFlags did not reset to omit SEARCH_FLAG_CONFIDENTIAL and SEARCH_FLAG_RODC_ATTRIBUTE ({search_flags})")
>+
>+        # work out the original 'searchFlags' value before we overwrite it
>+        old_value = self.get_attr_search_flags(self.attr_dn)
>+
>+        self.set_attr_search_flags(self.attr_dn, str(self.flag_under_test))
>+
>+        # reset the value after the test completes
>+        self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value)
>+
>+    def add_attr(self, dn, attr, value):
>+        m = Message()
>+        m.dn = dn
>+        m[attr] = MessageElement(value, FLAG_MOD_ADD, attr)
>+        self.ldb_admin.modify(m)
>+
>+    def set_attr_search_flags(self, attr_dn, flags):
>+        """Modifies the searchFlags for an object in the schema"""
>+        m = Message()
>+        m.dn = Dn(self.ldb_admin, attr_dn)
>+        m['searchFlags'] = MessageElement(flags, FLAG_MOD_REPLACE,
>+                                          'searchFlags')
>+        self.ldb_admin.modify(m)
>+
>+        # note we have to update the schema for this change to take effect (on
>+        # Windows, at least)
>+        self.ldb_admin.set_schema_update_now()
>+
>+    def get_attr_search_flags(self, attr_dn):
>+        res = self.ldb_admin.search(attr_dn, scope=SCOPE_BASE,
>+                                    attrs=['searchFlags'])
>+        return res[0]['searchFlags'][0]
>+
>+    def find_under_current_ou(self, res):
>+        for msg in res:
>+            if msg.dn == self.conf_dn:
>+                return msg
>+        self.fail(f"Failed to find object {self.conf_dn} in {len(res)} results")
>+
>+
>+class ConfidentialDirsyncTests(SpecialDirsyncTests):
>+
>+    def setUp(self):
>+        self.flag_under_test = SEARCH_FLAG_CONFIDENTIAL
>+        super().setUp()
>+
>+    def test_unicodePwd_normal(self):
>         res = self.ldb_admin.search(self.base_dn,
>                                     attrs=["unicodePwd", "supplementalCredentials", "samAccountName"],
>-                                    expression="(samAccountName=krbtgt)",
>-                                    controls=["dirsync:1:0:0"])
>+                                    expression=f"(samAccountName={self.conf_user})")
>+
>+        msg = res[0]
>+
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get("unicodePwd") is None)
>+        self.assertTrue(msg.get("supplementalCredentials") is None)
>+
>+    def _test_dirsync_unicodePwd(self, ldb_conn, control=None, insist_on_empty_element=False):
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=["unicodePwd", "supplementalCredentials", "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})",
>+                         controls=[control])
>+
>+        msg = self.find_under_current_ou(res)
> 
>-        self.assertTrue(len(res) == 1)
>+        self.assertTrue("samAccountName" in msg)
>         # This form ensures this is a case insensitive comparison
>-        self.assertTrue("samAccountName" in res[0])
>-        self.assertTrue(res[0].get("samAccountName"))
>-        self.assertTrue(res[0].get("unicodePwd") is None)
>-        self.assertTrue(res[0].get("supplementalCredentials") is None)
>+        self.assertTrue(msg.get("samAccountName"))
>+        if insist_on_empty_element:
>+            self.assertTrue(msg.get("unicodePwd") is not None)
>+            self.assertEqual(len(msg.get("unicodePwd")), 0)
>+            self.assertTrue(msg.get("supplementalCredentials") is not None)
>+            self.assertEqual(len(msg.get("supplementalCredentials")), 0)
>+        else:
>+            self.assertTrue(msg.get("unicodePwd") is None
>+                            or len(msg.get("unicodePwd")) == 0)
>+            self.assertTrue(msg.get("supplementalCredentials") is None
>+                            or len(msg.get("supplementalCredentials")) == 0)
>+
>+    def test_dirsync_unicodePwd_OBJ_SEC(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:1:0")
>+
>+    def test_dirsync_unicodePwd_OBJ_SEC_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:1:0", insist_on_empty_element=True)
>+
>+    def test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:1:0")
>+
>+    def test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:1:0", insist_on_empty_element=True)
>+
>+    def test_dirsync_unicodePwd_with_GET_CHANGES(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:0:0")
>+
>+    def test_dirsync_unicodePwd_with_GET_CHANGES_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_unicodePwd(ldb_conn, control="dirsync:1:0:0", insist_on_empty_element=True)
>+
>+    def test_normal(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=[self.conf_attr, "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})")
>+
>+        msg = res[0]
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get(self.conf_attr) is None)
>+
>+    def _test_dirsync_OBJECT_SECURITY(self, ldb_conn, insist_on_empty_element=False):
>+        res = ldb_conn.search(self.base_dn,
>+                              attrs=[self.conf_attr, "samAccountName"],
>+                              expression=f"(samAccountName={self.conf_user})",
>+                              controls=["dirsync:1:1:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        if insist_on_empty_element:
>+            self.assertTrue(msg.get(self.conf_attr) is not None)
>+            self.assertEqual(len(msg.get(self.conf_attr)), 0)
>+        else:
>+            self.assertTrue(msg.get(self.conf_attr) is None
>+                            or len(msg.get(self.conf_attr)) == 0)
>+
>+    def test_dirsync_OBJECT_SECURITY(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def test_dirsync_OBJECT_SECURITY_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn, insist_on_empty_element=True)
>+
>+    def test_dirsync_with_GET_CHANGES(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=[self.conf_attr, "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})",
>+                         controls=["dirsync:1:0:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get(self.conf_attr))
>+        self.assertEqual(len(msg.get(self.conf_attr)), 1)
>+
>+    def test_dirsync_with_GET_CHANGES_OBJECT_SECURITY(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def test_dirsync_with_GET_CHANGES_OBJECT_SECURITY_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn, insist_on_empty_element=True)
>+
>+class FilteredDirsyncTests(SpecialDirsyncTests):
>+
>+    def setUp(self):
>+        self.flag_under_test = SEARCH_FLAG_RODC_ATTRIBUTE
>+        super().setUp()
>+
>+    def test_attr(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=[self.conf_attr, "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})")
>+
>+        msg = res[0]
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get(self.conf_attr))
>+        self.assertEqual(len(msg.get(self.conf_attr)), 1)
>+
>+    def _test_dirsync_OBJECT_SECURITY(self, ldb_conn):
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=[self.conf_attr, "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})",
>+                         controls=["dirsync:1:1:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get(self.conf_attr))
>+        self.assertEqual(len(msg.get(self.conf_attr)), 1)
>+
>+    def test_dirsync_OBJECT_SECURITY(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def test_dirsync_OBJECT_SECURITY_with_GET_CHANGES(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def _test_dirsync_with_GET_CHANGES(self, insist_on_empty_element=False):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         expression=f"(samAccountName={self.conf_user})",
>+                         controls=["dirsync:1:0:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        if insist_on_empty_element:
>+            self.assertTrue(msg.get(self.conf_attr) is not None)
>+            self.assertEqual(len(msg.get(self.conf_attr)), 0)
>+        else:
>+            self.assertTrue(msg.get(self.conf_attr) is None
>+                            or len(msg.get(self.conf_attr)) == 0)
>+
>+    def test_dirsync_with_GET_CHANGES(self):
>+        self._test_dirsync_with_GET_CHANGES()
>+
>+    def test_dirsync_with_GET_CHANGES_insist_on_empty_element(self):
>+        self._test_dirsync_with_GET_CHANGES(insist_on_empty_element=True)
>+
>+    def test_dirsync_with_GET_CHANGES_attr(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        try:
>+            res = ldb_conn.search(self.base_dn,
>+                                  attrs=[self.conf_attr, "samAccountName"],
>+                                  expression=f"(samAccountName={self.conf_user})",
>+                                  controls=["dirsync:1:0:0"])
>+            self.fail("ldb.search() should have failed with LDAP_INSUFFICIENT_ACCESS_RIGHTS")
>+        except ldb.LdbError as e:
>+            (errno, errstr) = e.args
>+            self.assertEqual(errno, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS)
>+
>+class ConfidentialFilteredDirsyncTests(SpecialDirsyncTests):
>+
>+    def setUp(self):
>+        self.flag_under_test = SEARCH_FLAG_RODC_ATTRIBUTE|SEARCH_FLAG_CONFIDENTIAL
>+        super().setUp()
>+
>+    def test_attr(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         attrs=["unicodePwd", "supplementalCredentials", "samAccountName"],
>+                         expression=f"(samAccountName={self.conf_user})")
>+
>+        msg = res[0]
>+        self.assertTrue(msg.get("samAccountName"))
>+        self.assertTrue(msg.get(self.conf_attr) is None)
>+
>+    def _test_dirsync_OBJECT_SECURITY(self, ldb_conn, insist_on_empty_element=False):
>+        res = ldb_conn.search(self.base_dn,
>+                              attrs=[self.conf_attr, "samAccountName"],
>+                              expression=f"(samAccountName={self.conf_user})",
>+                              controls=["dirsync:1:1:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        self.assertTrue("samAccountName" in msg)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        if insist_on_empty_element:
>+            self.assertTrue(msg.get(self.conf_attr) is not None)
>+            self.assertEqual(len(msg.get(self.conf_attr)), 0)
>+        else:
>+            self.assertTrue(msg.get(self.conf_attr) is None
>+                            or len(msg.get(self.conf_attr)) == 0)
>+
>+    def test_dirsync_OBJECT_SECURITY(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def test_dirsync_OBJECT_SECURITY_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.simple_user, self.simple_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn, insist_on_empty_element=True)
>+
>+    def test_dirsync_OBJECT_SECURITY_with_GET_CHANGES(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn)
>+
>+    def test_dirsync_OBJECT_SECURITY_with_GET_CHANGES_insist_on_empty_element(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        self._test_dirsync_OBJECT_SECURITY(ldb_conn, insist_on_empty_element=True)
>+
>+    def _test_dirsync_with_GET_CHANGES(self, insist_on_empty_element=False):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        res = ldb_conn.search(self.base_dn,
>+                         expression=f"(samAccountName={self.conf_user})",
>+                         controls=["dirsync:1:0:0"])
>+
>+        msg = self.find_under_current_ou(res)
>+        # This form ensures this is a case insensitive comparison
>+        self.assertTrue(msg.get("samAccountName"))
>+        if insist_on_empty_element:
>+            self.assertTrue(msg.get(self.conf_attr) is not None)
>+            self.assertEqual(len(msg.get(self.conf_attr)), 0)
>+        else:
>+            self.assertTrue(msg.get(self.conf_attr) is None
>+                            or len(msg.get(self.conf_attr)) == 0)
>+
>+    def test_dirsync_with_GET_CHANGES(self):
>+        self._test_dirsync_with_GET_CHANGES()
>+
>+    def test_dirsync_with_GET_CHANGES_insist_on_empty_element(self):
>+        self._test_dirsync_with_GET_CHANGES(insist_on_empty_element=True)
>+
>+    def test_dirsync_with_GET_CHANGES_attr(self):
>+        ldb_conn = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass)
>+        try:
>+            res = ldb_conn.search(self.base_dn,
>+                                  attrs=[self.conf_attr, "samAccountName"],
>+                                  expression=f"(samAccountName={self.conf_user})",
>+                                  controls=["dirsync:1:0:0"])
>+            self.fail("ldb.search() should have failed with LDAP_INSUFFICIENT_ACCESS_RIGHTS")
>+        except ldb.LdbError as e:
>+            (errno, errstr) = e.args
>+            self.assertEqual(errno, ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS)
>+
> 
> if not getattr(opts, "listtests", False):
>     lp = sambaopts.get_loadparm()
>-- 
>2.25.1
>
>
>From e233376f19ab985df4f9e02bc5da533a8a8b5e9c Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 17:58:27 +1200
>Subject: [PATCH 9/9] CVE-2023-4154: Unimplement the original DirSync behaviour
> without LDAP_DIRSYNC_OBJECT_SECURITY
>
>This makes LDAP_DIRSYNC_OBJECT_SECURITY the only behaviour provided by
>Samba.
>
>Having a second access control system withing the LDAP stack is unsafe
>and this layer is incomplete.
>
>The current system gives all accounts that have been given the
>GUID_DRS_GET_CHANGES extended right SYSTEM access.  Currently in Samba
>this equates to full access to passwords as well as "RODC Filtered
>attributes" (often used with confidential attributes).
>
>Rather than attempting to correctly filter for secrets (passwords) and
>these filtered attributes, as well as preventing search expressions for
>both, we leave this complexity to the acl_read module which has this
>facility already well tested.
>
>The implication is that callers will only see and filter by attribute
>in DirSync that they could without DirSync.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/dirsync             |  3 +--
> source4/dsdb/samdb/ldb_modules/dirsync.c | 22 +++++++++++-----------
> 2 files changed, 12 insertions(+), 13 deletions(-)
>
>diff --git a/selftest/knownfail.d/dirsync b/selftest/knownfail.d/dirsync
>index db098549a08..fcf4d469d6e 100644
>--- a/selftest/knownfail.d/dirsync
>+++ b/selftest/knownfail.d/dirsync
>@@ -1,12 +1,11 @@
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_OBJ_SEC_insist_on_empty_element$.*$
>-^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC_insist_on_empty_element$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_insist_on_empty_element$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES_OBJECT_SECURITY_insist_on_empty_element$.*$
>+^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_with_GET_CHANGES_insist_on_empty_element$.*$
>-^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element$.*$
> ^samba4.ldap.dirsync.python$.*$.__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES$.*$
>diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
>index b3c463741c8..fbb75790095 100644
>--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
>+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
>@@ -56,7 +56,6 @@ struct dirsync_context {
> 	bool linkIncrVal;
> 	bool localonly;
> 	bool partial;
>-	bool assystem;
> 	int functional_level;
> 	const struct GUID *our_invocation_id;
> 	const struct dsdb_schema *schema;
>@@ -872,10 +871,6 @@ static int dirsync_search_callback(struct ldb_request *req, struct ldb_reply *ar
> 			DSDB_SEARCH_SHOW_DELETED |
> 			DSDB_SEARCH_SHOW_EXTENDED_DN;
> 
>-		if (dsc->assystem) {
>-			flags = flags | DSDB_FLAG_AS_SYSTEM;
>-		}
>-
> 		ret = dsdb_module_search_tree(dsc->module, dsc, &res,
> 					dn, LDB_SCOPE_BASE,
> 					req->op.search.tree,
>@@ -1102,16 +1097,21 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
> 			return LDB_ERR_OPERATIONS_ERROR;
> 		}
> 		objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]);
>+
>+		/*
>+		 * While we never use the answer to this for access
>+		 * control (after CVE-2023-4154), we return a
>+		 * different error message depending on if the user
>+		 * was granted GUID_DRS_GET_CHANGES to provide a closer
>+		 * emulation and keep some tests passing.
>+		 *
>+		 * (Samba's ACL logic is not well suited to redacting
>+		 * only the secret and RODC filtered attributes).
>+		 */
> 		ret = acl_check_extended_right(dsc, module, req, objectclass,
> 					       sd, acl_user_token(module),
> 					       GUID_DRS_GET_CHANGES, SEC_ADS_CONTROL_ACCESS, sid);
> 
>-		if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
>-			return ret;
>-		}
>-		dsc->assystem = true;
>-		ret = ldb_request_add_control(req, LDB_CONTROL_AS_SYSTEM_OID, false, NULL);
>-
> 		if (ret != LDB_SUCCESS) {
> 			return ret;
> 		}
>-- 
>2.25.1
>

Flags:

metze: review+
abartlet: ci-passed+

Actions: View

Attachments on bug 15424: 18025 | 18026 | 18027 | 18040 | 18075 | 18076 | 18084 | 18085 | 18086 | 18087 | 18088 | 18089 | 18090 | 18091 | 18092 | 18098