Attachment 10084 Details for Bug 10700 – Patches for v4-1-test

[patch] Patches for v4-1-test

tmp41.diff (text/plain), 12.35 KB, created by Stefan Metzmacher on 2014-07-08 13:32:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-07-08 13:32:37 UTC

Size: 12.35 KB

patch

obsolete

>From 274f4a5b95391d2c19c5ae785a8085393ee1d1b1 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 18 Aug 2013 19:37:56 +0000
>Subject: [PATCH 1/6] samdb: Fix CID 1034910 Dereference before null check
>
>strncmp("tdb://", sam_name, 6) dereferences sam_name. Check for
>NULL before that.
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 6417d9e0355f840ca4cf3b740ad5aabfc534d834)
>---
> source4/dsdb/samdb/ldb_modules/partition_metadata.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>index 5826ac2..c67d6cf 100644
>--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>@@ -199,13 +199,13 @@ static int partition_metadata_open(struct ldb_module *module, bool create)
> 	}
> 
> 	sam_name = (const char *)ldb_get_opaque(ldb, "ldb_url");
>-	if (strncmp("tdb://", sam_name, 6) == 0) {
>-		sam_name += 6;
>-	}
> 	if (!sam_name) {
> 		talloc_free(tmp_ctx);
> 		return ldb_operr(ldb);
> 	}
>+	if (strncmp("tdb://", sam_name, 6) == 0) {
>+		sam_name += 6;
>+	}
> 	filename = talloc_asprintf(tmp_ctx, "%s.d/metadata.tdb", sam_name);
> 	if (!filename) {
> 		talloc_free(tmp_ctx);
>-- 
>1.9.1
>
>
>From 9dbd89a36e83f1847e3cb441591ac49e8c440161 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 25 Sep 2013 17:09:58 -0700
>Subject: [PATCH 2/6] dsdb: Provide a clearer error when we fail to store the
> sequence number in metadata.tdb
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 6da2dcd17ee46d339d7d80df3dccd456703e7fe2)
>---
> .../dsdb/samdb/ldb_modules/partition_metadata.c    | 25 ++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>index c67d6cf..db1815a 100644
>--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>@@ -129,9 +129,13 @@ static int partition_metadata_set_uint64(struct ldb_module *module,
> 	}
> 
> 	if (tdb_store(tdb, tdb_key, tdb_data, tdb_flag) != 0) {
>+		int ret;
>+		char *error_string = talloc_asprintf(tmp_ctx, "%s: tdb_store of key %s failed: %s",
>+						     tdb_name(tdb), key, tdb_errorstr(tdb));
>+		ret = ldb_module_error(module, LDB_ERR_OPERATIONS_ERROR,
>+				       error_string);
> 		talloc_free(tmp_ctx);
>-		return ldb_module_error(module, LDB_ERR_OPERATIONS_ERROR,
>-					tdb_errorstr(tdb));
>+		return ret;
> 	}
> 
> 	talloc_free(tmp_ctx);
>@@ -242,9 +246,11 @@ static int partition_metadata_open(struct ldb_module *module, bool create)
> 	if (data->metadata->db == NULL) {
> 		talloc_free(tmp_ctx);
> 		if (create) {
>-			ldb_debug(ldb, LDB_DEBUG_ERROR,
>-				  "partition_metadata: Unable to create %s",
>-				  filename);
>+			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to create %s",
>+					       filename);
>+		} else {
>+			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to open %s",
>+					       filename);
> 		}
> 		return LDB_ERR_OPERATIONS_ERROR;
> 	}
>@@ -295,9 +301,16 @@ int partition_metadata_init(struct ldb_module *module)
> 	}
> 
> 	/* metadata.tdb does not exist, create it */
>-	DEBUG(2, ("partition_metadata: Migrating partition metadata\n"));
>+	DEBUG(2, ("partition_metadata: Migrating partition metadata: "
>+		  "open of metadata.tdb gave: %s\n",
>+		  ldb_errstring(ldb_module_get_ctx(module))));
> 	ret = partition_metadata_open(module, true);
> 	if (ret != LDB_SUCCESS) {
>+		ldb_asprintf_errstring(ldb_module_get_ctx(module),
>+				       "partition_metadata: "
>+				       "Migrating partition metadata: "
>+				       "create of metadata.tdb gave: %s\n",
>+				       ldb_errstring(ldb_module_get_ctx(module)));
> 		talloc_free(data->metadata);
> 		data->metadata = NULL;
> 		goto end;
>-- 
>1.9.1
>
>
>From b43542a949077f696cad818ee554cdf686f4abc0 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Fri, 23 May 2014 16:41:33 +1200
>Subject: [PATCH 3/6] dsdb: Do not give an error is metadata.tdb does not yet
> exist
>
>Change-Id: I88ee188c776364fd66da388ce01fc9288aa2ded0
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>(cherry picked from commit 822b4927288231b7a90579af9792608a0bdef706)
>---
> source4/dsdb/samdb/ldb_modules/schema_load.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
>index faaf3f2..3397669 100644
>--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
>+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
>@@ -113,8 +113,8 @@ static int schema_metadata_get_uint64(struct ldb_module *module,
> 	TALLOC_CTX *tmp_ctx;
> 
> 	if (!data || !data->metadata) {
>-		return ldb_module_error(module, LDB_ERR_OPERATIONS_ERROR,
>-					"schema: metadata tdb not initialized");
>+		*value = default_value;
>+		return LDB_SUCCESS;
> 	}
> 
> 	tmp_ctx = talloc_new(NULL);
>-- 
>1.9.1
>
>
>From 656547efb2bb4d380cf579fbe5766ec7ecbbdc79 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Wed, 5 Feb 2014 14:53:26 +1300
>Subject: [PATCH 4/6] dsdb: Return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS rather
> than OPERATIONS_ERROR on EACCES and EPERM
>
>This makes provision errors clearer in Samba.
>
>Andrew Bartlett
>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Jelmer Vernooij <jelmer@samba.org>
>(cherry picked from commit 262c3de3f880bb08b1220d1e755bb31365dab49b)
>---
> source4/dsdb/samdb/ldb_modules/partition_metadata.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/partition_metadata.c b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>index db1815a..b3b5744 100644
>--- a/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>+++ b/source4/dsdb/samdb/ldb_modules/partition_metadata.c
>@@ -246,11 +246,14 @@ static int partition_metadata_open(struct ldb_module *module, bool create)
> 	if (data->metadata->db == NULL) {
> 		talloc_free(tmp_ctx);
> 		if (create) {
>-			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to create %s",
>-					       filename);
>+			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to create %s: %s",
>+					       filename, strerror(errno));
> 		} else {
>-			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to open %s",
>-					       filename);
>+			ldb_asprintf_errstring(ldb, "partition_metadata: Unable to open %s: %s",
>+					       filename, strerror(errno));
>+		}
>+		if (errno == EACCES || errno == EPERM) {
>+			return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
> 		}
> 		return LDB_ERR_OPERATIONS_ERROR;
> 	}
>-- 
>1.9.1
>
>
>From f572bdb02a307ea6fd4f1f79c7e13bb78d925042 Mon Sep 17 00:00:00 2001
>From: Nadezhda Ivanova <nivanova@symas.com>
>Date: Tue, 15 Oct 2013 02:06:38 +0300
>Subject: [PATCH 5/6] s4-dsacl: Fixed incorrect handling of privileges in
> sec_access_check_ds
>
>Restore and backup privileges are not relevant to ldap
>access checks, and the TakeOwnership privilege should
>grant write_owner right
>
>Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit daefca2a1aaa9f4e0ca2f17ef4c9a71412c081ea)
>---
> libcli/security/access_check.c    | 12 ++++--------
> source4/dsdb/tests/python/acl.py  | 26 ++++++++++++++++++++++++++
> source4/dsdb/tests/python/ldap.py |  6 +++++-
> 3 files changed, 35 insertions(+), 9 deletions(-)
>
>diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
>index 2425e8a..2be5928 100644
>--- a/libcli/security/access_check.c
>+++ b/libcli/security/access_check.c
>@@ -436,14 +436,10 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
> 		bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
> 	}
> 
>-	/* TODO: remove this, as it is file server specific */
>-	if ((bits_remaining & SEC_RIGHTS_PRIV_RESTORE) &&
>-	    security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
>-		bits_remaining &= ~(SEC_RIGHTS_PRIV_RESTORE);
>-	}
>-	if ((bits_remaining & SEC_RIGHTS_PRIV_BACKUP) &&
>-	    security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
>-		bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP);
>+	/* SEC_PRIV_TAKE_OWNERSHIP grants SEC_STD_WRITE_OWNER */
>+	if ((bits_remaining & (SEC_STD_WRITE_OWNER)) &&
>+	    security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
>+		bits_remaining &= ~(SEC_STD_WRITE_OWNER);
> 	}
> 
> 	/* a NULL dacl allows access */
>diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
>index ecda3c5..7439be6 100755
>--- a/source4/dsdb/tests/python/acl.py
>+++ b/source4/dsdb/tests/python/acl.py
>@@ -1250,6 +1250,32 @@ class AclRenameTests(AclTests):
>         res = self.ldb_admin.search(self.base_dn, expression="(distinguishedName=%s)" % ou3_dn)
>         self.assertNotEqual(len(res), 0)
> 
>+    def test_rename_u9(self):
>+        """Rename 'User object' cross OU, with explicit deny on sd and dc"""
>+        ou1_dn = "OU=test_rename_ou1," + self.base_dn
>+        ou2_dn = "OU=test_rename_ou2," + self.base_dn
>+        user_dn = "CN=test_rename_user2," + ou1_dn
>+        rename_user_dn = "CN=test_rename_user5," + ou2_dn
>+        # Create OU structure
>+        self.ldb_admin.create_ou(ou1_dn)
>+        self.ldb_admin.create_ou(ou2_dn)
>+        self.ldb_admin.newuser(self.testuser2, self.user_pass, userou=self.ou1)
>+        mod = "(D;;SD;;;DA)"
>+        self.sd_utils.dacl_add_ace(user_dn, mod)
>+        mod = "(D;;DC;;;DA)"
>+        self.sd_utils.dacl_add_ace(ou1_dn, mod)
>+        # Rename 'User object' having SD and CC to AU
>+        try:
>+            self.ldb_admin.rename(user_dn, rename_user_dn)
>+        except LdbError, (num, _):
>+            self.assertEquals(num, ERR_INSUFFICIENT_ACCESS_RIGHTS)
>+        else:
>+            self.fail()
>+        #add an allow ace so we can delete this ou
>+        mod = "(A;;DC;;;DA)"
>+        self.sd_utils.dacl_add_ace(ou1_dn, mod)
>+
>+
> #tests on Control Access Rights
> class AclCARTests(AclTests):
> 
>diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py
>index 63c422a7..643830f 100755
>--- a/source4/dsdb/tests/python/ldap.py
>+++ b/source4/dsdb/tests/python/ldap.py
>@@ -2649,7 +2649,7 @@ nTSecurityDescriptor:: """ + desc_base64)
>         user_dn = "CN=%s,CN=Users,%s" % (user_name, self.base_dn)
>         delete_force(self.ldb, user_dn)
>         try:
>-            sddl = "O:DUG:DUD:PAI(A;;RPWP;;;AU)S:PAI"
>+            sddl = "O:DUG:DUD:AI(A;;RPWP;;;AU)S:PAI"
>             desc = security.descriptor.from_sddl(sddl, security.dom_sid('S-1-5-21'))
>             desc_base64 = base64.b64encode( ndr_pack(desc) )
>             self.ldb.add_ldif("""
>@@ -2659,6 +2659,10 @@ sAMAccountName: """ + user_name + """
> nTSecurityDescriptor:: """ + desc_base64)
>             res = self.ldb.search(base=user_dn, attrs=["nTSecurityDescriptor"])
>             self.assertTrue("nTSecurityDescriptor" in res[0])
>+            desc = res[0]["nTSecurityDescriptor"][0]
>+            desc = ndr_unpack(security.descriptor, desc)
>+            desc_sddl = desc.as_sddl(self.domain_sid)
>+            self.assertTrue("O:S-1-5-21-513G:S-1-5-21-513D:AI(A;;RPWP;;;AU)" in desc_sddl)
>         finally:
>             delete_force(self.ldb, user_dn)
> 
>-- 
>1.9.1
>
>
>From fcc7b3de235249f4598bbda74d7272ae51f0811e Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Mon, 9 Sep 2013 09:57:27 +1200
>Subject: [PATCH 6/6] s4-rpc_server/drsuapi: Print ldb error showing why we
> failed to perform the access check
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit f75dc8f4a54581ed207e7caa2e52211ea24e3554)
>---
> source4/rpc_server/drsuapi/drsutil.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/rpc_server/drsuapi/drsutil.c b/source4/rpc_server/drsuapi/drsutil.c
>index d3b8d23..2762a4e 100644
>--- a/source4/rpc_server/drsuapi/drsutil.c
>+++ b/source4/rpc_server/drsuapi/drsutil.c
>@@ -170,7 +170,7 @@ static WERROR drs_security_access_check_log(struct ldb_context *sam_ctx,
> 		security_token_debug(2, 0, token);
> 		return WERR_DS_DRA_ACCESS_DENIED;
> 	} else if (ret != LDB_SUCCESS) {
>-		DEBUG(1,("Failed to perform access check on %s\n", ldb_dn_get_linearized(dn)));
>+		DEBUG(1,("Failed to perform access check on %s: %s\n", ldb_dn_get_linearized(dn), ldb_strerror(ret)));
> 		return WERR_DS_DRA_INTERNAL_ERROR;
> 	}
> 	return WERR_OK;
>-- 
>1.9.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 10700: 10084