Attachment 18090 Details for Bug 15424 – Patch in master backported to Samba 4.16 (v5.1)

[patch] Patch in master backported to Samba 4.16 (v5.1)

CVE-2023-4154-dirsync-4.16-v5.1.patch (text/plain), 124.23 KB, created by Andrew Bartlett on 2023-09-11 20:36:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2023-09-11 20:36:23 UTC

Size: 124.23 KB

patch

obsolete

>From e7e0f9eef625db2564db071ec1abcebe22277c2a Mon Sep 17 00:00:00 2001
>From: Christian Merten <christian@merten.dev>
>Date: Mon, 19 Sep 2022 22:47:10 +0200
>Subject: [PATCH 01/21] libcli security_descriptor: Add function to delete a
> given ace from a security descriptor
>
>Two functions have been added to delete a given ace from the SACL or the DACL of a security descriptor.
>
>Signed-off-by: Christian Merten <christian@merten.dev>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 7efe673fbdcd27ddd23f36281c5f5338681a68fe)
>---
> libcli/security/security_descriptor.c | 66 +++++++++++++++++++++++++++
> libcli/security/security_descriptor.h |  4 ++
> 2 files changed, 70 insertions(+)
>
>diff --git a/libcli/security/security_descriptor.c b/libcli/security/security_descriptor.c
>index ba142016389..64c2d027876 100644
>--- a/libcli/security/security_descriptor.c
>+++ b/libcli/security/security_descriptor.c
>@@ -419,6 +419,72 @@ NTSTATUS security_descriptor_sacl_del(struct security_descriptor *sd,
> 	return security_descriptor_acl_del(sd, true, trustee);
> }
> 
>+/*
>+  delete the given ACE in the SACL or DACL of a security_descriptor
>+*/
>+static NTSTATUS security_descriptor_acl_del_ace(struct security_descriptor *sd,
>+						bool sacl_del,
>+						const struct security_ace *ace)
>+{
>+	uint32_t i;
>+	bool found = false;
>+	struct security_acl *acl = NULL;
>+
>+	if (sacl_del) {
>+		acl = sd->sacl;
>+	} else {
>+		acl = sd->dacl;
>+	}
>+
>+	if (acl == NULL) {
>+		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
>+	}
>+
>+	for (i=0;i<acl->num_aces;i++) {
>+		if (security_ace_equal(ace, &acl->aces[i])) {
>+			ARRAY_DEL_ELEMENT(acl->aces, i, acl->num_aces);
>+			acl->num_aces--;
>+			if (acl->num_aces == 0) {
>+				acl->aces = NULL;
>+			}
>+			found = true;
>+			i--;
>+		}
>+	}
>+
>+	if (!found) {
>+		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
>+	}
>+
>+	acl->revision = SECURITY_ACL_REVISION_NT4;
>+
>+	for (i=0;i<acl->num_aces;i++) {
>+		switch (acl->aces[i].type) {
>+		case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
>+		case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
>+		case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT:
>+		case SEC_ACE_TYPE_SYSTEM_ALARM_OBJECT:
>+			acl->revision = SECURITY_ACL_REVISION_ADS;
>+			return NT_STATUS_OK;
>+		default:
>+			break; /* only for the switch statement */
>+		}
>+	}
>+
>+	return NT_STATUS_OK;
>+}
>+
>+NTSTATUS security_descriptor_dacl_del_ace(struct security_descriptor *sd,
>+					  const struct security_ace *ace)
>+{
>+	return security_descriptor_acl_del_ace(sd, false, ace);
>+}
>+
>+NTSTATUS security_descriptor_sacl_del_ace(struct security_descriptor *sd,
>+					  const struct security_ace *ace)
>+{
>+	return security_descriptor_acl_del_ace(sd, true, ace);
>+}
> /*
>   compare two security ace structures
> */
>diff --git a/libcli/security/security_descriptor.h b/libcli/security/security_descriptor.h
>index 7e6df87fefa..46545321d15 100644
>--- a/libcli/security/security_descriptor.h
>+++ b/libcli/security/security_descriptor.h
>@@ -39,6 +39,10 @@ NTSTATUS security_descriptor_dacl_del(struct security_descriptor *sd,
> 				      const struct dom_sid *trustee);
> NTSTATUS security_descriptor_sacl_del(struct security_descriptor *sd,
> 				      const struct dom_sid *trustee);
>+NTSTATUS security_descriptor_dacl_del_ace(struct security_descriptor *sd,
>+					  const struct security_ace *ace);
>+NTSTATUS security_descriptor_sacl_del_ace(struct security_descriptor *sd,
>+					  const struct security_ace *ace);
> bool security_ace_equal(const struct security_ace *ace1, 
> 			const struct security_ace *ace2);
> bool security_acl_equal(const struct security_acl *acl1, 
>-- 
>2.25.1
>
>
>From cf638f096add4032561dadb4682ce27348d601a2 Mon Sep 17 00:00:00 2001
>From: Christian Merten <christian@merten.dev>
>Date: Mon, 19 Sep 2022 23:01:34 +0200
>Subject: [PATCH 02/21] librpc ndr/py_security: Export ACE deletion functions
> to python
>
>Exported security_descriptor_sacl_del and security_descriptor_dacl_del as new methods of the
>security descriptor class to python.
>
>Signed-off-by: Christian Merten <christian@merten.dev>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 84a54d2fa2b1590fdb4e2ea986ded9c39a82cf78)
>---
> source4/librpc/ndr/py_security.c | 52 +++++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
>diff --git a/source4/librpc/ndr/py_security.c b/source4/librpc/ndr/py_security.c
>index e79e7170812..e61b994d7cb 100644
>--- a/source4/librpc/ndr/py_security.c
>+++ b/source4/librpc/ndr/py_security.c
>@@ -234,6 +234,52 @@ static PyObject *py_descriptor_sacl_del(PyObject *self, PyObject *args)
> 	Py_RETURN_NONE;
> }
> 
>+static PyObject *py_descriptor_dacl_del_ace(PyObject *self, PyObject *args)
>+{
>+	struct security_descriptor *desc = pytalloc_get_ptr(self);
>+	NTSTATUS status;
>+	struct security_ace *ace = NULL;
>+	PyObject *py_ace = Py_None;
>+
>+	if (!PyArg_ParseTuple(args, "O!", &security_ace_Type, &py_ace))
>+		return NULL;
>+
>+	if (!PyObject_TypeCheck(py_ace, &security_ace_Type)) {
>+		PyErr_SetString(PyExc_TypeError,
>+				"expected security.security_ace "
>+				"for first argument to .dacl_del_ace");
>+		return NULL;
>+	}
>+
>+	ace = pytalloc_get_ptr(py_ace);
>+	status = security_descriptor_dacl_del_ace(desc, ace);
>+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
>+	Py_RETURN_NONE;
>+}
>+
>+static PyObject *py_descriptor_sacl_del_ace(PyObject *self, PyObject *args)
>+{
>+	struct security_descriptor *desc = pytalloc_get_ptr(self);
>+	NTSTATUS status;
>+	struct security_ace *ace = NULL;
>+	PyObject *py_ace = Py_None;
>+
>+	if (!PyArg_ParseTuple(args, "O!", &security_ace_Type, &py_ace))
>+		return NULL;
>+
>+	if (!PyObject_TypeCheck(py_ace, &security_ace_Type)) {
>+		PyErr_SetString(PyExc_TypeError,
>+				"expected security.security_ace "
>+				"for first argument to .sacl_del_ace");
>+		return NULL;
>+	}
>+
>+	ace = pytalloc_get_ptr(py_ace);
>+	status = security_descriptor_sacl_del_ace(desc, ace);
>+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
>+	Py_RETURN_NONE;
>+}
>+
> static PyObject *py_descriptor_new(PyTypeObject *self, PyObject *args, PyObject *kwargs)
> {
> 	return pytalloc_steal(self, security_descriptor_initialise(NULL));
>@@ -302,7 +348,11 @@ static PyMethodDef py_descriptor_extra_methods[] = {
> 		NULL },
> 	{ "sacl_del", (PyCFunction)py_descriptor_sacl_del, METH_VARARGS,
> 		NULL },
>-	{ "from_sddl", (PyCFunction)py_descriptor_from_sddl, METH_VARARGS|METH_CLASS, 
>+	{ "dacl_del_ace", (PyCFunction)py_descriptor_dacl_del_ace, METH_VARARGS,
>+		NULL },
>+	{ "sacl_del_ace", (PyCFunction)py_descriptor_sacl_del_ace, METH_VARARGS,
>+		NULL },
>+	{ "from_sddl", (PyCFunction)py_descriptor_from_sddl, METH_VARARGS|METH_CLASS,
> 		NULL },
> 	{ "as_sddl", (PyCFunction)py_descriptor_as_sddl, METH_VARARGS,
> 		NULL },
>-- 
>2.25.1
>
>
>From 682c6d76aa300ce154d203b0bafa1c710746b546 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 1 Mar 2023 14:49:06 +1300
>Subject: [PATCH 03/21] CVE-2023-4154 dsdb: Remove remaining references to
> DC_MODE_RETURN_NONE and DC_MODE_RETURN_ALL
>
>The confidential_attrs test no longer uses DC_MODE_RETURN_NONE we can now
>remove the complexity.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>(cherry picked from commit 82d2ec786f7e75ff6f34eb3357964345b10de091)
>---
> .../dsdb/tests/python/confidential_attr.py    | 86 ++++---------------
> 1 file changed, 16 insertions(+), 70 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index 6889d5a5560..ac83f488061 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -70,20 +70,6 @@ lp = sambaopts.get_loadparm()
> creds = credopts.get_credentials(lp)
> creds.set_gensec_features(creds.get_gensec_features() | gensec.FEATURE_SEAL)
> 
>-# When a user does not have access rights to view the objects' attributes,
>-# Windows and Samba behave slightly differently.
>-# A windows DC will always act as if the hidden attribute doesn't exist AT ALL
>-# (for an unprivileged user). So, even for a user that lacks access rights,
>-# the inverse/'!' queries should return ALL objects. This is similar to the
>-# kludgeaclredacted behaviour on Samba.
>-# However, on Samba (for implementation simplicity) we never return a matching
>-# result for an unprivileged user.
>-# Either approach is OK, so long as it gets applied consistently and we don't
>-# disclose any sensitive details by varying what gets returned by the search.
>-DC_MODE_RETURN_NONE = 0
>-DC_MODE_RETURN_ALL = 1
>-
>-
> #
> # Tests start here
> #
>@@ -193,25 +179,6 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>         # reset the value after the test completes
>         self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value)
> 
>-    # The behaviour of the DC can differ in some cases, depending on whether
>-    # we're talking to a Windows DC or a Samba DC
>-    def guess_dc_mode(self):
>-        # if we're in selftest, we can be pretty sure it's a Samba DC
>-        if os.environ.get('SAMBA_SELFTEST') == '1':
>-            return DC_MODE_RETURN_NONE
>-
>-        searches = self.get_negative_match_all_searches()
>-        res = self.ldb_user.search(self.test_dn, expression=searches[0],
>-                                   scope=SCOPE_SUBTREE)
>-
>-        # we default to DC_MODE_RETURN_NONE (samba).Update this if it
>-        # looks like we're talking to a Windows DC
>-        if len(res) == self.total_objects:
>-            return DC_MODE_RETURN_ALL
>-
>-        # otherwise assume samba DC behaviour
>-        return DC_MODE_RETURN_NONE
>-
>     def get_user_dn(self, name):
>         return "CN={0},{1}".format(name, self.ou)
> 
>@@ -359,7 +326,7 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>         return expected_results
> 
>     # Returns the expected negative (i.e. '!') search behaviour when talking to
>-    # a DC with DC_MODE_RETURN_ALL behaviour, i.e. we assert that users
>+    # a DC, i.e. we assert that users
>     # without rights always see ALL objects in '!' searches
>     def negative_searches_return_all(self, has_rights_to=0,
>                                      total_objects=None):
>@@ -409,32 +376,24 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>     # and what access rights the user has.
>     # Note we only handle has_rights_to="all", 1 (the test object), or 0 (i.e.
>     # we don't have rights to any objects)
>-    def negative_search_expected_results(self, has_rights_to, dc_mode,
>-                                         total_objects=None):
>+    def negative_search_expected_results(self, has_rights_to, total_objects=None):
> 
>         if has_rights_to == "all":
>             expect_results = self.negative_searches_all_rights(total_objects)
> 
>-        # if it's a Samba DC, we only expect the 'match-all' searches to return
>-        # the objects that we have access rights to (all others are hidden).
>-        # Whereas Windows 'hides' the objects by always returning all of them
>-        elif dc_mode == DC_MODE_RETURN_NONE:
>-            expect_results = self.negative_searches_return_none(has_rights_to)
>         else:
>             expect_results = self.negative_searches_return_all(has_rights_to,
>                                                                total_objects)
>         return expect_results
> 
>-    def assert_negative_searches(self, has_rights_to=0,
>-                                 dc_mode=DC_MODE_RETURN_NONE, samdb=None):
>+    def assert_negative_searches(self, has_rights_to=0, samdb=None):
>         """Asserts user without rights cannot see objects in '!' searches"""
> 
>         if samdb is None:
>             samdb = self.ldb_user
> 
>         # build a dictionary of key=search-expr, value=expected_num assertions
>-        expected_results = self.negative_search_expected_results(has_rights_to,
>-                                                                 dc_mode)
>+        expected_results = self.negative_search_expected_results(has_rights_to)
> 
>         for search, expected_num in expected_results.items():
>             self.assert_search_result(expected_num, search, samdb)
>@@ -490,8 +449,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
>         self.make_attr_confidential()
> 
>         self.assert_conf_attr_searches(has_rights_to=0)
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
> 
>         # sanity-check we haven't hidden the attribute from the admin as well
>@@ -503,8 +461,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
>         self.make_attr_confidential()
> 
>         self.assert_conf_attr_searches(has_rights_to=0)
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
> 
>         # apply the allow ACE to the object under test
>@@ -513,7 +470,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
>         # the user should now be able to see the attribute for the one object
>         # we gave it rights to
>         self.assert_conf_attr_searches(has_rights_to=1)
>-        self.assert_negative_searches(has_rights_to=1, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=1)
>         self.assert_attr_visible(expect_attr=True)
> 
>         # sanity-check the admin can still see the attribute
>@@ -566,8 +523,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
>         self.make_attr_confidential()
> 
>         self.assert_conf_attr_searches(has_rights_to=0)
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
> 
>         # apply the ACE to the object under test
>@@ -575,7 +531,7 @@ class ConfidentialAttrTest(ConfidentialAttrCommon):
> 
>         # this should make no difference to the user's ability to see the attr
>         self.assert_conf_attr_searches(has_rights_to=0)
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
> 
>         # sanity-check the admin can still see the attribute
>@@ -707,8 +663,7 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
>         return expected_results
> 
>     # override method specifically for deny ACL test cases
>-    def assert_negative_searches(self, has_rights_to=0,
>-                                 dc_mode=DC_MODE_RETURN_NONE, samdb=None):
>+    def assert_negative_searches(self, has_rights_to=0, samdb=None):
>         """Asserts user without rights cannot see objects in '!' searches"""
> 
>         if samdb is None:
>@@ -719,12 +674,9 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
>         # assert this if the '!'/negative search behaviour is to suppress any
>         # objects we don't have access rights to)
>         excl_testobj = False
>-        if has_rights_to != "all" and dc_mode == DC_MODE_RETURN_NONE:
>-            excl_testobj = True
> 
>         # build a dictionary of key=search-expr, value=expected_num assertions
>-        expected_results = self.negative_search_expected_results(has_rights_to,
>-                                                                 dc_mode)
>+        expected_results = self.negative_search_expected_results(has_rights_to)
> 
>         for search, expected_num in expected_results.items():
>             self.assert_search_result(expected_num, search, samdb,
>@@ -741,9 +693,7 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
> 
>         # the user shouldn't be able to see the attribute anymore
>         self.assert_conf_attr_searches(has_rights_to="deny-one")
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to="deny-one",
>-                                      dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to="deny-one")
>         self.assert_attr_visible(expect_attr=False)
> 
>         # sanity-check we haven't hidden the attribute from the admin as well
>@@ -887,8 +837,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>                                   attrs=['name'])
> 
>     # override method specifically for dirsync (total object count differs)
>-    def assert_negative_searches(self, has_rights_to=0,
>-                                 dc_mode=DC_MODE_RETURN_NONE, samdb=None):
>+    def assert_negative_searches(self, has_rights_to=0, samdb=None):
>         """Asserts user without rights cannot see objects in '!' searches"""
> 
>         if samdb is None:
>@@ -898,7 +847,6 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         # here only includes the user objects (not the parent OU)
>         total_objects = len(self.all_users)
>         expected_results = self.negative_search_expected_results(has_rights_to,
>-                                                                 dc_mode,
>                                                                  total_objects)
> 
>         for search, expected_num in expected_results.items():
>@@ -917,8 +865,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
> 
>         self.assert_conf_attr_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
> 
>         # as a final sanity-check, make sure the admin can still see the attr
>         self.assert_conf_attr_searches(has_rights_to="all",
>@@ -1012,8 +959,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         # check we can't see the objects now, even with using dirsync controls
>         self.assert_conf_attr_searches(has_rights_to=0)
>         self.assert_attr_visible(expect_attr=False)
>-        dc_mode = DC_MODE_RETURN_ALL
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
> 
>         # now delete the users (except for the user whose LDB connection
>         # we're currently using)
>@@ -1023,7 +969,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
> 
>         # check we still can't see the objects
>         self.assert_conf_attr_searches(has_rights_to=0)
>-        self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode)
>+        self.assert_negative_searches(has_rights_to=0)
> 
>     def test_timing_attack(self):
>         # Create the machine account.
>-- 
>2.25.1
>
>
>From 653914d376281b305c03ef9af50bf357ae60bb8e Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Fri, 27 Jan 2023 07:43:40 +1300
>Subject: [PATCH 04/21] CVE-2023-4154 s4:dsdb:tests: Refactor confidential
> attributes test
>
>Use more specific unittest methods, and remove unused code.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>(cherry picked from commit 2e5d08c908b3fa48b9b374279a331061cb77bce3)
>---
> .../dsdb/tests/python/confidential_attr.py    | 69 +++++--------------
> 1 file changed, 16 insertions(+), 53 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/confidential_attr.py b/source4/dsdb/tests/python/confidential_attr.py
>index ac83f488061..eb75da6374f 100755
>--- a/source4/dsdb/tests/python/confidential_attr.py
>+++ b/source4/dsdb/tests/python/confidential_attr.py
>@@ -24,7 +24,6 @@ import sys
> sys.path.insert(0, "bin/python")
> 
> import samba
>-import os
> import random
> import statistics
> import time
>@@ -136,9 +135,9 @@ 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.assertTrue(int(search_flags) & SEARCH_FLAG_CONFIDENTIAL == 0,
>-                        "{0} searchFlags already {1}".format(self.conf_attr,
>-                                                             search_flags))
>+        self.assertEqual(0, int(search_flags) & SEARCH_FLAG_CONFIDENTIAL,
>+                         "{0} searchFlags already {1}".format(self.conf_attr,
>+                                                              search_flags))
> 
>     def tearDown(self):
>         super(ConfidentialAttrCommon, self).tearDown()
>@@ -208,9 +207,9 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>         for attr in attr_filters:
>             res = samdb.search(self.test_dn, expression=expr,
>                                scope=SCOPE_SUBTREE, attrs=attr)
>-            self.assertTrue(len(res) == expected_num,
>-                            "%u results, not %u for search %s, attr %s" %
>-                            (len(res), expected_num, expr, str(attr)))
>+            self.assertEqual(len(res), expected_num,
>+                             "%u results, not %u for search %s, attr %s" %
>+                             (len(res), expected_num, expr, str(attr)))
> 
>     # return a selection of searches that match exactly against the test object
>     def get_exact_match_searches(self):
>@@ -352,25 +351,6 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
> 
>         return expected_results
> 
>-    # Returns the expected negative (i.e. '!') search behaviour when talking to
>-    # a DC with DC_MODE_RETURN_NONE behaviour, i.e. we assert that users
>-    # without rights cannot see objects in '!' searches at all
>-    def negative_searches_return_none(self, has_rights_to=0):
>-        expected_results = {}
>-
>-        # the 'match-all' searches should only return the objects we have
>-        # access rights to (if any)
>-        for search in self.get_negative_match_all_searches():
>-            expected_results[search] = has_rights_to
>-
>-        # for inverse matches, we should NOT be told about any objects at all
>-        inverse_searches = self.get_inverse_match_searches()
>-        inverse_searches += ["(!({0}=*))".format(self.conf_attr)]
>-        for search in inverse_searches:
>-            expected_results[search] = 0
>-
>-        return expected_results
>-
>     # Returns the expected negative (i.e. '!') search behaviour. This varies
>     # depending on what type of DC we're talking to (i.e. Windows or Samba)
>     # and what access rights the user has.
>@@ -403,7 +383,7 @@ class ConfidentialAttrCommon(samba.tests.TestCase):
>         # checks whether the confidential attribute is present
>         res = samdb.search(self.conf_dn, expression="(objectClass=*)",
>                            scope=SCOPE_SUBTREE, attrs=attrs)
>-        self.assertTrue(len(res) == 1)
>+        self.assertEqual(1, len(res))
> 
>         attr_returned = False
>         for msg in res:
>@@ -586,9 +566,9 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
>         for attr in attr_filters:
>             res = samdb.search(self.test_dn, expression=expr,
>                                scope=SCOPE_SUBTREE, attrs=attr)
>-            self.assertTrue(len(res) == expected_num,
>-                            "%u results, not %u for search %s, attr %s" %
>-                            (len(res), expected_num, expr, str(attr)))
>+            self.assertEqual(len(res), expected_num,
>+                             "%u results, not %u for search %s, attr %s" %
>+                             (len(res), expected_num, expr, str(attr)))
> 
>             # assert we haven't revealed the hidden test-object
>             if excl_testobj:
>@@ -604,7 +584,7 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
> 
>         # make sure the test object is not returned if we've been denied rights
>         # to it via an ACE
>-        excl_testobj = True if has_rights_to == "deny-one" else False
>+        excl_testobj = has_rights_to == "deny-one"
> 
>         # these first few searches we just expect to match against the one
>         # object under test that we're trying to guess the value of
>@@ -625,24 +605,6 @@ class ConfidentialAttrTestDenyAcl(ConfidentialAttrCommon):
>             self.assert_search_result(expected_num, search, samdb,
>                                       excl_testobj)
> 
>-    # override method specifically for deny ACL test cases. Instead of being
>-    # granted access to either no objects or only one, we are being denied
>-    # access to only one object (but can still access the rest).
>-    def negative_searches_return_none(self, has_rights_to=0):
>-        expected_results = {}
>-
>-        # on Samba we will see the objects we have rights to, but the one we
>-        # are denied access to will be hidden
>-        searches = self.get_negative_match_all_searches()
>-        searches += self.get_inverse_match_searches()
>-        for search in searches:
>-            expected_results[search] = self.total_objects - 1
>-
>-        # The wildcard returns the objects without this attribute as normal.
>-        search = "(!({0}=*))".format(self.conf_attr)
>-        expected_results[search] = self.total_objects - self.objects_with_attr
>-        return expected_results
>-
>     # override method specifically for deny ACL test cases
>     def negative_searches_return_all(self, has_rights_to=0,
>                                      total_objects=None):
>@@ -783,7 +745,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         for attr in self.attr_filters:
>             res = samdb.search(base_dn, expression=search, scope=SCOPE_SUBTREE,
>                                attrs=attr, controls=self.dirsync)
>-            self.assertTrue(len(res) == expected_num,
>+            self.assertEqual(len(res), expected_num,
>                             "%u results, not %u for search %s, attr %s" %
>                             (len(res), expected_num, search, str(attr)))
> 
>@@ -797,7 +759,8 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         expr = self.single_obj_filter
>         res = samdb.search(self.base_dn, expression=expr, scope=SCOPE_SUBTREE,
>                            attrs=attrs, controls=self.dirsync)
>-        self.assertTrue(len(res) == 1 or no_result_ok)
>+        if not no_result_ok:
>+            self.assertEqual(1, len(res))
> 
>         attr_returned = False
>         for msg in res:
>@@ -888,7 +851,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         search_flags = int(self.get_attr_search_flags(self.attr_dn))
> 
>         # check we've already set the confidential flag
>-        self.assertTrue(search_flags & SEARCH_FLAG_CONFIDENTIAL != 0)
>+        self.assertNotEqual(0, search_flags & SEARCH_FLAG_CONFIDENTIAL)
>         search_flags |= SEARCH_FLAG_PRESERVEONDELETE
> 
>         self.set_attr_search_flags(self.attr_dn, str(search_flags))
>@@ -964,7 +927,7 @@ class ConfidentialAttrTestDirsync(ConfidentialAttrCommon):
>         # now delete the users (except for the user whose LDB connection
>         # we're currently using)
>         for user in self.all_users:
>-            if user != self.user:
>+            if user is not self.user:
>                 self.ldb_admin.delete(self.get_user_dn(user))
> 
>         # check we still can't see the objects
>-- 
>2.25.1
>
>
>From 09d65978f2553a78d951c29de44883f8596268c3 Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Wed, 2 Aug 2023 10:44:32 +0200
>Subject: [PATCH 05/21] 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>
>
>[abartlet@samba.org adapted from commit b29793ffdee5d9b9c1c05830622e80f7faec7670
> but excluding changes to token_group.py due to conflicts in backporting to 4.16]
>---
> 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/user_account_control.py  |  2 +-
> 13 files changed, 37 insertions(+), 37 deletions(-)
>
>diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
>index 3e5f7a44a79..6d547a480a8 100755
>--- a/source4/dsdb/tests/python/acl.py
>+++ b/source4/dsdb/tests/python/acl.py
>@@ -174,7 +174,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)
>@@ -187,7 +187,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.assertTrue(len(res) > 0)
>@@ -248,7 +248,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)
>@@ -1133,7 +1133,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)
>@@ -1424,7 +1424,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"
>@@ -1880,7 +1880,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 b186e723f39..19cdf3e67d7 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 e1c49d7bffb..9b22177989b 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 98afcb39d1f..a387c5ec96c 100755
>--- a/source4/dsdb/tests/python/passwords.py
>+++ b/source4/dsdb/tests/python/passwords.py
>@@ -1059,12 +1059,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 d99247d18b1..ac1c25ac1f5 100755
>--- a/source4/dsdb/tests/python/sam.py
>+++ b/source4/dsdb/tests/python/sam.py
>@@ -3776,7 +3776,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 b67bf33b5f7..b8908e5007e 100755
>--- a/source4/dsdb/tests/python/sec_descriptor.py
>+++ b/source4/dsdb/tests/python/sec_descriptor.py
>@@ -684,7 +684,7 @@ class OwnerGroupDescriptorTests(DescriptorTests):
> 
>     # Tests for SCHEMA
> 
>-    # Defalt descriptor tests ##################################################################
>+    # Default descriptor tests ##################################################################
> 
>     def test_130(self):
>         user_name = "testuser1"
>@@ -938,7 +938,7 @@ class OwnerGroupDescriptorTests(DescriptorTests):
> 
>     # Tests for CONFIGURATION
> 
>-    # Defalt descriptor tests ##################################################################
>+    # Default descriptor tests ##################################################################
> 
>     def test_160(self):
>         user_name = "testuser1"
>@@ -1983,7 +1983,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
>@@ -1991,7 +1991,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
>@@ -1999,14 +1999,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/user_account_control.py b/source4/dsdb/tests/python/user_account_control.py
>index b427aaf1ea3..8f575686baf 100755
>--- a/source4/dsdb/tests/python/user_account_control.py
>+++ b/source4/dsdb/tests/python/user_account_control.py
>@@ -847,7 +847,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 f32d100c8018f8338cb9fe93565183fa323ad0a1 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 14 Feb 2023 17:19:27 +1300
>Subject: [PATCH 06/21] CVE-2023-4154 s4-dsdb: Remove
> DSDB_ACL_CHECKS_DIRSYNC_FLAG
>
>It's no longer used anywhere.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>[abartlet@samba.org backported to Samba 4.16 from commit 8b4e6f7b3fb8018cb64deef9b8e1cbc2e5ba12cf]
>---
> source4/dsdb/samdb/ldb_modules/dirsync.c | 11 ++---------
> source4/dsdb/samdb/samdb.h               |  1 -
> 2 files changed, 2 insertions(+), 10 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c
>index fa57af49e8f..b3c463741c8 100644
>--- a/source4/dsdb/samdb/ldb_modules/dirsync.c
>+++ b/source4/dsdb/samdb/ldb_modules/dirsync.c
>@@ -1005,7 +1005,6 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
> 	struct dirsync_context *dsc;
> 	struct ldb_context *ldb;
> 	struct ldb_parse_tree *new_tree = req->op.search.tree;
>-	uint32_t flags = 0;
> 	enum ndr_err_code ndr_err;
> 	DATA_BLOB blob;
> 	const char **attrs;
>@@ -1117,13 +1116,8 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
> 			return ret;
> 		}
> 		talloc_free(acl_res);
>-	} else {
>-		flags |= DSDB_ACL_CHECKS_DIRSYNC_FLAG;
>-
>-		if (ret != LDB_SUCCESS) {
>-			return ret;
>-		}
>-
>+	} else if (ret != LDB_SUCCESS) {
>+		return ret;
> 	}
> 
> 	dsc->functional_level = dsdb_functional_level(ldb);
>@@ -1394,7 +1388,6 @@ static int dirsync_ldb_search(struct ldb_module *module, struct ldb_request *req
> 				      req->controls,
> 				      dsc, dirsync_search_callback,
> 				      req);
>-	ldb_req_set_custom_flags(down_req, flags);
> 	LDB_REQ_SET_LOCATION(down_req);
> 	if (ret != LDB_SUCCESS) {
> 		return ret;
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index 3c4c78822bf..e90e02609f3 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -348,7 +348,6 @@ struct dsdb_extended_dn_store_format {
> 
> #define DSDB_OPAQUE_PARTITION_MODULE_MSG_OPAQUE_NAME "DSDB_OPAQUE_PARTITION_MODULE_MSG"
> 
>-#define DSDB_ACL_CHECKS_DIRSYNC_FLAG 0x1
> #define DSDB_SAMDB_MINIMUM_ALLOWED_RID   1000
> 
> #define DSDB_METADATA_SCHEMA_SEQ_NUM	"SCHEMA_SEQ_NUM"
>-- 
>2.25.1
>
>
>From 8a67125243ee807a223b99731c796d176fb1b54b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 10 Mar 2023 18:25:18 +0100
>Subject: [PATCH 07/21] CVE-2023-4154 python:sd_utils: introduce
> update_aces_in_dacl() helper
>
>This is a more generic api that can be re-used in other places
>as well in future. It operates on a security descriptor object instead of
>SDDL.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit 8411e6d302e25d10f1035ebbdcbde7308566e930)
>---
> python/samba/sd_utils.py | 124 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 112 insertions(+), 12 deletions(-)
>
>diff --git a/python/samba/sd_utils.py b/python/samba/sd_utils.py
>index 26e80ee2f4a..52a78de5d09 100644
>--- a/python/samba/sd_utils.py
>+++ b/python/samba/sd_utils.py
>@@ -21,8 +21,11 @@
> import samba
> from ldb import Message, MessageElement, Dn
> from ldb import FLAG_MOD_REPLACE, SCOPE_BASE
>-from samba.ndr import ndr_pack, ndr_unpack
>+from samba.ndr import ndr_pack, ndr_unpack, ndr_deepcopy
> from samba.dcerpc import security
>+from samba.ntstatus import (
>+    NT_STATUS_OBJECT_NAME_NOT_FOUND,
>+)
> 
> 
> class SDUtils(object):
>@@ -63,19 +66,116 @@ class SDUtils(object):
>         res = self.ldb.search(object_dn)
>         return ndr_unpack(security.dom_sid, res[0]["objectSid"][0])
> 
>+    def update_aces_in_dacl(self, dn, del_aces=None, add_aces=None,
>+                            sddl_attr=None, controls=None):
>+        if del_aces is None:
>+            del_aces=[]
>+        if add_aces is None:
>+            add_aces=[]
>+
>+        def ace_from_sddl(ace_sddl):
>+            ace_sd = security.descriptor.from_sddl("D:" + ace_sddl, self.domain_sid)
>+            assert(len(ace_sd.dacl.aces)==1)
>+            return ace_sd.dacl.aces[0]
>+
>+        if sddl_attr is None:
>+            if controls is None:
>+                controls=["sd_flags:1:%d" % security.SECINFO_DACL]
>+            sd = self.read_sd_on_dn(dn, controls=controls)
>+            if not sd.type & security.SEC_DESC_DACL_PROTECTED:
>+                # if the DACL is not protected remove all
>+                # inherited aces, as they will be re-inherited
>+                # on the server, we need a ndr_deepcopy in order
>+                # to avoid reference problems while deleting
>+                # the aces while looping over them
>+                dacl_copy = ndr_deepcopy(sd.dacl)
>+                for ace in dacl_copy.aces:
>+                    if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE:
>+                        try:
>+                            sd.dacl_del_ace(ace)
>+                        except samba.NTSTATUSError as err:
>+                            if err.args[0] != NT_STATUS_OBJECT_NAME_NOT_FOUND:
>+                                raise err
>+                            # dacl_del_ace may remove more than
>+                            # one ace, so we may not find it anymore
>+                            pass
>+        else:
>+            if controls is None:
>+                controls=[]
>+            res = self.ldb.search(dn, SCOPE_BASE, None,
>+                                  [sddl_attr], controls=controls)
>+            old_sddl = str(res[0][sddl_attr][0])
>+            sd = security.descriptor.from_sddl(old_sddl, self.domain_sid)
>+
>+        num_changes = 0
>+        del_ignored = []
>+        add_ignored = []
>+        inherited_ignored = []
>+
>+        for ace in del_aces:
>+            if isinstance(ace, str):
>+                ace = ace_from_sddl(ace)
>+            assert(isinstance(ace, security.ace))
>+
>+            if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE:
>+                inherited_ignored.append(ace)
>+                continue
>+
>+            if ace not in sd.dacl.aces:
>+                del_ignored.append(ace)
>+                continue
>+
>+            sd.dacl_del_ace(ace)
>+            num_changes += 1
>+
>+        for ace in add_aces:
>+            add_idx = -1
>+            if isinstance(ace, dict):
>+                if "idx" in ace:
>+                    add_idx = ace["idx"]
>+                ace = ace["ace"]
>+            if isinstance(ace, str):
>+                ace = ace_from_sddl(ace)
>+            assert(isinstance(ace, security.ace))
>+
>+            if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE:
>+                inherited_ignored.append(ace)
>+                continue
>+
>+            if ace in sd.dacl.aces:
>+                add_ignored.append(ace)
>+                continue
>+
>+            sd.dacl_add(ace, add_idx)
>+            num_changes += 1
>+
>+        if num_changes == 0:
>+            return del_ignored, add_ignored, inherited_ignored
>+
>+        if sddl_attr is None:
>+            self.modify_sd_on_dn(dn, sd, controls=controls)
>+        else:
>+            new_sddl = sd.as_sddl(self.domain_sid)
>+            m = Message()
>+            m.dn = dn
>+            m[sddl_attr] = MessageElement(new_sddl.encode('ascii'),
>+                                          FLAG_MOD_REPLACE,
>+                                          sddl_attr)
>+            self.ldb.modify(m, controls=controls)
>+
>+        return del_ignored, add_ignored, inherited_ignored
>+
>     def dacl_add_ace(self, object_dn, ace):
>-        """Add an ACE to an objects security descriptor
>+        """Add an ACE (or more) to an objects security descriptor
>         """
>-        desc = self.read_sd_on_dn(object_dn, ["show_deleted:1"])
>-        desc_sddl = desc.as_sddl(self.domain_sid)
>-        if ace in desc_sddl:
>-            return
>-        if desc_sddl.find("(") >= 0:
>-            desc_sddl = (desc_sddl[:desc_sddl.index("(")] + ace +
>-                         desc_sddl[desc_sddl.index("("):])
>-        else:
>-            desc_sddl = desc_sddl + ace
>-        self.modify_sd_on_dn(object_dn, desc_sddl, ["show_deleted:1"])
>+        ace_sd = security.descriptor.from_sddl("D:" + ace, self.domain_sid)
>+        add_aces = []
>+        add_idx = 0
>+        for ace in ace_sd.dacl.aces:
>+            add_aces.append({"idx": add_idx, "ace": ace})
>+            add_idx += 1
>+        _,_,_ = self.update_aces_in_dacl(object_dn, add_aces=add_aces,
>+                                         controls=["show_deleted:1"])
> 
>     def get_sd_as_sddl(self, object_dn, controls=[]):
>         """Return object nTSecutiryDescriptor in SDDL format
>-- 
>2.25.1
>
>
>From 3159779fa1396ee1ef91e6997d3adb3048c0661b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Mar 2023 18:03:10 +0100
>Subject: [PATCH 08/21] CVE-2023-4154 python:sd_utils: add
> dacl_{prepend,append,delete}_aces() helpers
>
>They better represent what they are doing, we keep dacl_add_ace()
>as wrapper of dacl_prepend_aces() in order to let existing callers
>work as before.
>
>In future it would be good to have a dacl_insert_aces() that
>would canonicalize the ace order before storing, but that a task
>for another day.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit a1109a9bf12e020636b8d66fc54984aac58bfe6b)
>---
> python/samba/sd_utils.py | 39 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
>diff --git a/python/samba/sd_utils.py b/python/samba/sd_utils.py
>index 52a78de5d09..462bbfbaf18 100644
>--- a/python/samba/sd_utils.py
>+++ b/python/samba/sd_utils.py
>@@ -165,17 +165,46 @@ class SDUtils(object):
> 
>         return del_ignored, add_ignored, inherited_ignored
> 
>-    def dacl_add_ace(self, object_dn, ace):
>-        """Add an ACE (or more) to an objects security descriptor
>+    def dacl_prepend_aces(self, object_dn, aces, controls=None):
>+        """Prepend an ACE (or more) to an objects security descriptor
>         """
>-        ace_sd = security.descriptor.from_sddl("D:" + ace, self.domain_sid)
>+        ace_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid)
>         add_aces = []
>         add_idx = 0
>         for ace in ace_sd.dacl.aces:
>             add_aces.append({"idx": add_idx, "ace": ace})
>             add_idx += 1
>-        _,_,_ = self.update_aces_in_dacl(object_dn, add_aces=add_aces,
>-                                         controls=["show_deleted:1"])
>+        _,ai,ii = self.update_aces_in_dacl(object_dn, add_aces=add_aces,
>+                                           controls=controls)
>+        return ai, ii
>+
>+    def dacl_add_ace(self, object_dn, ace):
>+        """Add an ACE (or more) to an objects security descriptor
>+        """
>+        _,_ = self.dacl_prepend_aces(object_dn, ace,
>+                                     controls=["show_deleted:1"])
>+
>+    def dacl_append_aces(self, object_dn, aces, controls=None):
>+        """Append an ACE (or more) to an objects security descriptor
>+        """
>+        ace_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid)
>+        add_aces = []
>+        for ace in ace_sd.dacl.aces:
>+            add_aces.append(ace)
>+        _,ai,ii = self.update_aces_in_dacl(object_dn, add_aces=add_aces,
>+                                           controls=controls)
>+        return ai, ii
>+
>+    def dacl_delete_aces(self, object_dn, aces, controls=None):
>+        """Delete an ACE (or more) to an objects security descriptor
>+        """
>+        del_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid)
>+        del_aces = []
>+        for ace in del_sd.dacl.aces:
>+            del_aces.append(ace)
>+        di,_,ii = self.update_aces_in_dacl(object_dn, del_aces=del_aces,
>+                                           controls=controls)
>+        return di, ii
> 
>     def get_sd_as_sddl(self, object_dn, controls=[]):
>         """Return object nTSecutiryDescriptor in SDDL format
>-- 
>2.25.1
>
>
>From 1ddbc7040790e55b5c817e99610abfe782f7e7ea Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Mar 2023 10:11:05 +0100
>Subject: [PATCH 09/21] CVE-2023-4154 py_security: allow idx argument to
> descriptor.[s|d]acl_add()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit 9ea06aaf9f57e3c7094553d9ac40fb73057a9b74)
>---
> source4/librpc/ndr/py_security.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/source4/librpc/ndr/py_security.c b/source4/librpc/ndr/py_security.c
>index e61b994d7cb..4a8271a11db 100644
>--- a/source4/librpc/ndr/py_security.c
>+++ b/source4/librpc/ndr/py_security.c
>@@ -175,12 +175,13 @@ static PyObject *py_descriptor_sacl_add(PyObject *self, PyObject *args)
> 	NTSTATUS status;
> 	struct security_ace *ace;
> 	PyObject *py_ace;
>+	Py_ssize_t idx = -1;
> 
>-	if (!PyArg_ParseTuple(args, "O", &py_ace))
>+	if (!PyArg_ParseTuple(args, "O|n", &py_ace, &idx))
> 		return NULL;
> 
> 	ace = pytalloc_get_ptr(py_ace);
>-	status = security_descriptor_sacl_add(desc, ace);
>+	status = security_descriptor_sacl_insert(desc, ace, idx);
> 	PyErr_NTSTATUS_IS_ERR_RAISE(status);
> 	Py_RETURN_NONE;
> }
>@@ -191,13 +192,14 @@ static PyObject *py_descriptor_dacl_add(PyObject *self, PyObject *args)
> 	NTSTATUS status;
> 	struct security_ace *ace;
> 	PyObject *py_ace;
>+	Py_ssize_t idx = -1;
> 
>-	if (!PyArg_ParseTuple(args, "O", &py_ace))
>+	if (!PyArg_ParseTuple(args, "O|n", &py_ace, &idx))
> 		return NULL;
> 
> 	ace = pytalloc_get_ptr(py_ace);
> 
>-	status = security_descriptor_dacl_add(desc, ace);
>+	status = security_descriptor_dacl_insert(desc, ace, idx);
> 	PyErr_NTSTATUS_IS_ERR_RAISE(status);
> 	Py_RETURN_NONE;
> }
>-- 
>2.25.1
>
>
>From e50e0879b8536057b589231eddc462ae5fcbcad8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 17 Mar 2023 14:08:34 +0100
>Subject: [PATCH 10/21] CVE-2023-4154 python/samba/ndr: add ndr_deepcopy()
> helper
>
>This uses ndr_pack/unpack in order to create a deep copy
>of the given object.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit 4627997ddae44265ad35b3234232eb74458c6c34)
>---
> python/samba/ndr.py | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/python/samba/ndr.py b/python/samba/ndr.py
>index 35b2414e8ae..8369abfb2d0 100644
>--- a/python/samba/ndr.py
>+++ b/python/samba/ndr.py
>@@ -56,6 +56,25 @@ def ndr_print(object):
>     return ndr_print()
> 
> 
>+def ndr_deepcopy(object):
>+    """Create a deep copy of a NDR object, using pack/unpack
>+
>+    :param object: Object to copy
>+    :return: The object copy
>+    """
>+    ndr_pack = getattr(object, "__ndr_pack__", None)
>+    if ndr_pack is None:
>+        raise TypeError("%r is not a NDR object" % object)
>+    data = ndr_pack()
>+    cls = type(object)
>+    copy = cls()
>+    ndr_unpack = getattr(copy, "__ndr_unpack__", None)
>+    if ndr_unpack is None:
>+        raise TypeError("%r is not a NDR object" % copy)
>+    ndr_unpack(data, allow_remaining=False)
>+    return copy
>+
>+
> def ndr_pack_in(object, bigendian=False, ndr64=False):
>     """Pack the input of an NDR function object.
> 
>-- 
>2.25.1
>
>
>From 12e2ecc331cd7292c43f08e91affcaaff830f842 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Mar 2023 09:57:43 +0100
>Subject: [PATCH 11/21] CVE-2023-4154 replace: add ARRAY_INSERT_ELEMENT()
> helper
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit 9d8ff0d1e0b2ba7c84af36e1931f5bc99902a44b)
>---
> lib/replace/replace.h | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
>diff --git a/lib/replace/replace.h b/lib/replace/replace.h
>index 3c938f26240..b5f835e4570 100644
>--- a/lib/replace/replace.h
>+++ b/lib/replace/replace.h
>@@ -871,6 +871,21 @@ typedef unsigned long long ptrdiff_t ;
> #define ARRAY_DEL_ELEMENT(a,i,n) \
> if((i)<((n)-1)){memmove(&((a)[(i)]),&((a)[(i)+1]),(sizeof(*(a))*((n)-(i)-1)));}
> 
>+/**
>+ * Insert an array element by moving the rest one up
>+ *
>+ */
>+#define ARRAY_INSERT_ELEMENT(__array,__old_last_idx,__new_elem,__new_idx) do { \
>+	if ((__new_idx) < (__old_last_idx)) { \
>+		const void *__src = &((__array)[(__new_idx)]); \
>+		void *__dst = &((__array)[(__new_idx)+1]); \
>+		size_t __num = (__old_last_idx)-(__new_idx); \
>+		size_t __len = sizeof(*(__array)) * __num; \
>+		memmove(__dst, __src, __len); \
>+	} \
>+	(__array)[(__new_idx)] = (__new_elem); \
>+} while(0)
>+
> /**
>  * Pointer difference macro
>  */
>-- 
>2.25.1
>
>
>From eef41cb126c01c8767e94027cb5ab4ab2ab9731d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Mar 2023 10:00:11 +0100
>Subject: [PATCH 12/21] CVE-2023-4154 libcli/security: prepare
> security_descriptor_acl_add() to place the ace at a position
>
>Often it is important to insert an ace at a specific position in the
>ACL. As a default we still append by default by using -1, which is the
>generic version of passing the number of existing aces.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit c3cb915a67aff6739b72b86d7d139609df309ada)
>---
> libcli/security/security_descriptor.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
>diff --git a/libcli/security/security_descriptor.c b/libcli/security/security_descriptor.c
>index 64c2d027876..8657c797364 100644
>--- a/libcli/security/security_descriptor.c
>+++ b/libcli/security/security_descriptor.c
>@@ -267,9 +267,11 @@ NTSTATUS security_descriptor_for_client(TALLOC_CTX *mem_ctx,
> 
> static NTSTATUS security_descriptor_acl_add(struct security_descriptor *sd,
> 					    bool add_to_sacl,
>-					    const struct security_ace *ace)
>+					    const struct security_ace *ace,
>+					    ssize_t _idx)
> {
> 	struct security_acl *acl = NULL;
>+	ssize_t idx;
> 
> 	if (add_to_sacl) {
> 		acl = sd->sacl;
>@@ -288,15 +290,28 @@ static NTSTATUS security_descriptor_acl_add(struct security_descriptor *sd,
> 		acl->aces     = NULL;
> 	}
> 
>+	if (_idx < 0) {
>+		idx = (acl->num_aces + 1) + _idx;
>+	} else {
>+		idx = _idx;
>+	}
>+
>+	if (idx < 0) {
>+		return NT_STATUS_ARRAY_BOUNDS_EXCEEDED;
>+	} else if (idx > acl->num_aces) {
>+		return NT_STATUS_ARRAY_BOUNDS_EXCEEDED;
>+	}
>+
> 	acl->aces = talloc_realloc(acl, acl->aces,
> 				   struct security_ace, acl->num_aces+1);
> 	if (acl->aces == NULL) {
> 		return NT_STATUS_NO_MEMORY;
> 	}
> 
>-	acl->aces[acl->num_aces] = *ace;
>+	ARRAY_INSERT_ELEMENT(acl->aces, acl->num_aces, *ace, idx);
>+	acl->num_aces++;
> 
>-	switch (acl->aces[acl->num_aces].type) {
>+	switch (acl->aces[idx].type) {
> 	case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
> 	case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
> 	case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT:
>@@ -307,8 +322,6 @@ static NTSTATUS security_descriptor_acl_add(struct security_descriptor *sd,
> 		break;
> 	}
> 
>-	acl->num_aces++;
>-
> 	if (add_to_sacl) {
> 		sd->sacl = acl;
> 		sd->type |= SEC_DESC_SACL_PRESENT;
>@@ -327,7 +340,7 @@ static NTSTATUS security_descriptor_acl_add(struct security_descriptor *sd,
> NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd,
> 				      const struct security_ace *ace)
> {
>-	return security_descriptor_acl_add(sd, true, ace);
>+	return security_descriptor_acl_add(sd, true, ace, -1);
> }
> 
> /*
>@@ -337,7 +350,7 @@ NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd,
> NTSTATUS security_descriptor_dacl_add(struct security_descriptor *sd,
> 				      const struct security_ace *ace)
> {
>-	return security_descriptor_acl_add(sd, false, ace);
>+	return security_descriptor_acl_add(sd, false, ace, -1);
> }
> 
> /*
>-- 
>2.25.1
>
>
>From ad3a5caba126d07111348d5736847799e82c198d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Mar 2023 10:03:44 +0100
>Subject: [PATCH 13/21] CVE-2023-4154 libcli/security: add
> security_descriptor_[s|d]acl_insert() helpers
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15424
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>(cherry picked from commit 2c02378029fff6636b8f19e45af78b265f2210ed)
>---
> libcli/security/security_descriptor.c | 28 +++++++++++++++++++++++++++
> libcli/security/security_descriptor.h |  6 ++++++
> 2 files changed, 34 insertions(+)
>
>diff --git a/libcli/security/security_descriptor.c b/libcli/security/security_descriptor.c
>index 8657c797364..08f2cf19ee8 100644
>--- a/libcli/security/security_descriptor.c
>+++ b/libcli/security/security_descriptor.c
>@@ -343,6 +343,20 @@ NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd,
> 	return security_descriptor_acl_add(sd, true, ace, -1);
> }
> 
>+/*
>+  insert an ACE at a given index to the SACL of a security_descriptor
>+
>+  idx can be negative, which means it's related to the new size from the
>+  end, so -1 means the ace is appended at the end.
>+*/
>+
>+NTSTATUS security_descriptor_sacl_insert(struct security_descriptor *sd,
>+					 const struct security_ace *ace,
>+					 ssize_t idx)
>+{
>+	return security_descriptor_acl_add(sd, true, ace, idx);
>+}
>+
> /*
>   add an ACE to the DACL of a security_descriptor
> */
>@@ -353,6 +367,20 @@ NTSTATUS security_descriptor_dacl_add(struct security_descriptor *sd,
> 	return security_descriptor_acl_add(sd, false, ace, -1);
> }
> 
>+/*
>+  insert an ACE at a given index to the DACL of a security_descriptor
>+
>+  idx can be negative, which means it's related to the new size from the
>+  end, so -1 means the ace is appended at the end.
>+*/
>+
>+NTSTATUS security_descriptor_dacl_insert(struct security_descriptor *sd,
>+					 const struct security_ace *ace,
>+					 ssize_t idx)
>+{
>+	return security_descriptor_acl_add(sd, false, ace, idx);
>+}
>+
> /*
>   delete the ACE corresponding to the given trustee in an ACL of a
>   security_descriptor
>diff --git a/libcli/security/security_descriptor.h b/libcli/security/security_descriptor.h
>index 46545321d15..354bc17e925 100644
>--- a/libcli/security/security_descriptor.h
>+++ b/libcli/security/security_descriptor.h
>@@ -33,8 +33,14 @@ NTSTATUS security_descriptor_for_client(TALLOC_CTX *mem_ctx,
> 					struct security_descriptor **_csd);
> NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd,
> 				      const struct security_ace *ace);
>+NTSTATUS security_descriptor_sacl_insert(struct security_descriptor *sd,
>+					 const struct security_ace *ace,
>+					 ssize_t idx);
> NTSTATUS security_descriptor_dacl_add(struct security_descriptor *sd,
> 				      const struct security_ace *ace);
>+NTSTATUS security_descriptor_dacl_insert(struct security_descriptor *sd,
>+					 const struct security_ace *ace,
>+					 ssize_t idx);
> NTSTATUS security_descriptor_dacl_del(struct security_descriptor *sd,
> 				      const struct dom_sid *trustee);
> NTSTATUS security_descriptor_sacl_del(struct security_descriptor *sd,
>-- 
>2.25.1
>
>
>From ba0db54ac26db192d8119e99d9da9e8b3abc0101 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 11:55:55 +1200
>Subject: [PATCH 14/21] 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 41ff3ef7151..9570af946c6 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 f81d663003408830099df7b3c6895939e9d5e299 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 13:15:40 +1200
>Subject: [PATCH 15/21] 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 8090041d0f2d681a76b2cb059c55e382ea848cb5 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 14:44:28 +1200
>Subject: [PATCH 16/21] 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 2d5e41db7f827f5f39857c7692ec184622ba067f Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 7 Aug 2023 11:56:56 +1200
>Subject: [PATCH 17/21] 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 8a041e18f0cae8ea9b532d38256b49a40c215936 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 11:18:46 +1200
>Subject: [PATCH 18/21] 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 f65ba48ca77f5d4651079bd06db2132f4727c97f Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 14:30:19 +1200
>Subject: [PATCH 19/21] 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 cb12513f512b9d6042f1eb9f473369183192b23f Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 22 Aug 2023 15:08:17 +1200
>Subject: [PATCH 20/21] 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 a2f53d6c3b3e8878ca0609ad2921102078ff9642 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 8 Aug 2023 17:58:27 +1200
>Subject: [PATCH 21/21] 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