[PATCH] Actually fix old DN string names in dbcheck
Andrew Bartlett
abartlet at samba.org
Fri Jun 29 03:39:01 UTC 2018
This patch by Tim is combined by tests I've written to prove we now
actually do this correctly.
Please read the patch for the details, but this is needed for the
rename DC work Tim is working on.
It has also annoyed users for a very long time.
We take care not to bump the replication vectors as we do not wish to
trigger replication for this fix.
Please reivew and push if CI indicates:
CI: https://gitlab.com/catalyst-samba/samba/pipelines/24801979
Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
-------------- next part --------------
From 809996713577d0cbe725186d95ab3b6cdd2f2e3f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 29 Jun 2018 14:53:19 +1200
Subject: [PATCH 1/2] dbcheck: Use symbolic control name for
DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS
While we do not wish to encourage use of this control, manually typed OIDs are
even more trouble, so pass out via pydsdb.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
python/samba/dbchecker.py | 2 +-
source4/dsdb/pydsdb.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 9d72fc6ca94..00194afffaf 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -765,7 +765,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
m = ldb.Message()
m.dn = obj.dn
m['value'] = ldb.MessageElement(forward_vals, ldb.FLAG_MOD_REPLACE, forward_attr)
- if self.do_modify(m, ["local_oid:1.3.6.1.4.1.7165.4.3.19.2:1"],
+ if self.do_modify(m, ["local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS],
"Failed to fix duplicate links in attribute '%s'" % forward_attr):
self.report("Fixed duplicate links in attribute '%s'" % (forward_attr))
duplicate_cache_key = "%s:%s" % (str(obj.dn), forward_attr)
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index d9177bbd1d7..d10c7ec3b9d 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1581,6 +1581,7 @@ MODULE_INIT_FUNC(dsdb)
ADD_DSDB_STRING(DSDB_SYNTAX_OR_NAME);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
+ ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
--
2.11.0
From 20efce4435da469f45f3435d6dd3c8acf0efbb56 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 25 May 2018 14:05:27 +1200
Subject: [PATCH 2/2] dbchecker: Fixing up incorrect DNs wasn't working
dbcheck would fail to fix up attributes where the extended DN's GUID is
correct, but the DN itself is incorrect. The code failed attempting to
remove the old/incorrect DN, e.g.
NOTE: old (due to rename or delete) DN string component for
objectCategory in object CN=alice,CN=Users,DC=samba,DC=example,DC=com -
<GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=bad,DC=com
Change DN to <GUID=7bfdf9d8-62f9-420c-8a71-e3d3e931c91e>;
CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com?
[y/N/all/none] y
Failed to fix old DN string on attribute objectCategory : (16,
"attribute 'objectCategory': no matching attribute value while deleting
attribute on 'CN=alice,CN=Users,DC=samba,DC=example,DC=com'")
The problem was the LDB message specified the value to delete with its
full DN, including the GUID. The LDB code then helpfully corrected this
value on the way through, so that the DN got updated to reflect the
correct DN (i.e. 'DC=example,DC=com') of the object matching that GUID,
rather than the incorrect DN (i.e. 'DC=bad,DC=com') that we were trying
to remove. Because the requested value and the existing DB value didn't
match, the operation failed.
We can avoid this problem by passing down just the DN (not the extended
DN) of the value we want to delete. Without the GUID portion of the DN,
the LDB code will no longer try to correct it on the way through, and
the dbcheck operation will succeed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13495
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Pair-programmed-with: Andrew Bartlett <abartlet at samba.org>
---
python/samba/dbchecker.py | 19 +++++--
source4/dsdb/pydsdb.c | 1 +
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 64 +++++++++++++++++++++
source4/dsdb/samdb/samdb.h | 3 +
...ected-after-dbcheck-oneway-link-corruption.ldif | 19 +++++++
...-dbcheck-link-output-oneway-link-corruption.txt | 5 ++
testprogs/blackbox/dbcheck-links.sh | 65 ++++++++++++++++++++++
7 files changed, 171 insertions(+), 5 deletions(-)
create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 00194afffaf..c64fd4c1159 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -656,7 +656,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
m.dn = dn
m['old_value'] = ldb.MessageElement(val, ldb.FLAG_MOD_DELETE, attrname)
m['new_value'] = ldb.MessageElement(str(dsdb_dn), ldb.FLAG_MOD_ADD, attrname)
- if self.do_modify(m, ["show_recycled:1"],
+ if self.do_modify(m, ["show_recycled:1",
+ "local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME],
"Failed to fix old DN string on attribute %s" % (attrname)):
self.report("Fixed old DN string on attribute %s" % (attrname))
@@ -1298,14 +1299,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
res[0].dn, "SID")
continue
+ # Only for non-links, not even forward-only links
+ # (otherwise this breaks repl_meta_data):
+ #
# Now we have checked the GUID and SID, offer to fix old
- # DN strings as a non-error (for forward links with no
+ # DN strings as a non-error (DNs, not links so no
# backlink). Samba does not maintain this string
# otherwise, so we don't increment error_count.
if reverse_link_name is None:
- if str(res[0].dn) != str(dsdb_dn.dn):
- self.err_dn_string_component_old(obj.dn, attrname, val, dsdb_dn,
- res[0].dn)
+ if linkID == 0 and str(res[0].dn) != str(dsdb_dn.dn):
+ # Pass in the old/bad DN without the <GUID=...> part,
+ # otherwise the LDB code will correct it on the way through
+ # (Note: we still want to preserve the DSDB DN prefix in the
+ # case of binary DNs)
+ bad_dn = dsdb_dn.prefix + dsdb_dn.dn.get_linearized()
+ self.err_dn_string_component_old(obj.dn, attrname, bad_dn,
+ dsdb_dn, res[0].dn)
continue
# check the reverse_link is correct if there should be one
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index d10c7ec3b9d..a25f3411ae7 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -1582,6 +1582,7 @@ MODULE_INIT_FUNC(dsdb)
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA);
ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS);
+ ADD_DSDB_STRING(DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
ADD_DSDB_STRING(DSDB_CONTROL_REPLMD_VANISH_LINKS);
ADD_DSDB_STRING(DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID);
ADD_DSDB_STRING(DSDB_CONTROL_SKIP_DUPLICATES_CHECK_OID);
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 7395f52524e..c4a41a28d04 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -3333,6 +3333,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
const struct ldb_message_element *guid_el = NULL;
struct ldb_control *sd_propagation_control;
struct ldb_control *fix_links_control = NULL;
+ struct ldb_control *fix_dn_name_control = NULL;
struct replmd_private *replmd_private =
talloc_get_type(ldb_module_get_private(module), struct replmd_private);
@@ -3391,6 +3392,69 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
return ldb_next_request(module, req);
}
+ fix_dn_name_control = ldb_request_get_control(req,
+ DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME);
+ if (fix_dn_name_control != NULL) {
+ struct dsdb_schema *schema = NULL;
+ const struct dsdb_attribute *sa = NULL;
+
+ if (req->op.mod.message->num_elements != 2) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[0].flags != LDB_FLAG_MOD_DELETE) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[1].flags != LDB_FLAG_MOD_ADD) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[0].num_values != 1) {
+ return ldb_module_operr(module);
+ }
+
+ if (req->op.mod.message->elements[1].num_values != 1) {
+ return ldb_module_operr(module);
+ }
+
+ schema = dsdb_get_schema(ldb, req);
+ if (schema == NULL) {
+ return ldb_module_operr(module);
+ }
+
+ if (ldb_attr_cmp(req->op.mod.message->elements[0].name,
+ req->op.mod.message->elements[1].name) != 0) {
+ return ldb_module_operr(module);
+ }
+
+ sa = dsdb_attribute_by_lDAPDisplayName(schema,
+ req->op.mod.message->elements[0].name);
+ if (sa == NULL) {
+ return ldb_module_operr(module);
+ }
+
+ if (sa->dn_format == DSDB_INVALID_DN) {
+ return ldb_module_operr(module);
+ }
+
+ if (sa->linkID != 0) {
+ return ldb_module_operr(module);
+ }
+
+ /*
+ * If we are run from dbcheck and we are not updating
+ * a link (as these would need to be sorted and so
+ * can't go via such a simple update, then do not
+ * trigger replicated updates and a new USN from this
+ * change, it wasn't a real change, just a new
+ * (correct) string DN
+ */
+
+ fix_dn_name_control->critical = false;
+ return ldb_next_request(module, req);
+ }
+
ldb_debug(ldb, LDB_DEBUG_TRACE, "replmd_modify\n");
guid_el = ldb_msg_find_element(req->op.mod.message, "objectGUID");
diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
index 28d1b5cb32f..805a1f65fa8 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -132,6 +132,9 @@ struct dsdb_control_password_change {
/* passed by dbcheck to fix duplicate linked attributes (bug #13095) */
#define DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS "1.3.6.1.4.1.7165.4.3.19.2"
+/* passed by dbcheck to fix the DN strong of a one-way-link (bug #13495) */
+#define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME "1.3.6.1.4.1.7165.4.3.19.3"
+
/* passed when importing plain text password on upgrades */
#define DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID "1.3.6.1.4.1.7165.4.3.20"
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
new file mode 100644
index 00000000000..b2142df7424
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-after-dbcheck-oneway-link-corruption.ldif
@@ -0,0 +1,19 @@
+# record 1
+dn: OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# record 2
+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
+seeAlso: OU=dangling-ou2,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
+
+# Referral
+ref: ldap:///DC=DomainDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# Referral
+ref: ldap:///DC=ForestDnsZones,DC=release-4-5-0-pre1,DC=samba,DC=corp
+
+# returned 5 records
+# 2 entries
+# 3 referrals
diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
new file mode 100644
index 00000000000..69d1e516d14
--- /dev/null
+++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-oneway-link-corruption.txt
@@ -0,0 +1,5 @@
+Checking 228 objects
+NOTE: old (due to rename or delete) DN string component for seeAlso in object OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp - OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+Change DN to <GUID=20600e7c-92bb-492e-9552-f3ed7f8a2cad>;OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
+Fixed old DN string on attribute seeAlso
+Checked 228 objects (0 errors)
diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh
index 778edf002c9..13811ddb461 100755
--- a/testprogs/blackbox/dbcheck-links.sh
+++ b/testprogs/blackbox/dbcheck-links.sh
@@ -205,6 +205,67 @@ check_expected_after_dbcheck_forward_link_corruption() {
fi
}
+oneway_link_corruption() {
+ #
+ # Step1: add OU "dangling-ou"
+ #
+ ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption.ldif
+ cat > $ldif <<EOF
+dn: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+changetype: add
+objectclass: organizationalUnit
+objectGUID: 20600e7c-92bb-492e-9552-f3ed7f8a2cad
+EOF
+
+ out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --relax $ldif)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+
+ #
+ # Step2: add msExchConfigurationContainer "dangling-msexch"
+ #
+ ldif=$PREFIX_ABS/${RELEASE}/oneway_link_corruption2.ldif
+ cat > $ldif <<EOF
+dn: OU=dangling-from,DC=release-4-5-0-pre1,DC=samba,DC=corp
+changetype: add
+objectclass: organizationalUnit
+seeAlso: OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp
+EOF
+
+ out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+
+ #
+ # Step3: rename dangling-ou to dangling-ou2
+ #
+ # Because this is a one-way link we don't fix it at runtime
+ #
+ out=$(TZ=UTC $ldbrename -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb OU=dangling-ou,DC=release-4-5-0-pre1,DC=samba,DC=corp OU=dangling-ou2,DC=release-4-5-0-pre1,DC=samba,DC=corp)
+ if [ "$?" != "0" ]; then
+ echo "ldbmodify returned:\n$out"
+ return 1
+ fi
+}
+
+dbcheck_oneway_link_corruption() {
+ dbcheck "-oneway-link-corruption" "0" ""
+ return $?
+}
+
+check_expected_after_dbcheck_oneway_link_corruption() {
+ tmpldif=$PREFIX_ABS/$RELEASE/expected-after-dbcheck-oneway-link-corruption.ldif.tmp
+ TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(|(ou=dangling-ou)(ou=dangling-ou2)(ou=dangling-from))' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --sorted seeAlso > $tmpldif
+ diff $tmpldif $release_dir/expected-after-dbcheck-oneway-link-corruption.ldif
+ if [ "$?" != "0" ]; then
+ return 1
+ fi
+}
+
dbcheck_dangling_multi_valued() {
$PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --fix --yes
@@ -276,6 +337,10 @@ if [ -d $release_dir ]; then
testit "dbcheck_forward_link_corruption" dbcheck_forward_link_corruption
testit "check_expected_after_dbcheck_forward_link_corruption" check_expected_after_dbcheck_forward_link_corruption
testit "forward_link_corruption_clean" dbcheck_clean
+ testit "oneway_link_corruption" oneway_link_corruption
+ testit "dbcheck_oneway_link_corruption" dbcheck_oneway_link_corruption
+ testit "check_expected_after_dbcheck_oneway_link_corruption" check_expected_after_dbcheck_oneway_link_corruption
+ testit "oneway_link_corruption_clean" dbcheck_clean
testit "dangling_one_way_link" dangling_one_way_link
testit "dbcheck_one_way" dbcheck_one_way
testit "dbcheck_clean2" dbcheck_clean
--
2.11.0
More information about the samba-technical
mailing list