[PATCH] Moving RID manager causes replication issues
Clive Ferreira
cliveferreira at catalyst.net.nz
Fri Oct 28 02:04:55 UTC 2016
Hi,
This patch relaxes the objectclass_attrs check which requires that all
mandatory attributes are present. If the attribute is mandatory, but is
not replicated (which is limited to only two attributes: rIDNextRid and
rIDPreviousAllocationPool) then we should let the modification proceed.
This change is required for moving the RID manager role and attempting
to allocate a new RID pool from it. Although this check could be passed
by actually setting the value consistently, it doesn't resolve existing
databases and the edge case is only for two attributes (used for RID
allocation). Furthermore, it removes the special case for requiring a
dbcheck control to avoid errors.
The new test passes against Windows, but errors out at the cleanup stage
where we move back the RID master role to the original DC (which might
be due to a time-delay in transfering roles).
The second patch was missing a sign-off by Bob, which he can hopefully
approve here.
Any thoughts would be appreciated.
Cheers,
Garming + Clive
-------------- next part --------------
From 520f149de670c0307a4c6fb376c83732acb1a4a4 Mon Sep 17 00:00:00 2001
From: Clive Ferreira <cliveferreira at catalyst.net.nz>
Date: Tue, 11 Oct 2016 15:33:06 +1300
Subject: [PATCH 1/5] objectclass_attrs: correctly indent a comment
Signed-off-by: Clive Ferreira <cliveferreira at catalyst.net.nz>
Pair-programmed-by: Garming Sam <garming at catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12394
---
source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
index c83c2e9..616cff8 100644
--- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
+++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
@@ -435,12 +435,13 @@ static int attr_handler2(struct oc_context *ac)
}
if (isSchemaAttr) {
- /* Before really adding an attribute in the database,
- * let's check that we can translate it into a dbsd_attribute and
- * that we can find a valid syntax object.
- * If not it's better to reject this attribute than not be able
- * to start samba next time due to schema being unloadable.
- */
+ /*
+ * Before really adding an attribute in the database,
+ * let's check that we can translate it into a dsdb_attribute and
+ * that we can find a valid syntax object.
+ * If not it's better to reject this attribute than not be able
+ * to start samba next time due to schema being unloadable.
+ */
struct dsdb_attribute *att = talloc(ac, struct dsdb_attribute);
const struct dsdb_syntax *attrSyntax;
WERROR status;
--
2.7.4
From 2a0961bdde03c329e2e116380357851b860ce05c Mon Sep 17 00:00:00 2001
From: Bob Campbell <bobcampbell at catalyst.net.nz>
Date: Mon, 10 Oct 2016 16:58:57 +1300
Subject: [PATCH 2/5] tests/getnc_exop: Improve the ridalloc test by performing
an alloc against a new master
Currently we fail against ourselves due to rIDNextRid and
rIDPreviousAllocationPool normally being unset, despite being mandatory
attributes (being the only attributes in this situation).
Pair-programmed-with: Garming Sam <garming at catalyst.net.nz>
Pair-programmed-with: Clive Ferreira <cliveferreira at catalyst.net.nz>
Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12394
---
selftest/knownfail | 1 +
source4/torture/drs/python/getnc_exop.py | 76 ++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/selftest/knownfail b/selftest/knownfail
index 976761b..2db5715 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -294,3 +294,4 @@
#ntvfs server blocks copychunk with execute access on read handle
^samba4.smb2.ioctl.copy_chunk_bad_access
^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
+^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaSyncTestCase.test_edit_rid_master.*
diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
index d058e66..94d1402 100644
--- a/source4/torture/drs/python/getnc_exop.py
+++ b/source4/torture/drs/python/getnc_exop.py
@@ -289,6 +289,82 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase, ExopBaseTest):
# We don't check the linked_attributes_count as if the domain
# has an RODC, it can gain links on the server account object
+ def test_edit_rid_master(self):
+ """Test doing a RID allocation after changing the RID master from the original one.
+ This should set rIDNextRID to 0 on the new RID master."""
+ # 1. a. Transfer role to non-RID master
+ # b. Check that it succeeds correctly
+ #
+ # 2. a. Call the RID alloc against the former master.
+ # b. Check that it succeeds.
+ fsmo_dn = ldb.Dn(self.ldb_dc1, "CN=RID Manager$,CN=System," + self.ldb_dc1.domain_dn())
+ (fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn)
+
+ # 1. Swap RID master role
+ m = ldb.Message()
+ m.dn = ldb.Dn(self.ldb_dc1, "")
+ m["becomeRidMaster"] = ldb.MessageElement("1", ldb.FLAG_MOD_REPLACE,
+ "becomeRidMaster")
+
+ # Make sure that ldb_dc1 == RID Master
+
+ server_dn = str(ldb.Dn(self.ldb_dc1, self.ldb_dc1.get_dsServiceName()).parent())
+
+ # self.ldb_dc1 == LOCALDC
+ if server_dn == fsmo_owner['server_dn']:
+ # ldb_dc1 == VAMPIREDC
+ ldb_dc1, ldb_dc2 = self.ldb_dc2, self.ldb_dc1
+ else:
+ # Otherwise switch the two
+ ldb_dc1, ldb_dc2 = self.ldb_dc1, self.ldb_dc2
+
+ try:
+ # ldb_dc1 is now RID MASTER (as VAMPIREDC)
+ ldb_dc1.modify(m)
+ except ldb.LdbError, (num, msg):
+ self.fail("Failed to reassign RID Master " + msg)
+
+ try:
+ # 2. Perform a RID alloc
+ req8 = self._exop_req8(dest_dsa=fsmo_owner["ntds_guid"],
+ invocation_id=fsmo_not_owner["invocation_id"],
+ nc_dn_str=fsmo_dn,
+ exop=drsuapi.DRSUAPI_EXOP_FSMO_RID_ALLOC)
+
+ (drs, drs_handle) = self._ds_bind(fsmo_not_owner["dns_name"])
+ # 3. Make sure the allocation succeeds
+ try:
+ (level, ctr) = drs.DsGetNCChanges(drs_handle, 8, req8)
+ except RuntimeError, e:
+ self.fail("RID allocation failed: " + str(e))
+
+ fsmo_dn = ldb.Dn(self.ldb_dc1, "CN=RID Manager$,CN=System," + self.ldb_dc1.domain_dn())
+
+ self.assertEqual(level, 6, "Expected level 6 response!")
+ self.assertEqual(ctr.source_dsa_guid, misc.GUID(fsmo_not_owner["ntds_guid"]))
+ self.assertEqual(ctr.source_dsa_invocation_id, misc.GUID(fsmo_not_owner["invocation_id"]))
+ ctr6 = ctr
+ self.assertEqual(ctr6.extended_ret, drsuapi.DRSUAPI_EXOP_ERR_SUCCESS)
+ self.assertEqual(ctr6.object_count, 3)
+ self.assertNotEqual(ctr6.first_object, None)
+ self.assertEqual(ldb.Dn(ldb_dc2, ctr6.first_object.object.identifier.dn), fsmo_dn)
+ self.assertNotEqual(ctr6.first_object.next_object, None)
+ self.assertNotEqual(ctr6.first_object.next_object.next_object, None)
+ second_object = ctr6.first_object.next_object.object
+ self.assertEqual(ldb.Dn(self.ldb_dc1, second_object.identifier.dn), fsmo_owner["rid_set_dn"])
+ third_object = ctr6.first_object.next_object.next_object.object
+ self.assertEqual(ldb.Dn(self.ldb_dc1, third_object.identifier.dn), fsmo_owner["server_acct_dn"])
+ finally:
+ # Swap the RID master back for other tests
+ m = ldb.Message()
+ m.dn = ldb.Dn(ldb_dc2, "")
+ m["becomeRidMaster"] = ldb.MessageElement("1", ldb.FLAG_MOD_REPLACE, "becomeRidMaster")
+ try:
+ ldb_dc2.modify(m)
+ except ldb.LdbError, (num, msg):
+ self.fail("Failed to restore RID Master " + msg)
+
+
class DrsReplicaPrefixMapTestCase(drs_base.DrsBaseTestCase, ExopBaseTest):
def setUp(self):
super(DrsReplicaPrefixMapTestCase, self).setUp()
--
2.7.4
From 852f606ce734f35af61c73b1d99f308300185ada Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 20 Oct 2016 16:19:43 +1300
Subject: [PATCH 3/5] tests/getnc_exop: Finish a comment in getnc_exop.py
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12394
---
source4/torture/drs/python/getnc_exop.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/torture/drs/python/getnc_exop.py b/source4/torture/drs/python/getnc_exop.py
index 94d1402..941d323 100644
--- a/source4/torture/drs/python/getnc_exop.py
+++ b/source4/torture/drs/python/getnc_exop.py
@@ -256,7 +256,7 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase, ExopBaseTest):
# has an RODC, it can gain links on the server account object
def test_do_ridalloc_get_anc(self):
- """Test doing a RID allocation with a valid destination DSA guid and """
+ """Test doing a RID allocation with a valid destination DSA guid and GET_ANC flag"""
fsmo_dn = ldb.Dn(self.ldb_dc1, "CN=RID Manager$,CN=System," + self.ldb_dc1.domain_dn())
(fsmo_owner, fsmo_not_owner) = self._determine_fSMORoleOwner(fsmo_dn)
--
2.7.4
From dd3ee7f8407174c1b8058d1a5d0dd55ae5a2774b Mon Sep 17 00:00:00 2001
From: Clive Ferreira <cliveferreira at catalyst.net.nz>
Date: Thu, 20 Oct 2016 16:20:49 +1300
Subject: [PATCH 4/5] typo: supprise -> surprise
Signed-off-by: Clive Ferreira <cliveferreira at catalyst.net.nz>
Pair-programmed-with: Garming Sam <garming at catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12394
---
source4/dsdb/samdb/ldb_modules/rootdse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c
index 6a1b8ef..86ca89f 100644
--- a/source4/dsdb/samdb/ldb_modules/rootdse.c
+++ b/source4/dsdb/samdb/ldb_modules/rootdse.c
@@ -1515,7 +1515,7 @@ static int rootdse_become_master(struct ldb_module *module,
/*
* We always delete the transaction, not commit it, because
- * this gives the least supprise to this supprising action (as
+ * this gives the least surprise to this surprising action (as
* we will never record anything done to this point
*/
rootdse_del_trans(module);
--
2.7.4
From 0f056d5d5ff0c92046bbb93ee2a432d995ee8db5 Mon Sep 17 00:00:00 2001
From: Clive Ferreira <cliveferreira at catalyst.net.nz>
Date: Tue, 11 Oct 2016 15:32:54 +1300
Subject: [PATCH 5/5] objectclass_attrs: Only abort on a missing attribute when
an attribute is both MUST and replicated
If an attribute is not replicated or constructed, it is quite normal for
it to be missing. This is the case with both rIDNextRid and
rIDPreviousAllocationPool. This currently prevents us switching the RID
master. On Windows, missing this attribute does not cause any problems
for the RID manager.
We may now remove the knownfail entry added earlier.
Signed-off-by: Clive Ferreira <cliveferreira at catalyst.net.nz>
Pair-programmed-with: Garming Sam <garming at catalyst.net.nz>
Pair-programmed-with: Bob Campbell <bobcampbell at catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12394
---
selftest/knownfail | 1 -
source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 23 ++++++++++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/selftest/knownfail b/selftest/knownfail
index 2db5715..976761b 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -294,4 +294,3 @@
#ntvfs server blocks copychunk with execute access on read handle
^samba4.smb2.ioctl.copy_chunk_bad_access
^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
-^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaSyncTestCase.test_edit_rid_master.*
diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
index 616cff8..e239fb9 100644
--- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
+++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
@@ -426,12 +426,23 @@ static int attr_handler2(struct oc_context *ac)
* replicated.
*/
if (found_must_contain[0] != NULL &&
- ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE") == 0 &&
- ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK) == NULL) {
- ldb_asprintf_errstring(ldb, "objectclass_attrs: at least one mandatory attribute ('%s') on entry '%s' wasn't specified!",
- found_must_contain[0],
- ldb_dn_get_linearized(msg->dn));
- return LDB_ERR_OBJECT_CLASS_VIOLATION;
+ ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE") == 0) {
+
+ for (i = 0; found_must_contain[i] != NULL; i++) {
+ const struct dsdb_attribute *broken_attr = dsdb_attribute_by_lDAPDisplayName(ac->schema,
+ found_must_contain[i]);
+
+ bool replicated = (broken_attr->systemFlags &
+ (DS_FLAG_ATTR_NOT_REPLICATED | DS_FLAG_ATTR_IS_CONSTRUCTED)) == 0;
+
+ if (replicated) {
+ ldb_asprintf_errstring(ldb, "objectclass_attrs: at least one mandatory "
+ "attribute ('%s') on entry '%s' wasn't specified!",
+ found_must_contain[i],
+ ldb_dn_get_linearized(msg->dn));
+ return LDB_ERR_OBJECT_CLASS_VIOLATION;
+ }
+ }
}
if (isSchemaAttr) {
--
2.7.4
More information about the samba-technical
mailing list