The branch, master has been updated via 53b7364 s4:dsdb/tests/sec_descriptor: verify the search of a windows dc join keeps working via e617a3f s4:dsdb/tests/sec_descriptor: verify the nTSecurityDescriptor and sd_flags interaction via 6bc2cae s4:dsdb/operational: fix stripping of the nTSecurityDescriptor attribute via 22bb2fd s4:dsdb/acl_read: return the nTSecurityDescriptor attr if the sd_flags control is given (bug #9470) via 4f8558f s4:dsdb/acl_read: give some variables a better name via db15fcf s4:dsdb/acl_read: fix the calculation of the attribute array for the sub search via e218161 s4:dsdb/acl_read: check the ldb_attr_list_copy_add() result via 6bcafce s4:dsdb/dirsync: fix potential talloc hierachy problems (bug #9470) from ade5bfd s4-torture: call the s4u2self tests with arcfour and aes.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 53b736444d55c4eed3abbc34974b655cc2607cd6 Author: Stefan Metzmacher <me...@samba.org> Date: Fri Dec 7 18:58:57 2012 +0100 s4:dsdb/tests/sec_descriptor: verify the search of a windows dc join keeps working This is a regression test for bug #9470. Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> Autobuild-User(master): Michael Adam <ob...@samba.org> Autobuild-Date(master): Mon Dec 10 15:41:12 CET 2012 on sn-devel-104 commit e617a3fecb797031cf5a6545d51d7e116716ab52 Author: Stefan Metzmacher <me...@samba.org> Date: Thu Dec 6 14:04:47 2012 +0100 s4:dsdb/tests/sec_descriptor: verify the nTSecurityDescriptor and sd_flags interaction This is a regression test for bug #9470. Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 6bc2caed8b3f153f92af013275f39c803f886a22 Author: Stefan Metzmacher <me...@samba.org> Date: Thu Dec 6 15:56:26 2012 +0100 s4:dsdb/operational: fix stripping of the nTSecurityDescriptor attribute If the sd_flags control is specified, we should return nTSecurityDescriptor only if the client asked for all attributes. If there's a list of only explicit attribute names, we should ignore the sd_flags control. Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 22bb2fd868b8df2244b801aeaa515a8a4036bce8 Author: Stefan Metzmacher <me...@samba.org> Date: Thu Dec 6 12:36:09 2012 +0100 s4:dsdb/acl_read: return the nTSecurityDescriptor attr if the sd_flags control is given (bug #9470) Not returning the nTSecurityDescriptor causes a lot of problems. Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 4f8558ffaf4c9fb9e350ec528ec1ce60de5f2e24 Author: Stefan Metzmacher <me...@samba.org> Date: Thu Dec 6 12:29:49 2012 +0100 s4:dsdb/acl_read: give some variables a better name Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit db15fcfa899e1fe4d6994f68ceb299921b8aa6f1 Author: Stefan Metzmacher <me...@samba.org> Date: Fri Dec 7 18:40:25 2012 +0100 s4:dsdb/acl_read: fix the calculation of the attribute array for the sub search Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit e2181617a00d7982e4e6ced1c51aa2ee8a40df26 Author: Stefan Metzmacher <me...@samba.org> Date: Fri Dec 7 18:39:29 2012 +0100 s4:dsdb/acl_read: check the ldb_attr_list_copy_add() result Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 6bcafceb750d5c4d24e2ddbef35b411bebccd66f Author: Stefan Metzmacher <me...@samba.org> Date: Fri Dec 7 19:02:10 2012 +0100 s4:dsdb/dirsync: fix potential talloc hierachy problems (bug #9470) Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> ----------------------------------------------------------------------- Summary of changes: source4/dsdb/samdb/ldb_modules/acl_read.c | 69 ++++++++++----- source4/dsdb/samdb/ldb_modules/dirsync.c | 6 +- source4/dsdb/samdb/ldb_modules/operational.c | 14 +++- source4/dsdb/tests/python/sec_descriptor.py | 123 ++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 26 deletions(-) Changeset truncated at 500 lines: diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c index 2b20f24..9955451 100644 --- a/source4/dsdb/samdb/ldb_modules/acl_read.c +++ b/source4/dsdb/samdb/ldb_modules/acl_read.c @@ -45,9 +45,9 @@ struct aclread_context { const char * const *attrs; const struct dsdb_schema *schema; uint32_t sd_flags; - bool sd; - bool instance_type; - bool object_sid; + bool added_nTSecurityDescriptor; + bool added_instanceType; + bool added_objectSid; bool indirsync; }; @@ -145,15 +145,15 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) is_instancetype = ldb_attr_cmp("instanceType", msg->elements[i].name) == 0; /* these attributes were added to perform access checks and must be removed */ - if (is_objectsid && ac->object_sid) { + if (is_objectsid && ac->added_objectSid) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } - if (is_instancetype && ac->instance_type) { + if (is_instancetype && ac->added_instanceType) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } - if (is_sd && ac->sd) { + if (is_sd && ac->added_nTSecurityDescriptor) { aclread_mark_inaccesslible(&msg->elements[i]); continue; } @@ -295,7 +295,11 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) uint32_t flags = ldb_req_get_custom_flags(req); struct ldb_result *res; struct aclread_private *p; + bool need_sd = false; + bool explicit_sd_flags = false; bool is_untrusted = ldb_req_is_untrusted(req); + static const char * const _all_attrs[] = { "*", NULL }; + bool all_attrs = false; const char * const *attrs = NULL; uint32_t instanceType; static const char *acl_attrs[] = { @@ -363,35 +367,58 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) if (!ac->schema) { return ldb_operr(ldb); } + + attrs = req->op.search.attrs; + if (attrs == NULL) { + all_attrs = true; + attrs = _all_attrs; + } else if (attrs[0] == NULL) { + all_attrs = true; + attrs = _all_attrs; + } else if (ldb_attr_in_list(attrs, "*")) { + all_attrs = true; + } + /* * In theory we should also check for the SD control but control verification is * expensive so we'd better had the ntsecuritydescriptor to the list of * searched attribute and then remove it ! */ - ac->sd_flags = dsdb_request_sd_flags(ac->req, NULL); + ac->sd_flags = dsdb_request_sd_flags(ac->req, &explicit_sd_flags); - ac->sd = !(ldb_attr_in_list(req->op.search.attrs, "nTSecurityDescriptor")); - if (req->op.search.attrs && !ldb_attr_in_list(req->op.search.attrs, "*")) { - if (!ldb_attr_in_list(req->op.search.attrs, "instanceType")) { - ac->instance_type = true; - attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "instanceType"); - } else { - attrs = req->op.search.attrs; + if (ldb_attr_in_list(attrs, "nTSecurityDescriptor")) { + need_sd = false; + } else if (explicit_sd_flags && all_attrs) { + need_sd = false; + } else { + need_sd = true; + } + + if (!all_attrs) { + if (!ldb_attr_in_list(attrs, "instanceType")) { + attrs = ldb_attr_list_copy_add(ac, attrs, "instanceType"); + if (attrs == NULL) { + return ldb_oom(ldb); + } + ac->added_instanceType = true; } if (!ldb_attr_in_list(req->op.search.attrs, "objectSid")) { - ac->object_sid = true; attrs = ldb_attr_list_copy_add(ac, attrs, "objectSid"); + if (attrs == NULL) { + return ldb_oom(ldb); + } + ac->added_objectSid = true; } } - if (ac->sd) { - /* avoid replacing all attributes with nTSecurityDescriptor - * if attribute list is empty */ - if (!attrs) { - attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "*"); - } + if (need_sd) { attrs = ldb_attr_list_copy_add(ac, attrs, "nTSecurityDescriptor"); + if (attrs == NULL) { + return ldb_oom(ldb); + } + ac->added_nTSecurityDescriptor = true; } + ac->attrs = req->op.search.attrs; ret = ldb_build_search_req_ex(&down_req, ldb, ac, diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index f75ec52..e769948 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -240,7 +240,7 @@ static int dirsync_filter_entry(struct ldb_request *req, talloc_steal(newmsg->elements, el->name); talloc_steal(newmsg->elements, el->values); - talloc_free(msg); + talloc_steal(newmsg->elements, msg); return ldb_module_send_entry(dsc->req, msg, controls); } @@ -653,6 +653,7 @@ skip_link: continue; } } + talloc_steal(newmsg->elements, msg); /* * Here we run through the list of attributes returned @@ -685,10 +686,9 @@ skip_link: if (val > dsc->highestUSN) { dsc->highestUSN = val; } - talloc_free(msg); return ldb_module_send_entry(dsc->req, newmsg, controls); } else { - talloc_free(msg); + talloc_free(newmsg); return LDB_SUCCESS; } } diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c index 4ce8b8f..c642ad8 100644 --- a/source4/dsdb/samdb/ldb_modules/operational.c +++ b/source4/dsdb/samdb/ldb_modules/operational.c @@ -721,10 +721,20 @@ static int operational_search_post_process(struct ldb_module *module, continue; } case OPERATIONAL_SD_FLAGS: - if (controls_flags->sd || - ldb_attr_in_list(attrs_from_user, operational_remove[i].attr)) { + if (ldb_attr_in_list(attrs_from_user, operational_remove[i].attr)) { continue; } + if (controls_flags->sd) { + if (attrs_from_user == NULL) { + continue; + } + if (attrs_from_user[0] == NULL) { + continue; + } + if (ldb_attr_in_list(attrs_from_user, "*")) { + continue; + } + } ldb_msg_remove_attr(msg, operational_remove[i].attr); break; } diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py index aff6040..78cd052 100755 --- a/source4/dsdb/tests/python/sec_descriptor.py +++ b/source4/dsdb/tests/python/sec_descriptor.py @@ -1848,6 +1848,129 @@ class SdFlagsDescriptorTests(DescriptorTests): self.assertFalse("S:" in desc_sddl) self.assertFalse("G:" in desc_sddl) + def test_311(self): + sd_flags = (SECINFO_OWNER | + SECINFO_GROUP | + SECINFO_DACL | + SECINFO_SACL) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + [], controls=None) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name"], controls=None) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertFalse("nTSecurityDescriptor" in res[0]) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + [], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["*"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "*"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["*", "nTSecurityDescriptor"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "name"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name", "nTSecurityDescriptor"], controls=["sd_flags:1:%d" % (sd_flags)]) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["name", "nTSecurityDescriptor"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, + ["nTSecurityDescriptor", "name"], controls=None) + self.assertTrue("nTSecurityDescriptor" in res[0]) + tmp = res[0]["nTSecurityDescriptor"][0] + sd = ndr_unpack(security.descriptor, tmp) + sddl = sd.as_sddl(self.sd_utils.domain_sid) + self.assertTrue("O:" in sddl) + self.assertTrue("G:" in sddl) + self.assertTrue("D:" in sddl) + self.assertTrue("S:" in sddl) + + def test_312(self): + """This search is done by the windows dc join...""" + + res = self.ldb_admin.search(self.base_dn, SCOPE_BASE, None, ["1.1"], + controls=["extended_dn:1:0", "sd_flags:1:0", "search_options:1:1"]) + self.assertFalse("nTSecurityDescriptor" in res[0]) class RightsAttributesTests(DescriptorTests): -- Samba Shared Repository