From 274f4a5b95391d2c19c5ae785a8085393ee1d1b1 Mon Sep 17 00:00:00 2001 From: Volker Lendecke 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 Reviewed-by: Andrew Bartlett (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 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 Reviewed-by: Stefan Metzmacher (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 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 Reviewed-by: Andreas Schneider (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 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 Signed-off-by: Andrew Bartlett Reviewed-by: Jelmer Vernooij (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 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 Reviewed-by: Andrew Bartlett (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 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 Reviewed-by: Stefan Metzmacher (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