Attachment 8294 Details for Bug 9470 – Patch for master (also applies to v4-0-test)

[patch] Patch for master (also applies to v4-0-test)

tmp.diff (text/plain), 12.89 KB, created by Stefan Metzmacher on 2012-12-06 15:37:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2012-12-06 15:37:48 UTC

Size: 12.89 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/4] 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 f52dfdc578056eec5f2cb2e5395e2524c595160e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 6 Dec 2012 12:36:09 +0100
>Subject: [PATCH 2/4] 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 aa61c27..5ab914f 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
>@@ -276,6 +276,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;
>@@ -344,14 +345,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 702f6534934fc00ab554a26972912f9bd9e6d0f9 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 6 Dec 2012 15:56:26 +0100
>Subject: [PATCH 3/4] 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 d95e08ba02a37b0f528d2f2c5f754278148a9573 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 6 Dec 2012 14:04:47 +0100
>Subject: [PATCH 4/4] 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
>

Actions: View

Attachments on bug 9470: 8294 | 8300 | 8305 | 8334