>From 5892e07958f2fec46009704a9ef80d53736ba02b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 12:19:21 +1300 Subject: [PATCH 01/25] linked_attributes test: pep8 tidy-up, remove unused imports Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index 9813ea0..bb3468a 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -4,9 +4,7 @@ import optparse import sys import os -import base64 -import random -import re +import itertools sys.path.insert(0, "bin/python") import samba @@ -19,8 +17,6 @@ import ldb from samba.samdb import SamDB from samba.dcerpc import misc -import time - parser = optparse.OptionParser("linked_attributes.py [options] ") sambaopts = options.SambaOptions(parser) parser.add_option_group(sambaopts) @@ -61,7 +57,7 @@ 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) + session_info=system_session(lp), lp=lp) self.base_dn = self.samdb.domain_dn() self.ou = "OU=la,%s" % self.base_dn @@ -85,8 +81,8 @@ class LATests(samba.tests.TestCase): def add_object(self, cn, objectclass): dn = "CN=%s,%s" % (cn, self.ou) self.samdb.add({'cn': cn, - 'objectclass': objectclass, - 'dn': dn}) + 'objectclass': objectclass, + 'dn': dn}) return dn @@ -250,10 +246,10 @@ class LATests(samba.tests.TestCase): self.samdb.delete(g2) self.assert_back_links(u1, [g1], show_deleted=1, show_recycled=1, show_deactivated_link=0, - reveal_internals=0) + reveal_internals=0) self.assert_back_links(u2, set(), show_deleted=1, show_recycled=1, show_deactivated_link=0, - reveal_internals=0) + reveal_internals=0) self.assert_forward_links(g1, [u1], show_deleted=1, show_recycled=1, show_deactivated_link=0, reveal_internals=0) @@ -318,6 +314,7 @@ class LATests(samba.tests.TestCase): show_deactivated_link=0, reveal_internals=0 ) + def test_la_links_delete_link_reveal(self): if opts.no_reveal_internals: print 'skipping because --no-reveal-internals' -- 2.7.4 >From 26538413b64da47f893964bf4af9a83ca425756b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 12:26:13 +1300 Subject: [PATCH 02/25] s4/linked_attribute tests: add multiple links and replace tests Also a "delete all" test. Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 132 +++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index bb3468a..c586082 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -107,6 +107,12 @@ class LATests(samba.tests.TestCase): m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_DELETE, attr) self.samdb.modify(m) + def replace_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_REPLACE, attr) + self.samdb.modify(m) + def attr_search(self, obj, expected, attr, scope=ldb.SCOPE_BASE, **controls): if opts.no_reveal_internals: @@ -298,6 +304,8 @@ class LATests(samba.tests.TestCase): self.assert_forward_links(g2, [u1]) self.remove_linked_attribute(g2, u1) self.assert_forward_links(g2, []) + self.remove_linked_attribute(g1, []) + self.assert_forward_links(g1, []) def _test_la_links_delete_link_reveal(self): u1, u2 = self.add_objects(2, 'user', 'u_del_link_reveal') @@ -373,6 +381,130 @@ class LATests(samba.tests.TestCase): show_deactivated_link=0, reveal_internals=0) + def test_multiple_links(self): + u1, u2, u3, u4 = self.add_objects(4, 'user', 'u_multiple_links') + g1, g2, g3 = self.add_objects(3, 'group', 'g_multiple_links') + + self.add_linked_attribute(g1, [u1, u2, u3, u4]) + self.add_linked_attribute(g2, [u3, u1]) + self.add_linked_attribute(g3, u2) + + self.assert_forward_links(g1, [u1, u2, u3, u4]) + self.assert_forward_links(g2, [u3, u1]) + self.assert_forward_links(g3, [u2]) + self.assert_back_links(u1, [g2, g1]) + self.assert_back_links(u2, [g3, g1]) + self.assert_back_links(u3, [g2, g1]) + self.assert_back_links(u4, [g1]) + + self.remove_linked_attribute(g2, [u1, u3]) + self.remove_linked_attribute(g1, [u1, u3]) + + self.assert_forward_links(g1, [u2, u4]) + self.assert_forward_links(g2, []) + self.assert_forward_links(g3, [u2]) + self.assert_back_links(u1, []) + self.assert_back_links(u2, [g3, g1]) + self.assert_back_links(u3, []) + self.assert_back_links(u4, [g1]) + + self.add_linked_attribute(g1, [u1, u3]) + self.add_linked_attribute(g2, [u3, u1]) + self.add_linked_attribute(g3, [u1, u3]) + + self.assert_forward_links(g1, [u1, u2, u3, u4]) + self.assert_forward_links(g2, [u1, u3]) + self.assert_forward_links(g3, [u1, u2, u3]) + self.assert_back_links(u1, [g1, g2, g3]) + self.assert_back_links(u2, [g3, g1]) + self.assert_back_links(u3, [g3, g2, g1]) + self.assert_back_links(u4, [g1]) + + def test_la_links_replace(self): + u1, u2, u3, u4 = self.add_objects(4, 'user', 'u_replace') + g1, g2, g3, g4 = self.add_objects(4, 'group', 'g_replace') + + self.add_linked_attribute(g1, [u1, u2]) + self.add_linked_attribute(g2, [u1, u3]) + self.add_linked_attribute(g3, u1) + + self.replace_linked_attribute(g1, [u2]) + self.replace_linked_attribute(g2, [u2, u3]) + self.replace_linked_attribute(g3, [u1, u3]) + self.replace_linked_attribute(g4, [u4]) + + self.assert_forward_links(g1, [u2]) + self.assert_forward_links(g2, [u3, u2]) + self.assert_forward_links(g3, [u3, u1]) + self.assert_forward_links(g4, [u4]) + self.assert_back_links(u1, [g3]) + self.assert_back_links(u2, [g1, g2]) + self.assert_back_links(u3, [g2, g3]) + self.assert_back_links(u4, [g4]) + + self.replace_linked_attribute(g1, [u1, u2, u3]) + self.replace_linked_attribute(g2, [u1]) + self.replace_linked_attribute(g3, [u2]) + self.replace_linked_attribute(g4, []) + + self.assert_forward_links(g1, [u1, u2, u3]) + self.assert_forward_links(g2, [u1]) + self.assert_forward_links(g3, [u2]) + self.assert_forward_links(g4, []) + self.assert_back_links(u1, [g1, g2]) + self.assert_back_links(u2, [g1, g3]) + self.assert_back_links(u3, [g1]) + self.assert_back_links(u4, []) + + + def test_la_links_replace2(self): + users = self.add_objects(12, 'user', 'u_replace2') + g1, = self.add_objects(1, 'group', 'g_replace2') + + self.add_linked_attribute(g1, users[:6]) + self.assert_forward_links(g1, users[:6]) + self.replace_linked_attribute(g1, users) + self.assert_forward_links(g1, users) + self.replace_linked_attribute(g1, users[6:]) + self.assert_forward_links(g1, users[6:]) + self.remove_linked_attribute(g1, users[6:9]) + self.assert_forward_links(g1, users[9:]) + self.remove_linked_attribute(g1, users[9:]) + self.assert_forward_links(g1, []) + + def test_la_links_permutations(self): + """Make sure the order in which we add links doesn't matter.""" + users = self.add_objects(3, 'user', 'u_permutations') + groups = self.add_objects(6, 'group', 'g_permutations') + + for g, p in zip(groups, itertools.permutations(users)): + self.add_linked_attribute(g, p) + + # everyone should be in every group + for g in groups: + self.assert_forward_links(g, users) + + for u in users: + self.assert_back_links(u, groups) + + for g, p in zip(groups[::-1], itertools.permutations(users)): + self.replace_linked_attribute(g, p) + + for g in groups: + self.assert_forward_links(g, users) + + for u in users: + self.assert_back_links(u, groups) + + for g, p in zip(groups, itertools.permutations(users)): + self.remove_linked_attribute(g, p) + + for g in groups: + self.assert_forward_links(g, []) + + for u in users: + self.assert_back_links(u, []) + 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') -- 2.7.4 >From 08e70c696c4066bbba9d9af10fdffd21410bf23d Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 12 Jan 2017 11:53:15 +1300 Subject: [PATCH 03/25] s4/linked_attribute tests: remove unused code We don't test for sort order because we don't depend on it. So this test was never used. Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index c586082..c16ab34 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -505,28 +505,6 @@ class LATests(samba.tests.TestCase): for u in users: self.assert_back_links(u, []) - 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) - def test_one_way_attributes(self): e1, e2 = self.add_objects(2, 'msExchConfigurationContainer', 'e_one_way') -- 2.7.4 >From bfe66ac6c69315cc9043e0f655ae1677ecc73682 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 12 Jan 2017 11:57:17 +1300 Subject: [PATCH 04/25] s4/linked_attribute tests: compare link lists in sorted order This isn't functionally different[1] from the previous use of set(), but it makes the error output easier to read. [1] OK, it will also show duplicates, which we really don't expect and would definitely want to see. Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index c16ab34..4f65df9 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -127,8 +127,7 @@ class LATests(samba.tests.TestCase): controls=controls) return res - def assert_links(self, obj, expected, attr, sorted=False, msg='', - **kwargs): + def assert_links(self, obj, expected, attr, msg='', **kwargs): res = self.attr_search(obj, expected, attr, **kwargs) if len(expected) == 0: @@ -141,9 +140,8 @@ class LATests(samba.tests.TestCase): except KeyError: self.fail("missing attr '%s' on %s" % (attr, obj)) - if sorted == False: - results = set(results) - expected = set(expected) + expected = sorted(expected) + results = sorted(results) if expected != results: print msg -- 2.7.4 >From cbe860f6f111c8697877d49fa67ae351ab336a8f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 4 Jan 2017 21:27:58 +1300 Subject: [PATCH 05/25] selftest: Do not test for link ordering in tombstones_expunge test By testing only for the DNs that are returned we do not change the strictness of the test, because it is a test of the match rule which applies to the whole object, not the returned values. However, when this code asserted the returned order of the links, it prevents us from changing this order. This order was not deterministic across DCs but as this test ran against an offline DB, it was able to assume a particular order. Signed-off-by: Andrew Bartlett --- .../release-4-5-0-pre1/expected-match-rule-links.ldif | 15 --------------- testprogs/blackbox/tombstones-expunge.sh | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-match-rule-links.ldif b/source4/selftest/provisions/release-4-5-0-pre1/expected-match-rule-links.ldif index 1553c1b..b207c7d 100644 --- a/source4/selftest/provisions/release-4-5-0-pre1/expected-match-rule-links.ldif +++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-match-rule-links.ldif @@ -1,41 +1,26 @@ # record 1 dn: CN=swimmers,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=fred,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=user1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 2 dn: CN=Domain Users,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=fred-clone,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 3 dn: CN=ddg\0ADEL:fb8c2fe3-5448-43de-99f9-e1d3b9357cfc,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 4 dn: CN=dsg\0ADEL:6d66d0ef-cad7-4e5d-b1b6-4a233a21c269,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 5 dn: CN=gdg\0ADEL:e0f581e7-14ee-4fc2-839c-8f46f581c72a,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 6 dn: CN=gsg\0ADEL:91aa85cc-fc19-4b8c-9fc7-aaba425439c7,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 7 dn: CN=udg\0ADEL:7cff5537-51b1-4d26-a295-0225dbea8525,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # record 8 dn: CN=usg\0ADEL:d012e8f5-a4bd-40ea-a2a1-68ff2508847d,CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp -member: CN=User1 UT. Tester,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp # Referral ref: ldap:///CN=Configuration,DC=release-4-5-0-pre1,DC=samba,DC=corp diff --git a/testprogs/blackbox/tombstones-expunge.sh b/testprogs/blackbox/tombstones-expunge.sh index e7e861f..3f7d708 100755 --- a/testprogs/blackbox/tombstones-expunge.sh +++ b/testprogs/blackbox/tombstones-expunge.sh @@ -110,7 +110,7 @@ remove_one_user() { check_match_rule_links() { tmpldif=$PREFIX_ABS/$RELEASE/expected-match-rule-links.ldif.tmp - TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(member:1.3.6.1.4.1.7165.4.5.2:=131139216000000000)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted member > $tmpldif + TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(member:1.3.6.1.4.1.7165.4.5.2:=131139216000000000)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted no_attrs > $tmpldif diff $tmpldif $release_dir/expected-match-rule-links.ldif if [ "$?" != "0" ]; then return 1 -- 2.7.4 >From 798b3b7949d924ad34835181e434c49a08c539ce Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2016 15:32:59 +1300 Subject: [PATCH 06/25] schema: Set flag into @INDEXLIST to indicate we support feature flags Because @INDEXLIST is rewritten by all Samba versions, we can detect that we have opened the database with an older version that does not support the feature flags by the absense of this in @INDEXLIST Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall --- source4/dsdb/samdb/samdb.h | 1 + source4/dsdb/schema/schema_set.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 176d065..c03d5c2 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -314,4 +314,5 @@ struct dsdb_extended_sec_desc_propagation_op { */ #define DSDB_FLAG_INTERNAL_FORCE_META_DATA 0x10000 +#define SAMBA_FEATURES_SUPPORTED_FLAG "@SAMBA_FEATURES_SUPPORTED" #endif /* __SAMDB_H__ */ diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c index 6f03046..2e688d0 100644 --- a/source4/dsdb/schema/schema_set.c +++ b/source4/dsdb/schema/schema_set.c @@ -111,6 +111,11 @@ static int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb, struc goto op_error; } + ret = ldb_msg_add_string(msg_idx, SAMBA_FEATURES_SUPPORTED_FLAG, "1"); + if (ret != LDB_SUCCESS) { + goto op_error; + } + for (attr = schema->attributes; attr; attr = attr->next) { const char *syntax = attr->syntax->ldb_syntax; -- 2.7.4 >From b17854cf0ef1df075c7d607ad6598a14414fb814 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 12 Jan 2017 16:51:45 +1300 Subject: [PATCH 07/25] samba_dsdb: Use and maintain compatibleFeatures and incompatibleFeatures in @SAMBA_DSDB This will allow us to determine if the database still contains unsorted links, as that information allows us to make the code handling links much more efficient. We won't add the actual flag until all the code is in place. Signed-off-by: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 24 ++++- source4/dsdb/samdb/ldb_modules/samba_dsdb.c | 123 +++++++++++++++++++++++- source4/dsdb/samdb/samdb.h | 5 + 3 files changed, 148 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 7d12c23..28c8adf 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -69,6 +69,7 @@ struct replmd_private { } *ncs; struct ldb_dn *schema_dn; bool originating_updates; + bool sorted_links; }; struct la_entry { @@ -236,16 +237,37 @@ static int replmd_init(struct ldb_module *module) { struct replmd_private *replmd_private; struct ldb_context *ldb = ldb_module_get_ctx(module); - + static const char *samba_dsdb_attrs[] = { SAMBA_COMPATIBLE_FEATURES_ATTR, NULL }; + struct ldb_dn *samba_dsdb_dn; + struct ldb_result *res; + int ret; + TALLOC_CTX *frame = talloc_stackframe(); replmd_private = talloc_zero(module, struct replmd_private); if (replmd_private == NULL) { ldb_oom(ldb); + TALLOC_FREE(frame); return LDB_ERR_OPERATIONS_ERROR; } ldb_module_set_private(module, replmd_private); replmd_private->schema_dn = ldb_get_schema_basedn(ldb); + samba_dsdb_dn = ldb_dn_new(frame, ldb, "@SAMBA_DSDB"); + if (!samba_dsdb_dn) { + TALLOC_FREE(frame); + return ldb_oom(ldb); + } + + ret = dsdb_module_search_dn(module, frame, &res, samba_dsdb_dn, + samba_dsdb_attrs, DSDB_FLAG_NEXT_MODULE, NULL); + if (ret == LDB_SUCCESS) { + replmd_private->sorted_links + = ldb_msg_check_string_attribute(res->msgs[0], + SAMBA_COMPATIBLE_FEATURES_ATTR, + SAMBA_SORTED_LINKS_FEATURE); + } + TALLOC_FREE(frame); + return ldb_next_init(module); } diff --git a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c index 21168a9..93ccfb9 100644 --- a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c +++ b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c @@ -231,11 +231,11 @@ static int set_ldap_credentials(struct ldb_context *ldb, bool use_external) static int samba_dsdb_init(struct ldb_module *module) { struct ldb_context *ldb = ldb_module_get_ctx(module); - int ret, len, i; + int ret, len, i, j; TALLOC_CTX *tmp_ctx = talloc_new(module); struct ldb_result *res; struct ldb_message *rootdse_msg = NULL, *partition_msg; - struct ldb_dn *samba_dsdb_dn, *partition_dn; + struct ldb_dn *samba_dsdb_dn, *partition_dn, *indexlist_dn; struct ldb_module *backend_module, *module_chain; const char **final_module_list, **reverse_module_list; /* @@ -317,11 +317,16 @@ static int samba_dsdb_init(struct ldb_module *module) static const char *openldap_backend_modules[] = { "dsdb_flags_ignore", "entryuuid", "simple_dn", NULL }; - static const char *samba_dsdb_attrs[] = { "backendType", NULL }; + static const char *samba_dsdb_attrs[] = { "backendType", + SAMBA_COMPATIBLE_FEATURES_ATTR, + SAMBA_INCOMPATIBLE_FEATURES_ATTR, NULL }; + static const char *indexlist_attrs[] = { SAMBA_FEATURES_SUPPORTED_FLAG, NULL }; static const char *partition_attrs[] = { "ldapBackend", NULL }; const char *backendType, *backendUrl; bool use_sasl_external = false; + const char *current_supportedFeatures[] = {SAMBA_SORTED_LINKS_FEATURE}; + if (!tmp_ctx) { return ldb_oom(ldb); } @@ -338,6 +343,12 @@ static int samba_dsdb_init(struct ldb_module *module) return ldb_oom(ldb); } + indexlist_dn = ldb_dn_new(tmp_ctx, ldb, "@INDEXLIST"); + if (!samba_dsdb_dn) { + talloc_free(tmp_ctx); + return ldb_oom(ldb); + } + partition_dn = ldb_dn_new(tmp_ctx, ldb, DSDB_PARTITION_DN); if (!partition_dn) { talloc_free(tmp_ctx); @@ -357,7 +368,113 @@ static int samba_dsdb_init(struct ldb_module *module) if (ret == LDB_ERR_NO_SUCH_OBJECT) { backendType = "ldb"; } else if (ret == LDB_SUCCESS) { + struct ldb_message_element *incompatibleFeatures; + struct ldb_message_element *old_compatibleFeatures; + backendType = ldb_msg_find_attr_as_string(res->msgs[0], "backendType", "ldb"); + + old_compatibleFeatures = ldb_msg_find_element(res->msgs[0], + SAMBA_COMPATIBLE_FEATURES_ATTR); + if (old_compatibleFeatures) { + struct ldb_message *features_msg; + struct ldb_message_element *features_el; + int samba_options_supported = 0; + ret = dsdb_module_search_dn(module, tmp_ctx, &res, + indexlist_dn, + indexlist_attrs, + DSDB_FLAG_NEXT_MODULE, NULL); + if (ret == LDB_SUCCESS) { + samba_options_supported + = ldb_msg_find_attr_as_int(res->msgs[0], + SAMBA_FEATURES_SUPPORTED_FLAG, + 0); + + } else if (ret == LDB_ERR_NO_SUCH_OBJECT) { + /* + * If we don't have @INDEXLIST yet, then we + * are so early in set-up that we know this is + * a blank DB, so no need to wripe out old + * features + */ + samba_options_supported = 1; + } + + features_msg = ldb_msg_new(res); + if (features_msg == NULL) { + return ldb_module_operr(module); + } + features_msg->dn = samba_dsdb_dn; + + ldb_msg_add_empty(features_msg, SAMBA_COMPATIBLE_FEATURES_ATTR, + LDB_FLAG_MOD_DELETE, &features_el); + + if (samba_options_supported == 1) { + for (i = 0; + old_compatibleFeatures && i < old_compatibleFeatures->num_values; + i++) { + for (j = 0; + j < ARRAY_SIZE(current_supportedFeatures); j++) { + if (strcmp((char *)old_compatibleFeatures->values[i].data, + current_supportedFeatures[j]) == 0) { + break; + } + } + if (j == ARRAY_SIZE(current_supportedFeatures)) { + /* + * Add to list of features to remove + * (rather than all features) + */ + ret = ldb_msg_add_value(features_msg, SAMBA_COMPATIBLE_FEATURES_ATTR, + &old_compatibleFeatures->values[i], + NULL); + if (ret != LDB_SUCCESS) { + return ret; + } + } + } + + if (features_el->num_values > 0) { + /* Delete by list */ + ret = ldb_next_start_trans(module); + if (ret != LDB_SUCCESS) { + return ret; + } + ret = dsdb_module_modify(module, features_msg, DSDB_FLAG_NEXT_MODULE, NULL); + if (ret != LDB_SUCCESS) { + ldb_next_del_trans(module); + return ret; + } + ret = ldb_next_end_trans(module); + if (ret != LDB_SUCCESS) { + return ret; + } + } + } else { + /* Delete all */ + ret = ldb_next_start_trans(module); + if (ret != LDB_SUCCESS) { + return ret; + } + ret = dsdb_module_modify(module, features_msg, DSDB_FLAG_NEXT_MODULE, NULL); + if (ret != LDB_SUCCESS) { + ldb_next_del_trans(module); + return ret; + } + ret = ldb_next_end_trans(module); + if (ret != LDB_SUCCESS) { + return ret; + } + } + } + incompatibleFeatures = ldb_msg_find_element(res->msgs[0], SAMBA_INCOMPATIBLE_FEATURES_ATTR); + if (incompatibleFeatures != NULL) { + ldb_set_errstring(ldb, "This Samba database was created with " + "a newer Samba version and is marked with " + "incompaibleFeatures in @SAMBA_DSDB. " + "This database can not safely be read by this Samba version"); + return LDB_ERR_OPERATIONS_ERROR; + } + } else { talloc_free(tmp_ctx); return ret; diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index c03d5c2..87a1d01 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -315,4 +315,9 @@ struct dsdb_extended_sec_desc_propagation_op { #define DSDB_FLAG_INTERNAL_FORCE_META_DATA 0x10000 #define SAMBA_FEATURES_SUPPORTED_FLAG "@SAMBA_FEATURES_SUPPORTED" +#define SAMBA_COMPATIBLE_FEATURES_ATTR "compatibleFeatures" +#define SAMBA_INCOMPATIBLE_FEATURES_ATTR "incompatibleFeatures" + +#define SAMBA_SORTED_LINKS_FEATURE "sortedLinks" + #endif /* __SAMDB_H__ */ -- 2.7.4 >From 6fb2a130634b7ad6d9e9a6c8308ce1098bd409e3 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 17 Dec 2016 22:04:59 +1300 Subject: [PATCH 08/25] replmd: check whether list is already sorted in get_parsed_dns() If they are we can avoid the sort. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 14 ++++++++++---- 1 file changed, 10 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 28c8adf..4c51732 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1849,6 +1849,7 @@ static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx, const char *ldap_oid, struct ldb_request *parent) { unsigned int i; + bool values_are_sorted = true; struct ldb_context *ldb = ldb_module_get_ctx(module); if (el == NULL) { @@ -1898,13 +1899,18 @@ static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx, } else if (!NT_STATUS_IS_OK(status)) { return LDB_ERR_OPERATIONS_ERROR; } - + if (i > 0 && values_are_sorted) { + int cmp = parsed_dn_compare(p, &(*pdn)[i - 1]); + if (cmp < 0) { + values_are_sorted = false; + } + } /* keep a pointer to the original ldb_val */ p->v = v; } - - TYPESAFE_QSORT(*pdn, el->num_values, parsed_dn_compare); - + if (! values_are_sorted) { + TYPESAFE_QSORT(*pdn, el->num_values, parsed_dn_compare); + } return LDB_SUCCESS; } -- 2.7.4 >From ed38b5762d5c27876383ad1178f277e360681621 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 23 Dec 2016 14:18:13 +1300 Subject: [PATCH 09/25] Fix some whitespace in repl_meta_data.c Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 4c51732..b143f10 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1544,7 +1544,7 @@ static int replmd_update_rpmd(struct ldb_module *module, bool rmd_is_provided; bool rmd_is_just_resorted = false; const char *not_rename_attrs[4 + msg->num_elements]; - + if (rename_attrs) { attrs = rename_attrs; } else { @@ -4150,9 +4150,9 @@ static int replmd_op_possible_conflict_callback(struct ldb_request *req, struct * We are on an RODC, or were a GC for this * partition, so we have to fail this until * someone who owns the partition sorts it - * out + * out */ - ldb_asprintf_errstring(ldb_module_get_ctx(ar->module), + ldb_asprintf_errstring(ldb_module_get_ctx(ar->module), "Conflict adding object '%s' from incoming replication as we are read only for the partition. \n" " - We must fail the operation until a master for this partition resolves the conflict", ldb_dn_get_linearized(conflict_dn)); @@ -4574,7 +4574,7 @@ static int replmd_replicated_apply_search_for_parent_callback(struct ldb_request ldb_dn_get_linearized(parent_msg->dn)); return ldb_module_done(ar->req, NULL, NULL, LDB_ERR_OPERATIONS_ERROR); } - + ret = dsdb_wellknown_dn(ldb_module_get_ctx(ar->module), msg, nc_root, DS_GUID_LOSTANDFOUND_CONTAINER, @@ -4704,7 +4704,7 @@ static int replmd_replicated_apply_search_for_parent(struct replmd_replicated_re ar->req); LDB_REQ_SET_LOCATION(search_req); - ret = dsdb_request_add_controls(search_req, + ret = dsdb_request_add_controls(search_req, DSDB_SEARCH_SHOW_RECYCLED| DSDB_SEARCH_SHOW_DELETED| DSDB_SEARCH_SHOW_EXTENDED_DN); -- 2.7.4 >From 73cbbea068e86fefe37b4443d69278eb20572799 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 13:17:25 +1300 Subject: [PATCH 10/25] replmd: pass replmd_private down to replmd_add_backlink() This is not much saving, but we are soon going to need replmd_private in the intermediate layers (e.g. replmd_modify_la_add). Signed-off-by: Andrew Bartlett Pair-programmed-with: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 97 +++++++++++++++++++------ 1 file changed, 73 insertions(+), 24 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index b143f10..94018fb 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -432,14 +432,16 @@ static int replmd_process_backlink(struct ldb_module *module, struct la_backlink add a backlink to the list of backlinks to add/delete in the prepare commit */ -static int replmd_add_backlink(struct ldb_module *module, const struct dsdb_schema *schema, - struct GUID *forward_guid, struct GUID *target_guid, - bool active, const struct dsdb_attribute *schema_attr, bool immediate) +static int replmd_add_backlink(struct ldb_module *module, + struct replmd_private *replmd_private, + const struct dsdb_schema *schema, + struct GUID *forward_guid, + struct GUID *target_guid, bool active, + const struct dsdb_attribute *schema_attr, + bool immediate) { const struct dsdb_attribute *target_attr; struct la_backlink *bl; - struct replmd_private *replmd_private = - talloc_get_type_abort(ldb_module_get_private(module), struct replmd_private); target_attr = dsdb_attribute_by_linkID(schema, schema_attr->linkID ^ 1); if (!target_attr) { @@ -866,7 +868,9 @@ static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct ds This involves setting up the right meta-data in extended DN components, and creating backlinks to the object */ -static int replmd_add_fix_la(struct ldb_module *module, struct ldb_message_element *el, +static int replmd_add_fix_la(struct ldb_module *module, + struct replmd_private *replmd_private, + struct ldb_message_element *el, uint64_t seq_num, const struct GUID *invocationId, NTTIME now, struct GUID *guid, const struct dsdb_attribute *sa, struct ldb_request *parent) { @@ -912,7 +916,9 @@ static int replmd_add_fix_la(struct ldb_module *module, struct ldb_message_eleme return ret; } - ret = replmd_add_backlink(module, schema, guid, &target_guid, true, sa, false); + ret = replmd_add_backlink(module, replmd_private, + schema, guid, &target_guid, true, sa, + false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -1094,7 +1100,8 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req) } if (sa->linkID != 0 && functional_level > DS_DOMAIN_FUNCTION_2000) { - ret = replmd_add_fix_la(module, e, ac->seq_num, + ret = replmd_add_fix_la(module, replmd_private, e, + ac->seq_num, &ac->our_invocation_id, now, &guid, sa, req); if (ret != LDB_SUCCESS) { @@ -2138,6 +2145,7 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d handle adding a linked attribute */ static int replmd_modify_la_add(struct ldb_module *module, + struct replmd_private *replmd_private, const struct dsdb_schema *schema, struct ldb_message *msg, struct ldb_message_element *el, @@ -2229,7 +2237,9 @@ static int replmd_modify_la_add(struct ldb_module *module, } } - ret = replmd_add_backlink(module, schema, msg_guid, &dns[i].guid, true, schema_attr, true); + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &dns[i].guid, + true, schema_attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2265,6 +2275,7 @@ static int replmd_modify_la_add(struct ldb_module *module, handle deleting all active linked attributes */ static int replmd_modify_la_delete(struct ldb_module *module, + struct replmd_private *replmd_private, const struct dsdb_schema *schema, struct ldb_message *msg, struct ldb_message_element *el, @@ -2385,7 +2396,12 @@ static int replmd_modify_la_delete(struct ldb_module *module, el->flags = LDB_FLAG_MOD_REPLACE; 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); + ret = replmd_add_backlink(module, + replmd_private, + schema, msg_guid, + &old_dns[i].guid, + false, schema_attr, + true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2398,8 +2414,16 @@ static int replmd_modify_la_delete(struct ldb_module *module, 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); + /* The element is in the delete list. + mark it dead. */ + ret = replmd_add_backlink(module, + replmd_private, + schema, + msg_guid, + &old_dns[i].guid, + false, + schema_attr, + true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2442,7 +2466,9 @@ static int replmd_modify_la_delete(struct ldb_module *module, talloc_free(tmp_ctx); return ret; } - ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, true); + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &p->guid, + false, schema_attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2465,6 +2491,7 @@ static int replmd_modify_la_delete(struct ldb_module *module, handle replacing a linked attribute */ static int replmd_modify_la_replace(struct ldb_module *module, + struct replmd_private *replmd_private, const struct dsdb_schema *schema, struct ldb_message *msg, struct ldb_message_element *el, @@ -2525,7 +2552,9 @@ static int replmd_modify_la_replace(struct ldb_module *module, if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue; - ret = replmd_add_backlink(module, schema, msg_guid, &old_dns[i].guid, false, schema_attr, false); + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &old_dns[i].guid, + false, schema_attr, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2580,7 +2609,9 @@ static int replmd_modify_la_replace(struct ldb_module *module, num_new_values++; } - ret = replmd_add_backlink(module, schema, msg_guid, &dns[i].guid, true, schema_attr, false); + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &dns[i].guid, + true, schema_attr, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2619,6 +2650,7 @@ static int replmd_modify_la_replace(struct ldb_module *module, handle linked attributes in modify requests */ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, + struct replmd_private *replmd_private, struct ldb_message *msg, uint64_t seq_num, time_t t, struct ldb_request *parent) @@ -2700,13 +2732,22 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, old_el = ldb_msg_find_element(old_msg, el->name); switch (el->flags & LDB_FLAG_MOD_MASK) { case LDB_FLAG_MOD_REPLACE: - ret = replmd_modify_la_replace(module, schema, msg, el, old_el, schema_attr, seq_num, t, &old_guid, parent); + ret = replmd_modify_la_replace(module, replmd_private, + schema, msg, el, old_el, + schema_attr, seq_num, t, + &old_guid, parent); break; case LDB_FLAG_MOD_DELETE: - ret = replmd_modify_la_delete(module, schema, msg, el, old_el, schema_attr, seq_num, t, &old_guid, parent); + ret = replmd_modify_la_delete(module, replmd_private, + schema, msg, el, old_el, + schema_attr, seq_num, t, + &old_guid, parent); break; case LDB_FLAG_MOD_ADD: - ret = replmd_modify_la_add(module, schema, msg, el, old_el, schema_attr, seq_num, t, &old_guid, parent); + ret = replmd_modify_la_add(module, replmd_private, + schema, msg, el, old_el, + schema_attr, seq_num, t, + &old_guid, parent); break; default: ldb_asprintf_errstring(ldb, @@ -2842,7 +2883,8 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req) return ret; } - ret = replmd_modify_handle_linked_attribs(module, msg, ac->seq_num, t, req); + ret = replmd_modify_handle_linked_attribs(module, replmd_private, + msg, ac->seq_num, t, req); if (ret != LDB_SUCCESS) { talloc_free(ac); return ret; @@ -6079,6 +6121,7 @@ static int replmd_extended_replicated_objects(struct ldb_module *module, struct process one linked attribute structure */ static int replmd_process_linked_attribute(struct ldb_module *module, + struct replmd_private *replmd_private, struct la_entry *la_entry, struct ldb_request *parent) { @@ -6359,7 +6402,9 @@ linked_attributes[0]: if (!(rmd_flags & DSDB_RMD_FLAG_DELETED)) { /* remove the existing backlink */ - ret = replmd_add_backlink(module, schema, &la->identifier->guid, &guid, false, attr, true); + ret = replmd_add_backlink(module, replmd_private, + schema, &la->identifier->guid, + &guid, false, attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -6379,7 +6424,9 @@ linked_attributes[0]: if (active) { /* add the new backlink */ - ret = replmd_add_backlink(module, schema, &la->identifier->guid, &guid, true, attr, true); + ret = replmd_add_backlink(module, replmd_private, + schema, &la->identifier->guid, + &guid, true, attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -6413,8 +6460,9 @@ linked_attributes[0]: } if (active) { - ret = replmd_add_backlink(module, schema, &la->identifier->guid, &guid, - true, attr, true); + ret = replmd_add_backlink(module, replmd_private, + schema, &la->identifier->guid, + &guid, true, attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -6521,7 +6569,8 @@ static int replmd_prepare_commit(struct ldb_module *module) for (la = DLIST_TAIL(replmd_private->la_list); la; la=prev) { prev = DLIST_PREV(la); DLIST_REMOVE(replmd_private->la_list, la); - ret = replmd_process_linked_attribute(module, la, NULL); + ret = replmd_process_linked_attribute(module, replmd_private, + la, NULL); if (ret != LDB_SUCCESS) { replmd_txn_cleanup(replmd_private); return ret; -- 2.7.4 >From 54794274db136e124300e4fbd743db2760596172 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 15:08:00 +1300 Subject: [PATCH 11/25] replmd_check_upgrade_links() only checks the first DN This assumes the links (on an object in the database) are either all in the old format or all in the new. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 25 +++++++++++++++++++++---- 1 file changed, 21 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 94018fb..328cf75 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2015,7 +2015,9 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d /* check if any links need upgrading from w2k format - The parent_ctx is the ldb_message_element which contains the values array that dns[i].v points at, and which should be used for allocating any new value. + The parent_ctx is the ldb_message_element which contains the values array + that dns[i].v points at, and which should be used for allocating any new + value. */ static int replmd_check_upgrade_links(struct parsed_dn *dns, uint32_t count, struct ldb_message_element *parent_ctx, const struct GUID *invocation_id) { @@ -2025,14 +2027,29 @@ static int replmd_check_upgrade_links(struct parsed_dn *dns, uint32_t count, str uint32_t version; int ret; - status = dsdb_get_extended_dn_uint32(dns[i].dsdb_dn->dn, &version, "RMD_VERSION"); + status = dsdb_get_extended_dn_uint32(dns[i].dsdb_dn->dn, + &version, "RMD_VERSION"); if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + /* + * We optimistically assume they are all the same; if + * the first one is fixed, they are all fixed. + * + * If the first one was *not* fixed and we find a + * later one that is, that is an occasion to shout + * with DEBUG(0). + */ + if (i == 0) { + return LDB_SUCCESS; + } + DEBUG(0, ("Mixed w2k and fixed format " + "linked attributes\n")); continue; } /* it's an old one that needs upgrading */ - ret = replmd_update_la_val(parent_ctx->values, dns[i].v, dns[i].dsdb_dn, dns[i].dsdb_dn, invocation_id, - 1, 1, 0, 0, false); + ret = replmd_update_la_val(parent_ctx->values, dns[i].v, + dns[i].dsdb_dn, dns[i].dsdb_dn, + invocation_id, 1, 1, 0, 0, false); if (ret != LDB_SUCCESS) { return ret; } -- 2.7.4 >From 41187e298997dc7f64c87cdf9c14bf149ec1bd1d Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 16:15:42 +1300 Subject: [PATCH 12/25] fix variable names in replmd_update_la_val Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 328cf75..eea77e4 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2014,12 +2014,12 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d /* check if any links need upgrading from w2k format - - The parent_ctx is the ldb_message_element which contains the values array - that dns[i].v points at, and which should be used for allocating any new - value. */ -static int replmd_check_upgrade_links(struct parsed_dn *dns, uint32_t count, struct ldb_message_element *parent_ctx, const struct GUID *invocation_id) +static int replmd_check_upgrade_links(struct ldb_context *ldb, + struct parsed_dn *dns, uint32_t count, + struct ldb_message_element *el, + const struct GUID *invocation_id, + const char *ldap_oid) { uint32_t i; for (i=0; ivalues, dns[i].v, + ret = replmd_update_la_val(el->values, dns[i].v, dns[i].dsdb_dn, dns[i].dsdb_dn, invocation_id, 1, 1, 0, 0, false); if (ret != LDB_SUCCESS) { -- 2.7.4 >From 0de45d6b311e2d38d36c798e7e616f8a1f110bee Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 15 Dec 2016 14:39:33 +1300 Subject: [PATCH 13/25] binsearch: clarify variable name in greater-than-or-equal search The exact match variable was called "result" following the other macros, which confused me for a moment. Signed-off-by: Douglas Bagnall --- lib/util/binsearch.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/util/binsearch.h b/lib/util/binsearch.h index 001f2c5..454e5ce 100644 --- a/lib/util/binsearch.h +++ b/lib/util/binsearch.h @@ -86,31 +86,31 @@ like BINARY_ARRAY_SEARCH_V, but if an exact result is not found, the 'next' argument will point to the element after the place where the exact result would have been. If an exact result is found, 'next' will be NULL. If the - target is beyond the end of the list, both 'result' and 'next' will be NULL. + target is beyond the end of the list, both 'exact' and 'next' will be NULL. Unlike other binsearch macros, where there are several elements that compare - the same, the result will always point to the first one. + the same, the exact result will always point to the first one. If you don't care to distinguish between the 'greater than' and 'equals' - cases, you can use the same pointer for both 'result' and 'next'. + cases, you can use the same pointer for both 'exact' and 'next'. As with all the binsearch macros, the comparison function is always called with the search term first. */ #define BINARY_ARRAY_SEARCH_GTE(array, array_size, target, comparison_fn, \ - result, next) do { \ + exact, next) do { \ int32_t _b, _e; \ - (result) = NULL; (next) = NULL; \ + (exact) = NULL; (next) = NULL; \ if ((array_size) > 0) { \ for (_b = 0, _e = (array_size)-1; _b <= _e; ) { \ int32_t _i = (_b + _e) / 2; \ int _r = comparison_fn(target, array[_i]); \ if (_r == 0) { \ - (result) = &array[_i]; \ + (exact) = &array[_i]; \ _e = _i - 1; \ } else if (_r < 0) { _e = _i - 1; \ } else { _b = _i + 1; } \ } \ - if ((result) == NULL &&_b < (array_size)) { \ + if ((exact) == NULL &&_b < (array_size)) { \ (next) = &array[_b]; \ } } } while (0) -- 2.7.4 >From 319d0444816c3157986be013f6dfdee00fdf1867 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 22 Dec 2016 16:09:22 +1300 Subject: [PATCH 14/25] binsearch: make BINARY_ARRAY_SEARCH_GTE compare against a pointer This is in preparation for improvements in our handling of linked attributes where we make changes to the pointer in the process of comparing it (for caching purposes). Signed-off-by: Douglas Bagnall --- lib/util/binsearch.h | 2 +- lib/util/tests/binsearch.c | 9 +++++++-- source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/util/binsearch.h b/lib/util/binsearch.h index 454e5ce..4204c9c 100644 --- a/lib/util/binsearch.h +++ b/lib/util/binsearch.h @@ -103,7 +103,7 @@ if ((array_size) > 0) { \ for (_b = 0, _e = (array_size)-1; _b <= _e; ) { \ int32_t _i = (_b + _e) / 2; \ - int _r = comparison_fn(target, array[_i]); \ + int _r = comparison_fn(target, &array[_i]); \ if (_r == 0) { \ (exact) = &array[_i]; \ _e = _i - 1; \ diff --git a/lib/util/tests/binsearch.c b/lib/util/tests/binsearch.c index c6ef47e..b3ecda1 100644 --- a/lib/util/tests/binsearch.c +++ b/lib/util/tests/binsearch.c @@ -31,6 +31,11 @@ static int int_cmp(int a, int b) return a - b; } +static int int_cmp_p(int a, int *b) +{ + return a - *b; +} + static bool test_binsearch_v(struct torture_context *tctx) { int array[] = { -11, -7, 0, 1, 723, 1000000}; @@ -72,7 +77,7 @@ static bool test_binsearch_gte(struct torture_context *tctx) i, target); BINARY_ARRAY_SEARCH_GTE(array, a_len, target, - int_cmp, result, next); + int_cmp_p, result, next); if (result == NULL) { /* we think there is no exact match */ @@ -134,7 +139,7 @@ static bool test_binsearch_gte(struct torture_context *tctx) i, target); BINARY_ARRAY_SEARCH_GTE(array, a_len, target, - int_cmp, result, result); + int_cmp_p, result, result); if (result == NULL) { /* we think the target is greater than all elements */ diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c index bd8df7d..5b744e3 100644 --- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c +++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c @@ -232,7 +232,7 @@ static int send_referrals(struct results_store *store, /* vlv_value_compare() is used in a binary search */ static int vlv_value_compare(struct vlv_sort_context *target, - struct GUID guid) + struct GUID *guid) { struct ldb_result *result = NULL; struct ldb_message_element *el = NULL; @@ -243,7 +243,7 @@ static int vlv_value_compare(struct vlv_sort_context *target, NULL }; - ret = vlv_search_by_dn_guid(ac->module, ac, &result, &guid, attrs); + ret = vlv_search_by_dn_guid(ac->module, ac, &result, guid, attrs); if (ret != LDB_SUCCESS) { target->status = ret; @@ -259,7 +259,7 @@ static int vlv_value_compare(struct vlv_sort_context *target, /* The same as vlv_value_compare() but sorting in the opposite direction. */ static int vlv_value_compare_rev(struct vlv_sort_context *target, - struct GUID guid) + struct GUID *guid) { return -vlv_value_compare(target, guid); } -- 2.7.4 >From 0fcf077b0f7632e7b154f5b8f5f2482b1b73ac12 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 12:12:23 +1300 Subject: [PATCH 15/25] replmd: parsed_dn_find() finds insertion point as well as exact hit This will allow us to maintain the list of links in sorted order. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 232 ++++++++++++++++++++---- 1 file changed, 201 insertions(+), 31 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index eea77e4..1c88ddc 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1816,35 +1816,134 @@ struct parsed_dn { struct ldb_val *v; }; +struct compare_ctx { + struct GUID *guid; + struct ldb_context *ldb; + TALLOC_CTX *mem_ctx; + const char *ldap_oid; + int err; + const struct GUID *invocation_id; +}; + + static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2) { return GUID_compare(&pdn1->guid, &pdn2->guid); } -static int GUID_compare_struct(struct GUID *g1, struct GUID g2) +static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, + struct parsed_dn *p) { - return GUID_compare(g1, &g2); + /* + * This works like a standard compare function in its return values, + * but has an extra trick to deal with errors: zero is returned and + * ctx->err is set to the ldb error code. + * + * That is, if (as is expected in most cases) you get a non-zero + * result, you don't need to check for errors. + */ + NTSTATUS status; + + if (GUID_all_zero(&p->guid)) { + + status = dsdb_get_extended_dn_guid(p->dsdb_dn->dn, &p->guid, "GUID"); + if (!NT_STATUS_IS_OK(status)) { + ctx->err = LDB_ERR_OPERATIONS_ERROR; + return 0; + } + } + + return GUID_compare(ctx->guid, &p->guid); } -static struct parsed_dn *parsed_dn_find(struct parsed_dn *pdn, - unsigned int count, struct GUID *guid, - struct ldb_dn *dn) + + +static int parsed_dn_find(struct ldb_context *ldb, struct parsed_dn *pdn, + unsigned int count, + struct GUID *guid, + struct ldb_dn *target_dn, + struct parsed_dn **exact, + struct parsed_dn **next, + const char *ldap_oid) { - struct parsed_dn *ret; unsigned int i; - if (dn && GUID_all_zero(guid)) { - /* when updating a link using DRS, we sometimes get a - NULL GUID. We then need to try and match by DN */ - for (i=0; idn, dn) == 0) { - dsdb_get_extended_dn_guid(pdn[i].dsdb_dn->dn, guid, "GUID"); - return &pdn[i]; + struct compare_ctx ctx; + if (pdn == NULL) { + *exact = NULL; + *next = NULL; + return LDB_SUCCESS; + } + + if (unlikely(GUID_all_zero(guid))) { + /* + * When updating a link using DRS, we sometimes get a NULL + * GUID when a forward link has been deleted and its GUID has + * for some reason been forgotten. The best we can do is try + * and match by DN via a linear search. Note that this + * probably only happens in the ADD case, in which we only + * allow modification of link if it is already deleted, so + * this seems very close to an elaborate NO-OP, but we are not + * quite prepared to declare it so. + * + * If the DN is not in our list, we have to add it to the + * beginning of the list, where it would naturally sort. + */ + struct parsed_dn *p; + if (target_dn == NULL) { + /* We don't know the target DN, so we can't search for DN */ + DEBUG(1, ("parsed_dn_find has a NULL GUID for a linked " + "attribute but we don't have a DN to compare " + "it with\n")); + return LDB_ERR_OPERATIONS_ERROR; + } + *exact = NULL; + *next = NULL; + + DEBUG(3, ("parsed_dn_find has a NULL GUID for a link to DN " + "%s; searching through links for it", + ldb_dn_get_linearized(target_dn))); + + for (i = 0; i < count; i++) { + int cmp; + p = &pdn[i]; + cmp = ldb_dn_compare(p->dsdb_dn->dn, target_dn); + if (cmp == 0) { + dsdb_get_extended_dn_guid(p->dsdb_dn->dn, + &p->guid, "GUID"); + *exact = p; + return LDB_SUCCESS; } } - return NULL; + /* + * Here we have a null guid which doesn't match any existing + * link. This is a bit unexpected because null guids occur + * when a forward link has been deleted and we are replicating + * that deletion. + * + * The best thing to do is weep into the logs and add the + * offending link to the beginning of the list which is + * at least the correct sort position. + */ + DEBUG(1, ("parsed_dn_find has been given a NULL GUID for a " + "link to unknown DN %s\n", + ldb_dn_get_linearized(target_dn))); + *next = pdn; + return LDB_SUCCESS; } - BINARY_ARRAY_SEARCH(pdn, count, guid, guid, GUID_compare_struct, ret); - return ret; + + ctx.guid = guid; + ctx.ldb = ldb; + ctx.mem_ctx = pdn; + ctx.ldap_oid = ldap_oid; + ctx.err = 0; + + BINARY_ARRAY_SEARCH_GTE(pdn, count, &ctx, la_guid_compare_with_trusted_dn, + *exact, *next); + + if (ctx.err != 0) { + return ctx.err; + } + return LDB_SUCCESS; } /* @@ -2212,7 +2311,17 @@ static int replmd_modify_la_add(struct ldb_module *module, /* for each new value, see if it exists already with the same GUID */ for (i=0; inum_values; i++) { - struct parsed_dn *p = parsed_dn_find(old_dns, old_num_values, &dns[i].guid, NULL); + struct parsed_dn *p; + struct parsed_dn *next; + int err = parsed_dn_find(ldb, old_dns, old_num_values, + &dns[i].guid, + dns[i].dsdb_dn->dn, + &p, &next, + schema_attr->syntax->ldap_oid); + if (err != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return err; + } if (p == NULL) { /* this is a new linked attribute value */ new_values = talloc_realloc(tmp_ctx, new_values, struct ldb_val, num_new_values+1); @@ -2232,7 +2341,6 @@ static int replmd_modify_la_add(struct ldb_module *module, /* this is only allowed if the GUID was previously deleted. */ uint32_t rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn); - if (!(rmd_flags & DSDB_RMD_FLAG_DELETED)) { struct GUID_txt_buf guid_str; ldb_asprintf_errstring(ldb, "Attribute %s already exists for target GUID %s", @@ -2371,9 +2479,18 @@ static int replmd_modify_la_delete(struct ldb_module *module, for (i=0; i < num_to_delete; i++) { struct parsed_dn *p = &dns[i]; struct parsed_dn *p2; + struct parsed_dn *next = NULL; uint32_t rmd_flags; + ret = parsed_dn_find(ldb, old_dns, old_el->num_values, + &p->guid, + NULL, + &p2, &next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } - p2 = parsed_dn_find(old_dns, old_el->num_values, &p->guid, NULL); if (!p2) { struct GUID_txt_buf buf; ldb_asprintf_errstring(ldb, "Attribute %s doesn't exist for target GUID %s", @@ -2429,8 +2546,20 @@ static int replmd_modify_la_delete(struct ldb_module *module, } else { unsigned int num_values = 0; unsigned int j = 0; + struct parsed_dn *exact = NULL, *next = NULL; + for (i = 0; i < old_el->num_values; i++) { - if (parsed_dn_find(dns, num_to_delete, &old_dns[i].guid, NULL) != NULL) { + ret = parsed_dn_find(ldb, dns, num_to_delete, + &old_dns[i].guid, + NULL, + &exact, &next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + if (exact != NULL) { /* The element is in the delete list. mark it dead. */ ret = replmd_add_backlink(module, @@ -2468,10 +2597,22 @@ static int replmd_modify_la_delete(struct ldb_module *module, */ for (i=0; inum_values; i++) { struct parsed_dn *p = &old_dns[i]; + struct parsed_dn *exact = NULL, *next = NULL; uint32_t rmd_flags; - if (num_to_delete && parsed_dn_find(dns, num_to_delete, &p->guid, NULL) == NULL) { - continue; + if (num_to_delete != 0) { + ret = parsed_dn_find(ldb, dns, num_to_delete, + &p->guid, + NULL, + &exact, &next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + if (exact == NULL) { + continue; + } } rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn); @@ -2564,7 +2705,7 @@ static int replmd_modify_la_replace(struct ldb_module *module, /* mark all the old ones as deleted */ for (i=0; idsdb_dn->dn); if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue; @@ -2577,8 +2718,16 @@ static int replmd_modify_la_replace(struct ldb_module *module, return ret; } - p = parsed_dn_find(dns, el->num_values, &old_p->guid, NULL); - if (p) { + ret = parsed_dn_find(ldb, dns, el->num_values, + &old_p->guid, + NULL, + &exact, &next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + if (exact) { /* we don't delete it if we are re-adding it */ continue; } @@ -2595,11 +2744,22 @@ static int replmd_modify_la_replace(struct ldb_module *module, * to old_el */ for (i=0; inum_values; i++) { - struct parsed_dn *p = &dns[i], *old_p; + struct parsed_dn *p = &dns[i]; + struct parsed_dn *old_p = NULL, *next = NULL; + + if (old_dns) { + ret = parsed_dn_find(ldb, old_dns, old_num_values, + &p->guid, + NULL, + &old_p, &next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + } - if (old_dns && - (old_p = parsed_dn_find(old_dns, - old_num_values, &p->guid, NULL)) != NULL) { + if (old_p != NULL) { /* update in place */ ret = replmd_update_la_val(old_el->values, old_p->v, p->dsdb_dn, old_p->dsdb_dn, invocation_id, @@ -6159,7 +6319,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module, struct ldb_result *target_res; const char *attrs[4]; const char *attrs2[] = { "isDeleted", "isRecycled", NULL }; - struct parsed_dn *pdn_list, *pdn; + struct parsed_dn *pdn_list, *pdn, *next; struct GUID guid = GUID_zero(); NTSTATUS ntstatus; bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false; @@ -6383,7 +6543,17 @@ linked_attributes[0]: } /* see if this link already exists */ - pdn = parsed_dn_find(pdn_list, old_el->num_values, &guid, dsdb_dn->dn); + ret = parsed_dn_find(ldb, pdn_list, old_el->num_values, + &guid, + dsdb_dn->dn, + &pdn, &next, + attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + if (pdn != NULL) { /* see if this update is newer than what we have already */ struct GUID invocation_id = GUID_zero(); -- 2.7.4 >From 75b33673103d3192cf5df841d2a3a7daf05342b5 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 15:09:15 +1300 Subject: [PATCH 16/25] replmd_check_upgrade_links() parses DNs as necessary Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 23 +++++++++++++++++++---- 1 file changed, 19 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 1c88ddc..6a1bd51 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2125,6 +2125,13 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb, NTSTATUS status; uint32_t version; int ret; + if (dns[i].dsdb_dn == NULL) { + dns[i].dsdb_dn = dsdb_dn_parse(dns, ldb, dns[i].v, + ldap_oid); + if (dns[i].dsdb_dn == NULL) { + return LDB_ERR_INVALID_DN_SYNTAX; + } + } status = dsdb_get_extended_dn_uint32(dns[i].dsdb_dn->dn, &version, "RMD_VERSION"); @@ -2303,7 +2310,9 @@ static int replmd_modify_la_add(struct ldb_module *module, return LDB_ERR_OPERATIONS_ERROR; } - ret = replmd_check_upgrade_links(old_dns, old_num_values, old_el, invocation_id); + ret = replmd_check_upgrade_links(ldb, old_dns, old_num_values, + old_el, invocation_id, + schema_attr->syntax->ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2457,7 +2466,9 @@ static int replmd_modify_la_delete(struct ldb_module *module, return LDB_ERR_OPERATIONS_ERROR; } - ret = replmd_check_upgrade_links(old_dns, old_el->num_values, old_el, invocation_id); + ret = replmd_check_upgrade_links(ldb, old_dns, old_el->num_values, + old_el, invocation_id, + schema_attr->syntax->ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2696,7 +2707,9 @@ static int replmd_modify_la_replace(struct ldb_module *module, return LDB_ERR_OPERATIONS_ERROR; } - ret = replmd_check_upgrade_links(old_dns, old_num_values, old_el, invocation_id); + ret = replmd_check_upgrade_links(ldb, old_dns, old_num_values, + old_el, invocation_id, + schema_attr->syntax->ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -6445,7 +6458,9 @@ linked_attributes[0]: return LDB_ERR_OPERATIONS_ERROR; } - ret = replmd_check_upgrade_links(pdn_list, old_el->num_values, old_el, our_invocation_id); + ret = replmd_check_upgrade_links(ldb, pdn_list, old_el->num_values, + old_el, our_invocation_id, + attr->syntax->ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; -- 2.7.4 >From 99dc52cee2800e094fc5ba323c2db581738c531a Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 16:24:23 +1300 Subject: [PATCH 17/25] replmd linked attributes: lazy parsing for trusted DNs If we know that links from the database are in sorted order (via the replmd_private->sorted_links flag), we can avoid actually parsing them until it is absolutely necessary. In many cases we are adding a single link to a long list. The location of the single link is found via a binary search, so we end up parsing log(N) DNs instead of N. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 99 ++++++++++++++++++++++++- 1 file changed, 97 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 6a1bd51..f6e0399 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1825,6 +1825,25 @@ struct compare_ctx { const struct GUID *invocation_id; }; +/* When a parsed_dn comes from the database, sometimes it is not really parsed. */ + +static int really_parse_trusted_dn(TALLOC_CTX *mem_ctx, struct ldb_context *ldb, + struct parsed_dn *pdn, const char *ldap_oid) +{ + NTSTATUS status; + struct dsdb_dn *dsdb_dn = dsdb_dn_parse_trusted(mem_ctx, ldb, pdn->v, + ldap_oid); + if (dsdb_dn == NULL) { + return LDB_ERR_INVALID_DN_SYNTAX; + } + + status = dsdb_get_extended_dn_guid(dsdb_dn->dn, &pdn->guid, "GUID"); + if (!NT_STATUS_IS_OK(status)) { + return LDB_ERR_OPERATIONS_ERROR; + } + pdn->dsdb_dn = dsdb_dn; + return LDB_SUCCESS; +} static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2) { @@ -1841,11 +1860,23 @@ static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, * * That is, if (as is expected in most cases) you get a non-zero * result, you don't need to check for errors. + * + * We assume the second argument refers to a DN is from the database + * and has a GUID -- but this GUID might not have been parsed out yet. */ NTSTATUS status; if (GUID_all_zero(&p->guid)) { + if (p->dsdb_dn == NULL) { + p->dsdb_dn = dsdb_dn_parse_trusted(ctx->mem_ctx, ctx->ldb, p->v, + ctx->ldap_oid); + if (p->dsdb_dn == NULL) { + ctx->err = LDB_ERR_INVALID_DN_SYNTAX; + return 0; + } + } + status = dsdb_get_extended_dn_guid(p->dsdb_dn->dn, &p->guid, "GUID"); if (!NT_STATUS_IS_OK(status)) { ctx->err = LDB_ERR_OPERATIONS_ERROR; @@ -1906,6 +1937,13 @@ static int parsed_dn_find(struct ldb_context *ldb, struct parsed_dn *pdn, for (i = 0; i < count; i++) { int cmp; p = &pdn[i]; + if (p->dsdb_dn == NULL) { + p->dsdb_dn = dsdb_dn_parse_trusted(pdn, ldb, + p->v, ldap_oid); + if (p->dsdb_dn == NULL) { + return LDB_ERR_OPERATIONS_ERROR; + } + } cmp = ldb_dn_compare(p->dsdb_dn->dn, target_dn); if (cmp == 0) { dsdb_get_extended_dn_guid(p->dsdb_dn->dn, @@ -2021,6 +2059,49 @@ static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx, } /* + * Get a series of trusted message element values. The result is sorted by + * GUID, even though the GUIDs might not be known. That works because we trust + * the database to give us the elements like that if the + * replmd_private->sorted_links flag is set. + */ +static int get_parsed_dns_trusted(struct ldb_module *module, + struct replmd_private *replmd_private, + TALLOC_CTX *mem_ctx, + struct ldb_message_element *el, + struct parsed_dn **pdn, + const char *ldap_oid, + struct ldb_request *parent) +{ + unsigned int i; + + if (el == NULL) { + *pdn = NULL; + return LDB_SUCCESS; + } + + if (!replmd_private->sorted_links) { + /* We need to sort the list. This is the slow old path we want + to avoid. + */ + return get_parsed_dns(module, mem_ctx, el, pdn, ldap_oid, + parent); + } + /* Here we get a list of 'struct parsed_dns' without the parsing */ + *pdn = talloc_zero_array(mem_ctx, struct parsed_dn, + el->num_values); + if (!*pdn) { + ldb_module_oom(module); + return LDB_ERR_OPERATIONS_ERROR; + } + + for (i = 0; i < el->num_values; i++) { + (*pdn)[i].v = &el->values[i]; + } + + return LDB_SUCCESS; +} + +/* build a new extended DN, including all meta data fields RMD_FLAGS = DSDB_RMD_FLAG_* bits @@ -2111,6 +2192,7 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d uint64_t seq_num, uint64_t local_usn, NTTIME nttime, uint32_t version, bool deleted); + /* check if any links need upgrading from w2k format */ @@ -2264,6 +2346,8 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d return LDB_SUCCESS; } + + /* handle adding a linked attribute */ @@ -2292,13 +2376,24 @@ static int replmd_modify_la_add(struct ldb_module *module, unix_to_nt_time(&now, t); - ret = get_parsed_dns(module, tmp_ctx, el, &dns, schema_attr->syntax->ldap_oid, parent); + /* get the DNs to be added, fully parsed. + * + * We need full parsing because they came off the wire and we don't + * trust them, besides which we need their details to know where to put + * them. + */ + ret = get_parsed_dns(module, tmp_ctx, el, &dns, + schema_attr->syntax->ldap_oid, parent); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - ret = get_parsed_dns(module, tmp_ctx, old_el, &old_dns, schema_attr->syntax->ldap_oid, parent); + /* get the existing DNs, lazily parsed */ + ret = get_parsed_dns_trusted(module, replmd_private, + tmp_ctx, old_el, &old_dns, + schema_attr->syntax->ldap_oid, parent); + if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; -- 2.7.4 >From ddf1c14ea08ee18623654e9470ba6c8c6131534d Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 13:57:21 +1300 Subject: [PATCH 18/25] replmd linked attrs: fully parse dn for upgrade check Elsewhere we use the dsdb_dn pointer as a flag indicating parsed-ness, so we have to be consistent. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index f6e0399..6d0e8c2 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2208,9 +2208,9 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb, uint32_t version; int ret; if (dns[i].dsdb_dn == NULL) { - dns[i].dsdb_dn = dsdb_dn_parse(dns, ldb, dns[i].v, - ldap_oid); - if (dns[i].dsdb_dn == NULL) { + ret = really_parse_trusted_dn(dns, ldb, &dns[i], + ldap_oid); + if (ret != LDB_SUCCESS) { return LDB_ERR_INVALID_DN_SYNTAX; } } -- 2.7.4 >From 676e3fb6fcb8dfd446924714626ed998053035fb Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 13:20:41 +1300 Subject: [PATCH 19/25] replmd: rework replmd_modify_la_add to merge efficiently Because both the list of added links and the list of existing links are sorted, it is possible to interlace the two and obtain a merged sorted list. We avoid a great amount of talloc_realloc()ing by observing that the merged list can't be longer than the sum of the two lists. In the (common) case where there are many existing links but few being added, we avoid parsing most of the existing link DNs and GUIDs if the sorted_links feature flag is set. Signed-off-by: Douglas Bagnall Pair-programmed-with: Garming Sam --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 157 +++++++++++++++++------- 1 file changed, 114 insertions(+), 43 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 6d0e8c2..0248e74 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2363,13 +2363,14 @@ static int replmd_modify_la_add(struct ldb_module *module, struct GUID *msg_guid, struct ldb_request *parent) { - unsigned int i; + unsigned int i, j; struct parsed_dn *dns, *old_dns; TALLOC_CTX *tmp_ctx = talloc_new(msg); int ret; struct ldb_val *new_values = NULL; - unsigned int num_new_values = 0; - unsigned old_num_values = old_el?old_el->num_values:0; + unsigned old_num_values = old_el ? old_el->num_values : 0; + unsigned num_values = 0; + unsigned max_num_values; const struct GUID *invocation_id; struct ldb_context *ldb = ldb_module_get_ctx(module); NTTIME now; @@ -2413,42 +2414,63 @@ static int replmd_modify_la_add(struct ldb_module *module, return ret; } - /* for each new value, see if it exists already with the same GUID */ - for (i=0; inum_values; i++) { - struct parsed_dn *p; + max_num_values = old_num_values + el->num_values; + if (max_num_values < old_num_values) { + DEBUG(0, ("we seem to have overflow in replmd_modify_la_add. " + "old values: %u, new values: %u, sum: %u", + old_num_values, el->num_values, max_num_values)); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + + new_values = talloc_zero_array(tmp_ctx, struct ldb_val, max_num_values); + + if (new_values == NULL) { + ldb_module_oom(module); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + + /* + * For each new value, find where it would go in the list. If there is + * a matching GUID there, we update the existing value; otherwise we + * put it in place. + */ + j = 0; + for (i = 0; i < el->num_values; i++) { + struct parsed_dn *exact; struct parsed_dn *next; + unsigned offset; int err = parsed_dn_find(ldb, old_dns, old_num_values, &dns[i].guid, dns[i].dsdb_dn->dn, - &p, &next, + &exact, &next, schema_attr->syntax->ldap_oid); if (err != LDB_SUCCESS) { talloc_free(tmp_ctx); return err; } - if (p == NULL) { - /* this is a new linked attribute value */ - new_values = talloc_realloc(tmp_ctx, new_values, struct ldb_val, num_new_values+1); - if (new_values == NULL) { - ldb_module_oom(module); - talloc_free(tmp_ctx); - return LDB_ERR_OPERATIONS_ERROR; - } - ret = replmd_build_la_val(new_values, &new_values[num_new_values], dns[i].dsdb_dn, - invocation_id, seq_num, seq_num, now, 0, false); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - num_new_values++; - } else { - /* this is only allowed if the GUID was - previously deleted. */ - uint32_t rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn); + + if (exact != NULL) { + /* + * We are trying to add one that exists, which is only + * allowed if it was previously deleted. + * + * When we do undelete a link we change it in place. + * It will be copied across into the right spot in due + * course. + */ + uint32_t rmd_flags; + rmd_flags = dsdb_dn_rmd_flags(exact->dsdb_dn->dn); + if (!(rmd_flags & DSDB_RMD_FLAG_DELETED)) { struct GUID_txt_buf guid_str; - ldb_asprintf_errstring(ldb, "Attribute %s already exists for target GUID %s", - el->name, GUID_buf_string(&p->guid, &guid_str)); + ldb_asprintf_errstring(ldb, + "Attribute %s already " + "exists for target GUID %s", + el->name, + GUID_buf_string(&exact->guid, + &guid_str)); talloc_free(tmp_ctx); /* error codes for 'member' need to be special cased */ @@ -2458,37 +2480,86 @@ static int replmd_modify_la_add(struct ldb_module *module, return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; } } - ret = replmd_update_la_val(old_el->values, p->v, dns[i].dsdb_dn, p->dsdb_dn, - invocation_id, seq_num, seq_num, now, 0, false); + + ret = replmd_update_la_val(new_values, exact->v, + dns[i].dsdb_dn, + exact->dsdb_dn, + invocation_id, seq_num, + seq_num, now, 0, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } + + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, + &dns[i].guid, true, + schema_attr, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + continue; + } + /* + * Here we don't have an exact match. + * + * If next is NULL, this one goes beyond the end of the + * existing list, so we need to add all of those ones first. + * + * If next is not NULL, we need to add all the ones before + * next. + */ + if (next == NULL) { + offset = old_num_values; + } else { + /* next should have been parsed, but let's make sure */ + if (next->dsdb_dn == NULL) { + ret = really_parse_trusted_dn(tmp_ctx, ldb, next, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + return ret; + } + } + offset = MIN(next - old_dns, old_num_values); + } + + /* put all the old ones before next on the list */ + for (; j < offset; j++) { + new_values[num_values] = *old_dns[j].v; + num_values++; } ret = replmd_add_backlink(module, replmd_private, schema, msg_guid, &dns[i].guid, true, schema_attr, true); + /* Make the new linked attribute ldb_val. */ + ret = replmd_build_la_val(new_values, &new_values[num_values], + dns[i].dsdb_dn, invocation_id, + seq_num, seq_num, + now, 0, false); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + num_values++; if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } } - - /* add the new ones on to the end of the old values, constructing a new el->values */ - el->values = talloc_realloc(msg->elements, old_el?old_el->values:NULL, - struct ldb_val, - old_num_values+num_new_values); - if (el->values == NULL) { - ldb_module_oom(module); - return LDB_ERR_OPERATIONS_ERROR; + /* copy the rest of the old ones (if any) */ + for (; j < old_num_values; j++) { + new_values[num_values] = *old_dns[j].v; + num_values++; } - memcpy(&el->values[old_num_values], new_values, num_new_values*sizeof(struct ldb_val)); - el->num_values = old_num_values + num_new_values; - - talloc_steal(msg->elements, el->values); - talloc_steal(el->values, new_values); + talloc_steal(msg->elements, new_values); + if (old_el != NULL) { + talloc_steal(msg->elements, old_el->values); + } + el->values = new_values; + el->num_values = num_values; talloc_free(tmp_ctx); -- 2.7.4 >From 5a87fa2e9574abd4e3dbf39ad5f14e7f736028fb Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 4 Jan 2017 14:09:00 +1300 Subject: [PATCH 20/25] repl_meta_data: linked attributes use DRS sort order Links come over the wire as if sorted by memcmp() on the binary blobs, not as sorted by GUID_compare(). Until a few patches ago, a newly joined DC would have its linked attributes in the memcmp order. This restores that behaviour. This comparison could be made more efficient by storing the GUID in the original state, but it does not seem to be a bottleneck. Signed-off-by: Andrew Bartlett Pair-programmed-with: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 28 +++++++++++++++++++++++-- 1 file changed, 26 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 0248e74..9175b7d 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1845,9 +1845,33 @@ static int really_parse_trusted_dn(TALLOC_CTX *mem_ctx, struct ldb_context *ldb, return LDB_SUCCESS; } +/* + * We choose, as the sort order, the same order as is used in DRS replication, + * which is the memcmp() order of the NDR GUID, not that obtained from + * GUID_compare(). + * + * This means that sorted links will be in the same order as a new DC would + * see them. + */ +static int ndr_guid_compare(struct GUID *guid1, struct GUID *guid2) +{ + uint8_t v1_data[16]; + struct ldb_val v1 = data_blob_const(v1_data, sizeof(v1_data)); + uint8_t v2_data[16]; + struct ldb_val v2 = data_blob_const(v2_data, sizeof(v2_data)); + + /* This can't fail */ + ndr_push_struct_into_fixed_blob(&v1, guid1, + (ndr_push_flags_fn_t)ndr_push_GUID); + /* This can't fail */ + ndr_push_struct_into_fixed_blob(&v2, guid2, + (ndr_push_flags_fn_t)ndr_push_GUID); + return data_blob_cmp(&v1, &v2); +} + static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2) { - return GUID_compare(&pdn1->guid, &pdn2->guid); + return ndr_guid_compare(&pdn1->guid, &pdn2->guid); } static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, @@ -1884,7 +1908,7 @@ static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, } } - return GUID_compare(ctx->guid, &p->guid); + return ndr_guid_compare(ctx->guid, &p->guid); } -- 2.7.4 >From 087597239a2767cd1820dbbf619ba9a68e16fa32 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 09:49:38 +1300 Subject: [PATCH 21/25] replmd: rearrange nothing-to-delete logic Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 9175b7d..6ae9e9c 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2623,13 +2623,12 @@ static int replmd_modify_la_delete(struct ldb_module *module, unix_to_nt_time(&now, t); - /* check if there is nothing to delete */ - if ((!old_el || old_el->num_values == 0) && - el->num_values == 0) { - return LDB_SUCCESS; - } - - if (!old_el || old_el->num_values == 0) { + if (old_el == NULL || old_el->num_values == 0) { + /* there is nothing to delete... */ + if (el->num_values == 0) { + /* and we're deleting nothing, so that's OK */ + return LDB_SUCCESS; + } return LDB_ERR_NO_SUCH_ATTRIBUTE; } -- 2.7.4 >From add6eaed15e9e912efb71ce4365d78212240db2c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 11:00:57 +1300 Subject: [PATCH 22/25] replmd: simplify and optimise replmd_modify_la_delete With the old binary search, we didn't get a pointer to the found value, just a yes or no answer as to its existence. That meant we ended up searching in both directions to find the links to be deleted. As a consequence we needed to parse out the GUID of every existing link, even if it wasn't being deleted. Here we do it in one pass. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 253 ++++++++++++------------ 1 file changed, 129 insertions(+), 124 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 6ae9e9c..cb02b38 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2619,13 +2619,14 @@ static int replmd_modify_la_delete(struct ldb_module *module, struct ldb_control *vanish_links_ctrl = NULL; bool vanish_links = false; unsigned int num_to_delete = el->num_values; + uint32_t rmd_flags; NTTIME now; unix_to_nt_time(&now, t); if (old_el == NULL || old_el->num_values == 0) { /* there is nothing to delete... */ - if (el->num_values == 0) { + if (num_to_delete == 0) { /* and we're deleting nothing, so that's OK */ return LDB_SUCCESS; } @@ -2637,13 +2638,17 @@ static int replmd_modify_la_delete(struct ldb_module *module, return LDB_ERR_OPERATIONS_ERROR; } - ret = get_parsed_dns(module, tmp_ctx, el, &dns, schema_attr->syntax->ldap_oid, parent); + ret = get_parsed_dns(module, tmp_ctx, el, &dns, + schema_attr->syntax->ldap_oid, parent); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - ret = get_parsed_dns(module, tmp_ctx, old_el, &old_dns, schema_attr->syntax->ldap_oid, parent); + ret = get_parsed_dns_trusted(module, replmd_private, + tmp_ctx, old_el, &old_dns, + schema_attr->syntax->ldap_oid, parent); + if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2671,30 +2676,83 @@ static int replmd_modify_la_delete(struct ldb_module *module, } } + /* we empty out el->values here to avoid damage if we return early. */ 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 < num_to_delete; i++) { + /* + * If vanish links is set, we are actually removing members of + * old_el->values; otherwise we are just marking them deleted. + * + * There is a special case when no values are given: we remove them + * all. When we have the vanish_links control we just have to remove + * the backlinks and change our element to replace the existing values + * with the empty list. + */ + + if (num_to_delete == 0) { + for (i = 0; i < old_el->num_values; i++) { + struct parsed_dn *p = &old_dns[i]; + if (p->dsdb_dn == NULL) { + ret = really_parse_trusted_dn(tmp_ctx, ldb, p, + schema_attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + return ret; + } + } + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &p->guid, + false, schema_attr, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + if (vanish_links) { + continue; + } + + 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; + } + } + + if (vanish_links) { + el->flags = LDB_FLAG_MOD_REPLACE; + talloc_free(tmp_ctx); + return LDB_SUCCESS; + } + } + + + for (i = 0; i < num_to_delete; i++) { struct parsed_dn *p = &dns[i]; - struct parsed_dn *p2; + struct parsed_dn *exact = NULL; struct parsed_dn *next = NULL; - uint32_t rmd_flags; ret = parsed_dn_find(ldb, old_dns, old_el->num_values, &p->guid, NULL, - &p2, &next, + &exact, &next, schema_attr->syntax->ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - - if (!p2) { + if (exact == NULL) { struct GUID_txt_buf buf; - ldb_asprintf_errstring(ldb, "Attribute %s doesn't exist for target GUID %s", - el->name, GUID_buf_string(&p->guid, &buf)); + 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; @@ -2703,17 +2761,45 @@ static int replmd_modify_la_delete(struct ldb_module *module, return LDB_ERR_NO_SUCH_ATTRIBUTE; } } - rmd_flags = dsdb_dn_rmd_flags(p2->dsdb_dn->dn); + + if (vanish_links) { + if (CHECK_DEBUGLVL(5)) { + rmd_flags = dsdb_dn_rmd_flags(exact->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); + DEBUG(5, ("Deleting deleted linked " + "attribute %s to %s, because " + "vanish_links control is set\n", + el->name, guid_str)); + } + } + + /* remove the backlink */ + ret = replmd_add_backlink(module, + replmd_private, + schema, msg_guid, + &p->guid, + false, schema_attr, + true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + /* We flag the deletion and tidy it up later. */ + exact->v = NULL; + continue; + } + + rmd_flags = dsdb_dn_rmd_flags(exact->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", + ldb_asprintf_errstring(ldb, "Attribute %s already " + "deleted for target GUID %s", el->name, guid_str); if (ldb_attr_cmp(el->name, "member") == 0) { talloc_free(tmp_ctx); @@ -2723,116 +2809,35 @@ static int replmd_modify_la_delete(struct ldb_module *module, return LDB_ERR_NO_SUCH_ATTRIBUTE; } } - } - - if (vanish_links) { - if (num_to_delete == old_el->num_values || num_to_delete == 0) { - el->flags = LDB_FLAG_MOD_REPLACE; - for (i = 0; i < old_el->num_values; i++) { - ret = replmd_add_backlink(module, - replmd_private, - schema, msg_guid, - &old_dns[i].guid, - false, schema_attr, - true); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - } + ret = replmd_update_la_val(old_el->values, exact->v, + exact->dsdb_dn, exact->dsdb_dn, + invocation_id, seq_num, seq_num, + now, 0, true); + if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); - return LDB_SUCCESS; - } else { - unsigned int num_values = 0; - unsigned int j = 0; - struct parsed_dn *exact = NULL, *next = NULL; - - for (i = 0; i < old_el->num_values; i++) { - ret = parsed_dn_find(ldb, dns, num_to_delete, - &old_dns[i].guid, - NULL, - &exact, &next, - schema_attr->syntax->ldap_oid); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - - if (exact != NULL) { - /* The element is in the delete list. - mark it dead. */ - ret = replmd_add_backlink(module, - replmd_private, - 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; + return ret; } - } else { - - /* 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; inum_values; i++) { - struct parsed_dn *p = &old_dns[i]; - struct parsed_dn *exact = NULL, *next = NULL; - uint32_t rmd_flags; - - if (num_to_delete != 0) { - ret = parsed_dn_find(ldb, dns, num_to_delete, - &p->guid, - NULL, - &exact, &next, - schema_attr->syntax->ldap_oid); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - if (exact == NULL) { - continue; - } - } - - rmd_flags = dsdb_dn_rmd_flags(p->dsdb_dn->dn); - if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue; + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, &p->guid, + false, schema_attr, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + } - 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, replmd_private, - schema, msg_guid, &p->guid, - false, schema_attr, true); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; + if (vanish_links) { + unsigned j = 0; + for (i = 0; i < old_el->num_values; i++) { + if (old_dns[i].v != NULL) { + old_el->values[j] = *old_dns[i].v; + j++; } } + old_el->num_values = j; } + el->values = talloc_steal(msg->elements, old_el->values); el->num_values = old_el->num_values; -- 2.7.4 >From 874804121cac80ff8e7b49b9e6d71223d3802bb4 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 17:49:24 +1300 Subject: [PATCH 23/25] replmd linked attributes: use really_parse_trusted_dn everywhere This function fills out the DN and GUID fields of an unparsed parsed_dn struct, which was happening in a few other places already. In some places the GUID was not being filled out, which would probably cause problems if the sorted_links switch was turned on. Signed-off-by: Douglas Bagnall --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 30 +++++++------------------ 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index cb02b38..08ec951 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1888,26 +1888,14 @@ static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, * We assume the second argument refers to a DN is from the database * and has a GUID -- but this GUID might not have been parsed out yet. */ - NTSTATUS status; - - if (GUID_all_zero(&p->guid)) { - - if (p->dsdb_dn == NULL) { - p->dsdb_dn = dsdb_dn_parse_trusted(ctx->mem_ctx, ctx->ldb, p->v, - ctx->ldap_oid); - if (p->dsdb_dn == NULL) { - ctx->err = LDB_ERR_INVALID_DN_SYNTAX; - return 0; - } - } - - status = dsdb_get_extended_dn_guid(p->dsdb_dn->dn, &p->guid, "GUID"); - if (!NT_STATUS_IS_OK(status)) { - ctx->err = LDB_ERR_OPERATIONS_ERROR; + if (p->dsdb_dn == NULL) { + int ret = really_parse_trusted_dn(ctx->mem_ctx, ctx->ldb, p, + ctx->ldap_oid); + if (ret != LDB_SUCCESS) { + ctx->err = ret; return 0; } } - return ndr_guid_compare(ctx->guid, &p->guid); } @@ -1962,16 +1950,14 @@ static int parsed_dn_find(struct ldb_context *ldb, struct parsed_dn *pdn, int cmp; p = &pdn[i]; if (p->dsdb_dn == NULL) { - p->dsdb_dn = dsdb_dn_parse_trusted(pdn, ldb, - p->v, ldap_oid); - if (p->dsdb_dn == NULL) { + int ret = really_parse_trusted_dn(pdn, ldb, p, ldap_oid); + if (ret != LDB_SUCCESS) { return LDB_ERR_OPERATIONS_ERROR; } } + cmp = ldb_dn_compare(p->dsdb_dn->dn, target_dn); if (cmp == 0) { - dsdb_get_extended_dn_guid(p->dsdb_dn->dn, - &p->guid, "GUID"); *exact = p; return LDB_SUCCESS; } -- 2.7.4 >From 1bade61a66ab96f8424fd8e7d3489db8277713c8 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 16:38:03 +1300 Subject: [PATCH 24/25] replmd linked_attributes: maintain sorted links in replace We use a merge-like algorithm, which gives us a slight algorithmic improvement (O(m + n) vs O(m log(n) + n log(m))) and keeps the results sorted. Here's an example. There are existing links to {A C D* F*} where D* and F* represent deleted links, and we want to replace them with {B C E F}. existing: A C D* E F* | | | replacements: B C E F result: A* B C D* E F This is what happens to each link: A gets deleted to A*. B gets added. C is retained, with possible extended DN changes. D* stays in the list as a deleted link E is retained like C F is undeleted. Backlinks are created in the case of B and F The backlink for A is deleted The backlinks are not changed for C and E or D* (D* has none) Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 244 ++++++++++++++---------- 1 file changed, 142 insertions(+), 102 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 08ec951..d1449d1 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2851,32 +2851,61 @@ static int replmd_modify_la_replace(struct ldb_module *module, struct GUID *msg_guid, struct ldb_request *parent) { - unsigned int i; + unsigned int i, old_i, new_i; struct parsed_dn *dns, *old_dns; TALLOC_CTX *tmp_ctx = talloc_new(msg); int ret; const struct GUID *invocation_id; struct ldb_context *ldb = ldb_module_get_ctx(module); struct ldb_val *new_values = NULL; - unsigned int num_new_values = 0; - unsigned int old_num_values = old_el?old_el->num_values:0; + const char *ldap_oid = schema_attr->syntax->ldap_oid; + unsigned int old_num_values; + unsigned int repl_num_values; + unsigned int max_num_values; NTTIME now; unix_to_nt_time(&now, t); - /* check if there is nothing to replace */ - if ((!old_el || old_el->num_values == 0) && - el->num_values == 0) { + /* + * The replace operation is unlike the replace and delete cases in that + * we need to look at every existing link to see whether it is being + * retained or deleted. In other words, we can't avoid parsing the GUIDs. + * + * As we are trying to combine two sorted lists, the algorithm we use + * is akin to the merge phase of a merge sort. We interleave the two + * lists, doing different things depending on which side the current + * item came from. + * + * There are three main cases, with some sub-cases. + * + * - a DN is in the old list but not the new one. It needs to be + * marked as deleted (but left in the list). + * - maybe it is already deleted, and we have less to do. + * + * - a DN is in both lists. The old data gets replaced by the new, + * and the list doesn't grow. The old link may have been marked as + * deleted, in which case we undelete it. + * + * - a DN is in the new list only. We add it in the right place. + */ + + old_num_values = old_el ? old_el->num_values : 0; + repl_num_values = el->num_values; + max_num_values = old_num_values + repl_num_values; + + if (max_num_values == 0) { + /* There is nothing to do! */ return LDB_SUCCESS; } - ret = get_parsed_dns(module, tmp_ctx, el, &dns, schema_attr->syntax->ldap_oid, parent); + ret = get_parsed_dns(module, tmp_ctx, el, &dns, ldap_oid, parent); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - ret = get_parsed_dns(module, tmp_ctx, old_el, &old_dns, schema_attr->syntax->ldap_oid, parent); + ret = get_parsed_dns(module, tmp_ctx, old_el, &old_dns, + ldap_oid, parent); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2888,128 +2917,139 @@ static int replmd_modify_la_replace(struct ldb_module *module, } ret = replmd_check_upgrade_links(ldb, old_dns, old_num_values, - old_el, invocation_id, - schema_attr->syntax->ldap_oid); + old_el, invocation_id, ldap_oid); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - /* mark all the old ones as deleted */ - for (i=0; idsdb_dn->dn); - - if (rmd_flags & DSDB_RMD_FLAG_DELETED) continue; + new_values = talloc_array(tmp_ctx, struct ldb_val, max_num_values); + if (new_values == NULL) { + ldb_module_oom(module); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } - ret = replmd_add_backlink(module, replmd_private, - schema, msg_guid, &old_dns[i].guid, - false, schema_attr, false); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; + old_i = 0; + new_i = 0; + for (i = 0; i < max_num_values; i++) { + int cmp; + struct parsed_dn *old_p, *new_p; + if (old_i < old_num_values && new_i < repl_num_values) { + old_p = &old_dns[old_i]; + new_p = &dns[new_i]; + cmp = parsed_dn_compare(old_p, new_p); + } else if (old_i < old_num_values) { + /* the new list is empty, read the old list */ + old_p = &old_dns[old_i]; + new_p = NULL; + cmp = -1; + } else if (new_i < repl_num_values) { + /* the old list is empty, read new list */ + old_p = NULL; + new_p = &dns[new_i]; + cmp = 1; + } else { + break; } - ret = parsed_dn_find(ldb, dns, el->num_values, - &old_p->guid, - NULL, - &exact, &next, - schema_attr->syntax->ldap_oid); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - if (exact) { - /* we don't delete it if we are re-adding it */ - continue; - } + if (cmp < 0) { + /* + * An old ones that come before the next replacement + * (if any). We mark it as deleted and add it to the + * final list. + */ + uint32_t rmd_flags = dsdb_dn_rmd_flags(old_p->dsdb_dn->dn); + if ((rmd_flags & DSDB_RMD_FLAG_DELETED) == 0) { + ret = replmd_update_la_val(new_values, old_p->v, + old_p->dsdb_dn, + old_p->dsdb_dn, + invocation_id, + seq_num, seq_num, + now, 0, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } - ret = replmd_update_la_val(old_el->values, old_p->v, old_p->dsdb_dn, old_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, replmd_private, + schema, msg_guid, + &old_p->guid, false, + schema_attr, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + } + new_values[i] = *old_p->v; + old_i++; + } else if (cmp == 0) { + /* + * We are overwriting one. + * If it is deleted, we need to add a backlink. + */ + uint32_t rmd_flags; - /* for each new value, either update its meta-data, or add it - * to old_el - */ - for (i=0; inum_values; i++) { - struct parsed_dn *p = &dns[i]; - struct parsed_dn *old_p = NULL, *next = NULL; - - if (old_dns) { - ret = parsed_dn_find(ldb, old_dns, old_num_values, - &p->guid, - NULL, - &old_p, &next, - schema_attr->syntax->ldap_oid); + ret = replmd_update_la_val(new_values, old_p->v, + new_p->dsdb_dn, + old_p->dsdb_dn, + invocation_id, + seq_num, seq_num, + now, 0, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - } - if (old_p != NULL) { - /* update in place */ - ret = replmd_update_la_val(old_el->values, old_p->v, p->dsdb_dn, - old_p->dsdb_dn, invocation_id, - seq_num, seq_num, now, 0, false); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; + rmd_flags = dsdb_dn_rmd_flags(old_p->dsdb_dn->dn); + if ((rmd_flags & DSDB_RMD_FLAG_DELETED) != 0) { + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, + &new_p->guid, true, + schema_attr, true); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } } + + new_values[i] = *old_p->v; + old_i++; + new_i++; } else { - /* add a new one */ - new_values = talloc_realloc(tmp_ctx, new_values, struct ldb_val, - num_new_values+1); - if (new_values == NULL) { - ldb_module_oom(module); + /* + * Replacements that don't match an existing one. We + * just add them to the final list. + */ + ret = replmd_build_la_val(new_values, + new_p->v, + new_p->dsdb_dn, + invocation_id, + seq_num, seq_num, + now, 0, false); + if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); - return LDB_ERR_OPERATIONS_ERROR; + return ret; } - ret = replmd_build_la_val(new_values, &new_values[num_new_values], dns[i].dsdb_dn, - invocation_id, seq_num, seq_num, now, 0, false); + ret = replmd_add_backlink(module, replmd_private, + schema, msg_guid, + &new_p->guid, true, + schema_attr, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } - num_new_values++; - } - - ret = replmd_add_backlink(module, replmd_private, - schema, msg_guid, &dns[i].guid, - true, schema_attr, false); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; + new_values[i] = *new_p->v; + new_i++; } } - - /* add the new values to the end of old_el */ - if (num_new_values != 0) { - el->values = talloc_realloc(msg->elements, old_el?old_el->values:NULL, - struct ldb_val, old_num_values+num_new_values); - if (el->values == NULL) { - ldb_module_oom(module); - return LDB_ERR_OPERATIONS_ERROR; - } - memcpy(&el->values[old_num_values], &new_values[0], - sizeof(struct ldb_val)*num_new_values); - el->num_values = old_num_values + num_new_values; - talloc_steal(msg->elements, new_values); - } else { - el->values = old_el->values; - el->num_values = old_el->num_values; - talloc_steal(msg->elements, el->values); + if (old_el != NULL) { + talloc_steal(msg->elements, old_el->values); } - + el->values = talloc_steal(msg->elements, new_values); + el->num_values = i; talloc_free(tmp_ctx); - /* we now tell the backend to replace all existing values - with the one we have constructed */ el->flags = LDB_FLAG_MOD_REPLACE; return LDB_SUCCESS; -- 2.7.4 >From 7f7e4abbbcda86a3cdc58fef98b6379064b70424 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 11 Jan 2017 09:24:06 +1300 Subject: [PATCH 25/25] Switch on the sortedLinks Flag on new databases Signed-off-by: Andrew Bartlett --- source4/setup/provision_init.ldif | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/setup/provision_init.ldif b/source4/setup/provision_init.ldif index 3d14578..486458a 100644 --- a/source4/setup/provision_init.ldif +++ b/source4/setup/provision_init.ldif @@ -27,6 +27,7 @@ disallowDNFilter: TRUE dn: @SAMBA_DSDB backendType: ${BACKEND_TYPE} serverRole: ${SERVER_ROLE} +compatibleFeatures: sortedLinks dn: @MODULES @LIST: samba_dsdb -- 2.7.4