diff options
author | Karolin Seeger <kseeger@samba.org> | 2020-01-21 11:01:42 +0100 |
---|---|---|
committer | Karolin Seeger <kseeger@samba.org> | 2020-01-21 11:01:42 +0100 |
commit | 5f73530222071af7cf6d9fa044cde86217fec112 (patch) | |
tree | debd07b558859b81bf7815d2638ab8a5a60686a8 | |
parent | c5dee3fcee618c471d6bec02340eddef5dd68855 (diff) | |
parent | 01a4dd8ea2b7503270221beef02d21b0a2bc5ffa (diff) | |
download | samba-5f73530222071af7cf6d9fa044cde86217fec112.tar.gz |
Merge tag 'samba-4.11.5' into v4-11-test
samba: tag release samba-4.11.5
-rw-r--r-- | WHATSNEW.txt | 76 | ||||
-rw-r--r-- | lib/util/charset/convert_string.c | 38 | ||||
-rw-r--r-- | source4/dsdb/kcc/scavenge_dns_records.c | 51 | ||||
-rw-r--r-- | source4/dsdb/samdb/ldb_modules/acl_util.c | 4 | ||||
-rw-r--r-- | source4/dsdb/samdb/ldb_modules/descriptor.c | 291 | ||||
-rw-r--r-- | source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 55 | ||||
-rw-r--r-- | source4/dsdb/samdb/samdb.h | 2 | ||||
-rwxr-xr-x | source4/selftest/tests.py | 5 | ||||
-rw-r--r-- | source4/torture/drs/python/repl_secdesc.py | 400 |
9 files changed, 751 insertions, 171 deletions
diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 830081446ab..99272550643 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,4 +1,76 @@ ============================== + Release Notes for Samba 4.11.5 + January 21, 2020 + ============================== + + +This is a security release in order to address the following defects: + +o CVE-2019-14902: Replication of ACLs set to inherit down a subtree on AD + Directory not automatic. +o CVE-2019-14907: Crash after failed character conversion at log level 3 or + above. +o CVE-2019-19344: Use after free during DNS zone scavenging in Samba AD DC. + + +======= +Details +======= + +o CVE-2019-14902: + The implementation of ACL inheritance in the Samba AD DC was not complete, + and so absent a 'full-sync' replication, ACLs could get out of sync between + domain controllers. + +o CVE-2019-14907: + When processing untrusted string input Samba can read past the end of the + allocated buffer when printing a "Conversion error" message to the logs. + +o CVE-2019-19344: + During DNS zone scavenging (of expired dynamic entries) there is a read of + memory after it has been freed. + +For more details and workarounds, please refer to the security advisories. + + +Changes since 4.11.4: +--------------------- + +o Andrew Bartlett <abartlet@samba.org> + * BUG 12497: CVE-2019-14902: Replication of ACLs down subtree on AD Directory + not automatic. + * BUG 14208: CVE-2019-14907: lib/util: Do not print the failed to convert + string into the logs. + +o Gary Lockyer <gary@catalyst.net.nz> + * BUG 14050: CVE-2019-19344: kcc dns scavenging: Fix use after free in + dns_tombstone_records_zone. + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical IRC channel on irc.freenode.net. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the "Samba 4.1 and newer" product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + + ============================== Release Notes for Samba 4.11.4 December 16, 2019 ============================== @@ -76,8 +148,8 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- + ============================== Release Notes for Samba 4.11.3 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 | @@ -1211,26 +1245,55 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, NULL, /* parent_req */ "(objectClass=*)"); if (ret != LDB_SUCCESS) { + /* + * LDB_ERR_NO_SUCH_OBJECT, say if the DN was a deleted + * object, is ignored by the caller + */ return ret; } if (res->count == 0 && !change->force_self) { + /* All done, no children */ TALLOC_FREE(res); return LDB_SUCCESS; - } else if (res->count == 0 && change->force_self) { - scope = LDB_SCOPE_BASE; - } else { - scope = LDB_SCOPE_SUBTREE; } /* + * First, if we are in force_self mode (eg renamed under new + * parent) then apply the SD to the top object + */ + if (change->force_self) { + ret = descriptor_sd_propagation_object(module, + guid_res->msgs[0], + &stop); + if (ret != LDB_SUCCESS) { + TALLOC_FREE(guid_res); + return ret; + } + + if (stop == true && !change->force_children) { + /* There was no change, nothing more to do */ + TALLOC_FREE(guid_res); + return LDB_SUCCESS; + } + + if (res->count == 0) { + /* All done! */ + TALLOC_FREE(guid_res); + return LDB_SUCCESS; + } + } + + /* + * Look for children + * * Note: that we do not search for deleted/recycled objects */ ret = dsdb_module_search(module, change, &res, - change->dn, - scope, + guid_res->msgs[0]->dn, + LDB_SCOPE_SUBTREE, no_attrs, DSDB_FLAG_NEXT_MODULE | DSDB_FLAG_AS_SYSTEM, @@ -1243,83 +1306,39 @@ static int descriptor_sd_propagation_recursive(struct ldb_module *module, TYPESAFE_QSORT(res->msgs, res->count, descriptor_sd_propagation_msg_sort); - for (c = change->children; c; c = c->next) { - struct ldb_message *msg = NULL; - - BINARY_ARRAY_SEARCH_P(res->msgs, res->count, dn, c->dn, - descriptor_sd_propagation_dn_sort, - msg); - - if (msg == NULL) { - ldb_debug(ldb, LDB_DEBUG_WARNING, - "descriptor_sd_propagation_recursive: " - "%s not found under %s", - ldb_dn_get_linearized(c->dn), - ldb_dn_get_linearized(change->dn)); - continue; - } - - msg->elements = (struct ldb_message_element *)c; - } - - DLIST_ADD(stopped_stack, change); - - if (change->force_self) { - i = 0; - } else { - i = 1; - } - - for (; i < res->count; i++) { - struct descriptor_changes *cur; - bool stop = false; - - cur = talloc_get_type(res->msgs[i]->elements, - struct descriptor_changes); - res->msgs[i]->elements = NULL; - res->msgs[i]->num_elements = 0; - - if (cur != NULL) { - DLIST_REMOVE(change->children, cur); - } - - for (c = stopped_stack; c; c = stopped_stack) { - ret = ldb_dn_compare_base(c->dn, - res->msgs[i]->dn); - if (ret == 0) { - break; - } - - c->stopped_dn = NULL; - DLIST_REMOVE(stopped_stack, c); - } - - if (cur != NULL) { - DLIST_ADD(stopped_stack, cur); - } - - if (stopped_stack->stopped_dn != NULL) { - ret = ldb_dn_compare_base(stopped_stack->stopped_dn, + /* We start from 1, the top object has been done */ + for (i = 1; i < res->count; i++) { + /* + * ldb_dn_compare_base() does not match for NULL but + * this is clearer + */ + if (stopped_dn != NULL) { + ret = ldb_dn_compare_base(stopped_dn, res->msgs[i]->dn); + /* + * Skip further processing of this + * sub-subtree + */ if (ret == 0) { continue; } - stopped_stack->stopped_dn = NULL; } - - ret = descriptor_sd_propagation_object(module, res->msgs[i], + ret = descriptor_sd_propagation_object(module, + res->msgs[i], &stop); if (ret != LDB_SUCCESS) { return ret; } - if (cur != NULL && cur->force_children) { - continue; - } - if (stop) { - stopped_stack->stopped_dn = res->msgs[i]->dn; - continue; + /* + * If this child didn't change, then nothing + * under it needs to change + * + * res has been sorted into tree order so the + * next few entries can be skipped + */ + stopped_dn = res->msgs[i]->dn; } } diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 1d800feb0c1..a2bef0256bc 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -5590,9 +5590,19 @@ static int replmd_replicated_apply_add(struct replmd_replicated_request *ar) replmd_ldb_message_sort(msg, ar->schema); if (!remote_isDeleted) { + /* + * Ensure any local ACL inheritence is applied from + * the parent object. + * + * This is needed because descriptor is above + * repl_meta_data in the module stack, so this will + * not be trigered 'naturally' by the flow of + * operations. + */ ret = dsdb_module_schedule_sd_propagation(ar->module, ar->objs->partition_dn, - msg->dn, true); + ar->objs->objects[ar->index_current].object_guid, + true); if (ret != LDB_SUCCESS) { return replmd_replicated_request_error(ar, ret); } @@ -6188,6 +6198,19 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) * replmd_replicated_apply_search_callback()) */ ret = replmd_replicated_handle_rename(ar, msg, ar->req, &renamed); + + /* + * This looks strange, but we must set this after any + * rename, otherwise the SD propegation will not + * happen (which might matter if we have a new parent) + * + * The additional case of calling + * replmd_op_name_modify_callback (below) is: + * - a no-op if there was no name change + * and + * - called in the default case regardless. + */ + renamed = true; } if (ret != LDB_SUCCESS) { @@ -6353,13 +6376,39 @@ static int replmd_replicated_apply_merge(struct replmd_replicated_request *ar) ar->index_current, msg->num_elements); if (renamed) { - sd_updated = true; + /* + * This is an new name for this object, so we must + * inherit from the parent + * + * This is needed because descriptor is above + * repl_meta_data in the module stack, so this will + * not be trigered 'naturally' by the flow of + * operations. + */ + ret = dsdb_module_schedule_sd_propagation(ar->module, + ar->objs->partition_dn, + ar->objs->objects[ar->index_current].object_guid, + true); + if (ret != LDB_SUCCESS) { + return ldb_operr(ldb); + } } if (sd_updated && !isDeleted) { + /* + * This is an existing object, so there is no need to + * inherit from the parent, but we must inherit any + * incoming changes to our child objects. + * + * This is needed because descriptor is above + * repl_meta_data in the module stack, so this will + * not be trigered 'naturally' by the flow of + * operations. + */ ret = dsdb_module_schedule_sd_propagation(ar->module, ar->objs->partition_dn, - msg->dn, true); + ar->objs->objects[ar->index_current].object_guid, + false); if (ret != LDB_SUCCESS) { return ldb_operr(ldb); } diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index fd8d4e4497e..b0fdfeb3967 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -292,7 +292,7 @@ struct dsdb_fsmo_extended_op { #define DSDB_EXTENDED_SEC_DESC_PROPAGATION_OID "1.3.6.1.4.1.7165.4.4.7" struct dsdb_extended_sec_desc_propagation_op { struct ldb_dn *nc_root; - struct ldb_dn *dn; + struct GUID guid; bool include_self; }; diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index f8fb0074c02..ae2b10ae659 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1128,6 +1128,11 @@ for env in ['vampire_dc', 'promoted_dc']: extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) + planoldpythontestsuite(env, "repl_secdesc", + name="samba4.drs.repl_secdesc.python(%s)" % env, + extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], + environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, + extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) planoldpythontestsuite(env, "repl_move", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], name="samba4.drs.repl_move.python(%s)" % env, diff --git a/source4/torture/drs/python/repl_secdesc.py b/source4/torture/drs/python/repl_secdesc.py new file mode 100644 index 00000000000..58212907e23 --- /dev/null +++ b/source4/torture/drs/python/repl_secdesc.py @@ -0,0 +1,400 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +# +# Unix SMB/CIFS implementation. +# Copyright (C) Catalyst.Net Ltd. 2017 +# Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +import drs_base +import ldb +import samba +from samba import sd_utils +from ldb import LdbError + +class ReplAclTestCase(drs_base.DrsBaseTestCase): + + def setUp(self): + super(ReplAclTestCase, self).setUp() + self.mod = "(A;CIOI;GA;;;SY)" + self.mod_becomes = "(A;OICIIO;GA;;;SY)" + self.mod_inherits_as = "(A;OICIIOID;GA;;;SY)" + + self.sd_utils_dc1 = sd_utils.SDUtils(self.ldb_dc1) + self.sd_utils_dc2 = sd_utils.SDUtils(self.ldb_dc2) + + self.ou = samba.tests.create_test_ou(self.ldb_dc1, + "test_acl_inherit") + + # disable replication for the tests so we can control at what point + # the DCs try to replicate + self._disable_all_repl(self.dnsname_dc1) + self._disable_all_repl(self.dnsname_dc2) + + # make sure DCs are synchronized before the test + self._net_drs_replicate(DC=self.dnsname_dc2, fromDC=self.dnsname_dc1, forced=True) + self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2, forced=True) + + def tearDown(self): + self.ldb_dc1.delete(self.ou, ["tree_delete:1"]) + + # re-enable replication + self._enable_all_repl(self.dnsname_dc1) + self._enable_all_repl(self.dnsname_dc2) + + super(ReplAclTestCase, self).tearDown() + + def test_acl_inheirt_new_object_1_pass(self): + # Set the inherited ACL on the parent OU + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set stuck as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Make a new object + dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Assert ACL replicated as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(self.ou)) + + # Confirm inherited ACLs are identical and were inherited + + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), + self.sd_utils_dc2.get_sd_as_sddl(dn)) + + def test_acl_inheirt_new_object(self): + # Set the inherited ACL on the parent OU + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set stuck as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Make a new object + dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Assert ACL replicated as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(self.ou)) + + # Confirm inherited ACLs are identical and were inheritied + + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), + self.sd_utils_dc2.get_sd_as_sddl(dn)) + + def test_acl_inherit_existing_object(self): + # Make a new object + dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) + + try: + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=dn, + attrs=[]) + self.fail() + except LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm it is now replicated + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=dn, + attrs=[]) + + # Set the inherited ACL on the parent OU + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set stuck as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm inherited ACLs are identical and were inherited + + # Assert ACL replicated as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(self.ou)) + + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), + self.sd_utils_dc2.get_sd_as_sddl(dn)) + + def test_acl_inheirt_existing_object_1_pass(self): + # Make a new object + dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + self.ldb_dc1.add({"dn": dn, "objectclass": "organizationalUnit"}) + + try: + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=dn, + attrs=[]) + self.fail() + except LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) + + # Set the inherited ACL on the parent OU + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Assert ACL replicated as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(self.ou)) + + # Confirm inherited ACLs are identical and were inherited + + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(dn), + self.sd_utils_dc2.get_sd_as_sddl(dn)) + + def test_acl_inheirt_renamed_object(self): + # Make a new object + new_ou = samba.tests.create_test_ou(self.ldb_dc1, + "acl_test_l2") + + sub_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + + try: + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=new_ou, + attrs=[]) + self.fail() + except LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm it is now replicated + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=new_ou, + attrs=[]) + + # Set the inherited ACL on the parent OU on DC1 + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Assert ACL replicated as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(self.ou)) + + # Rename to under self.ou + + self.ldb_dc1.rename(new_ou, sub_ou_dn) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm inherited ACLs are identical and were inherited + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), + self.sd_utils_dc2.get_sd_as_sddl(sub_ou_dn)) + + + def test_acl_inheirt_renamed_child_object(self): + # Make a new OU + new_ou = samba.tests.create_test_ou(self.ldb_dc1, + "acl_test_l2") + + # Here is where the new OU will end up at the end. + sub2_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l2,%s" % self.ou) + + sub3_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % new_ou) + sub3_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l3,%s" % sub2_ou_dn_final) + + self.ldb_dc1.add({"dn": sub3_ou_dn, + "objectclass": "organizationalUnit"}) + + sub4_ou_dn = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn) + sub4_ou_dn_final = ldb.Dn(self.ldb_dc1, "OU=l4,%s" % sub3_ou_dn_final) + + self.ldb_dc1.add({"dn": sub4_ou_dn, + "objectclass": "organizationalUnit"}) + + try: + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=new_ou, + attrs=[]) + self.fail() + except LdbError as err: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_NO_SUCH_OBJECT) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm it is now replicated + self.ldb_dc2.search(scope=ldb.SCOPE_BASE, + base=new_ou, + attrs=[]) + + # + # Given a tree new_ou -> l3 -> l4 + # + + # Set the inherited ACL on the grandchild OU (l3) on DC1 + self.sd_utils_dc1.dacl_add_ace(sub3_ou_dn, self.mod) + + # Assert ACL set stuck as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn)) + + # Rename new_ou (l2) to under self.ou (this must happen second). If the + # inheritence between l3 and l4 is name-based, this could + # break. + + # The tree is now self.ou -> l2 -> l3 -> l4 + + self.ldb_dc1.rename(new_ou, sub2_ou_dn_final) + + # Assert ACL set remained as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Confirm set ACLs (on l3 ) are identical and were inherited + self.assertIn(self.mod_becomes, + self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub3_ou_dn_final), + self.sd_utils_dc2.get_sd_as_sddl(sub3_ou_dn_final)) + + # Confirm inherited ACLs (from l3 to l4) are identical + # and where inherited + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub4_ou_dn_final), + self.sd_utils_dc2.get_sd_as_sddl(sub4_ou_dn_final)) + + + def test_acl_inheirt_renamed_object_in_conflict(self): + # Make a new object to be renamed under self.ou + new_ou = samba.tests.create_test_ou(self.ldb_dc1, + "acl_test_l2") + + # Make a new OU under self.ou (on DC2) + sub_ou_dn = ldb.Dn(self.ldb_dc2, "OU=l2,%s" % self.ou) + self.ldb_dc2.add({"dn": sub_ou_dn, + "objectclass": "organizationalUnit"}) + + # Set the inherited ACL on the parent OU + self.sd_utils_dc1.dacl_add_ace(self.ou, self.mod) + + # Assert ACL set stuck as expected + self.assertIn(self.mod_becomes, + self.sd_utils_dc1.get_sd_as_sddl(self.ou)) + + # Replicate to DC2 + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + # Rename to under self.ou + self.ldb_dc1.rename(new_ou, sub_ou_dn) + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) + + # Replicate to DC2 (will cause a conflict, DC1 to win, version + # is higher since named twice) + + self._net_drs_replicate(DC=self.dnsname_dc2, + fromDC=self.dnsname_dc1, + forced=True) + + children = self.ldb_dc2.search(scope=ldb.SCOPE_ONELEVEL, + base=self.ou, + attrs=[]) + for child in children: + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc2.get_sd_as_sddl(child.dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn), + self.sd_utils_dc2.get_sd_as_sddl(child.dn)) + + # Replicate back + self._net_drs_replicate(DC=self.dnsname_dc1, + fromDC=self.dnsname_dc2, + forced=True) + + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(sub_ou_dn)) + + for child in children: + self.assertIn(self.mod_inherits_as, + self.sd_utils_dc1.get_sd_as_sddl(child.dn)) + self.assertEquals(self.sd_utils_dc1.get_sd_as_sddl(child.dn), + self.sd_utils_dc2.get_sd_as_sddl(child.dn)) |