Attachment 15670 Details for Bug 12497 – a patch that passes the tests

[patch] a patch that passes the tests

abartlet-bug-12497-inherit.patch (text/plain), 24.62 KB, created by Andrew Bartlett on 2019-12-06 05:40:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2019-12-06 05:40:42 UTC

Size: 24.62 KB

patch

obsolete

>From da124cda05f043e36726815fd7a8f62fddc58621 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Thu, 28 Nov 2019 17:16:16 +1300
>Subject: [PATCH 01/11] selftest: Add test for replication of inherited
> security descriptors
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/selftest/tests.py                  |   5 +
> source4/torture/drs/python/repl_secdesc.py | 257 +++++++++++++++++++++
> 2 files changed, 262 insertions(+)
> create mode 100644 source4/torture/drs/python/repl_secdesc.py
>
>diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
>index 642dc680fa4..a10f56bf0f1 100755
>--- a/source4/selftest/tests.py
>+++ b/source4/selftest/tests.py
>@@ -1129,6 +1129,11 @@ for env in ['vampire_dc', 'promoted_dc']:
>                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>                            environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'},
>                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
>+    planoldpythontestsuite(env, "repl_secdesc",
>+                           name="samba4.drs.repl_secdesc.python(%s)" % env,
>+                           extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>+                           environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'},
>+                           extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
>     planoldpythontestsuite(env, "repl_move",
>                            extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
>                            name="samba4.drs.repl_move.python(%s)" % env,
>diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py
>new file mode 100644
>index 00000000000..f6dd36f9226
>--- /dev/null
>+++ b/source4/torture/drs/python/repl_secdesc.py
>@@ -0,0 +1,257 @@
>+#!/usr/bin/env python3
>+# -*- coding: utf-8 -*-
>+#
>+# Unix SMB/CIFS implementation.
>+# Copyright (C) Catalyst.Net Ltd. 2017
>+# Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019
>+#
>+# 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
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+#
>+import drs_base
>+import ldb
>+import samba
>+from samba import sd_utils
>+from ldb import LdbError
>+
>+class ReplAclTestCase(drs_base.DrsBaseTestCase):
>+
>+    def setUp(self):
>+        super(ReplAclTestCase, self).setUp()
>+        self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1)
>+        self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2)
>+
>+        self.ou = samba.tests.create_test_ou(self.ldb_dc1,
>+                                             "test_acl_inherit")
>+
>+        # disable replication for the tests so we can control at what point
>+        # the DCs try to replicate
>+        self._disable_all_repl(self.dnsname_dc1)
>+        self._disable_all_repl(self.dnsname_dc2)
>+
>+        # make sure DCs are synchronized before the test
>+        self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1, forced=True)
>+        self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2, forced=True)
>+
>+    def tearDown(self):
>+        self.ldb_dc1.delete(self.ou, ["tree_delete:1"])
>+
>+        # re-enable replication
>+        self._enable_all_repl(self.dnsname_dc1)
>+        self._enable_all_repl(self.dnsname_dc2)
>+
>+        super(ReplAclTestCase, self).tearDown()
>+
>+    def test_acl_inheirt_new_object_1_pass(self):
>+        # Set the inherited ACL on the parent OU
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Make a new object
>+        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
>+        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm inherited ACLs are identical
>+
>+        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
>+                          self.sd_utils_dc2.get_sd_as_sddl(dn))
>+
>+    def test_acl_inheirt_new_object(self):
>+        # Set the inherited ACL on the parent OU
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Make a new object
>+        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
>+        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm inherited ACLs are identical
>+
>+        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
>+                          self.sd_utils_dc2.get_sd_as_sddl(dn))
>+
>+    def test_acl_inherit_existing_object(self):
>+        # Make a new object
>+        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
>+        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
>+
>+        try:
>+            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
>+                                base=dn,
>+                                attrs=[])
>+            self.fail()
>+        except LdbError as err:
>+            enum = err.args[0]
>+            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm it is now replicated
>+        self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
>+                            base=dn,
>+                            attrs=[])
>+            
>+        # Set the inherited ACL on the parent OU
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm inherited ACLs are identical
>+
>+        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
>+                          self.sd_utils_dc2.get_sd_as_sddl(dn))
>+
>+    def test_acl_inheirt_existing_object_1_pass(self):
>+        # Make a new object
>+        dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
>+        self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"})
>+
>+        try:
>+            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
>+                                base=dn,
>+                                attrs=[])
>+            self.fail()
>+        except LdbError as err:
>+            enum = err.args[0]
>+            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
>+
>+        # Set the inherited ACL on the parent OU
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm inherited ACLs are identical
>+
>+        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn),
>+                          self.sd_utils_dc2.get_sd_as_sddl(dn))
>+
>+    def test_acl_inheirt_renamed_object(self):
>+        # Make a new object
>+        new_ou = samba.tests.create_test_ou(self.ldb_dc1,
>+                                            "acl_test_l2")
>+
>+        sub_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou)
>+
>+        try:
>+            self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
>+                                base=new_ou,
>+                                attrs=[])
>+            self.fail()
>+        except LdbError as err:
>+            enum = err.args[0]
>+            self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT)
>+            
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm it is now replicated
>+        self.ldb_dc2.search(scope=ldb.SCOPE_BASE,
>+                            base=new_ou,
>+                            attrs=[])
>+
>+        # Set the inherited ACL on the parent OU on DC1
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Rename to under self.ou
>+
>+        self.ldb_dc1.rename(new_ou, sub_ou_dn)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Confirm inherited ACLs are identical
>+        self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
>+                          self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn))
>+
>+
>+    def test_acl_inheirt_renamed_object_in_conflict(self):
>+        # Make a new object to be renamed under self.ou
>+        new_ou = samba.tests.create_test_ou(self.ldb_dc1,
>+                                            "acl_test_l2")
>+
>+        # Make a new OU under self.ou (on DC2)
>+        sub_ou_dn = ldb.Dn(self.ldb_dc2, "OU=l2,%s" % self.ou)
>+        self.ldb_dc2.add({"dn": sub_ou_dn,
>+                          "objectclass": "organizationalUnit"})
>+
>+        # Set the inherited ACL on the parent OU
>+        mod =  "(A;CIOI;GA;;;SY)"
>+        self.sd_utils_dc1.dacl_add_ace(self.ou, mod)
>+
>+        # Replicate to DC2
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        # Rename to under self.ou
>+        self.ldb_dc1.rename(new_ou, sub_ou_dn)
>+
>+        # Replicate to DC2 (will cause a conflict)
>+
>+        self._net_drs_replicate(DC=self.dnsname_dc2,
>+                                fromDC=self.dnsname_dc1,
>+                                forced=True)
>+
>+        children = self.ldb_dc2.search(scope=ldb.SCOPE_ONELEVEL,
>+                                       base=self.ou,
>+                                       attrs=[])
>+        for child in children:
>+            self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn),
>+                              self.sd_utils_dc2.get_sd_as_sddl(child.dn))
>+
>+        # Replicate back
>+        self._net_drs_replicate(DC=self.dnsname_dc1,
>+                                fromDC=self.dnsname_dc2,
>+                                forced=True)
>+
>+        for child in children:
>+            self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn),
>+                              self.sd_utils_dc2.get_sd_as_sddl(child.dn))
>-- 
>2.17.1
>
>
>From 66fe58445918403ad8a6cf7851404cf00c3acbfd Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 15:44:32 +1300
>Subject: [PATCH 02/11] dsdb: Explain that
> descriptor_sd_propagation_recursive() is proctected by a transaction
>
>This means we can trust the DB did not change between the two search
>requests.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index 9018b750ab5..fb2854438e1 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -1199,6 +1199,9 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
> 	 * LDB_SCOPE_SUBTREE searches are expensive.
> 	 *
> 	 * Note: that we do not search for deleted/recycled objects
>+	 *
>+	 * We know this is safe against a rename race as we are in the
>+	 * prepare_commit(), so must be in a transaction.
> 	 */
> 	ret = dsdb_module_search(module,
> 				 change,
>-- 
>2.17.1
>
>
>From 94504fe086c51b434c9b8aa4f4b31f130bcbdd07 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 15:50:35 +1300
>Subject: [PATCH 03/11] dsdb: Fix issue where inherited Security Descriptors
> were not replicated.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> .../dsdb/samdb/ldb_modules/repl_meta_data.c   | 25 +++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index c16ea7bb616..01ab441b37e 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -5634,9 +5634,19 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar)
> 	replmd_ldb_message_sort(msg, ar->schema);
> 
> 	if (!remote_isDeleted) {
>+		/*
>+		 * Ensure any local ACL inheritence is applied from
>+		 * the parent object.
>+		 *
>+		 * This is needed because descriptor is above
>+		 * repl_meta_data in the module stack, so this will
>+		 * not be trigered 'naturally' by the flow of
>+		 * operations.
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(ar->module,
> 							  ar->objs->partition_dn,
>-							  msg->dn, true);
>+							  msg->dn,
>+							  true);
> 		if (ret != LDB_SUCCESS) {
> 			return replmd_replicated_request_error(ar, ret);
> 		}
>@@ -6320,9 +6330,20 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 	}
> 
> 	if (sd_updated && !isDeleted) {
>+		/*
>+		 * This is an existing object, so there is no need to
>+		 * inherit from the parent, but we must inherit any
>+		 * incoming changes to our child objects.
>+		 *
>+		 * This is needed because descriptor is above
>+		 * repl_meta_data in the module stack, so this will
>+		 * not be trigered 'naturally' by the flow of
>+		 * operations.
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(ar->module,
> 							  ar->objs->partition_dn,
>-							  msg->dn, true);
>+							  msg->dn,
>+							  false);
> 		if (ret != LDB_SUCCESS) {
> 			return ldb_operr(ldb);
> 		}
>-- 
>2.17.1
>
>
>From ca86b6b52aec1d31132f8d9d3a51dac44e38cfb6 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 26 Nov 2019 16:17:32 +1300
>Subject: [PATCH 04/11] dsdb: Add comments explaining why SD propagation needs
> to be done here
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index fb2854438e1..7070affa645 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -876,6 +876,9 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
> 			return ldb_oom(ldb);
> 		}
> 
>+		/*
>+		 * Force SD propagation on children of this record
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
> 							  dn, false);
> 		if (ret != LDB_SUCCESS) {
>@@ -966,6 +969,10 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
> 			return ldb_oom(ldb);
> 		}
> 
>+		/*
>+		 * Force SD propagation on this record (get a new
>+		 * inherited SD from the potentially new parent
>+		 */
> 		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
> 							  newdn, true);
> 		if (ret != LDB_SUCCESS) {
>-- 
>2.17.1
>
>
>From 3a8aad91fb4c11487f6ce980cd6f228a22386e58 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 17:54:23 +1300
>Subject: [PATCH 05/11] dsdb: Ensure we honour both change->force_self and
> change->force_children
>
>If we are renaming a DN we can be in a situation where we need to
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/descriptor.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
>index 7070affa645..b9f465fc36f 100644
>--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
>+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
>@@ -1291,6 +1291,13 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module,
> 
> 		if (cur != NULL) {
> 			DLIST_REMOVE(change->children, cur);
>+		} else if (i == 0) {
>+			/*
>+			 * in the change->force_self case
>+			 * res->msgs[0]->elements was not overwritten,
>+			 * so set cur here
>+			 */
>+			cur = change;
> 		}
> 
> 		for (c = stopped_stack; c; c = stopped_stack) {
>-- 
>2.17.1
>
>
>From 184b4b33a6d4723bf2d6121d4ac4c6535a5a1e54 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 17:55:13 +1300
>Subject: [PATCH 06/11] repl_meta_data: Add comment explaining what is being
> renamed after the conflict is resolved
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 01ab441b37e..26e6075f6b4 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6007,7 +6007,10 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 		 ldb_dn_get_linearized(ar->search_msg->dn),
> 		 ldb_dn_get_linearized(msg->dn)));
> 
>-
>+	/*
>+	 * With the other record out of the way, do the rename we had
>+	 * at the top again
>+	 */
> 	ret = dsdb_module_rename(ar->module, ar->search_msg->dn, msg->dn,
> 				 DSDB_FLAG_NEXT_MODULE, ar->req);
> 	if (ret != LDB_SUCCESS) {
>-- 
>2.17.1
>
>
>From 9f62aa28e51f6d303fa973b67ed759ca3d2c7a90 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 18:05:54 +1300
>Subject: [PATCH 07/11] repl_meta_data: schedule SD propagation to a renamed DN
>
>We need to check the SD of the parent if we rename, it is not the same as an incoming SD change.
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 26e6075f6b4..d92dea973ee 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6329,7 +6329,22 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 		  ar->index_current, msg->num_elements);
> 
> 	if (renamed) {
>-		sd_updated = true;
>+		/*
>+		 * This is an new name for this object, so we must
>+		 * inherit from the parent
>+		 *
>+		 * This is needed because descriptor is above
>+		 * repl_meta_data in the module stack, so this will
>+		 * not be trigered 'naturally' by the flow of
>+		 * operations.
>+		 */
>+		ret = dsdb_module_schedule_sd_propagation(ar->module,
>+							  ar->objs->partition_dn,
>+							  msg->dn,
>+							  true);
>+		if (ret != LDB_SUCCESS) {
>+			return ldb_operr(ldb);
>+		}
> 	}
> 
> 	if (sd_updated && !isDeleted) {
>-- 
>2.17.1
>
>
>From 323cda26d9975093f54f1a5054a6baa4ac00d4b0 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 18:08:42 +1300
>Subject: [PATCH 08/11] selftest: add better comment
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/torture/drs/python/repl_secdesc.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py
>index f6dd36f9226..7fffe820e24 100644
>--- a/source4/torture/drs/python/repl_secdesc.py
>+++ b/source4/torture/drs/python/repl_secdesc.py
>@@ -234,7 +234,8 @@ class ReplAclTestCase(drs_base.DrsBaseTestCase):
>         # Rename to under self.ou
>         self.ldb_dc1.rename(new_ou, sub_ou_dn)
> 
>-        # Replicate to DC2 (will cause a conflict)
>+        # Replicate to DC2 (will cause a conflict, DC1 to win, version
>+        # is higher since named twice)
> 
>         self._net_drs_replicate(DC=self.dnsname_dc2,
>                                 fromDC=self.dnsname_dc1,
>-- 
>2.17.1
>
>
>From 16c3566e26f60425a3bb38bd9d1cb615c6903919 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 18:26:42 +1300
>Subject: [PATCH 09/11] repl_meta_data: Set renamed = true (and so do SD
> inheritance) after any rename
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index d92dea973ee..eda7335b36c 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -6164,6 +6164,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 		 * replmd_replicated_apply_search_callback())
> 		 */
> 		ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed);
>+		renamed = true;
> 	}
> 
> 	if (ret != LDB_SUCCESS) {
>-- 
>2.17.1
>
>
>From 69790d8af26e3863da141afb212e154befb6115f Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 18:15:16 +1300
>Subject: [PATCH 10/11] repl_meta_data: Do not set *rename = true unless there
> has been a conflict on the incoming DN
>
>The normal case of a partner-sent rename is not a cause for updating the replPropertyMetaData
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index eda7335b36c..4863a54e516 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -5907,7 +5907,6 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 				 DSDB_FLAG_NEXT_MODULE, ar->req);
> 	if (ret == LDB_SUCCESS) {
> 		talloc_free(tmp_ctx);
>-		*renamed = true;
> 		return ret;
> 	}
> 
>-- 
>2.17.1
>
>
>From bf90eb381bc6deb60b62f7a7564d2e9cdc9a8210 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 6 Dec 2019 18:26:11 +1300
>Subject: [PATCH 11/11] repl_meta_data: Only reset replMetaData entry for name
> if we made a conflict name here
>
>We previously set it for any rename
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 4863a54e516..ae6c170ef53 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -5888,7 +5888,7 @@ static int replmd_replicated_apply_search_for_parent(struct replmd_replicated_re
> static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 					   struct ldb_message *msg,
> 					   struct ldb_request *parent,
>-					   bool *renamed)
>+					   bool *renamed_to_conflict)
> {
> 	int ret;
> 	TALLOC_CTX *tmp_ctx = talloc_new(msg);
>@@ -5957,7 +5957,7 @@ static int replmd_replicated_handle_rename(struct replmd_replicated_request *ar,
> 		}
> 
> 		msg->dn = new_dn;
>-		*renamed = true;
>+		*renamed_to_conflict = true;
> 		talloc_free(tmp_ctx);
> 		return LDB_SUCCESS;
> 	}
>@@ -6062,6 +6062,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 	bool take_remote_isDeleted = false;
> 	bool sd_updated = false;
> 	bool renamed = false;
>+	bool renamed_to_conflict = false;
> 	bool is_schema_nc = false;
> 	NTSTATUS nt_status;
> 	const struct ldb_val *old_rdn, *new_rdn;
>@@ -6162,7 +6163,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 		 * the peer has an older name to what we have (see
> 		 * replmd_replicated_apply_search_callback())
> 		 */
>-		ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed);
>+		ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed_to_conflict);
> 		renamed = true;
> 	}
> 
>@@ -6175,7 +6176,7 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar)
> 		return replmd_replicated_request_werror(ar, WERR_DS_DRA_DB_ERROR);
> 	}
> 
>-	if (renamed == true) {
>+	if (renamed_to_conflict == true) {
> 		/*
> 		 * Set the callback to one that will fix up the name
> 		 * metadata on the new conflict DN
>-- 
>2.17.1
>

Actions: View

Attachments on bug 12497: 15633 | 15670 | 15678 | 15679 | 15680 | 15681 | 15685 | 15686 | 15687 | 15688 | 15689 | 15706 | 15708 | 15738