>From fcd6922f28d026681ab4f40d580999f14847aa72 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 12:26:13 +1300 Subject: [PATCH 01/30] 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 bcc17586c9a451491f847488eab2d781e2c0d74f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 12 Jan 2017 11:53:15 +1300 Subject: [PATCH 02/30] 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 c42922e676d62dcfa7404f77c7765c34114c0ee2 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 12 Jan 2017 11:57:17 +1300 Subject: [PATCH 03/30] 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 2cbc7998fc40899ffc92ed6558167fa35edfc575 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 1 Feb 2017 14:19:36 +1300 Subject: [PATCH 04/30] s4/linked_attribute tests: test with the relax control We had a theory this caused problems. It didn't, but the tests are still worthwhile. Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 81 ++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index 4f65df9..a604619 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -95,23 +95,26 @@ class LATests(samba.tests.TestCase): objectclass)) return dns - def add_linked_attribute(self, src, dest, attr='member'): + def add_linked_attribute(self, src, dest, attr='member', + controls=None): m = ldb.Message() m.dn = ldb.Dn(self.samdb, src) m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_ADD, attr) - self.samdb.modify(m) + self.samdb.modify(m, controls=controls) - def remove_linked_attribute(self, src, dest, attr='member'): + def remove_linked_attribute(self, src, dest, attr='member', + controls=None): m = ldb.Message() m.dn = ldb.Dn(self.samdb, src) m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_DELETE, attr) - self.samdb.modify(m) + self.samdb.modify(m, controls=controls) - def replace_linked_attribute(self, src, dest, attr='member'): + def replace_linked_attribute(self, src, dest, attr='member', + controls=None): m = ldb.Message() m.dn = ldb.Dn(self.samdb, src) m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_REPLACE, attr) - self.samdb.modify(m) + self.samdb.modify(m, controls=controls) def attr_search(self, obj, expected, attr, scope=ldb.SCOPE_BASE, **controls): @@ -503,6 +506,72 @@ class LATests(samba.tests.TestCase): for u in users: self.assert_back_links(u, []) + def test_la_links_relaxed(self): + """Check that the relax control doesn't mess with linked attributes.""" + relax_control = ['relax:0'] + + users = self.add_objects(10, 'user', 'u_relax') + groups = self.add_objects(3, 'group', 'g_relax') + g_relax1, g_relax2, g_uptight = groups + + # g_relax1 has all users added at once + # g_relax2 gets them one at a time in reverse order + # g_uptight never relaxes + + self.add_linked_attribute(g_relax1, users[:5], controls=relax_control) + + for u in reversed(users[:5]): + self.add_linked_attribute(g_relax2, u, controls=relax_control) + self.add_linked_attribute(g_uptight, u) + + for g in groups: + self.assert_forward_links(g, users[:5]) + + self.add_linked_attribute(g, users[5:7]) + self.assert_forward_links(g, users[:7]) + + for u in users[7:]: + self.add_linked_attribute(g, u) + + self.assert_forward_links(g, users) + + for u in users: + self.assert_back_links(u, groups) + + # try some replacement permutations + import random + random.seed(1) + users2 = users[:] + for i in range(5): + random.shuffle(users2) + self.replace_linked_attribute(g_relax1, users2, + controls=relax_control) + + self.assert_forward_links(g_relax1, users) + + for i in range(5): + random.shuffle(users2) + self.remove_linked_attribute(g_relax2, users2, + controls=relax_control) + self.remove_linked_attribute(g_uptight, users2) + + self.replace_linked_attribute(g_relax1, [], controls=relax_control) + + random.shuffle(users2) + self.add_linked_attribute(g_relax2, users2, + controls=relax_control) + self.add_linked_attribute(g_uptight, users2) + self.replace_linked_attribute(g_relax1, users2, + controls=relax_control) + + self.assert_forward_links(g_relax1, users) + self.assert_forward_links(g_relax2, users) + self.assert_forward_links(g_uptight, users) + + for u in users: + self.assert_back_links(u, groups) + + def test_one_way_attributes(self): e1, e2 = self.add_objects(2, 'msExchConfigurationContainer', 'e_one_way') -- 2.7.4 >From 47bfe8ab5d189fd070221e77c4e8488d46fd5134 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 2 Feb 2017 13:57:16 +1300 Subject: [PATCH 05/30] s4/linked_attribute tests: try adding linked attributes directly Previously we have only added linked attributes using a modify. Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett --- source4/dsdb/tests/python/linked_attributes.py | 64 ++++++++++++++++++++------ 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index a604619..305ff63 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -74,25 +74,24 @@ class LATests(samba.tests.TestCase): if not opts.no_cleanup: self.samdb.delete(self.ou, ['tree_delete:1']) - def delete_user(self, user): - self.samdb.delete(user['dn']) - del self.users[self.users.index(user)] - - def add_object(self, cn, objectclass): + def add_object(self, cn, objectclass, more_attrs={}): dn = "CN=%s,%s" % (cn, self.ou) - self.samdb.add({'cn': cn, - 'objectclass': objectclass, - 'dn': dn}) + attrs = {'cn': cn, + 'objectclass': objectclass, + 'dn': dn} + attrs.update(more_attrs) + self.samdb.add(attrs) return dn - def add_objects(self, n, objectclass, prefix=None): + def add_objects(self, n, objectclass, prefix=None, more_attrs={}): if prefix is None: prefix = objectclass dns = [] for i in range(n): dns.append(self.add_object("%s%d" % (prefix, i + 1), - objectclass)) + objectclass, + more_attrs=more_attrs)) return dns def add_linked_attribute(self, src, dest, attr='member', @@ -511,16 +510,17 @@ class LATests(samba.tests.TestCase): relax_control = ['relax:0'] users = self.add_objects(10, 'user', 'u_relax') - groups = self.add_objects(3, 'group', 'g_relax') + groups = self.add_objects(3, 'group', 'g_relax', + more_attrs={'member': users[:2]}) g_relax1, g_relax2, g_uptight = groups # g_relax1 has all users added at once # g_relax2 gets them one at a time in reverse order # g_uptight never relaxes - self.add_linked_attribute(g_relax1, users[:5], controls=relax_control) + self.add_linked_attribute(g_relax1, users[2:5], controls=relax_control) - for u in reversed(users[:5]): + for u in reversed(users[2:5]): self.add_linked_attribute(g_relax2, u, controls=relax_control) self.add_linked_attribute(g_uptight, u) @@ -571,6 +571,44 @@ class LATests(samba.tests.TestCase): for u in users: self.assert_back_links(u, groups) + def test_add_all_at_once(self): + """All these other tests are creating linked attributes after the + objects are there. We want to test creating them all at once + using LDIF. + """ + users = self.add_objects(7, 'user', 'u_all_at_once') + g1, g3 = self.add_objects(2, 'group', 'g_all_at_once', + more_attrs={'member': users}) + (g2,) = self.add_objects(1, 'group', 'g_all_at_once2', + more_attrs={'member': users[:5]}) + + self.assert_forward_links(g1, users) + self.assert_forward_links(g2, users[:5]) + self.assert_forward_links(g3, users) + for u in users[:5]: + self.assert_back_links(u, [g1, g2, g3]) + for u in users[5:]: + self.assert_back_links(u, [g1, g3]) + + self.remove_linked_attribute(g2, users[0]) + self.remove_linked_attribute(g2, users[1]) + self.add_linked_attribute(g2, users[1]) + self.add_linked_attribute(g2, users[5]) + self.add_linked_attribute(g2, users[6]) + + self.assert_forward_links(g1, users) + self.assert_forward_links(g2, users[1:]) + + for u in users[1:]: + self.remove_linked_attribute(g2, u) + self.remove_linked_attribute(g1, users) + + for u in users: + self.samdb.delete(u) + + self.assert_forward_links(g1, []) + self.assert_forward_links(g2, []) + self.assert_forward_links(g3, []) def test_one_way_attributes(self): e1, e2 = self.add_objects(2, 'msExchConfigurationContainer', -- 2.7.4 >From a224441db97855d312330299ec4d1b45516d9af0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 1 Feb 2017 14:21:22 +1300 Subject: [PATCH 06/30] s4/linked_attribute tests: remove helper function unused parameter Signed-off-by: Douglas Bagnall --- source4/dsdb/tests/python/linked_attributes.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/tests/python/linked_attributes.py b/source4/dsdb/tests/python/linked_attributes.py index 305ff63..1833600 100644 --- a/source4/dsdb/tests/python/linked_attributes.py +++ b/source4/dsdb/tests/python/linked_attributes.py @@ -115,8 +115,7 @@ class LATests(samba.tests.TestCase): m[attr] = ldb.MessageElement(dest, ldb.FLAG_MOD_REPLACE, attr) self.samdb.modify(m, controls=controls) - def attr_search(self, obj, expected, attr, scope=ldb.SCOPE_BASE, - **controls): + def attr_search(self, obj, attr, scope=ldb.SCOPE_BASE, **controls): if opts.no_reveal_internals: if 'reveal_internals' in controls: del controls['reveal_internals'] @@ -130,7 +129,7 @@ class LATests(samba.tests.TestCase): return res def assert_links(self, obj, expected, attr, msg='', **kwargs): - res = self.attr_search(obj, expected, attr, **kwargs) + res = self.attr_search(obj, attr, **kwargs) if len(expected) == 0: if attr in res[0]: -- 2.7.4 >From bed933097a8c80778e9f1ff4e82d88650e66b6c2 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 4 Jan 2017 21:27:58 +1300 Subject: [PATCH 07/30] 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 b01b87e953f6dad08b8fcda88de44c9e9747abb9 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 2 Dec 2016 15:32:59 +1300 Subject: [PATCH 08/30] 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 4bf94104d3d9ce8c78c16429094161b6e59009a0 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 12 Jan 2017 16:51:45 +1300 Subject: [PATCH 09/30] 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 95fba6029c8f2e3cad2e184e76785f8ff3c296e0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 17 Dec 2016 22:04:59 +1300 Subject: [PATCH 10/30] 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 961fc428d2320e6c7866cd94ff0d4bdc660f3b5a Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 23 Dec 2016 14:18:13 +1300 Subject: [PATCH 11/30] 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 cc4e994ef7595134f3c7ee291d802e8fe7364656 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 13:17:25 +1300 Subject: [PATCH 12/30] 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 2eafe38c6ba8adeaa2d4fa2e90f45891a764ca12 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 15:08:00 +1300 Subject: [PATCH 13/30] 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 bf8ab4b65e0da880760c58e7033eeb0d7123757c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 16:15:42 +1300 Subject: [PATCH 14/30] 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 f7dd39489c738da9042c54641aec7796b34c27e9 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 15 Dec 2016 14:39:33 +1300 Subject: [PATCH 15/30] 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 abe5423469c8fede5fa6b3562dbe0c5553cc0611 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 22 Dec 2016 16:09:22 +1300 Subject: [PATCH 16/30] 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 44d764235b85a9ce9607b7f544bc6c2c7c53ac1f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 12:12:23 +1300 Subject: [PATCH 17/30] 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 ce8a616371d1007f32875c6dc74248651c0ff3cd Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 15:09:15 +1300 Subject: [PATCH 18/30] 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 06efa3065a780d90bf33a74b3e960ce479b5a74b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 16:24:23 +1300 Subject: [PATCH 19/30] 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 82fa19d530def9babb0cb69f4b1e77667c23b042 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 13:57:21 +1300 Subject: [PATCH 20/30] 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 f4880d9f092819c90b58f43c5d3cdccf502317f9 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 29 Dec 2016 13:20:41 +1300 Subject: [PATCH 21/30] 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 eeb9816b0764925a99debf6dedf0a584cd5a8da6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 4 Jan 2017 14:09:00 +1300 Subject: [PATCH 22/30] 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 6d589b6a437ec943851555bb1bb10ba89cc2a366 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 09:49:38 +1300 Subject: [PATCH 23/30] 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 54cde3f1a16cb5399c30057e8f738e5c117410fa Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 11:00:57 +1300 Subject: [PATCH 24/30] 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 69250cfe6f96779d4bb3f29c8ee23b2141ea0f4d Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 11 Jan 2017 17:49:24 +1300 Subject: [PATCH 25/30] 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 af4f81714344d01281246578828ab4d1ffa4054c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 6 Jan 2017 16:38:03 +1300 Subject: [PATCH 26/30] 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 | 247 ++++++++++++++---------- 1 file changed, 145 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..75707d8 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,142 @@ 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 was previously + * deleted, we need to add a backlink. + * + * Note that if any RMD_FLAGs in an extended new DN + * will be ignored. + */ + 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 e3f064632cddc8f07a2850a814e15c160fbe037d Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 27 Jan 2017 17:46:22 +1300 Subject: [PATCH 27/30] replmd: keep links sorted in replmd_process_linked_attribute This is where linked attributes get added during a replication. Signed-off-by: Douglas Bagnall Pair-programmed-with: Garming Sam --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 33 +++++++++++++++++++++++-- 1 file changed, 31 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 75707d8..02063a6 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -6667,7 +6667,9 @@ linked_attributes[0]: } /* parse the existing links */ - ret = get_parsed_dns(module, tmp_ctx, old_el, &pdn_list, attr->syntax->ldap_oid, parent); + ret = get_parsed_dns_trusted(module, replmd_private, tmp_ctx, old_el, &pdn_list, + attr->syntax->ldap_oid, parent); + if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -6858,12 +6860,33 @@ linked_attributes[0]: } } } else { + unsigned offset; /* get a seq_num for this change */ ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } + /* + * We know where the new one needs to be, from the *next + * pointer into pdn_list. + */ + if (next == NULL) { + offset = old_el->num_values; + } else { + if (next->dsdb_dn == NULL) { + ret = really_parse_trusted_dn(tmp_ctx, ldb, next, + attr->syntax->ldap_oid); + if (ret != LDB_SUCCESS) { + return ret; + } + } + offset = next - pdn_list; + if (offset > old_el->num_values) { + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } + } old_el->values = talloc_realloc(msg->elements, old_el->values, struct ldb_val, old_el->num_values+1); @@ -6871,9 +6894,15 @@ linked_attributes[0]: ldb_module_oom(module); return LDB_ERR_OPERATIONS_ERROR; } + + if (offset != old_el->num_values) { + memmove(&old_el->values[offset + 1], &old_el->values[offset], + (old_el->num_values - offset) * sizeof(old_el->values[0])); + } + old_el->num_values++; - ret = replmd_build_la_val(tmp_ctx, &old_el->values[old_el->num_values-1], dsdb_dn, + ret = replmd_build_la_val(tmp_ctx, &old_el->values[offset], dsdb_dn, &la->meta_data.originating_invocation_id, la->meta_data.originating_usn, seq_num, la->meta_data.originating_change_time, -- 2.7.4 >From 33f40075311a1d38bf46977be9caca3f1623eaac Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 2 Feb 2017 16:37:58 +1300 Subject: [PATCH 28/30] replmd: treat a zero GUID as not present in get_parsed_dns This roughly follows the pattern in the 2009 commit 0d5d7f58473c989bff4 by the Andrews Tridgell and Bartlett, which dealt with zero GUIDs in replmd_add_fix_la(). That function is about to use get_parsed_dns() [see next commit], and the other users of get_parsed_dns don't really want to see zero guids, so it is simpler to test here. This makes hitting the GUID_all_zero branch of parsed_dn_find() even more unlikely. Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 02063a6..a5fa420 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2033,7 +2033,8 @@ static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx, dn = p->dsdb_dn->dn; status = dsdb_get_extended_dn_guid(dn, &p->guid, "GUID"); - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) || + unlikely(GUID_all_zero(&p->guid))) { /* we got a DN without a GUID - go find the GUID */ int ret = dsdb_module_guid_by_dn(module, dn, &p->guid, parent); if (ret != LDB_SUCCESS) { -- 2.7.4 >From 12c421bc90140a9c6251360baf96c679906d6aeb Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 1 Feb 2017 17:34:51 +1300 Subject: [PATCH 29/30] repl_md: get links in sorted order in replmd_add_fix_la This is where forward links get added when they get added with an object. Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 77 ++++++++++++------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index a5fa420..b2108ca 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -862,68 +862,72 @@ static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct ds const struct GUID *invocation_id, uint64_t seq_num, uint64_t local_usn, NTTIME nttime, uint32_t version, bool deleted); +struct parsed_dn { + struct dsdb_dn *dsdb_dn; + struct GUID guid; + struct ldb_val *v; +}; + +static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx, + struct ldb_message_element *el, struct parsed_dn **pdn, + const char *ldap_oid, struct ldb_request *parent); /* fix up linked attributes in replmd_add. 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, +static int replmd_add_fix_la(struct ldb_module *module, TALLOC_CTX *mem_ctx, 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) + struct GUID *guid, const struct dsdb_attribute *sa, + struct ldb_request *parent) { unsigned int i; - TALLOC_CTX *tmp_ctx = talloc_new(el->values); + TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx); struct ldb_context *ldb = ldb_module_get_ctx(module); - + struct parsed_dn *pdn; /* We will take a reference to the schema in replmd_add_backlink */ const struct dsdb_schema *schema = dsdb_get_schema(ldb, NULL); + struct ldb_val *new_values = NULL; - for (i=0; inum_values; i++) { - struct ldb_val *v = &el->values[i]; - struct dsdb_dn *dsdb_dn = dsdb_dn_parse(tmp_ctx, ldb, v, sa->syntax->ldap_oid); - struct GUID target_guid; - NTSTATUS status; - int ret; - - if (dsdb_dn == NULL) { - talloc_free(tmp_ctx); - return LDB_ERR_INVALID_DN_SYNTAX; - } + int ret = get_parsed_dns(module, tmp_ctx, el, &pdn, + sa->syntax->ldap_oid, parent); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } - /* note that the DN already has the extended - components from the extended_dn_store module */ - status = dsdb_get_extended_dn_guid(dsdb_dn->dn, &target_guid, "GUID"); - if (!NT_STATUS_IS_OK(status) || GUID_all_zero(&target_guid)) { - ret = dsdb_module_guid_by_dn(module, dsdb_dn->dn, &target_guid, parent); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - ret = dsdb_set_extended_dn_guid(dsdb_dn->dn, &target_guid, "GUID"); - if (ret != LDB_SUCCESS) { - talloc_free(tmp_ctx); - return ret; - } - } + new_values = talloc_array(tmp_ctx, struct ldb_val, el->num_values); + if (new_values == NULL) { + ldb_module_oom(module); + talloc_free(tmp_ctx); + return LDB_ERR_OPERATIONS_ERROR; + } - ret = replmd_build_la_val(el->values, v, dsdb_dn, invocationId, + for (i = 0; i < el->num_values; i++) { + struct parsed_dn *p = &pdn[i]; + ret = replmd_build_la_val(el->values, p->v, p->dsdb_dn, + invocationId, seq_num, seq_num, now, 0, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } + /* This is the only place we are doing deferred back-links */ ret = replmd_add_backlink(module, replmd_private, - schema, guid, &target_guid, true, sa, + schema, guid, &p->guid, true, sa, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } + + new_values[i] = *p->v; } + el->values = talloc_steal(mem_ctx, new_values); talloc_free(tmp_ctx); return LDB_SUCCESS; @@ -1100,7 +1104,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, replmd_private, e, + ret = replmd_add_fix_la(module, msg->elements, + replmd_private, e, ac->seq_num, &ac->our_invocation_id, now, &guid, sa, req); @@ -1810,12 +1815,6 @@ static int replmd_update_rpmd(struct ldb_module *module, return LDB_SUCCESS; } -struct parsed_dn { - struct dsdb_dn *dsdb_dn; - struct GUID guid; - struct ldb_val *v; -}; - struct compare_ctx { struct GUID *guid; struct ldb_context *ldb; -- 2.7.4 >From 2c6807db9ac65f8f31825bbf0c3ea923d393bced Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 11 Jan 2017 09:24:06 +1300 Subject: [PATCH 30/30] 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