[PATCH] Remove linked attributes when objects are tombstoned

Garming Sam garming at catalyst.net.nz
Thu Jul 14 12:30:01 UTC 2016


Hi,

Recently, we've found that linked attributes are a recurring problem in 
any realistic domain. They're time consuming to deal with and parse, 
easily being one of the biggest factors for performance issues in AD.

Fortunately, we've found one place in Samba where they are completely 
unnecessary. When objects are deleted, their linked attributes should be 
removed completely and not simply marked deleted as they are currently 
(performing unnecessary existence checks and would contribute to 
unnecessary DRS traffic and CPU time).

There are a number of fixes required to get this right.
1) Including a control to override the default 'mark as deleted' behaviour.
2) Using the control in the repl_meta_data module as they occur
3) Using the control in dbcheck to cleanup any existing occurrences

As part of the fixes, one-way links were also found to be implemented 
incorrectly. When the target of a one-way link is deleted, the link is 
always displayed (with the deleted DN). We also discovered some oddities 
around pseudo one-way links, but we don't have tests for fixes for those 
yet (and it's unlikely to affect performance or have any major overall 
impact).

The patches attached are most of the important changes (and the rest 
include a DB dump which is too large to attach) with the remaining in my 
dbchecker-fixes-2 branch. They should all pass autobuild right now.

https://git.samba.org/?p=garming/samba.git;a=shortlog;h=refs/heads/dbchecker-fixes-2


Any thoughts on the patches would be appreciated, otherwise, we'll try 
to get them in soon!

Cheers,

Garming


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
-------------- next part --------------
From 232fc10bea0394be850b43ec19b22b65a04c7a40 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 13 Jul 2016 17:41:51 +1200
Subject: [PATCH 01/15] dbcheck: Script swallows input when given a carriage return

Signed-off-by: Garming Sam <garming at samba.org>
---
 python/samba/dbchecker.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 039f841..3d33c38 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -434,7 +434,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
     def err_deleted_dn(self, dn, attrname, val, dsdb_dn, correct_dn):
         """handle a DN pointing to a deleted object"""
         self.report("ERROR: target DN is deleted for %s in object %s - %s" % (attrname, dn, val))
-        self.report("Target GUID points at deleted DN %s" % correct_dn)
+        self.report("Target GUID points at deleted DN %r" % str(correct_dn))
         if not self.confirm_all('Remove DN link?', 'remove_all_deleted_DN_links'):
             self.report("Not removing")
             return
-- 
1.7.0.4


From 81a84b866760f534c76b8da45603157720f4854e Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 14 Jul 2016 12:28:58 +1200
Subject: [PATCH 02/15] match_rules: Fix a duplicated check

Signed-off-by: Garming Sam <garming at samba.org>
---
 lib/ldb-samba/tests/match_rules.py |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ldb-samba/tests/match_rules.py b/lib/ldb-samba/tests/match_rules.py
index 9c92b8b..c2c44c6 100755
--- a/lib/ldb-samba/tests/match_rules.py
+++ b/lib/ldb-samba/tests/match_rules.py
@@ -722,7 +722,7 @@ class MatchRulesTests(samba.tests.TestCase):
         self.assertEqual(len(res1), 2)
         dn_list = [str(res.dn).lower() for res in res1]
         self.assertTrue(("CN=e1,%s" % self.ou).lower() in dn_list)
-        self.assertTrue(("CN=e1,%s" % self.ou).lower() in dn_list)
+        self.assertTrue(("CN=e2,%s" % self.ou).lower() in dn_list)
 
         res1 = self.ldb.search(self.ou,
                         scope=SCOPE_ONELEVEL,
@@ -730,7 +730,7 @@ class MatchRulesTests(samba.tests.TestCase):
         self.assertEqual(len(res1), 2)
         dn_list = [str(res.dn).lower() for res in res1]
         self.assertTrue(("CN=e1,%s" % self.ou).lower() in dn_list)
-        self.assertTrue(("CN=e1,%s" % self.ou).lower() in dn_list)
+        self.assertTrue(("CN=e2,%s" % self.ou).lower() in dn_list)
 
     def test_not_linked_attrs(self):
         res1 = self.ldb.search(self.base_dn,
-- 
1.7.0.4


From 7304e96ec9456c5f02fffca6dd09c742d3edd85f Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 14 Jul 2016 12:27:32 +1200
Subject: [PATCH 03/15] match_rules: Make cleanup faster and more efficient

Signed-off-by: Garming Sam <garming at samba.org>
---
 lib/ldb-samba/tests/match_rules.py |   42 +-----------------------------------
 1 files changed, 1 insertions(+), 41 deletions(-)

diff --git a/lib/ldb-samba/tests/match_rules.py b/lib/ldb-samba/tests/match_rules.py
index c2c44c6..517eede 100755
--- a/lib/ldb-samba/tests/match_rules.py
+++ b/lib/ldb-samba/tests/match_rules.py
@@ -271,27 +271,7 @@ class MatchRulesTests(samba.tests.TestCase):
 
     def tearDown(self):
         super(MatchRulesTests, self).tearDown()
-        delete_force(self.ldb, "cn=u4,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u3,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u2,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u1,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=g4,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g3,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g2,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g1,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=c1,%s" % self.ou_computers)
-        delete_force(self.ldb, "cn=c2,%s" % self.ou_computers)
-        delete_force(self.ldb, "cn=c3,%s" % self.ou_computers)
-        delete_force(self.ldb, self.ou_users)
-        delete_force(self.ldb, self.ou_groups)
-        delete_force(self.ldb, self.ou_computers)
-        delete_force(self.ldb, "OU=o4,OU=o3,OU=o2,OU=o1,%s" % self.ou)
-        delete_force(self.ldb, "OU=o3,OU=o2,OU=o1,%s" % self.ou)
-        delete_force(self.ldb, "OU=o2,OU=o1,%s" % self.ou)
-        delete_force(self.ldb, "OU=o1,%s" % self.ou)
-        delete_force(self.ldb, "CN=e2,%s" % self.ou)
-        delete_force(self.ldb, "CN=e1,%s" % self.ou)
-        delete_force(self.ldb, self.ou)
+        self.ldb.delete(self.ou, controls=['tree_delete:0'])
 
     def test_u1_member_of_g4(self):
         # Search without transitive match must return 0 results
@@ -1139,26 +1119,6 @@ class MatchRuleConditionTests(samba.tests.TestCase):
         self.question = 6*(9-2)
         self.answer = 42
 
-    def tearDown(self):
-        super(MatchRuleConditionTests, self).tearDown()
-        delete_force(self.ldb, "cn=u4,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u3,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u2,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=u1,%s" % self.ou_users)
-        delete_force(self.ldb, "cn=g4,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g3,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g2,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=g1,%s" % self.ou_groups)
-        delete_force(self.ldb, "cn=c1,%s" % self.ou_computers)
-        delete_force(self.ldb, "cn=c2,%s" % self.ou_computers)
-        delete_force(self.ldb, "cn=c3,%s" % self.ou_computers)
-        delete_force(self.ldb, "cn=c4,%s" % self.ou_computers)
-        delete_force(self.ldb, self.ou_users)
-        delete_force(self.ldb, self.ou_groups)
-        delete_force(self.ldb, self.ou_computers)
-        delete_force(self.ldb, self.ou)
-
-
     def test_g1_members(self):
         res1 = self.ldb.search(self.ou,
                                 scope=SCOPE_SUBTREE,
-- 
1.7.0.4


From ac0f2ffa804d730c1d643eb17866c11df93dca9e Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 6 Jul 2016 14:09:19 +1200
Subject: [PATCH 04/15] tests/dsdb/sam.py: remove duplicated primaryGroupID change

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/dsdb/tests/python/sam.py |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/source4/dsdb/tests/python/sam.py b/source4/dsdb/tests/python/sam.py
index 8296167..f0ec8c9 100755
--- a/source4/dsdb/tests/python/sam.py
+++ b/source4/dsdb/tests/python/sam.py
@@ -433,15 +433,6 @@ class SamTests(samba.tests.TestCase):
           FLAG_MOD_REPLACE, "primaryGroupID")
         ldb.modify(m)
 
-        # Swap the groups (does not really make sense but does the same)
-        m = Message()
-        m.dn = Dn(ldb, "cn=ldaptestuser,cn=users," + self.base_dn)
-        m["primaryGroupID"] = MessageElement(str(group_rid_1),
-          FLAG_MOD_REPLACE, "primaryGroupID")
-        m["primaryGroupID"] = MessageElement(str(group_rid_2),
-          FLAG_MOD_REPLACE, "primaryGroupID")
-        ldb.modify(m)
-
         # Old primary group should contain a "member" attribute for the user,
         # the new shouldn't contain anymore one
         res1 = ldb.search("cn=ldaptestgroup, cn=users," + self.base_dn,
-- 
1.7.0.4


From 1160eadd9b4b07584c83cc2ae400fbf254176025 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 2 Jun 2016 09:25:00 +1200
Subject: [PATCH 05/15] s4/dsdb/repl_meta_data: use local bool version of flag

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index c6dc6c6..3f76908 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -6203,7 +6203,7 @@ linked_attributes[0]:
 					  la->meta_data.originating_usn, seq_num,
 					  la->meta_data.originating_change_time,
 					  la->meta_data.version,
-					  (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?false:true);
+					  !active);
 		if (ret != LDB_SUCCESS) {
 			talloc_free(tmp_ctx);
 			return ret;
-- 
1.7.0.4


From 0e4aa0d55a1afbfada030ef05fc7ba95ba232912 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 30 Jun 2016 15:43:33 +1200
Subject: [PATCH 06/15] replmd_modify_delete: check talloc_new()

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 3f76908..d952868 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2173,7 +2173,7 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 {
 	unsigned int i;
 	struct parsed_dn *dns, *old_dns;
-	TALLOC_CTX *tmp_ctx = talloc_new(msg);
+	TALLOC_CTX *tmp_ctx = NULL;
 	int ret;
 	const struct GUID *invocation_id;
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
@@ -2191,6 +2191,11 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 		return LDB_ERR_NO_SUCH_ATTRIBUTE;
 	}
 
+	tmp_ctx = talloc_new(msg);
+	if (tmp_ctx == NULL) {
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
 	ret = get_parsed_dns(module, tmp_ctx, el, &dns, schema_attr->syntax->ldap_oid, parent);
 	if (ret != LDB_SUCCESS) {
 		talloc_free(tmp_ctx);
-- 
1.7.0.4


From 6bacc6adc4c0124ac34b234b09e55fbdab87f37c Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 6 Jul 2016 11:53:19 +1200
Subject: [PATCH 07/15] repl_meta_data: free context on error in replmd_modify_la_delete()

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index d952868..efd1932 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2210,6 +2210,7 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 
 	invocation_id = samdb_ntds_invocation_id(ldb);
 	if (!invocation_id) {
+		talloc_free(tmp_ctx);
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
@@ -2234,8 +2235,10 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 			ldb_asprintf_errstring(ldb, "Attribute %s doesn't exist for target GUID %s",
 					       el->name, GUID_buf_string(&p->guid, &buf));
 			if (ldb_attr_cmp(el->name, "member") == 0) {
+				talloc_free(tmp_ctx);
 				return LDB_ERR_UNWILLING_TO_PERFORM;
 			} else {
+				talloc_free(tmp_ctx);
 				return LDB_ERR_NO_SUCH_ATTRIBUTE;
 			}
 		}
@@ -2245,8 +2248,10 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 			ldb_asprintf_errstring(ldb, "Attribute %s already deleted for target GUID %s",
 					       el->name, GUID_buf_string(&p->guid, &buf));
 			if (ldb_attr_cmp(el->name, "member") == 0) {
+				talloc_free(tmp_ctx);
 				return LDB_ERR_UNWILLING_TO_PERFORM;
 			} else {
+				talloc_free(tmp_ctx);
 				return LDB_ERR_NO_SUCH_ATTRIBUTE;
 			}
 		}
-- 
1.7.0.4


From bc668795394c7e72ed086d071ae8feaf1329e6ae Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 6 Jul 2016 11:54:25 +1200
Subject: [PATCH 08/15] dsdb: add vanish links control

Normally linked attributes are deleted by marking them as with RMD flags,
but sometimes we want them to vanish without trace. At those times we
set the DSDB_CONTROL_REPLMD_VANISH_LINKS control.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/common/util.c                      |    7 ++
 source4/dsdb/common/util.h                      |   33 +++---
 source4/dsdb/pydsdb.c                           |    1 +
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c |  129 ++++++++++++++++++-----
 source4/dsdb/samdb/samdb.h                      |    3 +
 source4/setup/schema_samba4.ldif                |    1 +
 6 files changed, 133 insertions(+), 41 deletions(-)

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index bd0b5a3..0bbf402 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -4288,6 +4288,13 @@ int dsdb_request_add_controls(struct ldb_request *req, uint32_t dsdb_flags)
 		}
 	}
 
+	if (dsdb_flags & DSDB_REPLMD_VANISH_LINKS) {
+		ret = ldb_request_add_control(req, DSDB_CONTROL_REPLMD_VANISH_LINKS, true, NULL);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+	}
+
 	if (dsdb_flags & DSDB_MODIFY_PARTIAL_REPLICA) {
 		ret = ldb_request_add_control(req, DSDB_CONTROL_PARTIAL_REPLICA, false, NULL);
 		if (ret != LDB_SUCCESS) {
diff --git a/source4/dsdb/common/util.h b/source4/dsdb/common/util.h
index 1085073..f2867a2 100644
--- a/source4/dsdb/common/util.h
+++ b/source4/dsdb/common/util.h
@@ -26,22 +26,23 @@
    flags for dsdb_request_add_controls(). For the module functions,
    the upper 16 bits are in dsdb/samdb/ldb_modules/util.h
 */
-#define DSDB_SEARCH_SEARCH_ALL_PARTITIONS     0x0001
-#define DSDB_SEARCH_SHOW_DELETED              0x0002
-#define DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT 0x0004
-#define DSDB_SEARCH_REVEAL_INTERNALS          0x0008
-#define DSDB_SEARCH_SHOW_EXTENDED_DN          0x0010
-#define DSDB_MODIFY_RELAX		      0x0020
-#define DSDB_MODIFY_PERMISSIVE		      0x0040
-#define DSDB_FLAG_AS_SYSTEM		      0x0080
-#define DSDB_TREE_DELETE		      0x0100
-#define DSDB_SEARCH_ONE_ONLY		      0x0200 /* give an error unless 1 record */
-#define DSDB_SEARCH_SHOW_RECYCLED	      0x0400
-#define DSDB_PROVISION			      0x0800
-#define DSDB_BYPASS_PASSWORD_HASH	      0x1000
-#define DSDB_SEARCH_NO_GLOBAL_CATALOG	      0x2000
-#define DSDB_MODIFY_PARTIAL_REPLICA	      0x4000
-#define DSDB_PASSWORD_BYPASS_LAST_SET         0x8000
+#define DSDB_SEARCH_SEARCH_ALL_PARTITIONS     0x00001
+#define DSDB_SEARCH_SHOW_DELETED              0x00002
+#define DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT 0x00004
+#define DSDB_SEARCH_REVEAL_INTERNALS          0x00008
+#define DSDB_SEARCH_SHOW_EXTENDED_DN          0x00010
+#define DSDB_MODIFY_RELAX		      0x00020
+#define DSDB_MODIFY_PERMISSIVE		      0x00040
+#define DSDB_FLAG_AS_SYSTEM		      0x00080
+#define DSDB_TREE_DELETE		      0x00100
+#define DSDB_SEARCH_ONE_ONLY		      0x00200 /* give an error unless 1 record */
+#define DSDB_SEARCH_SHOW_RECYCLED	      0x00400
+#define DSDB_PROVISION			      0x00800
+#define DSDB_BYPASS_PASSWORD_HASH	      0x01000
+#define DSDB_SEARCH_NO_GLOBAL_CATALOG	      0x02000
+#define DSDB_MODIFY_PARTIAL_REPLICA	      0x04000
+#define DSDB_PASSWORD_BYPASS_LAST_SET         0x08000
+#define DSDB_REPLMD_VANISH_LINKS              0x10000
 
 bool is_attr_in_list(const char * const * attrs, const char *attr);
 
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index faed682..efaf66b 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1323,6 +1323,7 @@ void initdsdb(void)
 	ADD_DSDB_STRING(DSDB_SYNTAX_OR_NAME);
 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
 	ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
+	ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
 	ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
 	ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
 
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index efd1932..824d278 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2177,6 +2177,9 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 	int ret;
 	const struct GUID *invocation_id;
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct ldb_control *vanish_links_ctrl = NULL;
+	bool vanish_links = false;
+	unsigned int num_to_delete = el->num_values;
 	NTTIME now;
 
 	unix_to_nt_time(&now, t);
@@ -2220,11 +2223,20 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 		return ret;
 	}
 
+	if (parent) {
+		vanish_links_ctrl = ldb_request_get_control(parent, DSDB_CONTROL_REPLMD_VANISH_LINKS);
+		if (vanish_links_ctrl) {
+			vanish_links = true;
+			vanish_links_ctrl->critical = false;
+		}
+	}
+
+	el->num_values = 0;
 	el->values = NULL;
 
 	/* see if we are being asked to delete any links that
 	   don't exist or are already deleted */
-	for (i=0; i<el->num_values; i++) {
+	for (i=0; i < num_to_delete; i++) {
 		struct parsed_dn *p = &dns[i];
 		struct parsed_dn *p2;
 		uint32_t rmd_flags;
@@ -2245,8 +2257,15 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 		rmd_flags = dsdb_dn_rmd_flags(p2->dsdb_dn->dn);
 		if (rmd_flags & DSDB_RMD_FLAG_DELETED) {
 			struct GUID_txt_buf buf;
+			const char *guid_str = GUID_buf_string(&p->guid, &buf);
+			if (vanish_links) {
+				DEBUG(0, ("Deleting deleted linked attribute %s to %s, "
+					  "because vanish_links control is set\n",
+					  el->name, guid_str));
+				continue;
+			}
 			ldb_asprintf_errstring(ldb, "Attribute %s already deleted for target GUID %s",
-					       el->name, GUID_buf_string(&p->guid, &buf));
+					       el->name, guid_str);
 			if (ldb_attr_cmp(el->name, "member") == 0) {
 				talloc_free(tmp_ctx);
 				return LDB_ERR_UNWILLING_TO_PERFORM;
@@ -2257,34 +2276,75 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 		}
 	}
 
-	/* for each new value, see if it exists already with the same GUID
-	   if it is not already deleted and matches the delete list then delete it
-	*/
-	for (i=0; i<old_el->num_values; i++) {
-		struct parsed_dn *p = &old_dns[i];
-		uint32_t rmd_flags;
+	if (vanish_links) {
+		if (num_to_delete == old_el->num_values || num_to_delete == 0) {
+			el->flags = LDB_FLAG_MOD_REPLACE;
 
-		if (el->num_values && parsed_dn_find(dns, el->num_values, &p->guid, NULL) == NULL) {
-			continue;
+			for (i = 0; i < old_el->num_values; i++) {
+				ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, true);
+				if (ret != LDB_SUCCESS) {
+					talloc_free(tmp_ctx);
+					return ret;
+				}
+			}
+			talloc_free(tmp_ctx);
+			return LDB_SUCCESS;
+		} else {
+			unsigned int num_values = 0;
+			unsigned int j = 0;
+			for (i = 0; i < old_el->num_values; i++) {
+				if (parsed_dn_find(dns, num_to_delete, &old_dns[i].guid, NULL) != NULL) {
+					/* The element is in the delete list. mark it dead. */
+					ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, true);
+					if (ret != LDB_SUCCESS) {
+						talloc_free(tmp_ctx);
+						return ret;
+					}
+					old_dns[i].v->length = 0;
+				} else {
+					num_values++;
+				}
+			}
+			for (i = 0; i < old_el->num_values; i++) {
+				if (old_el->values[i].length != 0) {
+					old_el->values[j] = old_el->values[i];
+					j++;
+					if (j == num_values) {
+						break;
+					}
+				}
+			}
+			old_el->num_values = num_values;
 		}
+	} else {
 
-		rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn);
-		if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue;
+		/* for each new value, see if it exists already with the same GUID
+		   if it is not already deleted and matches the delete list then delete it
+		*/
+		for (i=0; i<old_el->num_values; i++) {
+			struct parsed_dn *p = &old_dns[i];
+			uint32_t rmd_flags;
 
-		ret = replmd_update_la_val(old_el->values, p->v, p->dsdb_dn, p->dsdb_dn,
-					   invocation_id, seq_num, seq_num, now, 0, true);
-		if (ret != LDB_SUCCESS) {
-			talloc_free(tmp_ctx);
-			return ret;
-		}
+			if (num_to_delete && parsed_dn_find(dns, num_to_delete, &p->guid, NULL) == NULL) {
+				continue;
+			}
 
-		ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, true);
-		if (ret != LDB_SUCCESS) {
-			talloc_free(tmp_ctx);
-			return ret;
+			rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn);
+			if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue;
+
+			ret = replmd_update_la_val(old_el->values, p->v, p->dsdb_dn, p->dsdb_dn,
+						   invocation_id, seq_num, seq_num, now, 0, true);
+			if (ret != LDB_SUCCESS) {
+				talloc_free(tmp_ctx);
+				return ret;
+			}
+			ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, true);
+			if (ret != LDB_SUCCESS) {
+				talloc_free(tmp_ctx);
+				return ret;
+			}
 		}
 	}
-
 	el->values = talloc_steal(msg->elements, old_el->values);
 	el->num_values = old_el->num_values;
 
@@ -2691,6 +2751,11 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
 
 	/* current partition control is needed by "replmd_op_callback" */
 	if (ldb_request_get_control(req, DSDB_CONTROL_CURRENT_PARTITION_OID) == NULL) {
+		struct ldb_control *ctrl = ldb_request_get_control(down_req,
+								   DSDB_CONTROL_REPLMD_VANISH_LINKS);
+		if (ctrl) {
+			ctrl->critical = false;
+		}
 		ret = ldb_request_add_control(down_req,
 					      DSDB_CONTROL_CURRENT_PARTITION_OID,
 					      false, NULL);
@@ -3015,6 +3080,7 @@ static int replmd_delete_remove_link(struct ldb_module *module,
 		const struct dsdb_attribute *target_attr;
 		struct ldb_message_element *el2;
 		struct ldb_val dn_val;
+		uint32_t dsdb_flags = 0;
 
 		if (dsdb_dn_is_deleted_val(&el->values[i])) {
 			continue;
@@ -3058,7 +3124,13 @@ static int replmd_delete_remove_link(struct ldb_module *module,
 		el2->values = &dn_val;
 		el2->num_values = 1;
 
-		ret = dsdb_module_modify(module, msg, DSDB_FLAG_OWN_MODULE, parent);
+		/*
+		 * Ensure that we tell the modification to vanish any linked
+		 * attributes (not simply mark them as isDeleted = TRUE)
+		 */
+		dsdb_flags |= DSDB_REPLMD_VANISH_LINKS;
+
+		ret = dsdb_module_modify(module, msg, dsdb_flags|DSDB_FLAG_OWN_MODULE, parent);
 		if (ret != LDB_SUCCESS) {
 			talloc_free(tmp_ctx);
 			return ret;
@@ -3146,6 +3218,7 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request
 		NULL
 	};
 	unsigned int i, el_count = 0;
+	uint32_t dsdb_flags = 0;
 	enum deletion_state deletion_state, next_deletion_state;
 
 	if (ldb_dn_is_special(req->op.del.dn)) {
@@ -3505,6 +3578,12 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request
 				if (sa->searchFlags & SEARCH_FLAG_PRESERVEONDELETE) {
 					continue;
 				}
+			} else {
+				/*
+				 * Ensure that we tell the modification to vanish any linked
+				 * attributes (not simply mark them as isDeleted = TRUE)
+				 */
+				dsdb_flags |= DSDB_REPLMD_VANISH_LINKS;
 			}
 			ret = ldb_msg_add_empty(msg, el->name, LDB_FLAG_MOD_DELETE, &el);
 			if (ret != LDB_SUCCESS) {
@@ -3602,7 +3681,7 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request
 		msg->dn = new_dn;
 	}
 
-	ret = dsdb_module_modify(module, msg, DSDB_FLAG_OWN_MODULE, req);
+	ret = dsdb_module_modify(module, msg, dsdb_flags|DSDB_FLAG_OWN_MODULE, req);
 	if (ret != LDB_SUCCESS) {
 		ldb_asprintf_errstring(ldb, "replmd_delete: Failed to modify object %s in delete - %s",
 				       ldb_dn_get_linearized(old_dn), ldb_errstring(ldb));
diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
index 0970948..438960d 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -182,6 +182,9 @@ struct dsdb_control_password_user_account_control {
  */
 #define DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID "1.3.6.1.4.1.7165.4.3.28"
 
+/* passed when we want to thoroughly delete linked attributes */
+#define DSDB_CONTROL_REPLMD_VANISH_LINKS "1.3.6.1.4.1.7165.4.3.29"
+
 #define DSDB_EXTENDED_REPLICATED_OBJECTS_OID "1.3.6.1.4.1.7165.4.4.1"
 struct dsdb_extended_replicated_object {
 	struct ldb_message *msg;
diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
index ac56f51..604e115 100644
--- a/source4/setup/schema_samba4.ldif
+++ b/source4/setup/schema_samba4.ldif
@@ -214,6 +214,7 @@
 #Allocated: DSDB_CONTROL_PASSWORD_DEFAULT_LAST_SET_OID 1.3.6.1.4.1.7165.4.3.26
 #Allocated: DSDB_CONTROL_PASSWORD_USER_ACCOUNT_CONTROL_OID 1.3.6.1.4.1.7165.4.3.27
 #Allocated: DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID 1.3.6.1.4.1.7165.4.3.28
+#Allocated: DSDB_CONTROL_REPLMD_VANISH_LINKS 1.3.6.1.4.1.7165.4.3.29
 
 # Extended 1.3.6.1.4.1.7165.4.4.x
 #Allocated: DSDB_EXTENDED_REPLICATED_OBJECTS_OID 1.3.6.1.4.1.7165.4.4.1
-- 
1.7.0.4


From 6a06228f9ef9865ae4ad7cb09151a12df6621aa0 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 30 Jun 2016 16:35:08 +1200
Subject: [PATCH 09/15] dsdb tests: add linked attribute tests

Note that this test will not work properly across ldap as the
marked-deleted linked attributes will not appear.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 source4/dsdb/tests/python/linked_attributes.py |  363 ++++++++++++++++++++++++
 source4/selftest/tests.py                      |    1 +
 2 files changed, 364 insertions(+), 0 deletions(-)
 create mode 100644 source4/dsdb/tests/python/linked_attributes.py

diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py
new file mode 100644
index 0000000..c7afad4
--- /dev/null
+++ b/source4/dsdb/tests/python/linked_attributes.py
@@ -0,0 +1,363 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# Originally based on ./sam.py
+import optparse
+import sys
+import os
+import base64
+import random
+import re
+
+sys.path.insert(0, "bin/python")
+import samba
+from samba.tests.subunitrun import SubunitOptions, TestProgram
+
+import samba.getopt as options
+
+from samba.auth import system_session
+import ldb
+from samba.samdb import SamDB
+from samba.dcerpc import misc
+
+import time
+
+parser = optparse.OptionParser("linked_attributes.py [options] <host>")
+sambaopts = options.SambaOptions(parser)
+parser.add_option_group(sambaopts)
+parser.add_option_group(options.VersionOptions(parser))
+# use command line creds if available
+credopts = options.CredentialsOptions(parser)
+parser.add_option_group(credopts)
+subunitopts = SubunitOptions(parser)
+parser.add_option_group(subunitopts)
+
+parser.add_option('--delete-in-setup', action='store_true',
+                  help="cleanup in setup")
+
+parser.add_option('--no-cleanup', action='store_true',
+                  help="don't cleanup in teardown")
+
+parser.add_option('--no-reveal-internals', action='store_true',
+                  help="Only use windows compatible ldap controls")
+
+opts, args = parser.parse_args()
+
+if len(args) < 1:
+    parser.print_usage()
+    sys.exit(1)
+
+host = args[0]
+
+lp = sambaopts.get_loadparm()
+creds = credopts.get_credentials(lp)
+
+
+class LATestException(Exception):
+    pass
+
+
+class LATests(samba.tests.TestCase):
+
+    def setUp(self):
+        super(LATests, self).setUp()
+        self.samdb = SamDB(host, credentials=creds,
+                         session_info=system_session(lp), lp=lp)
+
+        self.base_dn = self.samdb.domain_dn()
+        self.ou = "OU=la,%s" % self.base_dn
+        if opts.delete_in_setup:
+            try:
+                self.samdb.delete(self.ou, ['tree_delete:1'])
+            except ldb.LdbError, e:
+                print "tried deleting %s, got error %s" % (self.ou, e)
+        self.samdb.add({'objectclass': 'organizationalUnit',
+                        'dn': self.ou})
+
+    def tearDown(self):
+        super(LATests, self).tearDown()
+        if not opts.no_cleanup:
+            self.samdb.delete(self.ou, ['tree_delete:1'])
+
+    def delete_user(self, user):
+        self.samdb.delete(user['dn'])
+        del self.users[self.users.index(user)]
+
+    def add_object(self, cn, objectclass):
+        dn = "CN=%s,%s" % (cn, self.ou)
+        self.samdb.add({'cn': cn,
+                      'objectclass': objectclass,
+                      'dn': dn})
+
+        return dn
+
+    def add_objects(self, n, objectclass, prefix=None):
+        if prefix is None:
+            prefix = objectclass
+        dns = []
+        for i in range(n):
+            dns.append(self.add_object("%s%d" % (prefix, i + 1),
+                                       objectclass))
+        return dns
+
+    def add_linked_attribute(self, src, dest, attr='member'):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.samdb, src)
+        m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_ADD, attr)
+        self.samdb.modify(m)
+
+    def remove_linked_attribute(self, src, dest, attr='member'):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.samdb, src)
+        m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_DELETE, attr)
+        self.samdb.modify(m)
+
+    def attr_search(self, obj, expected, attr, scope=ldb.SCOPE_BASE,
+                    **controls):
+        if opts.no_reveal_internals:
+            if 'reveal_internals' in controls:
+                del controls['reveal_internals']
+
+        controls = ['%s:%d' % (k, int(v)) for k, v in controls.items()]
+
+        res = self.samdb.search(obj,
+                                scope=scope,
+                                attrs=[attr],
+                                controls=controls)
+        return res
+
+    def assert_links(self, obj, expected, attr, sorted=False, msg='',
+                     **kwargs):
+        res = self.attr_search(obj, expected, attr, **kwargs)
+
+        if len(expected) == 0:
+            if attr in res[0]:
+                self.fail("found attr '%s' in %s" % (attr, res[0]))
+            return
+
+        try:
+            results = list([x[attr] for x in res][0])
+        except KeyError:
+            self.fail("missing attr '%s' on %s" % (attr, obj))
+
+        if sorted == False:
+            results = set(results)
+            expected = set(expected)
+
+        if expected != results:
+            print msg
+            print "expected %s" % expected
+            print "received %s" % results
+
+        self.assertEqual(results, expected)
+
+    def assert_back_links(self, obj, expected, attr='memberOf', **kwargs):
+        self.assert_links(obj, expected, attr=attr,
+                          msg='back links do not match', **kwargs)
+
+    def assert_forward_links(self, obj, expected, attr='member', **kwargs):
+        self.assert_links(obj, expected, attr=attr,
+                          msg='forward links do not match', **kwargs)
+
+    def get_object_guid(self, dn):
+        res = self.samdb.search(dn,
+                                scope=ldb.SCOPE_BASE,
+                                attrs=['objectGUID'])
+        return str(misc.GUID(res[0]['objectGUID'][0]))
+
+    def _test_la_backlinks(self, reveal=False):
+        tag = 'backlinks'
+        kwargs = {}
+        if reveal:
+            tag += '_reveal'
+            kwargs = {'reveal_internals': 0}
+
+        u1, u2 = self.add_objects(2, 'user', 'u_%s' % tag)
+        g1, g2 = self.add_objects(2, 'group', 'g_%s' % tag)
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.assert_back_links(u1, [g1, g2], **kwargs)
+        self.assert_back_links(u2, [g2], **kwargs)
+
+    def test_la_backlinks(self):
+        self._test_la_backlinks()
+
+    def test_la_backlinks_reveal(self):
+        if opts.no_reveal_internals:
+            print 'skipping because --no-reveal-internals'
+            return
+        self._test_la_backlinks(True)
+
+    def _test_la_backlinks_delete_group(self, reveal=False):
+        tag = 'del_group'
+        kwargs = {}
+        if reveal:
+            tag += '_reveal'
+            kwargs = {'reveal_internals': 0}
+
+        u1, u2 = self.add_objects(2, 'user', 'u_' + tag)
+        g1, g2 = self.add_objects(2, 'group', 'g_' + tag)
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(g2, ['tree_delete:1'])
+
+        self.assert_back_links(u1, [g1], **kwargs)
+        self.assert_back_links(u2, set(), **kwargs)
+
+    def test_la_backlinks_delete_group(self):
+        self._test_la_backlinks_delete_group()
+
+    def test_la_backlinks_delete_group_reveal(self):
+        if opts.no_reveal_internals:
+            print 'skipping because --no-reveal-internals'
+            return
+        self._test_la_backlinks_delete_group(True)
+
+    def test_links_all_delete_group(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_all_del_group')
+        g1, g2 = self.add_objects(2, 'group', 'g_all_del_group')
+        g2guid = self.get_object_guid(g2)
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(g2)
+        self.assert_back_links(u1, [g1], show_deleted=1, show_recycled=1,
+                               show_deactivated_link=0)
+        self.assert_back_links(u2, set(), show_deleted=1, show_recycled=1,
+                               show_deactivated_link=0)
+        self.assert_forward_links(g1, [u1], show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0)
+        self.assert_forward_links('<GUID=%s>' % g2guid,
+                                  [], show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0)
+
+    def test_links_all_delete_group_reveal(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_all_del_group_reveal')
+        g1, g2 = self.add_objects(2, 'group', 'g_all_del_group_reveal')
+        g2guid = self.get_object_guid(g2)
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(g2)
+        self.assert_back_links(u1, [g1], show_deleted=1, show_recycled=1,
+                               show_deactivated_link=0,
+                                  reveal_internals=0)
+        self.assert_back_links(u2, set(), show_deleted=1, show_recycled=1,
+                               show_deactivated_link=0,
+                                  reveal_internals=0)
+        self.assert_forward_links(g1, [u1], show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0,
+                                  reveal_internals=0)
+        self.assert_forward_links('<GUID=%s>' % g2guid,
+                                  [], show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0,
+                                  reveal_internals=0)
+
+    def test_la_links_delete_link(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_link')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_link')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.remove_linked_attribute(g2, u1)
+
+        self.assert_forward_links(g1, [u1])
+        self.assert_forward_links(g2, [u2])
+
+        self.add_linked_attribute(g2, u1)
+        self.assert_forward_links(g2, [u1, u2])
+        self.remove_linked_attribute(g2, u2)
+        self.assert_forward_links(g2, [u1])
+        self.remove_linked_attribute(g2, u1)
+        self.assert_forward_links(g2, [])
+
+    def test_la_links_delete_link_reveal(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_link_reveal')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_link_reveal')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.remove_linked_attribute(g2, u1)
+
+        self.assert_forward_links(g2, [u1, u2], show_deleted=1,
+                                  show_recycled=1,
+                                  show_deactivated_link=0,
+                                  reveal_internals=0
+        )
+
+    def test_la_links_delete_user(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_user')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_user')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(u1)
+
+        self.assert_forward_links(g1, [])
+        self.assert_forward_links(g2, [u2])
+
+    def test_la_links_delete_user_reveal(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_user_reveal')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_user_reveal')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(u1)
+
+        self.assert_forward_links(g2, [u2],
+                                  show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0,
+                                  reveal_internals=0)
+        self.assert_forward_links(g1, [],
+                                  show_deleted=1, show_recycled=1,
+                                  show_deactivated_link=0,
+                                  reveal_internals=0)
+
+    def _test_la_links_sort_order(self):
+        u1, u2, u3 = self.add_objects(3, 'user', 'u_sort_order')
+        g1, g2, g3 = self.add_objects(3, 'group', 'g_sort_order')
+
+        # Add these in a haphazard order
+        self.add_linked_attribute(g2, u3)
+        self.add_linked_attribute(g3, u2)
+        self.add_linked_attribute(g1, u3)
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+        self.add_linked_attribute(g3, u3)
+        self.add_linked_attribute(g3, u1)
+
+        self.assert_forward_links(g1, [u3, u1], sorted=True)
+        self.assert_forward_links(g2, [u3, u2, u1], sorted=True)
+        self.assert_forward_links(g3, [u3, u2, u1], sorted=True)
+
+        self.assert_back_links(u1, [g3, g2, g1], sorted=True)
+        self.assert_back_links(u2, [g3, g2], sorted=True)
+        self.assert_back_links(u3, [g3, g2, g1], sorted=True)
+
+
+if "://" not in host:
+    if os.path.isfile(host):
+        host = "tdb://%s" % host
+    else:
+        host = "ldap://%s" % host
+
+
+TestProgram(module=__name__, opts=subunitopts)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 1b13940..01cb87b 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -583,6 +583,7 @@ plantestsuite_loadlist("samba4.ldap.sites.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [
 
 plantestsuite_loadlist("samba4.ldap.sort.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/sort.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
 plantestsuite_loadlist("samba4.ldap.vlv.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/vlv.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
+plantestsuite_loadlist("samba4.ldap.linked_attributes.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(samba4srcdir, "dsdb/tests/python/linked_attributes.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
 
 for env in ["ad_dc_ntvfs", "fl2000dc", "fl2003dc", "fl2008r2dc"]:
     plantestsuite_loadlist("samba4.ldap_schema.python(%s)" % env, env, [python, os.path.join(samba4srcdir, "dsdb/tests/python/ldap_schema.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
-- 
1.7.0.4


From 988ae8fc985ca898194a17406c51ad26412507c2 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 14 Jul 2016 18:03:33 +1200
Subject: [PATCH 10/15] drs tests: querying linked attribute over DRS

Without the deactivated links control, we assert certain conditions over DRS
instead.
---
 source4/selftest/tests.py                          |    5 +
 .../torture/drs/python/linked_attributes_drs.py    |  212 ++++++++++++++++++++
 2 files changed, 217 insertions(+), 0 deletions(-)
 create mode 100644 source4/torture/drs/python/linked_attributes_drs.py

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 01cb87b..8b2512f 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -674,6 +674,11 @@ for env in ['vampire_dc', 'promoted_dc']:
                            name="samba4.drs.getnc_exop.python(%s)" % env,
                            environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
                            extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
+    planoldpythontestsuite(env, "linked_attributes_drs",
+                           extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')],
+                           name="samba4.drs.linked_attributes_drs.python(%s)" % env,
+                           environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()},
+                           extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD'])
 
 
 planoldpythontestsuite("chgdcpass:local", "samba.tests.blackbox.samba_dnsupdate",
diff --git a/source4/torture/drs/python/linked_attributes_drs.py b/source4/torture/drs/python/linked_attributes_drs.py
new file mode 100644
index 0000000..04d31c2
--- /dev/null
+++ b/source4/torture/drs/python/linked_attributes_drs.py
@@ -0,0 +1,212 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# Originally based on ./sam.py
+import sys
+import os
+import base64
+import random
+import re
+
+sys.path.insert(0, "bin/python")
+import samba
+from samba.tests.subunitrun import SubunitOptions, TestProgram
+
+import samba.getopt as options
+
+from samba.auth import system_session
+import ldb
+from samba.samdb import SamDB
+from samba.dcerpc import misc
+
+from samba.dcerpc import drsuapi, misc, drsblobs
+from samba.drs_utils import drs_DsBind
+from samba.ndr import ndr_unpack, ndr_pack
+
+import drs_base
+
+import time
+
+
+class LATestException(Exception):
+    pass
+
+class ExopBaseTest:
+    def _exop_req8(self, dest_dsa, invocation_id, nc_dn_str, exop,
+                   replica_flags=0, max_objects=0):
+        req8 = drsuapi.DsGetNCChangesRequest8()
+
+        req8.destination_dsa_guid = misc.GUID(dest_dsa) if dest_dsa else misc.GUID()
+        req8.source_dsa_invocation_id = misc.GUID(invocation_id)
+        req8.naming_context = drsuapi.DsReplicaObjectIdentifier()
+        req8.naming_context.dn = unicode(nc_dn_str)
+        req8.highwatermark = drsuapi.DsReplicaHighWaterMark()
+        req8.highwatermark.tmp_highest_usn = 0
+        req8.highwatermark.reserved_usn = 0
+        req8.highwatermark.highest_usn = 0
+        req8.uptodateness_vector = None
+        req8.replica_flags = replica_flags
+        req8.max_object_count = max_objects
+        req8.max_ndr_size = 402116
+        req8.extended_op = exop
+        req8.fsmo_info = 0
+        req8.partial_attribute_set = None
+        req8.partial_attribute_set_ex = None
+        req8.mapping_ctr.num_mappings = 0
+        req8.mapping_ctr.mappings = None
+
+        return req8
+
+    def _ds_bind(self, server_name):
+        binding_str = "ncacn_ip_tcp:%s[seal]" % server_name
+
+        drs = drsuapi.drsuapi(binding_str, self.get_loadparm(), self.get_credentials())
+        (drs_handle, supported_extensions) = drs_DsBind(drs)
+        return (drs, drs_handle)
+
+    
+class LATests(drs_base.DrsBaseTestCase, ExopBaseTest):
+
+    def setUp(self):
+        super(LATests, self).setUp()
+        # DrsBaseTestCase sets up self.ldb_dc1, self.ldb_dc2
+        # we're only using one
+        self.samdb = self.ldb_dc1
+        
+        self.base_dn = self.samdb.domain_dn()
+        self.ou = "OU=la,%s" % self.base_dn
+        if True:
+            try:
+                self.samdb.delete(self.ou, ['tree_delete:1'])
+            except ldb.LdbError, e:
+                pass
+        self.samdb.add({'objectclass': 'organizationalUnit',
+                        'dn': self.ou})
+
+        self.dc_guid = self.samdb.get_invocation_id()
+        self.drs, self.drs_handle = self._ds_bind(self.dnsname_dc1)
+
+    def tearDown(self):
+        super(LATests, self).tearDown()
+        try:
+            self.samdb.delete(self.ou, ['tree_delete:1'])
+        except ldb.LdbError, e:
+            pass
+
+    def delete_user(self, user):
+        self.samdb.delete(user['dn'])
+        del self.users[self.users.index(user)]
+
+    def add_object(self, cn, objectclass):
+        dn = "CN=%s,%s" % (cn, self.ou)
+        self.samdb.add({'cn': cn,
+                      'objectclass': objectclass,
+                      'dn': dn})
+
+        return dn
+
+    def add_objects(self, n, objectclass, prefix=None):
+        if prefix is None:
+            prefix = objectclass
+        dns = []
+        for i in range(n):
+            dns.append(self.add_object("%s%d" % (prefix, i + 1),
+                                       objectclass))
+        return dns
+
+    def add_linked_attribute(self, src, dest, attr='member'):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.samdb, src)
+        m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_ADD, attr)
+        self.samdb.modify(m)
+
+    def remove_linked_attribute(self, src, dest, attr='member'):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.samdb, src)
+        m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_DELETE, attr)
+        self.samdb.modify(m)
+
+    def attr_search(self, obj, expected, attr, scope=ldb.SCOPE_BASE):
+
+        req8 = self._exop_req8(dest_dsa=None,
+                               invocation_id=self.dc_guid,
+                               nc_dn_str=obj,
+                               exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ)
+
+        level, ctr = self.drs.DsGetNCChanges(self.drs_handle, 8, req8)
+        expected_attid = getattr(drsuapi, 'DRSUAPI_ATTID_' + attr)
+
+        links = []
+        for link in ctr.linked_attributes:
+            if link.attid == expected_attid:
+                unpacked = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
+                                      link.value.blob)
+                active = link.flags &  drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE
+                links.append((str(unpacked.dn), bool(active)))
+
+        return links
+
+
+    def assert_forward_links(self, obj, expected, attr='member'):
+        results = self.attr_search(obj, expected, attr)
+        self.assertEqual(len(results), len(expected))
+
+        for k, v in results:
+            self.assertTrue(k in expected) 
+            self.assertEqual(expected[k], v, "%s active flag should be %d, not %d" %
+                             (k, expected[k], v))
+
+    def get_object_guid(self, dn):
+        res = self.samdb.search(dn,
+                                scope=ldb.SCOPE_BASE,
+                                attrs=['objectGUID'])
+        return str(misc.GUID(res[0]['objectGUID'][0]))
+
+    def test_links_all_delete_group(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_all_del_group')
+        g1, g2 = self.add_objects(2, 'group', 'g_all_del_group')
+        g2guid = self.get_object_guid(g2)
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(g2)
+        self.assert_forward_links(g1, {u1: True})
+        res = self.samdb.search('<GUID=%s>' % g2guid,
+                                scope=ldb.SCOPE_BASE,
+                                controls=['show_deleted:1'])
+        new_dn = res[0].dn
+        self.assert_forward_links(new_dn, {})
+
+
+    def test_la_links_delete_link(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_link')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_link')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.remove_linked_attribute(g2, u1)
+
+        self.assert_forward_links(g1, {u1: True})
+        self.assert_forward_links(g2, {u1: False, u2: True})
+
+        self.add_linked_attribute(g2, u1)
+        self.remove_linked_attribute(g2, u2)
+        self.assert_forward_links(g2, {u1: True, u2: False})
+        self.remove_linked_attribute(g2, u1)
+        self.assert_forward_links(g2, {u1: False, u2: False})
+
+    def test_la_links_delete_user(self):
+        u1, u2 = self.add_objects(2, 'user', 'u_del_user')
+        g1, g2 = self.add_objects(2, 'group', 'g_del_user')
+
+        self.add_linked_attribute(g1, u1)
+        self.add_linked_attribute(g2, u1)
+        self.add_linked_attribute(g2, u2)
+
+        self.samdb.delete(u1)
+
+        self.assert_forward_links(g1, {})
+        self.assert_forward_links(g2, {u2: True})
-- 
1.7.0.4


From 4b8288faaf5d23aa7cfac42e5b5d943911f250e4 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 14 Jul 2016 16:56:50 +1200
Subject: [PATCH 11/15] link_attrs: Add tests for one way links (and pseudo one-way)

Tested against Win2012R2. The deactivated link control has no effect on either
one way links or pseudo ones (only two-way ones presumably).

Signed-off-by: Garming Sam <garming at samba.org>
---
 selftest/knownfail                             |    3 +
 source4/dsdb/tests/python/linked_attributes.py |   68 ++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 1a92a5d..74543dd 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -285,3 +285,6 @@
 
 # fl2000dc doesn't support AES
 ^samba4.krb5.kdc.*as-req-aes.*fl2000dc
+
+^samba4.ldap.linked_attributes.python\(ad_dc_ntvfs\).__main__.LATests.test_one_way_attributes\(ad_dc_ntvfs:local\)
+^samba4.ldap.linked_attributes.python\(ad_dc_ntvfs\).__main__.LATests.test_pretend_one_way_attributes\(ad_dc_ntvfs:local\)
diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py
index c7afad4..cea3a01 100644
--- a/source4/dsdb/tests/python/linked_attributes.py
+++ b/source4/dsdb/tests/python/linked_attributes.py
@@ -352,6 +352,74 @@ class LATests(samba.tests.TestCase):
         self.assert_back_links(u2, [g3, g2], sorted=True)
         self.assert_back_links(u3, [g3, g2, g1], sorted=True)
 
+    def test_one_way_attributes(self):
+        e1, e2 = self.add_objects(2, 'msExchConfigurationContainer',
+                                  'e_one_way')
+        guid = self.get_object_guid(e2)
+
+        self.add_linked_attribute(e1, e2, attr="addressBookRoots")
+        self.assert_forward_links(e1, [e2], attr='addressBookRoots')
+
+        self.samdb.delete(e2)
+
+        res = self.samdb.search("<GUID=%s>" % guid,
+                                scope=ldb.SCOPE_BASE,
+                                controls=['show_deleted:1',
+                                          'show_recycled:1'])
+
+        new_dn = str(res[0].dn)
+        self.assert_forward_links(e1, [new_dn], attr='addressBookRoots')
+        self.assert_forward_links(e1, [new_dn],
+                                  attr='addressBookRoots',
+                                  show_deactivated_link=0)
+
+    def test_one_way_attributes_delete_link(self):
+        e1, e2 = self.add_objects(2, 'msExchConfigurationContainer',
+                                  'e_one_way')
+        guid = self.get_object_guid(e2)
+
+        self.add_linked_attribute(e1, e2, attr="addressBookRoots")
+        self.assert_forward_links(e1, [e2], attr='addressBookRoots')
+
+        self.remove_linked_attribute(e1, e2, attr="addressBookRoots")
+
+        self.assert_forward_links(e1, [], attr='addressBookRoots')
+        self.assert_forward_links(e1, [], attr='addressBookRoots',
+                                  show_deactivated_link=0)
+
+    def test_pretend_one_way_attributes(self):
+        e1, e2 = self.add_objects(2, 'msExchConfigurationContainer',
+                                  'e_one_way')
+        guid = self.get_object_guid(e2)
+
+        self.add_linked_attribute(e1, e2, attr="addressBookRoots2")
+        self.assert_forward_links(e1, [e2], attr='addressBookRoots2')
+
+        self.samdb.delete(e2)
+        res = self.samdb.search("<GUID=%s>" % guid,
+                                scope=ldb.SCOPE_BASE,
+                                controls=['show_deleted:1',
+                                          'show_recycled:1'])
+
+        new_dn = str(res[0].dn)
+
+        self.assert_forward_links(e1, [], attr='addressBookRoots2')
+        self.assert_forward_links(e1, [], attr='addressBookRoots2',
+                                  show_deactivated_link=0)
+
+    def test_pretend_one_way_attributes_delete_link(self):
+        e1, e2 = self.add_objects(2, 'msExchConfigurationContainer',
+                                  'e_one_way')
+        guid = self.get_object_guid(e2)
+
+        self.add_linked_attribute(e1, e2, attr="addressBookRoots2")
+        self.assert_forward_links(e1, [e2], attr='addressBookRoots2')
+
+        self.remove_linked_attribute(e1, e2, attr="addressBookRoots2")
+
+        self.assert_forward_links(e1, [], attr='addressBookRoots2')
+        self.assert_forward_links(e1, [], attr='addressBookRoots2',
+                                  show_deactivated_link=0)
 
 if "://" not in host:
     if os.path.isfile(host):
-- 
1.7.0.4


From 3ee2bfdc9af301240f17a145ef265a1eb4fc7be9 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Wed, 13 Jul 2016 13:29:19 +1200
Subject: [PATCH 12/15] extended_dn_out: Force showing of one-way links if they exist

Signed-off-by: Garming Sam <garming at samba.org>
---
 selftest/knownfail                               |    3 ---
 source4/dsdb/samdb/ldb_modules/extended_dn_out.c |   12 +++++++-----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 74543dd..1a92a5d 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -285,6 +285,3 @@
 
 # fl2000dc doesn't support AES
 ^samba4.krb5.kdc.*as-req-aes.*fl2000dc
-
-^samba4.ldap.linked_attributes.python\(ad_dc_ntvfs\).__main__.LATests.test_one_way_attributes\(ad_dc_ntvfs:local\)
-^samba4.ldap.linked_attributes.python\(ad_dc_ntvfs\).__main__.LATests.test_pretend_one_way_attributes\(ad_dc_ntvfs:local\)
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
index 31835a7..d29a50c 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
@@ -325,7 +325,8 @@ struct extended_search_context {
    renames of the target
 */
 static int fix_one_way_link(struct extended_search_context *ac, struct ldb_dn *dn,
-			    bool is_deleted_objects, bool *remove_value)
+			    bool is_deleted_objects, bool *remove_value,
+			    uint32_t linkID)
 {
 	struct GUID guid;
 	NTSTATUS status;
@@ -348,9 +349,9 @@ static int fix_one_way_link(struct extended_search_context *ac, struct ldb_dn *d
 
 	search_flags = DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SEARCH_ALL_PARTITIONS | DSDB_SEARCH_ONE_ONLY;
 
-	if (ldb_request_get_control(ac->req, LDB_CONTROL_SHOW_DEACTIVATED_LINK_OID) ||
-	    is_deleted_objects) {
-		search_flags |= DSDB_SEARCH_SHOW_DELETED;
+	if (linkID == 0) {
+		/* You must ALWAYS show one-way links regardless of the state of the target */
+		search_flags |= (DSDB_SEARCH_SHOW_DELETED | DSDB_SEARCH_SHOW_RECYCLED);
 	}
 
 	ret = dsdb_module_search(ac->module, tmp_ctx, &res, NULL, LDB_SCOPE_SUBTREE, attrs,
@@ -611,7 +612,8 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
 			if (attribute->one_way_link &&
 			    strcasecmp(attribute->lDAPDisplayName, "objectCategory") != 0) {
 				bool remove_value;
-				ret = fix_one_way_link(ac, dn, is_deleted_objects, &remove_value);
+				ret = fix_one_way_link(ac, dn, is_deleted_objects, &remove_value,
+						       attribute->linkID);
 				if (ret != LDB_SUCCESS) {
 					talloc_free(dsdb_dn);
 					return ldb_module_done(ac->req, NULL, NULL, ret);
-- 
1.7.0.4


From 19bff532c52b332024735cd026e44dbe8d0a2761 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 30 Jun 2016 16:15:35 +1200
Subject: [PATCH 13/15] dbcheck: cache linkIDs and reverse attribute names

This avoids fetching the same same schema things again and again.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/dbchecker.py |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 3d33c38..ed5ee43 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -88,7 +88,7 @@ class dbcheck(object):
         self.fix_missing_deleted_objects = False
 
         self.dn_set = set()
-
+        self.link_id_cache = {}
         self.name_map = {}
         try:
             res = samdb.search(base="CN=DnsAdmins,CN=Users,%s" % samdb.domain_dn(), scope=ldb.SCOPE_BASE,
@@ -335,6 +335,17 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             return False
         return True
 
+    def get_attr_linkID_and_reverse_name(self, attrname):
+        if attrname in self.link_id_cache:
+            return self.link_id_cache[attrname]
+        linkID = self.samdb_schema.get_linkId_from_lDAPDisplayName(attrname)
+        if linkID:
+            revname = self.samdb_schema.get_backlink_from_lDAPDisplayName(attrname)
+        else:
+            revname = None
+        self.link_id_cache[attrname] = (linkID, revname)
+        return linkID, revname
+
     def err_empty_attribute(self, dn, attrname):
         '''fix empty attributes'''
         self.report("ERROR: Empty attribute %s in %s" % (attrname, dn))
@@ -448,7 +459,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
     def err_missing_dn_GUID(self, dn, attrname, val, dsdb_dn):
         """handle a missing target DN (both GUID and DN string form are missing)"""
         # check if its a backlink
-        linkID = self.samdb_schema.get_linkId_from_lDAPDisplayName(attrname)
+        linkID, _ = self.get_attr_linkID_and_reverse_name(attrname)
         if (linkID & 1 == 0) and str(dsdb_dn).find('\\0ADEL') == -1:
             self.report("Not removing dangling forward link")
             return
@@ -745,8 +756,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             else:
                 fixing_msDS_HasInstantiatedNCs = False
 
-            linkID = self.samdb_schema.get_linkId_from_lDAPDisplayName(attrname)
-            reverse_link_name = self.samdb_schema.get_backlink_from_lDAPDisplayName(attrname)
+            linkID, reverse_link_name = self.get_attr_linkID_and_reverse_name(attrname)
             if reverse_link_name is not None:
                 attrs.append(reverse_link_name)
 
@@ -1583,10 +1593,12 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 error_count += 1
                 continue
 
+            linkID, reverse_link_name = self.get_attr_linkID_and_reverse_name(attrname)
+
             flag = self.samdb_schema.get_systemFlags_from_lDAPDisplayName(attrname)
             if (not flag & dsdb.DS_FLAG_ATTR_NOT_REPLICATED
                 and not flag & dsdb.DS_FLAG_ATTR_IS_CONSTRUCTED
-                and not self.samdb_schema.get_linkId_from_lDAPDisplayName(attrname)):
+                and not linkID):
                 set_attrs_seen.add(str(attrname).lower())
 
             if syntax_oid in [ dsdb.DSDB_SYNTAX_BINARY_DN, dsdb.DSDB_SYNTAX_OR_NAME,
-- 
1.7.0.4


From 04e2053cc41c8ce3c2003c0515bd33f81fab6dc6 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at samba.org>
Date: Thu, 14 Jul 2016 13:53:23 +0200
Subject: [PATCH 14/15] Add dbcheck to flapping

---
 selftest/flapping |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/selftest/flapping b/selftest/flapping
index 4ea0a44..2c4d026 100644
--- a/selftest/flapping
+++ b/selftest/flapping
@@ -32,3 +32,4 @@
 #
 ^samba4.ldap.notification.python\(.*\).__main__.LDAPNotificationTest.test_max_search
 ^samba3.blackbox.smbclient_tar.* # fails very, very often on sn-devel
+^samba4.blackbox.dbcheck\(
-- 
1.7.0.4


From 18f8eba2e4c1015620e2eb9ff365e33700de15f2 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 30 Jun 2016 16:17:37 +1200
Subject: [PATCH 15/15] dbcheck: check for linked atributes that should not exist

In order to do this we need to use the reveal internals control, which
breaks the comparison against extended DNs. So we compare the
components instead.

Because this patch makes our code notice and fix stale one-way-links
(eg, after a rename) now, the renamedc test needs to be adjusted to
match.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/dbchecker.py      |  134 +++++++++++++++++++++++++++-------------
 testprogs/blackbox/renamedc.sh |    5 ++
 2 files changed, 95 insertions(+), 44 deletions(-)

diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index ed5ee43..ded4dc0 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -53,9 +53,12 @@ class dbcheck(object):
         self.fix_all_DN_GUIDs = False
         self.fix_all_binary_dn = False
         self.remove_all_deleted_DN_links = False
-        self.fix_all_target_mismatch = False
+        self.fix_all_string_dn_component_mismatch = False
+        self.fix_all_GUID_dn_component_mismatch = False
+        self.fix_all_SID_dn_component_mismatch = False
         self.fix_all_metadata = False
         self.fix_time_metadata = False
+        self.fix_undead_linked_attributes = False
         self.fix_all_missing_backlinks = False
         self.fix_all_orphaned_backlinks = False
         self.fix_rmd_flags = False
@@ -452,7 +455,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         m = ldb.Message()
         m.dn = dn
         m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
-        if self.do_modify(m, ["show_recycled:1", "local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK],
+        if self.do_modify(m, ["show_recycled:1",
+                              "local_oid:%s:0" % dsdb.DSDB_CONTROL_REPLMD_VANISH_LINKS],
                           "Failed to remove deleted DN attribute %s" % attrname):
             self.report("Removed deleted DN on attribute %s" % attrname)
 
@@ -511,21 +515,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to fix %s on attribute %s" % (errstr, attrname)):
             self.report("Fixed %s on attribute %s" % (errstr, attrname))
 
-    def err_dn_target_mismatch(self, dn, attrname, val, dsdb_dn, correct_dn, errstr):
+    def err_dn_component_target_mismatch(self, dn, attrname, val, dsdb_dn, correct_dn, mismatch_type):
         """handle a DN string being incorrect"""
-        self.report("ERROR: incorrect DN string component for %s in object %s - %s" % (attrname, dn, val))
+        self.report("ERROR: incorrect DN %s component for %s in object %s - %s" % (mismatch_type, attrname, dn, val))
         dsdb_dn.dn = correct_dn
 
-        if not self.confirm_all('Change DN to %s?' % str(dsdb_dn), 'fix_all_target_mismatch'):
-            self.report("Not fixing %s" % errstr)
+        if not self.confirm_all('Change DN to %s?' % str(dsdb_dn),
+                                'fix_all_%s_dn_component_mismatch' % mismatch_type):
+            self.report("Not fixing %s component mismatch" % mismatch_type)
             return
         m = ldb.Message()
         m.dn = dn
         m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
         m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname)
         if self.do_modify(m, ["show_recycled:1"],
-                          "Failed to fix incorrect DN string on attribute %s" % attrname):
-            self.report("Fixed incorrect DN string on attribute %s" % (attrname))
+                          "Failed to fix incorrect DN %s on attribute %s" % (mismatch_type, attrname)):
+            self.report("Fixed incorrect DN %s on attribute %s" % (mismatch_type, attrname))
 
     def err_unknown_attribute(self, obj, attrname):
         '''handle an unknown attribute error'''
@@ -540,6 +545,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           "Failed to remove unknown attribute %s" % attrname):
             self.report("Removed unknown attribute %s" % (attrname))
 
+    def err_undead_linked_attribute(self, obj, attrname, val):
+        '''handle a link that should not be there on a deleted object'''
+        self.report("ERROR: linked attribute '%s' to '%s' is present on "
+                    "deleted object %s" % (attrname, val, obj.dn))
+        if not self.confirm_all('Remove linked attribute %s' % attrname, 'fix_undead_linked_attributes'):
+            self.report("Not removing linked attribute %s" % attrname)
+            return
+        m = ldb.Message()
+        m.dn = obj.dn
+        m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
+
+        if self.do_modify(m, ["show_recycled:1", "show_deleted:1", "reveal_internals:0",
+                              "local_oid:%s:0" % dsdb.DSDB_CONTROL_REPLMD_VANISH_LINKS],
+                          "Failed to delete forward link %s" % attrname):
+            self.report("Fixed undead forward link %s" % (attrname))
+
     def err_missing_backlink(self, obj, attrname, val, backlink_name, target_dn):
         '''handle a missing backlink value'''
         self.report("ERROR: missing backlink attribute '%s' in %s for link %s in %s" % (backlink_name, target_dn, attrname, obj.dn))
@@ -735,6 +756,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
     def check_dn(self, obj, attrname, syntax_oid):
         '''check a DN attribute for correctness'''
         error_count = 0
+        obj_guid = obj['objectGUID'][0]
+
         for val in obj[attrname]:
             dsdb_dn = dsdb_Dn(self.samdb, val, syntax_oid)
 
@@ -747,7 +770,6 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 continue
 
             guidstr = str(misc.GUID(guid))
-
             attrs = ['isDeleted']
 
             if (str(attrname).lower() == 'msds-hasinstantiatedncs') and (obj.dn == self.ntds_dsa):
@@ -763,7 +785,9 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             # check its the right GUID
             try:
                 res = self.samdb.search(base="<GUID=%s>" % guidstr, scope=ldb.SCOPE_BASE,
-                                        attrs=attrs, controls=["extended_dn:1:1", "show_recycled:1"])
+                                        attrs=attrs, controls=["extended_dn:1:1", "show_recycled:1",
+                                                               "reveal_internals:0"
+                                        ])
             except ldb.LdbError, (enum, estr):
                 error_count += 1
                 self.err_incorrect_dn_GUID(obj.dn, attrname, val, dsdb_dn, "incorrect GUID")
@@ -782,43 +806,62 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             is_deleted = 'isDeleted' in obj and obj['isDeleted'][0].upper() == 'TRUE'
             target_is_deleted = 'isDeleted' in res[0] and res[0]['isDeleted'][0].upper() == 'TRUE'
 
-            # the target DN is not allowed to be deleted, unless the target DN is the
-            # special Deleted Objects container
-            if target_is_deleted and not is_deleted and not self.is_deleted_objects_dn(dsdb_dn):
+
+            if is_deleted and not obj.dn in self.deleted_objects_containers and linkID:
+                # A fully deleted object should not have any linked
+                # attributes. (MS-ADTS 3.1.1.5.5.1.1 Tombstone
+                # Requirements and 3.1.1.5.5.1.3 Recycled-Object
+                # Requirements)
+                self.err_undead_linked_attribute(obj, attrname, val)
+                error_count += 1
+                continue
+            elif target_is_deleted and not self.is_deleted_objects_dn(dsdb_dn) and linkID:
+                # the target DN is not allowed to be deleted, unless the target DN is the
+                # special Deleted Objects container
                 error_count += 1
                 self.err_deleted_dn(obj.dn, attrname, val, dsdb_dn, res[0].dn)
                 continue
 
             # check the DN matches in string form
-            if res[0].dn.extended_str() != dsdb_dn.dn.extended_str():
+            if str(res[0].dn) != str(dsdb_dn.dn):
                 error_count += 1
-                self.err_dn_target_mismatch(obj.dn, attrname, val, dsdb_dn,
-                                            res[0].dn, "incorrect string version of DN")
+                self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
+                                                      res[0].dn, "string")
+                continue
+
+            if res[0].dn.get_extended_component("GUID") != dsdb_dn.dn.get_extended_component("GUID"):
+                error_count += 1
+                self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
+                                                      res[0].dn, "GUID")
+                continue
+
+            if res[0].dn.get_extended_component("SID") != dsdb_dn.dn.get_extended_component("SID"):
+                error_count += 1
+                self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn,
+                                                      res[0].dn, "SID")
                 continue
 
-            if is_deleted and not target_is_deleted and reverse_link_name is not None:
-                revealed_dn = self.find_revealed_link(obj.dn, attrname, guid)
-                rmd_flags = revealed_dn.dn.get_extended_component("RMD_FLAGS")
-                if rmd_flags is not None and (int(rmd_flags) & 1) == 0:
-                    # the RMD_FLAGS for this link should be 1, as the target is deleted
-                    self.err_incorrect_rmd_flags(obj, attrname, revealed_dn)
-                    continue
 
             # check the reverse_link is correct if there should be one
             if reverse_link_name is not None:
                 match_count = 0
                 if reverse_link_name in res[0]:
                     for v in res[0][reverse_link_name]:
-                        if v == obj.dn.extended_str():
+                        v_guid = dsdb_Dn(self.samdb, v).dn.get_extended_component("GUID")
+                        if v_guid == obj_guid:
                             match_count += 1
                 if match_count != 1:
-                    error_count += 1
-                    if linkID & 1:
-                        self.err_orphaned_backlink(obj, attrname, val, reverse_link_name, dsdb_dn.dn)
-                    else:
-                        self.err_missing_backlink(obj, attrname, val, reverse_link_name, dsdb_dn.dn)
+                    if target_is_deleted:
+                        error_count += 1
+                        if linkID & 1:
+                            self.err_missing_backlink(obj, attrname, val, reverse_link_name, dsdb_dn.dn)
+                        else:
+                            self.err_orphaned_backlink(obj, attrname, val, reverse_link_name, dsdb_dn.dn)
                     continue
 
+
+
+
         return error_count
 
 
@@ -1375,6 +1418,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             attrs.append("systemFlags")
         if '*' in attrs:
             attrs.append("replPropertyMetaData")
+        else:
+            attrs.append("objectGUID")
 
         try:
             sd_flags = 0
@@ -1389,6 +1434,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                                         "show_recycled:1",
                                         "show_deleted:1",
                                         "sd_flags:1:%d" % sd_flags,
+                                        "reveal_internals:0",
                                     ],
                                     attrs=attrs)
         except ldb.LdbError, (enum, estr):
@@ -1425,7 +1471,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         systemFlags = 0
 
         for attrname in obj:
-            if attrname == 'dn':
+            if attrname == 'dn' or attrname == "distinguishedName":
                 continue
 
             if str(attrname).lower() == 'objectclass':
@@ -1478,8 +1524,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
                 else:
                     # Here we check that the first attid is 0
-                    # (objectClass) and that the last on is the RDN
-                    # from the DN.
+                    # (objectClass).
                     if list_attid_from_md[0] != 0:
                         error_count += 1
                         self.report("ERROR: Not fixing incorrect inital attributeID in '%s' on '%s', it should be objectClass" %
@@ -1605,23 +1650,24 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                                dsdb.DSDB_SYNTAX_STRING_DN, ldb.SYNTAX_DN ]:
                 # it's some form of DN, do specialised checking on those
                 error_count += self.check_dn(obj, attrname, syntax_oid)
+            else:
 
-            values = set()
-            # check for incorrectly normalised attributes
-            for val in obj[attrname]:
-                values.add(str(val))
+                values = set()
+                # check for incorrectly normalised attributes
+                for val in obj[attrname]:
+                    values.add(str(val))
 
-                normalised = self.samdb.dsdb_normalise_attributes(self.samdb_schema, attrname, [val])
-                if len(normalised) != 1 or normalised[0] != val:
-                    self.err_normalise_mismatch(dn, attrname, obj[attrname])
+                    normalised = self.samdb.dsdb_normalise_attributes(self.samdb_schema, attrname, [val])
+                    if len(normalised) != 1 or normalised[0] != val:
+                        self.err_normalise_mismatch(dn, attrname, obj[attrname])
+                        error_count += 1
+                        break
+
+                if len(obj[attrname]) != len(values):
+                    self.err_duplicate_values(dn, attrname, obj[attrname], list(values))
                     error_count += 1
                     break
 
-            if len(obj[attrname]) != len(values):
-                   self.err_duplicate_values(dn, attrname, obj[attrname], list(values))
-                   error_count += 1
-                   break
-
             if str(attrname).lower() == "instancetype":
                 calculated_instancetype = self.calculate_instancetype(dn)
                 if len(obj["instanceType"]) != 1 or obj["instanceType"][0] != str(calculated_instancetype):
diff --git a/testprogs/blackbox/renamedc.sh b/testprogs/blackbox/renamedc.sh
index 4f187a4..b81c501 100755
--- a/testprogs/blackbox/renamedc.sh
+++ b/testprogs/blackbox/renamedc.sh
@@ -64,6 +64,10 @@ testrenamedc2() {
 		-s $PREFIX/renamedc_test/etc/smb.conf
 }
 
+dbcheck_fix() {
+	$BINDIR/samba-tool dbcheck --cross-ncs -s $PREFIX/renamedc_test/etc/smb.conf --fix --yes
+}
+
 dbcheck() {
 	$BINDIR/samba-tool dbcheck --cross-ncs -s $PREFIX/renamedc_test/etc/smb.conf
 }
@@ -77,6 +81,7 @@ testit "confirmrenamedc_sAMAccountName" confirmrenamedc_sAMAccountName || failed
 testit "confirmrenamedc_dNSHostName" confirmrenamedc_dNSHostName || failed=`expr $failed + 1`
 testit "confirmrenamedc_rootdse_dnsHostName" confirmrenamedc_rootdse_dnsHostName || failed=`expr $failed + 1`
 testit "confirmrenamedc_rootdse_dsServiceName" confirmrenamedc_rootdse_dsServiceName || failed=`expr $failed + 1`
+testit_expect_failure "dbcheck_fix" dbcheck_fix || failed=`expr $failed + 1`
 testit "dbcheck" dbcheck || failed=`expr $failed + 1`
 testit "renamedc2" testrenamedc2 || failed=`expr $failed + 1`
 
-- 
1.7.0.4



More information about the samba-technical mailing list