dbcheck: Add explict tests for unknown and unsorted attributeID values
authorAndrew Bartlett <abartlet@samba.org>
Thu, 23 Jul 2015 04:01:14 +0000 (16:01 +1200)
committerStefan Metzmacher <metze@samba.org>
Mon, 24 Aug 2015 21:46:22 +0000 (23:46 +0200)
Unknown attributeID values would cause an exception previously, and
unsorted attributes cause a failure to replicate with Samba 4.2.

In commit 61b978872fe86906611f64430b2608f5e7ea7ad8 we started
to sort these values correctly, but previous versions of Samba
did not sort them correctly (we sorted high-bit-set values as
negative), and then after 9c9df40220234cba973e84b4985d90da1334a1d1
we stoped accepting these.

To ensure we are allowed to make this unusual change to the
replPropertyMetaData, a new OID is allocated and checked
for in repl_meta_data.c

BUG: https://bugzilla.samba.org/show_bug.cgi?id=10973

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
python/samba/dbchecker.py
python/samba/tests/dsdb.py
source4/dsdb/samdb/ldb_modules/repl_meta_data.c
source4/dsdb/samdb/samdb.h
source4/setup/schema_samba4.ldif

index 74e9678367f98276268a1238809811f20e192f0b..a97b58a0e041bf1a937b120e1155924afb7277f4 100644 (file)
@@ -24,6 +24,7 @@ from base64 import b64decode
 from samba import dsdb
 from samba import common
 from samba.dcerpc import misc
+from samba.dcerpc import drsuapi
 from samba.ndr import ndr_unpack, ndr_pack
 from samba.dcerpc import drsblobs
 from samba.common import dsdb_Dn
@@ -63,6 +64,7 @@ class dbcheck(object):
         self.move_to_lost_and_found = False
         self.fix_instancetype = False
         self.fix_replmetadata_zero_invocationid = False
+        self.fix_replmetadata_unsorted_attid = False
         self.fix_deleted_deleted_objects = False
         self.fix_dn = False
         self.fix_base64_userparameters = False
@@ -698,9 +700,11 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         return 0
 
     def process_metadata(self, val):
-        '''Read metadata properties and list attributes in it'''
+        '''Read metadata properties and list attributes in it.
+           raises KeyError if the attid is unknown.'''
 
         list_att = []
+        list_attid = []
 
         repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob, str(val))
         obj = repl.ctr
@@ -708,8 +712,9 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
         for o in repl.ctr.array:
             att = self.samdb_schema.get_lDAPDisplayName_by_attid(o.attid)
             list_att.append(att.lower())
+            list_attid.append(o.attid)
 
-        return list_att
+        return (list_att, list_attid)
 
 
     def fix_metadata(self, dn, attr):
@@ -989,11 +994,52 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             nmsg = ldb.Message()
             nmsg.dn = dn
             nmsg[attr] = ldb.MessageElement(replBlob, ldb.FLAG_MOD_REPLACE, attr)
-            if self.do_modify(nmsg, ["local_oid:1.3.6.1.4.1.7165.4.3.14:0"],
+            if self.do_modify(nmsg, ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA,
+                                     "local_oid:1.3.6.1.4.1.7165.4.3.14:0"],
                               "Failed to fix attribute %s" % attr):
                 self.report("Fixed attribute '%s' of '%s'\n" % (attr, dn))
 
 
+    def err_replmetadata_unknown_attid(self, dn, attr, repl_meta_data):
+        repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
+                          str(repl_meta_data))
+        ctr = repl.ctr
+        for o in ctr.array:
+            # Search for an invalid attid
+            try:
+                att = self.samdb_schema.get_lDAPDisplayName_by_attid(o.attid)
+            except KeyError:
+                self.report('ERROR: attributeID 0X%0X is not known in our schema, not fixing %s on %s\n' % (o.attid, attr, dn))
+                return
+
+
+    def err_replmetadata_unsorted_attid(self, dn, attr, repl_meta_data):
+        repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
+                          str(repl_meta_data))
+        ctr = repl.ctr
+        found = False
+
+        self.report('ERROR: unsorted attributeID values in %s on %s\n' % (attr, dn))
+        if not self.confirm_all('Fix %s on %s by sorting the attribute list?'
+                                % (attr, dn), 'fix_replmetadata_unsorted_attid'):
+            self.report('Not fixing %s on %s\n' % (attr, dn))
+            return
+
+        # Sort the array, except for the last element
+        ctr.array[:-1] = sorted(ctr.array[:-1], key=lambda o: o.attid)
+
+        replBlob = ndr_pack(repl)
+
+        nmsg = ldb.Message()
+        nmsg.dn = dn
+        nmsg[attr] = ldb.MessageElement(replBlob, ldb.FLAG_MOD_REPLACE, attr)
+        if self.do_modify(nmsg, ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA,
+                                 "local_oid:1.3.6.1.4.1.7165.4.3.14:0",
+                                 "local_oid:1.3.6.1.4.1.7165.4.3.25:0"],
+                          "Failed to fix attribute %s" % attr):
+            self.report("Fixed attribute '%s' of '%s'\n" % (attr, dn))
+
+
     def is_deleted_deleted_objects(self, obj):
         faulty = False
         if "description" not in obj:
@@ -1181,7 +1227,17 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                     # We don't continue, as we may also have other fixes for this attribute
                     # based on what other attributes we see.
 
-                list_attrs_from_md = self.process_metadata(obj[attrname])
+                try:
+                    (list_attrs_from_md, list_attid_from_md) = self.process_metadata(obj[attrname])
+                except KeyError:
+                    error_count += 1
+                    self.err_replmetadata_unknown_attid(dn, attrname, obj[attrname])
+                    continue
+
+                if sorted(list_attid_from_md[:-1]) != list_attid_from_md[:-1]:
+                    error_count += 1
+                    self.err_replmetadata_unsorted_attid(dn, attrname, obj[attrname])
+
                 got_repl_property_meta_data = True
                 continue
 
index f19853f970bd6bb20174e6e337f2b2e1de4959c7..a3b94fbe20675d093f51f4e3290a3cdd2c558041 100644 (file)
@@ -65,6 +65,30 @@ class DsdbTests(TestCase):
         msg["replPropertyMetaData"] = ldb.MessageElement(replBlob, ldb.FLAG_MOD_REPLACE, "replPropertyMetaData")
         self.assertRaises(ldb.LdbError, self.samdb.modify, msg, ["local_oid:1.3.6.1.4.1.7165.4.3.14:0"])
 
+    def test_error_replpropertymetadata_nochange(self):
+        res = self.samdb.search(expression="cn=Administrator",
+                            scope=ldb.SCOPE_SUBTREE,
+                            attrs=["replPropertyMetaData"])
+        repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
+                            str(res[0]["replPropertyMetaData"]))
+        replBlob = ndr_pack(repl)
+        msg = ldb.Message()
+        msg.dn = res[0].dn
+        msg["replPropertyMetaData"] = ldb.MessageElement(replBlob, ldb.FLAG_MOD_REPLACE, "replPropertyMetaData")
+        self.assertRaises(ldb.LdbError, self.samdb.modify, msg, ["local_oid:1.3.6.1.4.1.7165.4.3.14:0"])
+
+    def test_error_replpropertymetadata_allow_sort(self):
+        res = self.samdb.search(expression="cn=Administrator",
+                            scope=ldb.SCOPE_SUBTREE,
+                            attrs=["replPropertyMetaData"])
+        repl = ndr_unpack(drsblobs.replPropertyMetaDataBlob,
+                            str(res[0]["replPropertyMetaData"]))
+        replBlob = ndr_pack(repl)
+        msg = ldb.Message()
+        msg.dn = res[0].dn
+        msg["replPropertyMetaData"] = ldb.MessageElement(replBlob, ldb.FLAG_MOD_REPLACE, "replPropertyMetaData")
+        self.samdb.modify(msg, ["local_oid:1.3.6.1.4.1.7165.4.3.14:0", "local_oid:1.3.6.1.4.1.7165.4.3.25:0"])
+
     def test_twoatt_replpropertymetadata(self):
         res = self.samdb.search(expression="cn=Administrator",
                             scope=ldb.SCOPE_SUBTREE,
index 4ff13951f247e975c5141db79989309e541dcfa5..3310d307b01109eca6ad8d55d7187adf9c1d7d9d 100644 (file)
@@ -1383,6 +1383,7 @@ static int replmd_update_rpmd(struct ldb_module *module,
        struct ldb_message_element *objectclass_el;
        enum urgent_situation situation;
        bool rmd_is_provided;
+       bool rmd_is_just_resorted = false;
 
        if (rename_attrs) {
                attrs = rename_attrs;
@@ -1404,6 +1405,9 @@ static int replmd_update_rpmd(struct ldb_module *module,
 
        if (ldb_request_get_control(req, DSDB_CONTROL_CHANGEREPLMETADATA_OID)) {
                rmd_is_provided = true;
+               if (ldb_request_get_control(req, DSDB_CONTROL_CHANGEREPLMETADATA_RESORT_OID)) {
+                       rmd_is_just_resorted = true;
+               }
        } else {
                rmd_is_provided = false;
        }
@@ -1422,7 +1426,7 @@ static int replmd_update_rpmd(struct ldb_module *module,
                /* In this case the change_replmetadata control was supplied */
                /* We check that it's the only attribute that is provided
                 * (it's a rare case so it's better to keep the code simplier)
-                * We also check that the highest local_usn is bigger than
+                * We also check that the highest local_usn is bigger or the same as
                 * uSNChanged. */
                uint64_t db_seq;
                if( msg->num_elements != 1 ||
@@ -1449,7 +1453,6 @@ static int replmd_update_rpmd(struct ldb_module *module,
                                 ldb_dn_get_linearized(msg->dn)));
                        return LDB_ERR_OPERATIONS_ERROR;
                }
-               *seq_num = find_max_local_usn(omd);
 
                ret = dsdb_module_search_dn(module, msg, &res, msg->dn, attrs2,
                                            DSDB_FLAG_NEXT_MODULE |
@@ -1462,12 +1465,22 @@ static int replmd_update_rpmd(struct ldb_module *module,
                        return ret;
                }
 
-               db_seq = ldb_msg_find_attr_as_uint64(res->msgs[0], "uSNChanged", 0);
-               if (*seq_num <= db_seq) {
-                       DEBUG(0,(__location__ ": changereplmetada control provided but max(local_usn)"\
-                                             " is less or equal to uSNChanged (max = %lld uSNChanged = %lld)\n",
-                                (long long)*seq_num, (long long)db_seq));
-                       return LDB_ERR_OPERATIONS_ERROR;
+               if (rmd_is_just_resorted == false) {
+                       *seq_num = find_max_local_usn(omd);
+
+                       db_seq = ldb_msg_find_attr_as_uint64(res->msgs[0], "uSNChanged", 0);
+
+                       /*
+                        * The test here now allows for a new
+                        * replPropertyMetaData with no change, if was
+                        * just dbcheck re-sorting the values.
+                        */
+                       if (*seq_num <= db_seq) {
+                               DEBUG(0,(__location__ ": changereplmetada control provided but max(local_usn)" \
+                                        " is less than uSNChanged (max = %lld uSNChanged = %lld)\n",
+                                        (long long)*seq_num, (long long)db_seq));
+                               return LDB_ERR_OPERATIONS_ERROR;
+                       }
                }
 
        } else {
@@ -1547,7 +1560,7 @@ static int replmd_update_rpmd(struct ldb_module *module,
         * replmd_update_rpmd_element has done an update if the
         * seq_num is set
         */
-       if (*seq_num != 0) {
+       if (*seq_num != 0 || rmd_is_just_resorted == true) {
                struct ldb_val *md_value;
                struct ldb_message_element *el;
 
index a25fa13982200029b7d16e85beb0b17a8a5e0b31..324045a9329434973cfb3cd65411ca6c71b72997 100644 (file)
@@ -151,6 +151,12 @@ struct dsdb_control_password_change {
  */
 #define DSDB_CONTROL_RESTORE_TOMBSTONE_OID "1.3.6.1.4.1.7165.4.3.24"
 
+/**
+  OID used to allow the replacement of replPropertyMetaData.
+  It is used when the current replmetadata needs only to be re-sorted, but not edited.
+*/
+#define DSDB_CONTROL_CHANGEREPLMETADATA_RESORT_OID "1.3.6.1.4.1.7165.4.3.25"
+
 #define DSDB_EXTENDED_REPLICATED_OBJECTS_OID "1.3.6.1.4.1.7165.4.4.1"
 struct dsdb_extended_replicated_object {
        struct ldb_message *msg;
index bdcd6252fa626f8683858df6b5cb6a5051e86067..69aa363cc90b61351a8ce90b67b114725e0b788f 100644 (file)
 #Allocated: DSDB_CONTROL_SEC_DESC_PROPAGATION_OID 1.3.6.1.4.1.7165.4.3.21
 #Allocated: DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID 1.3.6.1.4.1.7165.4.3.23
 #Allocated: DSDB_CONTROL_RESTORE_TOMBSTONE_OID 1.3.6.1.4.1.7165.4.3.24
+#Allocated: DSDB_CONTROL_CHANGEREPLMETADATA_RESORT_OID 1.3.6.1.4.1.7165.4.3.25
 
 # Extended 1.3.6.1.4.1.7165.4.4.x
 #Allocated: DSDB_EXTENDED_REPLICATED_OBJECTS_OID 1.3.6.1.4.1.7165.4.4.1