[SCM] Samba Shared Repository - branch master updated

Karolin Seeger kseeger at samba.org
Tue Jan 21 11:39:05 UTC 2020


The branch, master has been updated
       via  13658324a3a CVE-2019-19344 kcc dns scavenging: Fix use after free in dns_tombstone_records_zone
       via  34a8cee348d CVE-2019-14907 lib/util: Do not print the failed to convert string into the logs
       via  86023642c39 repl_meta_data: Only reset replMetaData entry for name if we made a conflict name here
       via  9e126852a69 repl_meta_data: Do not set *rename = true unless there has been a conflict on the incoming DN
       via  512ea17983e repl_meta_data: Add comment explaining what is being renamed after the conflict is resolved
       via  2b1828276b3 CVE-2019-14902 dsdb: Change basis of descriptor module deferred processing to be GUIDs
       via  b7030f9a8bd CVE-2019-14902 repl_meta_data: Set renamed = true (and so do SD inheritance) after any rename
       via  4c62210098d CVE-2019-14902 repl_meta_data: Fix issue where inherited Security Descriptors were not replicated.
       via  520d2ae187e CVE-2019-14902 repl_meta_data: schedule SD propagation to a renamed DN
       via  3f3791765c6 CVE-2019-14902 dsdb: Ensure we honour both change->force_self and change->force_children
       via  5d714c1cea1 CVE-2019-14902 dsdb: Add comments explaining why SD propagation needs to be done here
       via  545d205e5b2 CVE-2019-14902 dsdb: Explain that descriptor_sd_propagation_recursive() is proctected by a transaction
       via  febe15ab2e1 selftest: Add test to confirm ACL inheritence really happens
       via  d64670bab82 CVE-2019-14902 selftest: Add test for a special case around replicated renames
       via  7b19e221aee CVE-2019-14902 selftest: Add test for replication of inherited security descriptors
      from  558bd7c83d0 util: Add detection of libunwind

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 13658324a3ab30213ff50c21308f287ef3a131fd
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Mon Dec 16 13:57:47 2019 +1300

    CVE-2019-19344 kcc dns scavenging: Fix use after free in dns_tombstone_records_zone
    
    ldb_msg_add_empty reallocates the underlying element array, leaving
    old_el pointing to freed memory.
    
    This patch takes two defensive copies of the ldb message, and performs
    the updates on them rather than the ldb messages in the result.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14050
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(master): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(master): Tue Jan 21 11:38:38 UTC 2020 on sn-devel-184

commit 34a8cee348d3dfea18e92a4ae829ae797a652192
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Nov 29 20:58:47 2019 +1300

    CVE-2019-14907 lib/util: Do not print the failed to convert string into the logs
    
    The string may be in another charset, or may be sensitive and
    certainly may not be terminated.  It is not safe to just print.
    
    Found by Robert Święcki using a fuzzer he wrote for smbd.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14208
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 86023642c3961f00d0e4e6c71086739d9d568276
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 18:26:11 2019 +1300

    repl_meta_data: Only reset replMetaData entry for name if we made a conflict name here
    
    We previously set it for any rename
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 9e126852a6912e545641a506491f425a987e3b80
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 18:15:16 2019 +1300

    repl_meta_data: Do not set *rename = true unless there has been a conflict on the incoming DN
    
    The normal case of a partner-sent rename is not a cause for updating the replPropertyMetaData
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 512ea17983e7cca78778d493c75b4401a438dfbb
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 17:55:13 2019 +1300

    repl_meta_data: Add comment explaining what is being renamed after the conflict is resolved
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 2b1828276b365a30131ac6ea543ac344941b8088
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Dec 12 14:44:57 2019 +1300

    CVE-2019-14902 dsdb: Change basis of descriptor module deferred processing to be GUIDs
    
    We can not process on the basis of a DN, as the DN may have changed in a rename,
    not only that this module can see, but also from repl_meta_data below.
    
    Therefore remove all the complex tree-based change processing, leaving only
    a tree-based sort of the possible objects to be changed, and a single
    stopped_dn variable containing the DN to stop processing below (after
    a no-op change).
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit b7030f9a8bd67f454c17d065d9af9199748aa6d3
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 18:26:42 2019 +1300

    CVE-2019-14902 repl_meta_data: Set renamed = true (and so do SD inheritance) after any rename
    
    Previously if there was a conflict, but the incoming object would still
    win, this was not marked as a rename, and so inheritence was not done.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 4c62210098df44c6c4cbd0a3d41734e11286106c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 26 15:50:35 2019 +1300

    CVE-2019-14902 repl_meta_data: Fix issue where inherited Security Descriptors were not replicated.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 520d2ae187e83b1cefb4d0c9bf823a051db6b14f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 18:05:54 2019 +1300

    CVE-2019-14902 repl_meta_data: schedule SD propagation to a renamed DN
    
    We need to check the SD of the parent if we rename, it is not the same as an incoming SD change.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 3f3791765c6fd554b8f7708df0101d3c58339a27
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Dec 6 17:54:23 2019 +1300

    CVE-2019-14902 dsdb: Ensure we honour both change->force_self and change->force_children
    
    If we are renaming a DN we can be in a situation where we need to
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 5d714c1cea1beadc9ca07935217e0174d9d48ac6
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 26 16:17:32 2019 +1300

    CVE-2019-14902 dsdb: Add comments explaining why SD propagation needs to be done here
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 545d205e5b203983855c10558ad3a326c13b712e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Nov 26 15:44:32 2019 +1300

    CVE-2019-14902 dsdb: Explain that descriptor_sd_propagation_recursive() is proctected by a transaction
    
    This means we can trust the DB did not change between the two search
    requests.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit febe15ab2e109721a7f8ef996c3fe4d82106c42c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Dec 16 11:29:27 2019 +1300

    selftest: Add test to confirm ACL inheritence really happens
    
    While we have a seperate test (sec_descriptor.py) that confirms inheritance in
    general we want to lock in these specific patterns as this test covers
    rename.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit d64670bab82f21fc16fb9ffb32f713c0cae2f9e3
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Dec 10 15:16:24 2019 +1300

    CVE-2019-14902 selftest: Add test for a special case around replicated renames
    
    It appears Samba is currently string-name based in the ACL inheritence code.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 7b19e221aeefb4a93990aa589636df471cb8d47e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Nov 28 17:16:16 2019 +1300

    CVE-2019-14902 selftest: Add test for replication of inherited security descriptors
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12497
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/util/charset/convert_string.c               |  38 +--
 source4/dsdb/kcc/scavenge_dns_records.c         |  51 ++-
 source4/dsdb/samdb/ldb_modules/acl_util.c       |   4 +-
 source4/dsdb/samdb/ldb_modules/descriptor.c     | 291 +++++++++--------
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c |  68 +++-
 source4/dsdb/samdb/samdb.h                      |   2 +-
 source4/selftest/tests.py                       |   5 +
 source4/torture/drs/python/repl_secdesc.py      | 400 ++++++++++++++++++++++++
 8 files changed, 684 insertions(+), 175 deletions(-)
 create mode 100644 source4/torture/drs/python/repl_secdesc.py


Changeset truncated at 500 lines:

diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c
index d274e305a0c..b725b53cb5a 100644
--- a/lib/util/charset/convert_string.c
+++ b/lib/util/charset/convert_string.c
@@ -293,31 +293,31 @@ bool convert_string_handle(struct smb_iconv_handle *ic,
 		switch(errno) {
 			case EINVAL:
 				reason="Incomplete multibyte sequence";
-				DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",
-					 reason, (const char *)src));
+				DBG_NOTICE("Conversion error: %s\n",
+					 reason);
 				break;
 			case E2BIG:
 			{
 				reason="No more room";
 				if (from == CH_UNIX) {
-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u - '%s' error: %s\n",
-						 charset_name(ic, from), charset_name(ic, to),
-						 (unsigned int)srclen, (unsigned int)destlen, (const char *)src, reason));
+					DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
+						   charset_name(ic, from), charset_name(ic, to),
+						   (unsigned int)srclen, (unsigned int)destlen, reason);
 				} else {
-					DEBUG(3,("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
-						 charset_name(ic, from), charset_name(ic, to),
-						 (unsigned int)srclen, (unsigned int)destlen, reason));
+					DBG_NOTICE("E2BIG: convert_string(%s,%s): srclen=%u destlen=%u error: %s\n",
+						   charset_name(ic, from), charset_name(ic, to),
+						   (unsigned int)srclen, (unsigned int)destlen, reason);
 				}
 				break;
 			}
 			case EILSEQ:
 				reason="Illegal multibyte sequence";
-				DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",
-					 reason, (const char *)src));
+				DBG_NOTICE("convert_string_internal: Conversion error: %s\n",
+					   reason);
 				break;
 			default:
-				DEBUG(0,("convert_string_internal: Conversion error: %s(%s)\n",
-					 reason, (const char *)src));
+				DBG_ERR("convert_string_internal: Conversion error: %s\n",
+					reason);
 				break;
 		}
 		/* smb_panic(reason); */
@@ -427,20 +427,22 @@ bool convert_string_talloc_handle(TALLOC_CTX *ctx, struct smb_iconv_handle *ic,
 		switch(errno) {
 			case EINVAL:
 				reason="Incomplete multibyte sequence";
-				DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
+				DBG_NOTICE("Conversion error: %s\n",
+					   reason);
 				break;
 			case E2BIG:
 				reason = "output buffer is too small";
-				DBG_NOTICE("convert_string_talloc: "
-					   "Conversion error: %s(%s)\n",
-					   reason, inbuf);
+				DBG_NOTICE("Conversion error: %s\n",
+					   reason);
 				break;
 			case EILSEQ:
 				reason="Illegal multibyte sequence";
-				DEBUG(3,("convert_string_talloc: Conversion error: %s(%s)\n",reason,inbuf));
+				DBG_NOTICE("Conversion error: %s\n",
+					   reason);
 				break;
 			default:
-				DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
+				DBG_ERR("Conversion error: %s\n",
+					reason);
 				break;
 		}
 		/* smb_panic(reason); */
diff --git a/source4/dsdb/kcc/scavenge_dns_records.c b/source4/dsdb/kcc/scavenge_dns_records.c
index 6c0684b3153..8e916cf7b06 100644
--- a/source4/dsdb/kcc/scavenge_dns_records.c
+++ b/source4/dsdb/kcc/scavenge_dns_records.c
@@ -128,6 +128,8 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 	struct ldb_message_element *el = NULL;
 	struct ldb_message_element *tombstone_el = NULL;
 	struct ldb_message_element *old_el = NULL;
+	struct ldb_message *new_msg = NULL;
+	struct ldb_message *old_msg = NULL;
 	int ret;
 	struct GUID guid;
 	struct GUID_txt_buf buf_guid;
@@ -184,12 +186,29 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 	 * change.  This prevents race conditions.
 	 */
 	for (i = 0; i < res->count; i++) {
-		old_el = ldb_msg_find_element(res->msgs[i], "dnsRecord");
+		old_msg = ldb_msg_copy(mem_ctx, res->msgs[i]);
+		if (old_msg == NULL) {
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+
+		old_el = ldb_msg_find_element(old_msg, "dnsRecord");
+		if (old_el == NULL) {
+			TALLOC_FREE(old_msg);
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+
 		old_el->flags = LDB_FLAG_MOD_DELETE;
+		new_msg = ldb_msg_copy(mem_ctx, old_msg);
+		if (new_msg == NULL) {
+			TALLOC_FREE(old_msg);
+			return NT_STATUS_INTERNAL_ERROR;
+		}
 
 		ret = ldb_msg_add_empty(
-		    res->msgs[i], "dnsRecord", LDB_FLAG_MOD_ADD, &el);
+		    new_msg, "dnsRecord", LDB_FLAG_MOD_ADD, &el);
 		if (ret != LDB_SUCCESS) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			return NT_STATUS_INTERNAL_ERROR;
 		}
 
@@ -197,12 +216,16 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 		status = copy_current_records(mem_ctx, old_el, el, t);
 
 		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			return NT_STATUS_INTERNAL_ERROR;
 		}
 
 		/* If nothing was expired, do nothing. */
 		if (el->num_values == old_el->num_values &&
 		    el->num_values != 0) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			continue;
 		}
 
@@ -213,14 +236,16 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 			el->values = tombstone_blob;
 			el->num_values = 1;
 
-			tombstone_el = ldb_msg_find_element(res->msgs[i],
+			tombstone_el = ldb_msg_find_element(new_msg,
 						  "dnsTombstoned");
 			if (tombstone_el == NULL) {
-				ret = ldb_msg_add_value(res->msgs[i],
+				ret = ldb_msg_add_value(new_msg,
 							"dnsTombstoned",
 							true_struct,
 							&tombstone_el);
 				if (ret != LDB_SUCCESS) {
+					TALLOC_FREE(old_msg);
+					TALLOC_FREE(new_msg);
 					return NT_STATUS_INTERNAL_ERROR;
 				}
 				tombstone_el->flags = LDB_FLAG_MOD_ADD;
@@ -234,13 +259,15 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 			 * Do not change the status of dnsTombstoned
 			 * if we found any live records
 			 */
-			ldb_msg_remove_attr(res->msgs[i],
+			ldb_msg_remove_attr(new_msg,
 					    "dnsTombstoned");
 		}
 
 		/* Set DN to the GUID in case the object was moved. */
-		el = ldb_msg_find_element(res->msgs[i], "objectGUID");
+		el = ldb_msg_find_element(new_msg, "objectGUID");
 		if (el == NULL) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			*error_string =
 			    talloc_asprintf(mem_ctx,
 					    "record has no objectGUID "
@@ -251,20 +278,24 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 
 		status = GUID_from_ndr_blob(el->values, &guid);
 		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			*error_string =
 			    discard_const_p(char, "Error: Invalid GUID.\n");
 			return NT_STATUS_INTERNAL_ERROR;
 		}
 
 		GUID_buf_string(&guid, &buf_guid);
-		res->msgs[i]->dn =
+		new_msg->dn =
 		    ldb_dn_new_fmt(mem_ctx, samdb, "<GUID=%s>", buf_guid.buf);
 
 		/* Remove the GUID so we're not trying to modify it. */
-		ldb_msg_remove_attr(res->msgs[i], "objectGUID");
+		ldb_msg_remove_attr(new_msg, "objectGUID");
 
-		ret = ldb_modify(samdb, res->msgs[i]);
+		ret = ldb_modify(samdb, new_msg);
 		if (ret != LDB_SUCCESS) {
+			TALLOC_FREE(old_msg);
+			TALLOC_FREE(new_msg);
 			*error_string =
 			    talloc_asprintf(mem_ctx,
 					    "Failed to modify dns record "
@@ -273,6 +304,8 @@ NTSTATUS dns_tombstone_records_zone(TALLOC_CTX *mem_ctx,
 					    ldb_errstring(samdb));
 			return NT_STATUS_INTERNAL_ERROR;
 		}
+		TALLOC_FREE(old_msg);
+		TALLOC_FREE(new_msg);
 	}
 
 	return NT_STATUS_OK;
diff --git a/source4/dsdb/samdb/ldb_modules/acl_util.c b/source4/dsdb/samdb/ldb_modules/acl_util.c
index 6d645b10fe2..b9931795e19 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_util.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_util.c
@@ -286,7 +286,7 @@ uint32_t dsdb_request_sd_flags(struct ldb_request *req, bool *explicit)
 
 int dsdb_module_schedule_sd_propagation(struct ldb_module *module,
 					struct ldb_dn *nc_root,
-					struct ldb_dn *dn,
+					struct GUID guid,
 					bool include_self)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
@@ -299,7 +299,7 @@ int dsdb_module_schedule_sd_propagation(struct ldb_module *module,
 	}
 
 	op->nc_root = nc_root;
-	op->dn = dn;
+	op->guid = guid;
 	op->include_self = include_self;
 
 	ret = dsdb_module_extended(module, op, NULL,
diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
index 9018b750ab5..daa08c2ebc7 100644
--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
@@ -46,9 +46,8 @@
 
 struct descriptor_changes {
 	struct descriptor_changes *prev, *next;
-	struct descriptor_changes *children;
 	struct ldb_dn *nc_root;
-	struct ldb_dn *dn;
+	struct GUID guid;
 	bool force_self;
 	bool force_children;
 	struct ldb_dn *stopped_dn;
@@ -771,7 +770,8 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
 				    current_attrs,
 				    DSDB_FLAG_NEXT_MODULE |
 				    DSDB_FLAG_AS_SYSTEM |
-				    DSDB_SEARCH_SHOW_RECYCLED,
+				    DSDB_SEARCH_SHOW_RECYCLED |
+				    DSDB_SEARCH_SHOW_EXTENDED_DN,
 				    req);
 	if (ret != LDB_SUCCESS) {
 		ldb_debug(ldb, LDB_DEBUG_ERROR,"descriptor_modify: Could not find %s\n",
@@ -832,7 +832,7 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
 		user_sd = old_sd;
 	}
 
-	sd = get_new_descriptor(module, dn, req,
+	sd = get_new_descriptor(module, current_res->msgs[0]->dn, req,
 				objectclass, parent_sd,
 				user_sd, old_sd, sd_flags);
 	if (sd == NULL) {
@@ -869,15 +869,32 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
 			return ldb_oom(ldb);
 		}
 	} else if (cmp_ret != 0) {
+		struct GUID guid;
 		struct ldb_dn *nc_root;
+		NTSTATUS status;
 
-		ret = dsdb_find_nc_root(ldb, msg, dn, &nc_root);
+		ret = dsdb_find_nc_root(ldb,
+					msg,
+					current_res->msgs[0]->dn,
+					&nc_root);
 		if (ret != LDB_SUCCESS) {
 			return ldb_oom(ldb);
 		}
 
-		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
-							  dn, false);
+		status = dsdb_get_extended_dn_guid(current_res->msgs[0]->dn,
+						   &guid,
+						   "GUID");
+		if (!NT_STATUS_IS_OK(status)) {
+			return ldb_operr(ldb);
+		}
+
+		/*
+		 * Force SD propagation on children of this record
+		 */
+		ret = dsdb_module_schedule_sd_propagation(module,
+							  nc_root,
+							  guid,
+							  false);
 		if (ret != LDB_SUCCESS) {
 			return ldb_operr(ldb);
 		}
@@ -960,16 +977,31 @@ static int descriptor_rename(struct ldb_module *module, struct ldb_request *req)
 
 	if (ldb_dn_compare(olddn, newdn) != 0) {
 		struct ldb_dn *nc_root;
+		struct GUID guid;
 
 		ret = dsdb_find_nc_root(ldb, req, newdn, &nc_root);
 		if (ret != LDB_SUCCESS) {
 			return ldb_oom(ldb);
 		}
 
-		ret = dsdb_module_schedule_sd_propagation(module, nc_root,
-							  newdn, true);
-		if (ret != LDB_SUCCESS) {
-			return ldb_operr(ldb);
+		ret = dsdb_module_guid_by_dn(module,
+					     olddn,
+					     &guid,
+					     req);
+		if (ret == LDB_SUCCESS) {
+			/*
+			 * Without disturbing any errors if the olddn
+			 * does not exit, force SD propagation on
+			 * this record (get a new inherited SD from
+			 * the potentially new parent
+			 */
+			ret = dsdb_module_schedule_sd_propagation(module,
+								  nc_root,
+								  guid,
+								  true);
+			if (ret != LDB_SUCCESS) {
+				return ldb_operr(ldb);
+			}
 		}
 	}
 
@@ -985,9 +1017,7 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
 	struct dsdb_extended_sec_desc_propagation_op *op;
 	TALLOC_CTX *parent_mem = NULL;
-	struct descriptor_changes *parent_change = NULL;
 	struct descriptor_changes *c;
-	int ret;
 
 	op = talloc_get_type(req->op.extended.data,
 			     struct dsdb_extended_sec_desc_propagation_op);
@@ -1004,32 +1034,6 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
 
 	parent_mem = descriptor_private->trans_mem;
 
-	for (c = descriptor_private->changes; c; c = c->next) {
-		ret = ldb_dn_compare(c->nc_root, op->nc_root);
-		if (ret != 0) {
-			continue;
-		}
-
-		ret = ldb_dn_compare(c->dn, op->dn);
-		if (ret == 0) {
-			if (op->include_self) {
-				c->force_self = true;
-			} else {
-				c->force_children = true;
-			}
-			return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
-		}
-
-		ret = ldb_dn_compare_base(c->dn, op->dn);
-		if (ret != 0) {
-			continue;
-		}
-
-		parent_mem = c;
-		parent_change = c;
-		break;
-	}
-
 	c = talloc_zero(parent_mem, struct descriptor_changes);
 	if (c == NULL) {
 		return ldb_module_oom(module);
@@ -1038,21 +1042,14 @@ static int descriptor_extended_sec_desc_propagation(struct ldb_module *module,
 	if (c->nc_root == NULL) {
 		return ldb_module_oom(module);
 	}
-	c->dn = ldb_dn_copy(c, op->dn);
-	if (c->dn == NULL) {
-		return ldb_module_oom(module);
-	}
+	c->guid = op->guid;
 	if (op->include_self) {
 		c->force_self = true;
 	} else {
 		c->force_children = true;
 	}
 
-	if (parent_change != NULL) {
-		DLIST_ADD_END(parent_change->children, c);
-	} else {
-		DLIST_ADD_END(descriptor_private->changes, c);
-	}
+	DLIST_ADD_END(descriptor_private->changes, c);
 
 	return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
 }
@@ -1172,38 +1169,75 @@ static int descriptor_sd_propagation_msg_sort(struct ldb_message **m1,
 	return ldb_dn_compare(dn2, dn1);
 }
 
-static int descriptor_sd_propagation_dn_sort(struct ldb_dn *dn1,
-					     struct ldb_dn *dn2)
-{
-	/*
-	 * This sorts in tree order, parents first
-	 */
-	return ldb_dn_compare(dn2, dn1);
-}
-
 static int descriptor_sd_propagation_recursive(struct ldb_module *module,
 					       struct descriptor_changes *change)
 {
-	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct ldb_result *guid_res = NULL;
 	struct ldb_result *res = NULL;
 	unsigned int i;
 	const char * const no_attrs[] = { "@__NONE__", NULL };
-	struct descriptor_changes *c;
-	struct descriptor_changes *stopped_stack = NULL;
-	enum ldb_scope scope;
+	struct ldb_dn *stopped_dn = NULL;
+	struct GUID_txt_buf guid_buf;
 	int ret;
+	bool stop = false;
 
 	/*
-	 * First confirm this object has children, or exists (depending on change->force_self)
+	 * First confirm this object has children, or exists
+	 * (depending on change->force_self)
 	 * 
 	 * LDB_SCOPE_SUBTREE searches are expensive.
 	 *
-	 * Note: that we do not search for deleted/recycled objects
+	 * We know this is safe against a rename race as we are in the
+	 * prepare_commit(), so must be in a transaction.
+	 */
+
+	/* Find the DN by GUID, as this is stable under rename */
+	ret = dsdb_module_search(module,
+				 change,
+				 &guid_res,
+				 change->nc_root,
+				 LDB_SCOPE_SUBTREE,
+				 no_attrs,
+				 DSDB_FLAG_NEXT_MODULE |
+				 DSDB_FLAG_AS_SYSTEM |
+				 DSDB_SEARCH_SHOW_DELETED |
+				 DSDB_SEARCH_SHOW_RECYCLED,
+				 NULL, /* parent_req */
+				 "(objectGUID=%s)",
+				 GUID_buf_string(&change->guid,
+						 &guid_buf));
+
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	if (guid_res->count != 1) {
+		/*
+		 * We were just given this GUID during the same
+		 * transaction, if it is missing this is a big
+		 * problem.
+		 *
+		 * Cleanup of tombstones does not trigger this module
+		 * as it just does a delete.
+		 */
+		ldb_asprintf_errstring(ldb_module_get_ctx(module),
+				       "failed to find GUID %s under %s "
+				       "for transaction-end SD inheritance: %d results",
+				       GUID_buf_string(&change->guid,
+						       &guid_buf),
+				       ldb_dn_get_linearized(change->nc_root),
+				       guid_res->count);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	/*
+	 * OK, so there was a parent, are there children?  Note: that
+	 * this time we do not search for deleted/recycled objects
 	 */
 	ret = dsdb_module_search(module,
 				 change,
 				 &res,
-				 change->dn,
+				 guid_res->msgs[0]->dn,
 				 LDB_SCOPE_ONELEVEL,
 				 no_attrs,
 				 DSDB_FLAG_NEXT_MODULE |


-- 
Samba Shared Repository



More information about the samba-cvs mailing list