From db271e6df948b7d7390b9694ce7117def8fd2003 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 21 Oct 2013 16:59:11 -0700 Subject: [PATCH 1/2] Fix bug 10196 - RW Deny for a specific user is not overriding RW Allow for a group. When the ID returned is ID_TYPE_BOTH we must *always* add it as both a user and a group, not just in the owning case. Otherwise DENY entries are not correctly processed. Confirmed by the reporter as fixing the problem. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10196 Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp (cherry picked from commit 14813e74431816cd894fb242ff5633c2cd14ddca) --- source3/smbd/posix_acls.c | 79 ++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index ad1431d..621457e 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1981,47 +1981,50 @@ static bool create_canon_ace_lists(files_struct *fsp, } if (unixid.type == ID_TYPE_BOTH) { - /* If it's the owning user, this is a - * user_obj, not a user. This way, we - * get a valid ACL for groups that own - * files, without putting user ACL - * entries in for groups otherwise */ - if (unixid.id == pst->st_ex_uid) { - current_ace->owner_type = UID_ACE; - current_ace->unix_ug.type = ID_TYPE_UID; - current_ace->unix_ug.id = unixid.id; - current_ace->type = SMB_ACL_USER_OBJ; - - /* Add the user object to the posix ACL, - and proceed to the group mapping - below. This handles the talloc_free - of current_ace if not added for some - reason */ - if (!add_current_ace_to_acl(fsp, - psa, - &file_ace, - &dir_ace, - &got_file_allow, - &got_dir_allow, - &all_aces_are_inherit_only, - current_ace)) { - free_canon_ace_list(file_ace); - free_canon_ace_list(dir_ace); - return false; - } - - if ((current_ace = talloc(talloc_tos(), - canon_ace)) == NULL) { - free_canon_ace_list(file_ace); - free_canon_ace_list(dir_ace); - DEBUG(0,("create_canon_ace_lists: " - "malloc fail.\n")); - return False; - } + /* + * We must add both a user and group + * entry POSIX_ACL. + * This is due to the fact that in POSIX + * user entries are more specific than + * groups. + */ + current_ace->owner_type = UID_ACE; + current_ace->unix_ug.type = ID_TYPE_UID; + current_ace->unix_ug.id = unixid.id; + current_ace->type = + (unixid.id == pst->st_ex_uid) ? + SMB_ACL_USER_OBJ : + SMB_ACL_USER; + + /* Add the user object to the posix ACL, + and proceed to the group mapping + below. This handles the talloc_free + of current_ace if not added for some + reason */ + if (!add_current_ace_to_acl(fsp, + psa, + &file_ace, + &dir_ace, + &got_file_allow, + &got_dir_allow, + &all_aces_are_inherit_only, + current_ace)) { + free_canon_ace_list(file_ace); + free_canon_ace_list(dir_ace); + return false; + } - ZERO_STRUCTP(current_ace); + if ((current_ace = talloc(talloc_tos(), + canon_ace)) == NULL) { + free_canon_ace_list(file_ace); + free_canon_ace_list(dir_ace); + DEBUG(0,("create_canon_ace_lists: " + "malloc fail.\n")); + return False; } + ZERO_STRUCTP(current_ace); + sid_copy(¤t_ace->trustee, &psa->trustee); current_ace->unix_ug.type = ID_TYPE_GID; -- 1.8.4.1 From 98d5a51bd850c591c405d6651c3bf4eea9e92cfd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 23 Oct 2013 15:06:40 -0700 Subject: [PATCH 2/2] Fix bug 10196 - RW Deny for a specific user is not overriding RW Allow for a group. Fix posix_acl tests to match the change in writing ACLs with ID_TYPE_BOTH. Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp (cherry picked from commit a1bc1c32e33508c45e614646d69a5f5d67ba22be) --- python/samba/tests/posixacl.py | 160 +++++++++++++++++++++++++++++------------ 1 file changed, 116 insertions(+), 44 deletions(-) diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py index f3a4772..bb104f7 100644 --- a/python/samba/tests/posixacl.py +++ b/python/samba/tests/posixacl.py @@ -336,7 +336,7 @@ class PosixAclMappingTests(TestCaseInTempDir): (AU_gid,AU_type) = s4_passdb.sid_to_id(AU_sid) self.assertEquals(AU_type, idmap.ID_TYPE_BOTH) - self.assertEquals(posix_acl.count, 9) + self.assertEquals(posix_acl.count, 13) self.assertEquals(posix_acl.acl[0].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[0].a_perm, 7) @@ -352,23 +352,39 @@ class PosixAclMappingTests(TestCaseInTempDir): self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ) self.assertEquals(posix_acl.acl[3].a_perm, 6) - self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[4].a_perm, 7) + self.assertEquals(posix_acl.acl[4].info.uid, BA_gid) - self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[5].a_perm, 5) - self.assertEquals(posix_acl.acl[5].info.gid, SO_gid) + self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[5].a_perm, 7) - self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[6].a_perm, 7) - self.assertEquals(posix_acl.acl[6].info.gid, SY_gid) + self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[6].a_perm, 5) + self.assertEquals(posix_acl.acl[6].info.uid, SO_gid) self.assertEquals(posix_acl.acl[7].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[7].a_perm, 5) - self.assertEquals(posix_acl.acl[7].info.gid, AU_gid) + self.assertEquals(posix_acl.acl[7].info.gid, SO_gid) - self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[8].a_perm, 7) + self.assertEquals(posix_acl.acl[8].info.uid, SY_gid) + + self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[9].a_perm, 7) + self.assertEquals(posix_acl.acl[9].info.gid, SY_gid) + + self.assertEquals(posix_acl.acl[10].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[10].a_perm, 5) + self.assertEquals(posix_acl.acl[10].info.uid, AU_gid) + + self.assertEquals(posix_acl.acl[11].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[11].a_perm, 5) + self.assertEquals(posix_acl.acl[11].info.gid, AU_gid) + + self.assertEquals(posix_acl.acl[12].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[12].a_perm, 7) # check that it matches: @@ -454,7 +470,7 @@ class PosixAclMappingTests(TestCaseInTempDir): (AU_gid,AU_type) = s4_passdb.sid_to_id(AU_sid) self.assertEquals(AU_type, idmap.ID_TYPE_BOTH) - self.assertEquals(posix_acl.count, 9) + self.assertEquals(posix_acl.count, 13) self.assertEquals(posix_acl.acl[0].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[0].a_perm, 7) @@ -470,23 +486,39 @@ class PosixAclMappingTests(TestCaseInTempDir): self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ) self.assertEquals(posix_acl.acl[3].a_perm, 7) - self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[4].a_perm, 7) + self.assertEquals(posix_acl.acl[4].info.uid, BA_gid) - self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[5].a_perm, 5) - self.assertEquals(posix_acl.acl[5].info.gid, SO_gid) + self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[5].a_perm, 7) - self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[6].a_perm, 7) - self.assertEquals(posix_acl.acl[6].info.gid, SY_gid) + self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[6].a_perm, 5) + self.assertEquals(posix_acl.acl[6].info.uid, SO_gid) self.assertEquals(posix_acl.acl[7].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[7].a_perm, 5) - self.assertEquals(posix_acl.acl[7].info.gid, AU_gid) + self.assertEquals(posix_acl.acl[7].info.gid, SO_gid) - self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[8].a_perm, 7) + self.assertEquals(posix_acl.acl[8].info.uid, SY_gid) + + self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[9].a_perm, 7) + self.assertEquals(posix_acl.acl[9].info.gid, SY_gid) + + self.assertEquals(posix_acl.acl[10].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[10].a_perm, 5) + self.assertEquals(posix_acl.acl[10].info.uid, AU_gid) + + self.assertEquals(posix_acl.acl[11].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[11].a_perm, 5) + self.assertEquals(posix_acl.acl[11].info.gid, AU_gid) + + self.assertEquals(posix_acl.acl[12].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[12].a_perm, 7) # check that it matches: @@ -534,7 +566,7 @@ class PosixAclMappingTests(TestCaseInTempDir): (PA_gid,PA_type) = s4_passdb.sid_to_id(PA_sid) self.assertEquals(PA_type, idmap.ID_TYPE_BOTH) - self.assertEquals(posix_acl.count, 10) + self.assertEquals(posix_acl.count, 15) self.assertEquals(posix_acl.acl[0].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[0].a_perm, 7) @@ -550,27 +582,47 @@ class PosixAclMappingTests(TestCaseInTempDir): self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ) self.assertEquals(posix_acl.acl[3].a_perm, 7) - self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[4].a_perm, 7) + self.assertEquals(posix_acl.acl[4].info.uid, BA_gid) - self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[5].a_perm, 5) - self.assertEquals(posix_acl.acl[5].info.gid, SO_gid) + self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[5].a_perm, 7) - self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[6].a_perm, 7) - self.assertEquals(posix_acl.acl[6].info.gid, SY_gid) + self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[6].a_perm, 5) + self.assertEquals(posix_acl.acl[6].info.uid, SO_gid) self.assertEquals(posix_acl.acl[7].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[7].a_perm, 5) - self.assertEquals(posix_acl.acl[7].info.gid, AU_gid) + self.assertEquals(posix_acl.acl[7].info.gid, SO_gid) - self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[8].a_perm, 7) - self.assertEquals(posix_acl.acl[8].info.gid, PA_gid) + self.assertEquals(posix_acl.acl[8].info.uid, SY_gid) - self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[9].a_perm, 7) + self.assertEquals(posix_acl.acl[9].info.gid, SY_gid) + + self.assertEquals(posix_acl.acl[10].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[10].a_perm, 5) + self.assertEquals(posix_acl.acl[10].info.uid, AU_gid) + + self.assertEquals(posix_acl.acl[11].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[11].a_perm, 5) + self.assertEquals(posix_acl.acl[11].info.gid, AU_gid) + + self.assertEquals(posix_acl.acl[12].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[12].a_perm, 7) + self.assertEquals(posix_acl.acl[12].info.uid, PA_gid) + + self.assertEquals(posix_acl.acl[13].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[13].a_perm, 7) + self.assertEquals(posix_acl.acl[13].info.gid, PA_gid) + + self.assertEquals(posix_acl.acl[14].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[14].a_perm, 7) # check that it matches: @@ -621,7 +673,7 @@ class PosixAclMappingTests(TestCaseInTempDir): (PA_gid,PA_type) = s4_passdb.sid_to_id(PA_sid) self.assertEquals(PA_type, idmap.ID_TYPE_BOTH) - self.assertEquals(posix_acl.count, 10) + self.assertEquals(posix_acl.count, 15) self.assertEquals(posix_acl.acl[0].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[0].a_perm, 7) @@ -637,27 +689,47 @@ class PosixAclMappingTests(TestCaseInTempDir): self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ) self.assertEquals(posix_acl.acl[3].a_perm, 6) - self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[4].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[4].a_perm, 7) + self.assertEquals(posix_acl.acl[4].info.uid, BA_gid) - self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[5].a_perm, 5) - self.assertEquals(posix_acl.acl[5].info.gid, SO_gid) + self.assertEquals(posix_acl.acl[5].a_type, smb_acl.SMB_ACL_GROUP_OBJ) + self.assertEquals(posix_acl.acl[5].a_perm, 7) - self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_GROUP) - self.assertEquals(posix_acl.acl[6].a_perm, 7) - self.assertEquals(posix_acl.acl[6].info.gid, SY_gid) + self.assertEquals(posix_acl.acl[6].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[6].a_perm, 5) + self.assertEquals(posix_acl.acl[6].info.uid, SO_gid) self.assertEquals(posix_acl.acl[7].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[7].a_perm, 5) - self.assertEquals(posix_acl.acl[7].info.gid, AU_gid) + self.assertEquals(posix_acl.acl[7].info.gid, SO_gid) - self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[8].a_type, smb_acl.SMB_ACL_USER) self.assertEquals(posix_acl.acl[8].a_perm, 7) - self.assertEquals(posix_acl.acl[8].info.gid, PA_gid) + self.assertEquals(posix_acl.acl[8].info.uid, SY_gid) - self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[9].a_type, smb_acl.SMB_ACL_GROUP) self.assertEquals(posix_acl.acl[9].a_perm, 7) + self.assertEquals(posix_acl.acl[9].info.gid, SY_gid) + + self.assertEquals(posix_acl.acl[10].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[10].a_perm, 5) + self.assertEquals(posix_acl.acl[10].info.uid, AU_gid) + + self.assertEquals(posix_acl.acl[11].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[11].a_perm, 5) + self.assertEquals(posix_acl.acl[11].info.gid, AU_gid) + + self.assertEquals(posix_acl.acl[12].a_type, smb_acl.SMB_ACL_USER) + self.assertEquals(posix_acl.acl[12].a_perm, 7) + self.assertEquals(posix_acl.acl[12].info.uid, PA_gid) + + self.assertEquals(posix_acl.acl[13].a_type, smb_acl.SMB_ACL_GROUP) + self.assertEquals(posix_acl.acl[13].a_perm, 7) + self.assertEquals(posix_acl.acl[13].info.gid, PA_gid) + + self.assertEquals(posix_acl.acl[14].a_type, smb_acl.SMB_ACL_MASK) + self.assertEquals(posix_acl.acl[14].a_perm, 7) # check that it matches: -- 1.8.4.1