From e2c060293daad3c7cc588b6cc5582f302fcbd798 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 29 Jun 2018 14:53:19 +1200 Subject: [PATCH 01/17] 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 Reviewed-by: Gary Lockyer (cherry picked from commit c7fd68088d84232a2f4074ca278b5448ef624afd) --- 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 b2b8b0c9558f..df2f6f614a37 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -759,7 +759,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 8cf3ef5e4f68..afc5394b641e 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -1569,6 +1569,7 @@ void initdsdb(void) 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.17.1 From d95c05c8dcdb7202058e30abf704a2eeb8505fd7 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Fri, 25 May 2018 14:05:27 +1200 Subject: [PATCH 02/17] 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 - ; CN=Person,CN=Schema,CN=Configuration,DC=samba,DC=bad,DC=com Change DN to ; 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 Signed-off-by: Andrew Bartlett Reviewed-by: Gary Lockyer Pair-programmed-with: Andrew Bartlett (cherry picked from commit 22208f52e6096fbe9413b8ff339d9446851e0874) --- python/samba/dbchecker.py | 19 ++++-- source4/dsdb/pydsdb.c | 1 + .../dsdb/samdb/ldb_modules/repl_meta_data.c | 64 ++++++++++++++++++ source4/dsdb/samdb/samdb.h | 3 + ...-after-dbcheck-oneway-link-corruption.ldif | 19 ++++++ ...eck-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 df2f6f614a37..34856108a227 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -650,7 +650,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)) @@ -1289,14 +1290,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 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 afc5394b641e..16cfa0364430 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -1570,6 +1570,7 @@ void initdsdb(void) 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 62f58addfde6..a2d803e6ed4d 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -3329,6 +3329,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); @@ -3387,6 +3388,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 eb5274028068..7b809c76415b 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -130,6 +130,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 000000000000..b2142df74247 --- /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 000000000000..69d1e516d14c --- /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 ;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 778edf002c90..13811ddb461b 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 < $ldif < $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.17.1 From 4e1141dccf1f61fee767f62aa655579890e40a51 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 15:35:52 +0200 Subject: [PATCH 03/17] schema_samba4.ldif: add allocation of DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME This was already allocated in source4/dsdb/samdb/samdb.h with commit 22208f52e6096fbe9413b8ff339d9446851e0874. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 0189f23f5bda263c7462366ee16b2fe4bcda0119) --- source4/setup/schema_samba4.ldif | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif index 6aafc9e5f493..40641c46dd71 100644 --- a/source4/setup/schema_samba4.ldif +++ b/source4/setup/schema_samba4.ldif @@ -214,6 +214,7 @@ #Allocated: DSDB_CONTROL_DBCHECK 1.3.6.1.4.1.7165.4.3.19 #Allocated: DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA 1.3.6.1.4.1.7165.4.3.19.1 #Allocated: DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS 1.3.6.1.4.1.7165.4.3.19.2 +#Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME 1.3.6.1.4.1.7165.4.3.19.3 #Allocated: DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID 1.3.6.1.4.1.7165.4.3.20 #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 -- 2.17.1 From 38ad525f045a4a644a04369690ec7a319a9e6239 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 17:13:13 +0200 Subject: [PATCH 04/17] s4:dsdb: fix comment on DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 60131b4452d43b3792e7f27a4190c88e7aabb1b4) --- source4/dsdb/samdb/samdb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 7b809c76415b..7d1164ffa49c 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -130,7 +130,7 @@ 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) */ +/* passed by dbcheck to fix the DN string 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 */ -- 2.17.1 From dd5092e64ea9822ba3a5894e1cccbda3356eecc2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 16 Oct 2018 15:16:18 +0200 Subject: [PATCH 05/17] testprogs/blackbox: add samba4.blackbox.test_primary_group test This demonstrates the bug, that happens when the primaryGroupID of a user is changed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 364ed537e0bcb3a97cae0f2d1ff72de9423ce0e6) --- .../samba4.blackbox.test_primary_group | 2 + source4/selftest/tests.py | 2 + testprogs/blackbox/test_primary_group.sh | 86 +++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 selftest/knownfail.d/samba4.blackbox.test_primary_group create mode 100755 testprogs/blackbox/test_primary_group.sh diff --git a/selftest/knownfail.d/samba4.blackbox.test_primary_group b/selftest/knownfail.d/samba4.blackbox.test_primary_group new file mode 100644 index 000000000000..779f6808c977 --- /dev/null +++ b/selftest/knownfail.d/samba4.blackbox.test_primary_group @@ -0,0 +1,2 @@ +^samba4.blackbox.test_primary_group.dbcheck.*run1 +^samba4.blackbox.test_primary_group.dbcheck.*run2 diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index fd9475156d9e..0841341d7592 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -406,6 +406,8 @@ for env in ["ad_member", "s4member", "ad_dc_ntvfs", "chgdcpass"]: plantestsuite("samba4.blackbox.samba_tool(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(samba4srcdir, "utils/tests/test_samba_tool.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', smbclient4]) plantestsuite("samba4.blackbox.net_rpc_user(ad_dc)", "ad_dc", [os.path.join(bbdir, "test_net_rpc_user.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$DOMAIN']) +plantestsuite("samba4.blackbox.test_primary_group", "ad_dc:local", [os.path.join(bbdir, "test_primary_group.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$DOMAIN', '$PREFIX_ABS']) + if have_heimdal_support: for env in ["ad_dc_ntvfs", "ad_dc"]: plantestsuite("samba4.blackbox.pkinit(%s:local)" % env, "%s:local" % env, [os.path.join(bbdir, "test_pkinit_heimdal.sh"), '$SERVER', 'pkinit', '$PASSWORD', '$REALM', '$DOMAIN', '$PREFIX/%s' % env, "aes256-cts-hmac-sha1-96", smbclient4, configuration]) diff --git a/testprogs/blackbox/test_primary_group.sh b/testprogs/blackbox/test_primary_group.sh new file mode 100755 index 000000000000..c2d803e1d15c --- /dev/null +++ b/testprogs/blackbox/test_primary_group.sh @@ -0,0 +1,86 @@ +#!/bin/bash + +if [ $# -lt 5 ]; then +cat < $ldif +rid=$(cat $ldif | sed -n 's/^objectSid: S-1-5-21-.*-.*-.*-//p') + +testit "search2" $VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName="$testuser" dn || failed=`expr $failed + 1` +ldif="${TMPDIR}/search2.ldif" +$VALGRIND $BINDIR/ldbsearch -H ldap://$SERVER_IP -U$USERNAME%$PASSWORD -d0 sAMAccountName=$testuser dn > $ldif +user_dn=$(cat $ldif | sed -n 's/^dn: //p') + +ldif="${TMPDIR}/modify1.ldif" +cat > $ldif < $ldif < Date: Mon, 8 Oct 2018 17:13:52 +0200 Subject: [PATCH 06/17] s4:dsdb: add DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID oid This will be used to fix missing components in future. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit bb9c9e49a5e82f19626cb1b12ec9189fff5114e8) --- source4/dsdb/pydsdb.c | 1 + source4/dsdb/samdb/samdb.h | 3 +++ source4/setup/schema_samba4.ldif | 1 + 3 files changed, 5 insertions(+) diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c index 16cfa0364430..7f25f7574dcb 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -1571,6 +1571,7 @@ void initdsdb(void) 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_DBCHECK_FIX_LINK_DN_SID); 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/samdb.h b/source4/dsdb/samdb/samdb.h index 7d1164ffa49c..2db8dd4d4d88 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -133,6 +133,9 @@ struct dsdb_control_password_change { /* passed by dbcheck to fix the DN string 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 by dbcheck to fix the DN SID of a one-way-link (bug #13418) */ +#define DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID "1.3.6.1.4.1.7165.4.3.19.4" + /* 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/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif index 40641c46dd71..219b232f7535 100644 --- a/source4/setup/schema_samba4.ldif +++ b/source4/setup/schema_samba4.ldif @@ -215,6 +215,7 @@ #Allocated: DSDB_CONTROL_DBCHECK_MODIFY_RO_REPLICA 1.3.6.1.4.1.7165.4.3.19.1 #Allocated: DSDB_CONTROL_DBCHECK_FIX_DUPLICATE_LINKS 1.3.6.1.4.1.7165.4.3.19.2 #Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME 1.3.6.1.4.1.7165.4.3.19.3 +#Allocated: DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID 1.3.6.1.4.1.7165.4.3.19.4 #Allocated: DSDB_CONTROL_PASSWORD_BYPASS_LAST_SET_OID 1.3.6.1.4.1.7165.4.3.20 #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 -- 2.17.1 From 3da79616e13797bd9b2f57de34454dc53926e90d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 17:14:28 +0200 Subject: [PATCH 07/17] dbchecker: improve verbose output of do_modify() This makes it easier to debug dbcheck problems. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c5c99b569569ce36cac94e967ca53e3182abd6f7) --- python/samba/dbchecker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index 34856108a227..189230503aca 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -378,10 +378,11 @@ systemFlags: -1946157056%s""" % (dn, guid_suffix), def do_modify(self, m, controls, msg, validate=True): '''perform a modify with optional verbose output''' + controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK] if self.verbose: self.report(self.samdb.write_ldif(m, ldb.CHANGETYPE_MODIFY)) + self.report("controls: %r" % controls) try: - controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK] self.samdb.modify(m, controls=controls, validate=validate) except Exception, err: if self.in_transaction: -- 2.17.1 From 0b9b83891131892ee07cd884635b6325ab84e162 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 08/17] dbchecker: Fix missing on linked attributes BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a801799ebe26780653f4ed3fa3fc633e31871f7d) --- python/samba/dbchecker.py | 42 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index 189230503aca..2619b9bc72e3 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -59,6 +59,7 @@ class dbcheck(object): self.fix_all_string_dn_component_mismatch = False self.fix_all_GUID_dn_component_mismatch = False self.fix_all_SID_dn_component_mismatch = False + self.fix_all_SID_dn_component_missing = False self.fix_all_old_dn_string_component_mismatch = False self.fix_all_metadata = False self.fix_time_metadata = False @@ -673,6 +674,38 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) "Failed to fix incorrect DN %s on attribute %s" % (mismatch_type, attrname)): self.report("Fixed incorrect DN %s on attribute %s" % (mismatch_type, attrname)) + def err_dn_component_missing_target_sid(self, dn, attrname, val, dsdb_dn, target_sid_blob): + """handle a DN string being incorrect""" + self.report("ERROR: missing DN SID component for %s in object %s - %s" % (attrname, dn, val)) + + if len(dsdb_dn.prefix) != 0: + self.report("Not fixing missing DN SID on DN+BINARY or DN+STRING") + return + + correct_dn = ldb.Dn(self.samdb, dsdb_dn.dn.extended_str()) + correct_dn.set_extended_component("SID", target_sid_blob) + + if not self.confirm_all('Change DN to %s?' % correct_dn.extended_str(), + 'fix_all_SID_dn_component_missing'): + self.report("Not fixing missing DN SID component") + return + + target_guid_blob = correct_dn.get_extended_component("GUID") + guid_sid_dn = ldb.Dn(self.samdb, "") + guid_sid_dn.set_extended_component("GUID", target_guid_blob) + guid_sid_dn.set_extended_component("SID", target_sid_blob) + + m = ldb.Message() + m.dn = dn + m['new_value'] = ldb.MessageElement(guid_sid_dn.extended_str(), ldb.FLAG_MOD_ADD, attrname) + controls = [ + "show_recycled:1", + "local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID + ] + if self.do_modify(m, controls, + "Failed to ADD missing DN SID on attribute %s" % (attrname)): + self.report("Fixed missing DN SID on attribute %s" % (attrname)) + def err_unknown_attribute(self, obj, attrname): '''handle an unknown attribute error''' self.report("ERROR: unknown attribute '%s' in %s" % (attrname, obj.dn)) @@ -1285,7 +1318,14 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) res[0].dn, "GUID") continue - if res[0].dn.get_extended_component("SID") != dsdb_dn.dn.get_extended_component("SID"): + target_sid = res[0].dn.get_extended_component("SID") + link_sid = dsdb_dn.dn.get_extended_component("SID") + if link_sid is None and target_sid is not None: + error_count += 1 + self.err_dn_component_missing_target_sid(obj.dn, attrname, val, + dsdb_dn, target_sid) + continue + if link_sid != target_sid: error_count += 1 self.err_dn_component_target_mismatch(obj.dn, attrname, val, dsdb_dn, res[0].dn, "SID") -- 2.17.1 From 912ebcbd1a4ee5cd84cb0192e7dc4da7542bf586 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 09/17] blackbox/dbcheck-links: Test broken links with missing on linked attributes BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit f81771c8593327e058b9cb4330d7e77083df3ea9) --- .../knownfail.d/samba4.blackbox.dbcheck-links | 6 + ...ink-output-missing-link-sid-corruption.txt | 8 ++ testprogs/blackbox/dbcheck-links.sh | 110 ++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt diff --git a/selftest/knownfail.d/samba4.blackbox.dbcheck-links b/selftest/knownfail.d/samba4.blackbox.dbcheck-links new file mode 100644 index 000000000000..ee207a53ab74 --- /dev/null +++ b/selftest/knownfail.d/samba4.blackbox.dbcheck-links @@ -0,0 +1,6 @@ +# The first one fails and all others are follow up failures... +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_missing_link_sid_corruption +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.missing_link_sid_clean +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3 +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_dangling_multi_valued +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_equal_or_too_many diff --git a/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt new file mode 100644 index 000000000000..34576157f25d --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt @@ -0,0 +1,8 @@ +Change DN to ;;;;;;;;;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES] +Change DN to ;;;;;;;;;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES] +Checked 231 objects (2 errors) +Checking 231 objects +ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - ;;;;;;;;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - ;;;;;;;;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +Fixed missing DN SID on attribute member +Fixed missing DN SID on attribute member diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh index 13811ddb461b..9798813004c5 100755 --- a/testprogs/blackbox/dbcheck-links.sh +++ b/testprogs/blackbox/dbcheck-links.sh @@ -131,6 +131,113 @@ check_expected_after_duplicate_links() { fi } +missing_link_sid_corruption() { + # Step1: add user "missingsidu1" + # + ldif=$PREFIX_ABS/${RELEASE}/missing_link_sid_corruption1.ldif + cat > $ldif < $ldif < $ldif < $ldif <;!!g' \ + -e 's!;!!g' \ + -e 's!RMD_ADDTIME=[1-9][0-9]*!RMD_ADDTIME=123456789000000000!g' \ + -e 's!RMD_CHANGETIME=[1-9][0-9]*!RMD_CHANGETIME=123456789000000000!g' \ + | cat + } > $ldif + + out=$(TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-5-0-PRE1,DC%3DSAMBA,DC%3DCORP.ldb $ldif) + if [ "$?" != "0" ]; then + echo "ldbmodify returned:\n$out" + return 1 + fi + + return 0 +} + +dbcheck_missing_link_sid_corruption() { + dbcheck "-missing-link-sid-corruption" "1" "" + return $? +} + forward_link_corruption() { # # Step1: add a duplicate forward link from @@ -344,6 +451,9 @@ if [ -d $release_dir ]; then testit "dangling_one_way_link" dangling_one_way_link testit "dbcheck_one_way" dbcheck_one_way testit "dbcheck_clean2" dbcheck_clean + testit "missing_link_sid_corruption" missing_link_sid_corruption + testit "dbcheck_missing_link_sid_corruption" dbcheck_missing_link_sid_corruption + testit "missing_link_sid_clean" dbcheck_clean testit "dangling_one_way_dn" dangling_one_way_dn testit "deleted_one_way_dn" deleted_one_way_dn testit "dbcheck_clean3" dbcheck_clean -- 2.17.1 From 05f06c474bda5f6ccf00d31c46e06377b8f08bf7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 10/17] s4:repl_meta_data: pass down struct replmd_replicated_request to replmd_modify_handle_linked_attribs() This will simplify further changes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 966c7febaf0245516481bde924ea6cd738eeb78b) --- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index a2d803e6ed4d..50ebb6148c86 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -3116,8 +3116,9 @@ static int replmd_modify_la_replace(struct ldb_module *module, */ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, struct replmd_private *replmd_private, + struct replmd_replicated_request *ac, struct ldb_message *msg, - uint64_t seq_num, time_t t, + time_t t, struct ldb_request *parent) { struct ldb_result *res; @@ -3126,8 +3127,6 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, struct ldb_context *ldb = ldb_module_get_ctx(module); struct ldb_message *old_msg; - const struct dsdb_schema *schema; - if (dsdb_functional_level(ldb) == DS_DOMAIN_FUNCTION_2000) { /* * Nothing special is required for modifying or vanishing links @@ -3161,10 +3160,6 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, if (ret != LDB_SUCCESS) { return ret; } - schema = dsdb_get_schema(ldb, res); - if (!schema) { - return LDB_ERR_OPERATIONS_ERROR; - } old_msg = res->msgs[0]; @@ -3173,7 +3168,7 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, struct ldb_message_element *old_el, *new_el; unsigned int mod_type = LDB_FLAG_MOD_TYPE(el->flags); const struct dsdb_attribute *schema_attr - = dsdb_attribute_by_lDAPDisplayName(schema, el->name); + = dsdb_attribute_by_lDAPDisplayName(ac->schema, el->name); if (!schema_attr) { ldb_asprintf_errstring(ldb, "%s: attribute %s is not a valid attribute in schema", @@ -3209,22 +3204,22 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, switch (mod_type) { case LDB_FLAG_MOD_REPLACE: ret = replmd_modify_la_replace(module, replmd_private, - schema, msg, el, old_el, - schema_attr, seq_num, t, + ac->schema, msg, el, old_el, + schema_attr, ac->seq_num, t, old_msg->dn, parent); break; case LDB_FLAG_MOD_DELETE: ret = replmd_modify_la_delete(module, replmd_private, - schema, msg, el, old_el, - schema_attr, seq_num, t, + ac->schema, msg, el, old_el, + schema_attr, ac->seq_num, t, old_msg->dn, parent); break; case LDB_FLAG_MOD_ADD: ret = replmd_modify_la_add(module, replmd_private, - schema, msg, el, old_el, - schema_attr, seq_num, t, + ac->schema, msg, el, old_el, + schema_attr, ac->seq_num, t, old_msg->dn, parent); break; @@ -3496,7 +3491,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req) } ret = replmd_modify_handle_linked_attribs(module, replmd_private, - msg, ac->seq_num, t, req); + ac, msg, t, req); if (ret != LDB_SUCCESS) { talloc_free(ac); return ret; -- 2.17.1 From b0f11b6f373773379b626e54e94eb899dce918c7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 11/17] s4:repl_meta_data: pass down struct replmd_replicated_request to replmd_modify_la_add() This will simplify further changes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 42e69a86ca583e3cb20c63b9c6930b4b3425485d) --- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 50ebb6148c86..d203c5712440 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2398,12 +2398,11 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d */ static int replmd_modify_la_add(struct ldb_module *module, struct replmd_private *replmd_private, - const struct dsdb_schema *schema, + struct replmd_replicated_request *ac, struct ldb_message *msg, struct ldb_message_element *el, struct ldb_message_element *old_el, const struct dsdb_attribute *schema_attr, - uint64_t seq_num, time_t t, struct ldb_dn *msg_dn, struct ldb_request *parent) @@ -2416,17 +2415,10 @@ static int replmd_modify_la_add(struct ldb_module *module, 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; unix_to_nt_time(&now, t); - invocation_id = samdb_ntds_invocation_id(ldb); - if (!invocation_id) { - talloc_free(tmp_ctx); - return LDB_ERR_OPERATIONS_ERROR; - } - /* get the DNs to be added, fully parsed. * * We need full parsing because they came off the wire and we don't @@ -2522,15 +2514,16 @@ static int replmd_modify_la_add(struct ldb_module *module, ret = replmd_update_la_val(new_values, exact->v, dns[i].dsdb_dn, exact->dsdb_dn, - invocation_id, seq_num, - seq_num, now, false); + &ac->our_invocation_id, + ac->seq_num, ac->seq_num, + now, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } ret = replmd_add_backlink(module, replmd_private, - schema, + ac->schema, msg_dn, &dns[i].guid, true, @@ -2572,14 +2565,14 @@ static int replmd_modify_la_add(struct ldb_module *module, } ret = replmd_add_backlink(module, replmd_private, - schema, msg_dn, + ac->schema, msg_dn, &dns[i].guid, true, schema_attr, parent); /* 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, now); + dns[i].dsdb_dn, &ac->our_invocation_id, + ac->seq_num, now); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -3218,8 +3211,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, break; case LDB_FLAG_MOD_ADD: ret = replmd_modify_la_add(module, replmd_private, - ac->schema, msg, el, old_el, - schema_attr, ac->seq_num, t, + ac, msg, el, old_el, + schema_attr, t, old_msg->dn, parent); break; -- 2.17.1 From 30203927397b626d9c5fd701e1ac1909dbd005d4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 19:34:08 +0200 Subject: [PATCH 12/17] s4:repl_meta_data: add missing \n to a DEBUG message in replmd_modify_la_add() BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 70a306d0bd6806d1fd00d45e3d8cc70c73d09f79) --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 2 +- 1 file changed, 1 insertion(+), 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 d203c5712440..385c831dd726 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2445,7 +2445,7 @@ static int replmd_modify_la_add(struct ldb_module *module, 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 values: %u, new values: %u, sum: %u\n", old_num_values, el->num_values, max_num_values)); talloc_free(tmp_ctx); return LDB_ERR_OPERATIONS_ERROR; -- 2.17.1 From a1819ac7710b98fcf5bce6aa41eaa8b21b708120 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 13/17] s4:repl_meta_data: pass down struct replmd_replicated_request to replmd_modify_la_delete() This will simplify further changes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 738b52eb0856c8fcdbb8589e8061bcc14700c23a) --- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 385c831dd726..b350e4969e4d 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2611,12 +2611,11 @@ static int replmd_modify_la_add(struct ldb_module *module, */ static int replmd_modify_la_delete(struct ldb_module *module, struct replmd_private *replmd_private, - const struct dsdb_schema *schema, + struct replmd_replicated_request *ac, struct ldb_message *msg, struct ldb_message_element *el, struct ldb_message_element *old_el, const struct dsdb_attribute *schema_attr, - uint64_t seq_num, time_t t, struct ldb_dn *msg_dn, struct ldb_request *parent) @@ -2630,16 +2629,10 @@ static int replmd_modify_la_delete(struct ldb_module *module, bool vanish_links = false; unsigned int num_to_delete = el->num_values; uint32_t rmd_flags; - const struct GUID *invocation_id; NTTIME now; unix_to_nt_time(&now, t); - invocation_id = samdb_ntds_invocation_id(ldb); - if (!invocation_id) { - return LDB_ERR_OPERATIONS_ERROR; - } - if (old_el == NULL || old_el->num_values == 0) { /* there is nothing to delete... */ if (num_to_delete == 0) { @@ -2703,7 +2696,7 @@ static int replmd_modify_la_delete(struct ldb_module *module, } } ret = replmd_add_backlink(module, replmd_private, - schema, msg_dn, &p->guid, + ac->schema, msg_dn, &p->guid, false, schema_attr, parent); if (ret != LDB_SUCCESS) { @@ -2721,8 +2714,9 @@ static int replmd_modify_la_delete(struct ldb_module *module, ret = replmd_update_la_val(old_el->values, p->v, p->dsdb_dn, p->dsdb_dn, - invocation_id, seq_num, - seq_num, now, true); + &ac->our_invocation_id, + ac->seq_num, ac->seq_num, + now, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -2784,7 +2778,7 @@ static int replmd_modify_la_delete(struct ldb_module *module, /* remove the backlink */ ret = replmd_add_backlink(module, replmd_private, - schema, + ac->schema, msg_dn, &p->guid, false, schema_attr, @@ -2818,14 +2812,15 @@ static int replmd_modify_la_delete(struct ldb_module *module, ret = replmd_update_la_val(old_el->values, exact->v, exact->dsdb_dn, exact->dsdb_dn, - invocation_id, seq_num, seq_num, + &ac->our_invocation_id, + ac->seq_num, ac->seq_num, now, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } ret = replmd_add_backlink(module, replmd_private, - schema, msg_dn, + ac->schema, msg_dn, &p->guid, false, schema_attr, parent); @@ -3204,8 +3199,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, break; case LDB_FLAG_MOD_DELETE: ret = replmd_modify_la_delete(module, replmd_private, - ac->schema, msg, el, old_el, - schema_attr, ac->seq_num, t, + ac, msg, el, old_el, + schema_attr, t, old_msg->dn, parent); break; -- 2.17.1 From ac7972f33f812b810459ad40b53952f5427915da Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 14/17] s4:repl_meta_data: pass down struct replmd_replicated_request to replmd_modify_la_replace() This will simplify further changes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 1ef145d9d72d847055f6aba8a0070b3e1cfdabbc) --- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index b350e4969e4d..2a3a86ed27ed 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2870,12 +2870,11 @@ static int replmd_modify_la_delete(struct ldb_module *module, */ static int replmd_modify_la_replace(struct ldb_module *module, struct replmd_private *replmd_private, - const struct dsdb_schema *schema, + struct replmd_replicated_request *ac, struct ldb_message *msg, struct ldb_message_element *el, struct ldb_message_element *old_el, const struct dsdb_attribute *schema_attr, - uint64_t seq_num, time_t t, struct ldb_dn *msg_dn, struct ldb_request *parent) @@ -2884,7 +2883,6 @@ static int replmd_modify_la_replace(struct ldb_module *module, 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; const char *ldap_oid = schema_attr->syntax->ldap_oid; @@ -2895,11 +2893,6 @@ static int replmd_modify_la_replace(struct ldb_module *module, unix_to_nt_time(&now, t); - invocation_id = samdb_ntds_invocation_id(ldb); - if (!invocation_id) { - return LDB_ERR_OPERATIONS_ERROR; - } - /* * 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 @@ -2999,8 +2992,8 @@ static int replmd_modify_la_replace(struct ldb_module *module, ret = replmd_update_la_val(new_values, old_p->v, old_p->dsdb_dn, old_p->dsdb_dn, - invocation_id, - seq_num, seq_num, + &ac->our_invocation_id, + ac->seq_num, ac->seq_num, now, true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); @@ -3008,7 +3001,7 @@ static int replmd_modify_la_replace(struct ldb_module *module, } ret = replmd_add_backlink(module, replmd_private, - schema, + ac->schema, msg_dn, &old_p->guid, false, schema_attr, @@ -3033,8 +3026,8 @@ static int replmd_modify_la_replace(struct ldb_module *module, ret = replmd_update_la_val(new_values, old_p->v, new_p->dsdb_dn, old_p->dsdb_dn, - invocation_id, - seq_num, seq_num, + &ac->our_invocation_id, + ac->seq_num, ac->seq_num, now, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); @@ -3044,7 +3037,7 @@ static int replmd_modify_la_replace(struct ldb_module *module, 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, + ac->schema, msg_dn, &new_p->guid, true, schema_attr, @@ -3066,14 +3059,14 @@ static int replmd_modify_la_replace(struct ldb_module *module, ret = replmd_build_la_val(new_values, new_p->v, new_p->dsdb_dn, - invocation_id, - seq_num, now); + &ac->our_invocation_id, + ac->seq_num, now); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; } ret = replmd_add_backlink(module, replmd_private, - schema, + ac->schema, msg_dn, &new_p->guid, true, schema_attr, @@ -3192,8 +3185,8 @@ static int replmd_modify_handle_linked_attribs(struct ldb_module *module, switch (mod_type) { case LDB_FLAG_MOD_REPLACE: ret = replmd_modify_la_replace(module, replmd_private, - ac->schema, msg, el, old_el, - schema_attr, ac->seq_num, t, + ac, msg, el, old_el, + schema_attr, t, old_msg->dn, parent); break; -- 2.17.1 From 7e80d8df6bd485448095e726010a8999923bc585 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 15/17] s4:repl_meta_data: add support for DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID This will be used by dbcheck in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 0386307e34097f5d9233c970983c7306d1705a87) --- .../knownfail.d/samba4.blackbox.dbcheck-links | 6 - .../samdb/ldb_modules/extended_dn_store.c | 7 + .../dsdb/samdb/ldb_modules/repl_meta_data.c | 145 ++++++++++++++++++ source4/dsdb/samdb/ldb_modules/samldb.c | 12 +- 4 files changed, 161 insertions(+), 9 deletions(-) delete mode 100644 selftest/knownfail.d/samba4.blackbox.dbcheck-links diff --git a/selftest/knownfail.d/samba4.blackbox.dbcheck-links b/selftest/knownfail.d/samba4.blackbox.dbcheck-links deleted file mode 100644 index ee207a53ab74..000000000000 --- a/selftest/knownfail.d/samba4.blackbox.dbcheck-links +++ /dev/null @@ -1,6 +0,0 @@ -# The first one fails and all others are follow up failures... -^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_missing_link_sid_corruption -^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.missing_link_sid_clean -^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_clean3 -^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_dangling_multi_valued -^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_equal_or_too_many diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c index a32ab8d6f935..8e5a2ba7ae94 100644 --- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c +++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c @@ -376,6 +376,7 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req unsigned int i, j; struct extended_dn_context *ac; struct ldb_control *fix_links_control = NULL; + struct ldb_control *fix_link_sid_ctrl = NULL; int ret; if (ldb_dn_is_special(req->op.mod.message->dn)) { @@ -400,6 +401,12 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req return ldb_next_request(module, req); } + fix_link_sid_ctrl = ldb_request_get_control(ac->req, + DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID); + if (fix_link_sid_ctrl != NULL) { + return ldb_next_request(module, req); + } + for (i=0; i < req->op.mod.message->num_elements; i++) { const struct ldb_message_element *el = &req->op.mod.message->elements[i]; const struct dsdb_attribute *schema_attr diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 2a3a86ed27ed..5bdc57366da6 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -112,6 +112,8 @@ struct replmd_replicated_request { bool is_urgent; bool isDeleted; + + bool fix_link_sid; }; static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar); @@ -2481,6 +2483,109 @@ static int replmd_modify_la_add(struct ldb_module *module, return err; } + if (ac->fix_link_sid) { + char *fixed_dnstring = NULL; + struct dom_sid tmp_sid = { 0, }; + DATA_BLOB sid_blob = data_blob_null; + enum ndr_err_code ndr_err; + NTSTATUS status; + int num; + + if (exact == NULL) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + if (dns[i].dsdb_dn->dn_format != DSDB_NORMAL_DN) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + /* + * Only "" is allowed. + * + * We get the GUID to just to find the old + * value and the SID in order to add it + * to the found value. + */ + + num = ldb_dn_get_comp_num(dns[i].dsdb_dn->dn); + if (num != 0) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + num = ldb_dn_get_extended_comp_num(dns[i].dsdb_dn->dn); + if (num != 2) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + status = dsdb_get_extended_dn_sid(exact->dsdb_dn->dn, + &tmp_sid, "SID"); + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + /* this is what we expect */ + } else if (NT_STATUS_IS_OK(status)) { + struct GUID_txt_buf guid_str; + ldb_debug_set(ldb, LDB_DEBUG_FATAL, + "i[%u] SID NOT MISSING... Attribute %s already " + "exists for target GUID %s, SID %s, DN: %s", + i, el->name, + GUID_buf_string(&exact->guid, + &guid_str), + dom_sid_string(tmp_ctx, &tmp_sid), + dsdb_dn_get_extended_linearized(tmp_ctx, + exact->dsdb_dn, 1)); + talloc_free(tmp_ctx); + return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + } else { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + status = dsdb_get_extended_dn_sid(dns[i].dsdb_dn->dn, + &tmp_sid, "SID"); + if (!NT_STATUS_IS_OK(status)) { + struct GUID_txt_buf guid_str; + ldb_asprintf_errstring(ldb, + "NO SID PROVIDED... Attribute %s already " + "exists for target GUID %s", + el->name, + GUID_buf_string(&exact->guid, + &guid_str)); + talloc_free(tmp_ctx); + return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + } + + ndr_err = ndr_push_struct_blob(&sid_blob, tmp_ctx, &tmp_sid, + (ndr_push_flags_fn_t)ndr_push_dom_sid); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + ret = ldb_dn_set_extended_component(exact->dsdb_dn->dn, "SID", &sid_blob); + data_blob_free(&sid_blob); + if (ret != LDB_SUCCESS) { + talloc_free(tmp_ctx); + return ret; + } + + fixed_dnstring = dsdb_dn_get_extended_linearized( + new_values, exact->dsdb_dn, 1); + if (fixed_dnstring == NULL) { + talloc_free(tmp_ctx); + return ldb_operr(ldb); + } + + /* + * We just replace the existing value... + */ + *exact->v = data_blob_string_const(fixed_dnstring); + + continue; + } + if (exact != NULL) { /* * We are trying to add one that exists, which is only @@ -3306,6 +3411,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req) struct ldb_control *sd_propagation_control; struct ldb_control *fix_links_control = NULL; struct ldb_control *fix_dn_name_control = NULL; + struct ldb_control *fix_dn_sid_control = NULL; struct replmd_private *replmd_private = talloc_get_type(ldb_module_get_private(module), struct replmd_private); @@ -3451,6 +3557,44 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req) return LDB_ERR_OPERATIONS_ERROR; } + fix_dn_sid_control = ldb_request_get_control(req, + DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID); + if (fix_dn_sid_control != NULL) { + const struct dsdb_attribute *sa = NULL; + + if (msg->num_elements != 1) { + talloc_free(ac); + return ldb_module_operr(module); + } + + if (msg->elements[0].flags != LDB_FLAG_MOD_ADD) { + talloc_free(ac); + return ldb_module_operr(module); + } + + if (msg->elements[0].num_values != 1) { + talloc_free(ac); + return ldb_module_operr(module); + } + + sa = dsdb_attribute_by_lDAPDisplayName(ac->schema, + msg->elements[0].name); + if (sa == NULL) { + talloc_free(ac); + return ldb_module_operr(module); + } + + if (sa->dn_format != DSDB_NORMAL_DN) { + talloc_free(ac); + return ldb_module_operr(module); + } + + fix_dn_sid_control->critical = false; + ac->fix_link_sid = true; + + goto handle_linked_attribs; + } + ldb_msg_remove_attr(msg, "whenChanged"); ldb_msg_remove_attr(msg, "uSNChanged"); @@ -3471,6 +3615,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req) return ret; } + handle_linked_attribs: ret = replmd_modify_handle_linked_attribs(module, replmd_private, ac, msg, t, req); if (ret != LDB_SUCCESS) { diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 734d0be26a93..81d8c96437c9 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -3770,9 +3770,15 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req) el = ldb_msg_find_element(ac->msg, "member"); if (el != NULL) { - ret = samldb_member_check(ac); - if (ret != LDB_SUCCESS) { - return ret; + struct ldb_control *fix_link_sid_ctrl = NULL; + + fix_link_sid_ctrl = ldb_request_get_control(ac->req, + DSDB_CONTROL_DBCHECK_FIX_LINK_DN_SID); + if (fix_link_sid_ctrl == NULL) { + ret = samldb_member_check(ac); + if (ret != LDB_SUCCESS) { + return ret; + } } } -- 2.17.1 From 92b652746a667f381890fc6649bb4e5ac8926025 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 24 Aug 2018 15:33:49 +0200 Subject: [PATCH 16/17] s4:samldb: internally use extended dns while changing the primaryGroupID field This is important, otherwise we'll loose the component of the linked attribute. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 7a36cb30b716d56b84e894851c1a18e9eb3a0964) --- .../samba4.blackbox.test_primary_group | 2 -- source4/dsdb/samdb/ldb_modules/samldb.c | 29 ++++++++++++++----- 2 files changed, 21 insertions(+), 10 deletions(-) delete mode 100644 selftest/knownfail.d/samba4.blackbox.test_primary_group diff --git a/selftest/knownfail.d/samba4.blackbox.test_primary_group b/selftest/knownfail.d/samba4.blackbox.test_primary_group deleted file mode 100644 index 779f6808c977..000000000000 --- a/selftest/knownfail.d/samba4.blackbox.test_primary_group +++ /dev/null @@ -1,2 +0,0 @@ -^samba4.blackbox.test_primary_group.dbcheck.*run1 -^samba4.blackbox.test_primary_group.dbcheck.*run2 diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 81d8c96437c9..49c63cf12720 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -1642,9 +1642,14 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) struct ldb_result *res, *group_res; struct ldb_message_element *el; struct ldb_message *msg; + uint32_t search_flags = + DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_EXTENDED_DN; uint32_t prev_rid, new_rid, uac; struct dom_sid *prev_sid, *new_sid; struct ldb_dn *prev_prim_group_dn, *new_prim_group_dn; + const char *new_prim_group_dn_ext_str = NULL; + struct ldb_dn *user_dn = NULL; + const char *user_dn_ext_str = NULL; int ret; const char * const noattrs[] = { NULL }; @@ -1658,10 +1663,15 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) /* Fetch information from the existing object */ ret = dsdb_module_search_dn(ac->module, ac, &res, ac->msg->dn, attrs, - DSDB_FLAG_NEXT_MODULE, ac->req); + search_flags, ac->req); if (ret != LDB_SUCCESS) { return ret; } + user_dn = res->msgs[0]->dn; + user_dn_ext_str = ldb_dn_get_extended_linearized(ac, user_dn, 1); + if (user_dn_ext_str == NULL) { + return ldb_operr(ldb); + } uac = ldb_msg_find_attr_as_uint(res->msgs[0], "userAccountControl", 0); @@ -1725,7 +1735,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) ret = dsdb_module_search(ac->module, ac, &group_res, ldb_get_default_basedn(ldb), LDB_SCOPE_SUBTREE, - noattrs, DSDB_FLAG_NEXT_MODULE, + noattrs, search_flags, ac->req, "(objectSid=%s)", ldap_encode_ndr_dom_sid(ac, prev_sid)); @@ -1745,7 +1755,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) ret = dsdb_module_search(ac->module, ac, &group_res, ldb_get_default_basedn(ldb), LDB_SCOPE_SUBTREE, - noattrs, DSDB_FLAG_NEXT_MODULE, + noattrs, search_flags, ac->req, "(objectSid=%s)", ldap_encode_ndr_dom_sid(ac, new_sid)); @@ -1758,11 +1768,16 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) return LDB_ERR_UNWILLING_TO_PERFORM; } new_prim_group_dn = group_res->msgs[0]->dn; + new_prim_group_dn_ext_str = ldb_dn_get_extended_linearized(ac, + new_prim_group_dn, 1); + if (new_prim_group_dn_ext_str == NULL) { + return ldb_operr(ldb); + } /* We need to be already a normal member of the new primary * group in order to be successful. */ el = samdb_find_attribute(ldb, res->msgs[0], "memberOf", - ldb_dn_get_linearized(new_prim_group_dn)); + new_prim_group_dn_ext_str); if (el == NULL) { return LDB_ERR_UNWILLING_TO_PERFORM; } @@ -1774,8 +1789,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) } msg->dn = new_prim_group_dn; - ret = samdb_msg_add_delval(ldb, msg, msg, "member", - ldb_dn_get_linearized(ac->msg->dn)); + ret = samdb_msg_add_delval(ldb, msg, msg, "member", user_dn_ext_str); if (ret != LDB_SUCCESS) { return ret; } @@ -1793,8 +1807,7 @@ static int samldb_prim_group_change(struct samldb_ctx *ac) } msg->dn = prev_prim_group_dn; - ret = samdb_msg_add_addval(ldb, msg, msg, "member", - ldb_dn_get_linearized(ac->msg->dn)); + ret = samdb_msg_add_addval(ldb, msg, msg, "member", user_dn_ext_str); if (ret != LDB_SUCCESS) { return ret; } -- 2.17.1 From 788461553711149f3dd52c79a11f52de40448fe7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Oct 2018 15:56:43 +1300 Subject: [PATCH 17/17] dsdb: Add comments explaining the limitations of our current backlink behaviour BUG: https://bugzilla.samba.org/show_bug.cgi?id=13418 Signed-off-by: Andrew Bartlett Reviewed-by: Tim Beale Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Tue Oct 30 10:32:51 CET 2018 on sn-devel-144 (cherry picked from commit 852e1db12b0afa04a738c03bb2609c084fe96a7f) --- .../samdb/ldb_modules/linked_attributes.c | 18 +++++++++++++- .../dsdb/samdb/ldb_modules/repl_meta_data.c | 24 ++++++++++++++----- testprogs/blackbox/test_primary_group.sh | 6 ++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/linked_attributes.c b/source4/dsdb/samdb/ldb_modules/linked_attributes.c index c6beb25e58b4..c3f1a1484baa 100644 --- a/source4/dsdb/samdb/ldb_modules/linked_attributes.c +++ b/source4/dsdb/samdb/ldb_modules/linked_attributes.c @@ -25,7 +25,23 @@ * * Component: ldb linked_attributes module * - * Description: Module to ensure linked attribute pairs remain in sync + * Description: Module to ensure linked attribute pairs (i.e. forward-links + * and backlinks) remain in sync. + * + * Backlinks are 'plain' links (without extra metadata). When the link target + * object is modified (e.g. renamed), we use the backlinks to keep the link + * source object updated. Note there are some cases where we can't do this: + * - one-way links, which don't have a corresponding backlink + * - two-way deactivated links, i.e. when a user is removed from a group, + * the forward 'member' link still exists (but is inactive), however, the + * 'memberOf' backlink is deleted. + * In these cases, we can end up with a dangling forward link which is + * incorrect (i.e. the target has been renamed or deleted). We have dbcheck + * rules to detect and fix this, and cope otherwise by filtering at runtime + * (i.e. in the extended_dn module). + * + * See also repl_meta_data.c, which handles updating links for deleted + * objects, as well as link changes received from another DC. * * Author: Andrew Bartlett */ diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 5bdc57366da6..c2eafd0a5215 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -4367,6 +4367,10 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request - preserved if in above list, or is rDN - remove all linked attribs from this object - remove all links from other objects to this object + (note we use the backlinks to do this, so we won't find one-way + links that still point to this object, or deactivated two-way + links, i.e. 'member' after the user has been removed from the + group) - add lastKnownParent - update replPropertyMetaData? @@ -4488,12 +4492,12 @@ static int replmd_delete_internals(struct ldb_module *module, struct ldb_request if (sa->linkID & 1) { /* - we have a backlink in this object - that needs to be removed. We're not - allowed to remove it directly - however, so we instead setup a - modify to delete the corresponding - forward link + * we have a backlink in this object + * that needs to be removed. We're not + * allowed to remove it directly + * however, so we instead setup a + * modify to delete the corresponding + * forward link */ ret = replmd_delete_remove_link(module, schema, replmd_private, @@ -7626,6 +7630,14 @@ static int replmd_delete_link_value(struct ldb_module *module, /* if the existing link is active, remove its backlink */ if (is_active) { + /* + * NOTE WELL: After this we will never (at runtime) be + * able to find this forward link (for instant + * removal) if/when the link target is deleted. + * + * We have dbcheck rules to cover this and cope otherwise + * by filtering at runtime (i.e. in the extended_dn module). + */ ret = replmd_add_backlink(module, replmd_private, schema, src_obj_dn, target_guid, false, attr, NULL); diff --git a/testprogs/blackbox/test_primary_group.sh b/testprogs/blackbox/test_primary_group.sh index c2d803e1d15c..0fbc287114ad 100755 --- a/testprogs/blackbox/test_primary_group.sh +++ b/testprogs/blackbox/test_primary_group.sh @@ -75,11 +75,15 @@ testit "delete '$testgroup'" $VALGRIND $PYTHON $BINDIR/samba-tool group delete " # # As we don't support phantom objects and virtual backlinks -# the deletion of the user and group cause dangling links, +# the deletion of the user prior to the group causes dangling links, # which are detected like this: # # WARNING: target DN is deleted for member in object # +# Specifically, this happens because after the member link is +# deactivated the memberOf is gone, and so there is no way to find the +# now redundant forward link to clean it up. +# testit_expect_failure "dbcheck run3" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member --fix --yes || failed=`expr $failed + 1` testit "dbcheck run4" $VALGRIND $PYTHON $BINDIR/samba-tool dbcheck --attrs=member || failed=`expr $failed + 1` -- 2.17.1