The Samba-Bugzilla – Attachment 8300 Details for
Bug 9470
MMC crashes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for master (also applies to v4-0-test) v2
tmp.diff (text/plain), 18.37 KB, created by
Stefan Metzmacher
on 2012-12-07 10:44:33 UTC
(
hide
)
Description:
Patch for master (also applies to v4-0-test) v2
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2012-12-07 10:44:33 UTC
Size:
18.37 KB
patch
obsolete
>From 884eed5f232926c814423422d9fc77b3d4ce7b78 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 6 Dec 2012 12:29:49 +0100 >Subject: [PATCH 1/7] s4:dsdb/acl_read: give some variables a better name > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > source4/dsdb/samdb/ldb_modules/acl_read.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c >index 92744f2..aa61c27 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; > }; > >@@ -136,15 +136,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; > } >@@ -275,6 +275,7 @@ 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 is_untrusted = ldb_req_is_untrusted(req); > const char * const *attrs = NULL; > uint32_t instanceType; >@@ -350,27 +351,28 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) > */ > ac->sd_flags = dsdb_request_sd_flags(ac->req, NULL); > >- ac->sd = !(ldb_attr_in_list(req->op.search.attrs, "nTSecurityDescriptor")); >+ need_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"); >+ ac->added_instanceType = true; > } else { > attrs = req->op.search.attrs; > } > if (!ldb_attr_in_list(req->op.search.attrs, "objectSid")) { >- ac->object_sid = true; > attrs = ldb_attr_list_copy_add(ac, attrs, "objectSid"); >+ ac->added_objectSid = true; > } > } > >- if (ac->sd) { >+ if (need_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, "*"); > } > attrs = ldb_attr_list_copy_add(ac, attrs, "nTSecurityDescriptor"); >+ ac->added_nTSecurityDescriptor = true; > } > ac->attrs = req->op.search.attrs; > ret = ldb_build_search_req_ex(&down_req, >-- >1.7.9.5 > > >From 754d4430fd7dea2f551f408875ce89fe0b70e751 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 7 Dec 2012 11:02:49 +0100 >Subject: [PATCH 2/7] s4:dsdb/acl_read: keep the ldb_message of the sub search > (bug #9470) > >Some modules might not allocate values on the correct memory context. > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > source4/dsdb/samdb/ldb_modules/acl_read.c | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c >index aa61c27..4c2529e 100644 >--- a/source4/dsdb/samdb/ldb_modules/acl_read.c >+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c >@@ -245,6 +245,11 @@ static int aclread_callback(struct ldb_request *req, struct ldb_reply *ares) > k++; > } > } >+ /* >+ * This should not be needed, but some modules >+ * may allocate values on the wrong context... >+ */ >+ talloc_steal(ret_msg->elements, msg); > } else { > ret_msg->elements = NULL; > } >-- >1.7.9.5 > > >From 559b354fee7b0f6f9d765c33f41858a4f29d3c20 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 6 Dec 2012 12:36:09 +0100 >Subject: [PATCH 3/7] 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, >e.g. it lets the MMC Users and Computers crash. > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > source4/dsdb/samdb/ldb_modules/acl_read.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c >index 4c2529e..e3768d1 100644 >--- a/source4/dsdb/samdb/ldb_modules/acl_read.c >+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c >@@ -281,6 +281,7 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *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); > const char * const *attrs = NULL; > uint32_t instanceType; >@@ -349,14 +350,16 @@ static int aclread_search(struct ldb_module *module, struct ldb_request *req) > if (!ac->schema) { > return ldb_operr(ldb); > } >- /* >- * 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); >- >- need_sd = !(ldb_attr_in_list(req->op.search.attrs, "nTSecurityDescriptor")); >+ >+ ac->sd_flags = dsdb_request_sd_flags(ac->req, &explicit_sd_flags); >+ if (ldb_attr_in_list(req->op.search.attrs, "nTSecurityDescriptor")) { >+ need_sd = false; >+ } else if (explicit_sd_flags) { >+ need_sd = false; >+ } else { >+ need_sd = true; >+ } >+ > if (req->op.search.attrs && !ldb_attr_in_list(req->op.search.attrs, "*")) { > if (!ldb_attr_in_list(req->op.search.attrs, "instanceType")) { > attrs = ldb_attr_list_copy_add(ac, req->op.search.attrs, "instanceType"); >-- >1.7.9.5 > > >From 672f94e5bfcacd455223bd72cd9df7051fbd81d8 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 7 Dec 2012 10:08:14 +0000 >Subject: [PATCH 4/7] s4:dsdb/schema_data.c: correctly move the CN=Aggregate > attributes to msg->elements[i].values (bug #9470) > >We should keep the talloc hierarchy sane. > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > source4/dsdb/samdb/ldb_modules/schema_data.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/schema_data.c b/source4/dsdb/samdb/ldb_modules/schema_data.c >index bc9488b..3ce7ef9 100644 >--- a/source4/dsdb/samdb/ldb_modules/schema_data.c >+++ b/source4/dsdb/samdb/ldb_modules/schema_data.c >@@ -405,7 +405,11 @@ static int generate_objectClasses(struct ldb_context *ldb, struct ldb_message *m > int ret; > > for (sclass = schema->classes; sclass; sclass = sclass->next) { >- ret = ldb_msg_add_string(msg, "objectClasses", schema_class_to_description(msg, sclass)); >+ char *v = schema_class_to_description(msg, sclass); >+ if (v == NULL) { >+ return ldb_oom(ldb); >+ } >+ ret = ldb_msg_add_steal_string(msg, "objectClasses", v); > if (ret != LDB_SUCCESS) { > return ret; > } >@@ -417,9 +421,13 @@ static int generate_attributeTypes(struct ldb_context *ldb, struct ldb_message * > { > const struct dsdb_attribute *attribute; > int ret; >- >+ > for (attribute = schema->attributes; attribute; attribute = attribute->next) { >- ret = ldb_msg_add_string(msg, "attributeTypes", schema_attribute_to_description(msg, attribute)); >+ char *v = schema_attribute_to_description(msg, attribute); >+ if (v == NULL) { >+ return ldb_oom(ldb); >+ } >+ ret = ldb_msg_add_steal_string(msg, "attributeTypes", v); > if (ret != LDB_SUCCESS) { > return ret; > } >@@ -461,7 +469,7 @@ static int generate_extendedAttributeInfo(struct ldb_context *ldb, > return ldb_oom(ldb); > } > >- ret = ldb_msg_add_string(msg, "extendedAttributeInfo", val); >+ ret = ldb_msg_add_steal_string(msg, "extendedAttributeInfo", val); > if (ret != LDB_SUCCESS) { > return ret; > } >@@ -483,7 +491,7 @@ static int generate_extendedClassInfo(struct ldb_context *ldb, > return ldb_oom(ldb); > } > >- ret = ldb_msg_add_string(msg, "extendedClassInfo", val); >+ ret = ldb_msg_add_steal_string(msg, "extendedClassInfo", val); > if (ret != LDB_SUCCESS) { > return ret; > } >@@ -521,7 +529,11 @@ static int generate_possibleInferiors(struct ldb_context *ldb, struct ldb_messag > } > > for (i=0;possibleInferiors[i];i++) { >- ret = ldb_msg_add_string(msg, "possibleInferiors", possibleInferiors[i]); >+ char *v = talloc_strdup(msg, possibleInferiors[i]); >+ if (v == NULL) { >+ return ldb_oom(ldb); >+ } >+ ret = ldb_msg_add_steal_string(msg, "possibleInferiors", v); > if (ret != LDB_SUCCESS) { > return ret; > } >-- >1.7.9.5 > > >From 203f2bd3df85218f2c6d48c890b00b8a6a0e6d84 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 7 Dec 2012 10:34:58 +0100 >Subject: [PATCH 5/7] s4:dsdb/schema: fix dsdb_schema_set_el_from_ldb_msg() > (bug #9470) > >We should always update the ts_last_change. > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > source4/dsdb/schema/schema_set.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > >diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c >index e226118..31862a4 100644 >--- a/source4/dsdb/schema/schema_set.c >+++ b/source4/dsdb/schema/schema_set.c >@@ -676,13 +676,6 @@ WERROR dsdb_schema_set_el_from_ldb_msg(struct ldb_context *ldb, struct dsdb_sche > { > const char* tstring; > time_t ts; >- if (samdb_find_attribute(ldb, msg, >- "objectclass", "attributeSchema") != NULL) { >- return dsdb_set_attribute_from_ldb(ldb, schema, msg); >- } else if (samdb_find_attribute(ldb, msg, >- "objectclass", "classSchema") != NULL) { >- return dsdb_set_class_from_ldb(schema, msg); >- } > tstring = ldb_msg_find_attr_as_string(msg, "whenChanged", NULL); > /* keep a trace of the ts of the most recently changed object */ > if (tstring) { >@@ -691,6 +684,13 @@ WERROR dsdb_schema_set_el_from_ldb_msg(struct ldb_context *ldb, struct dsdb_sche > schema->ts_last_change = ts; > } > } >+ if (samdb_find_attribute(ldb, msg, >+ "objectclass", "attributeSchema") != NULL) { >+ return dsdb_set_attribute_from_ldb(ldb, schema, msg); >+ } else if (samdb_find_attribute(ldb, msg, >+ "objectclass", "classSchema") != NULL) { >+ return dsdb_set_class_from_ldb(schema, msg); >+ } > /* Don't fail on things not classes or attributes */ > return WERR_OK; > } >-- >1.7.9.5 > > >From 5b0d5755fd18c94405ea3dd47b68f1ccb888dba6 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 6 Dec 2012 15:56:26 +0100 >Subject: [PATCH 6/7] 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 <metze@samba.org> >--- > source4/dsdb/samdb/ldb_modules/operational.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > >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; > } >-- >1.7.9.5 > > >From 99a407a0aca1c84062d584e568f320014ad04767 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 6 Dec 2012 14:04:47 +0100 >Subject: [PATCH 7/7] 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 <metze@samba.org> >--- > source4/dsdb/tests/python/sec_descriptor.py | 116 +++++++++++++++++++++++++++ > 1 file changed, 116 insertions(+) > >diff --git a/source4/dsdb/tests/python/sec_descriptor.py b/source4/dsdb/tests/python/sec_descriptor.py >index aff6040..cf213ab 100755 >--- a/source4/dsdb/tests/python/sec_descriptor.py >+++ b/source4/dsdb/tests/python/sec_descriptor.py >@@ -1848,6 +1848,122 @@ 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) > > class RightsAttributesTests(DescriptorTests): > >-- >1.7.9.5 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=8300">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 9470
:
8294
|
8300
|
8305
|
8334