From 51b99c03f0b34283fcf167ce02387d9ab8c7a9f9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 15:35:52 +0200 Subject: [PATCH 01/15] 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 648f04944b88..0ea51977775e 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 c15f7c2074ee2705ed8b91d330a9f81c12f501da Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 17:13:13 +0200 Subject: [PATCH 02/15] 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 805a1f65fa83..08dc67082a90 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -132,7 +132,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 f94b580c9c7c73ee656592fa1e9fda66ee477634 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 16 Oct 2018 15:16:18 +0200 Subject: [PATCH 03/15] 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 3a07eee4750b..03186a4c7671 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 04/15] 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 62d2a9120a91..729d1b99a292 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -1656,6 +1656,7 @@ MODULE_INIT_FUNC(dsdb) 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 08dc67082a90..e1b0e4aa4e3e 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -135,6 +135,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 0ea51977775e..a84675843658 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 1213f8622c5f597b48122a37e487babb0c5565a5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 8 Oct 2018 17:14:28 +0200 Subject: [PATCH 05/15] 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 c64fd4c1159a..c1613e47b303 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -383,10 +383,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 as err: if self.in_transaction: -- 2.17.1 From 0cae96b957c87e6415fc3a33697f69e83c226139 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 06/15] 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 c1613e47b303..a35c4075a43f 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -60,6 +60,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 @@ -679,6 +680,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)) @@ -1294,7 +1327,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 0be59f93431e2a02eeca652876e902db3ba301fd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 07/15] 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 8a8a515cc5f7d17f52eb25b97700ab47aaa1390f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 08/15] 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 c4a41a28d046..442bc1b8cb86 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -3120,8 +3120,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; @@ -3130,8 +3131,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 @@ -3165,10 +3164,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]; @@ -3177,7 +3172,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", @@ -3213,22 +3208,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; @@ -3500,7 +3495,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 1803c8f9536fa6ef3afa7f14cc8c70e9a069865f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 09/15] 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 442bc1b8cb86..3ce32e83f722 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2402,12 +2402,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) @@ -2420,17 +2419,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 @@ -2526,15 +2518,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, @@ -2576,14 +2569,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; @@ -3222,8 +3215,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 190f6e3fe331a3d735301535f21ab397f310ff2c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 19:34:08 +0200 Subject: [PATCH 10/15] 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 3ce32e83f722..de95fee3004d 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2449,7 +2449,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 5e560c00a563ff4d3ac0b86a046b58b2816101d2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 11/15] 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 de95fee3004d..870bba2931e1 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2615,12 +2615,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) @@ -2634,16 +2633,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) { @@ -2707,7 +2700,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) { @@ -2725,8 +2718,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; @@ -2788,7 +2782,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, @@ -2822,14 +2816,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); @@ -3208,8 +3203,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 b8437c5332fdac8818dc9f8da93b9ed959ad4033 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 18:43:25 +0200 Subject: [PATCH 12/15] 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 870bba2931e1..48a0f08a4fb1 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -2874,12 +2874,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) @@ -2888,7 +2887,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; @@ -2899,11 +2897,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 @@ -3003,8 +2996,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); @@ -3012,7 +3005,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, @@ -3037,8 +3030,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); @@ -3048,7 +3041,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, @@ -3070,14 +3063,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, @@ -3196,8 +3189,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 518ca5d3c6b8cbe59a516483c4fff056ee10aabf Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 12 Oct 2018 15:56:18 +0200 Subject: [PATCH 13/15] 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 a37b55c40469..6bfced3b7d2b 100644 --- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c +++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c @@ -722,6 +722,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)) { @@ -746,6 +747,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 48a0f08a4fb1..c6968ac45dca 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); @@ -2485,6 +2487,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 @@ -3310,6 +3415,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); @@ -3455,6 +3561,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"); @@ -3475,6 +3619,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 a7995e31cc18..30741f5cb7a8 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -3808,9 +3808,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 72b6d3351ab47e2f46a422060557f4a7167026e4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 24 Aug 2018 15:33:49 +0200 Subject: [PATCH 14/15] 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 30741f5cb7a8..e69228c32c75 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -1680,9 +1680,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 }; @@ -1696,10 +1701,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); @@ -1763,7 +1773,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)); @@ -1783,7 +1793,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)); @@ -1796,11 +1806,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; } @@ -1812,8 +1827,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; } @@ -1831,8 +1845,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 5441c664d38fa06b31128aa4713ff6cced3854f8 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 30 Oct 2018 15:56:43 +1300 Subject: [PATCH 15/15] 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 57d25076a755..2568d4d17900 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 c6968ac45dca..a24b2638d483 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -4378,6 +4378,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? @@ -4515,12 +4519,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, @@ -7666,6 +7670,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