Running Samba 3.0.25c compiled with ACL support on RHEL4 Linux. The server is a member server of a Samba domain. I'll attach a complete smb.conf later but the smb.conf options I think are relevant are here: map archive = no map acl inherit = no inherit acls = yes store dos attributes = yes To reproduce: 1. Create a directory with an ACL like the following. The important point is that the owning group has just execute permission and the default acl also grants only execute permission to the owning group: [root@rchs5bld samba-3.0.25c]# getfacl /home/group/xlstest/jpjtest/ getfacl: Removing leading '/' from absolute path names # file: home/group/xlstest/jpjtest # owner: jpjanosi # group: domain\040users user::rwx group::--x group:ltest:rwx mask::rwx other::--- default:user::rwx default:group::--x default:group:ltest:rwx default:mask::rwx default:other::--- 2. Create a file in that directory via a Windows client as a user with a primary group of domain users. The ACL matches the default acl as expected and the ownership is correct: [root@rchs5bld samba-3.0.25c]# getfacl /home/group/xlstest/jpjtest/testfile8 getfacl: Removing leading '/' from absolute path names # file: home/group/xlstest/jpjtest/testfile8 # owner: jpjanosi # group: domain\040users user::rwx group::--x group:ltest:rwx mask::rwx other::--- 3. User the security tab of the file properties window in explorer on a Windows XP client to add a user to ACL for the file. Only a user was added, none of the check boxes for existing ACEs were clicked or anything done in the advanced tab. The ACL now has rx for the owning group and r for other which means any domain user can read the file now since x is set down the tree for domain users: [root@rchs5bld samba-3.0.25c]# getfacl /home/group/xlstest/jpjtest/testfile8 getfacl: Removing leading '/' from absolute path names # file: home/group/xlstest/jpjtest/testfile8 # owner: jpjanosi # group: domain\040users user::rwx user:ronda:r-x group::r-x group:ltest:rwx mask::rwx other::r-- This happens because smbd/posix_acls.c:set_nt_acl calls smbd/posix_acls.c:append_parent_acl, which then calls smbd/dosmode.c:unixmode. This function starts with 666 for the mode it returns as its result: mode_t result = (S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH); With our config the these bits do not get masked off during this function and we the set_net_acl call gives the owning group and other read permission.
Created attachment 2900 [details] main smb.conf file
Created attachment 2901 [details] Included smb.conf file
Created attachment 2902 [details] Level 10 log of the nttrans set security desc operation
bmarsh was trying to duplicate this problem on 3.0.25b so we could report this to RedHat. I looked at this again and have the following comments: This was introduced in 3.0.25c so bmarsh could not duplicate it on 3.0.25b to report to RedHat for assistance. Here is the change to smbd/posix_acls.c that introduces the problem. *************** *** 3188,3193 **** --- 3403,3420 ---- create_file_sids(&sbuf, &file_owner_sid, &file_grp_sid); + if ((security_info_sent & DACL_SECURITY_INFORMATION) && + psd->dacl != NULL && + (psd->type & (SE_DESC_DACL_AUTO_INHERITED| + SE_DESC_DACL_AUTO_INHERIT_REQ))== + (SE_DESC_DACL_AUTO_INHERITED| + SE_DESC_DACL_AUTO_INHERIT_REQ) ) { + NTSTATUS status = append_parent_acl(fsp, &sbuf, psd, &psd); + if (!NT_STATUS_IS_OK(status)) { + return False; + } + } + acl_perms = unpack_canon_ace( fsp, &sbuf, &file_owner_sid, &file_grp_sid, &file_ace_list, &dir_ace_list, security_info_sent, psd); So I think there are two problem with this new code. First since we have "map acl inherit = no" we should not being trying to do win2k like inheritance of ACLs and so the if should fail and we shouldn't call append_parent_acl. The second bug is that append_parent_acl starts off with 666 for mode bits and builds from there because it calls smbd/dosmode.c:unixmode at the start.
I'm not convinced yet that we shouldn't be doing _any_ attempt of acl inheritance. I'm trying to think about this on systems where we simply don't have any xattrs to use for the mapping. The "map acl inherit" parm is merely about storing those inheritance bits to get a more accurate inheritance. In the "no" case we are inferring these bits...and perhaps we need to revisit this.
I should just add that the group mode bits still shouldn't be changing in this case, so your second point is still correct.
Ok, so as John pointed out to me, the key here is that the SE_DESC_DACL_PROTECTED flag is on, so the ACE for CREATOR_GROUP that has been set on the child shouldn't be overwritten by the inherited ACE for the same. Perhaps this points to a merge logic error instead?
Created attachment 3118 [details] wireshark capture of adding a user to the acl frame 510
Created attachment 3119 [details] Patch I cannot tell a lie :-). Jim told me how to fix this bug and was gracious enough to let me write the patch :-). Thanks Jim ! Let me know if this fixes it. Jeremy.
Created attachment 3120 [details] Replacement patch This is a commented version of the first patch. Jeremy.
Created attachment 3121 [details] Second part of the patch. Second part of the patch.
John, when you get a chance, can you verify? If you want to build yourself with just patches, you'll want: 0e7886a3ceb8406c5e331a66c0e6fb6ab4493a3e 485cedadb0e61775e6cb152f42f4dfdf17e82666 6b594996a8dff0c6c663752f06a994c95020d869 7a529c43181eb9b3926b214b2fe84aea06be7a3c 938f78546a4706f25d7b07efbca97a6b2d12d4b9 b83dfaf09679b0bbd7341230e1e96b53ae5289cb fc0508922417e9ef9a4450067d29d15121b52902 or just pull the latest.
Jim - Looks good. I pulled the latest 3.0 last night, the version is reported as 3.0.28a-GIT-f04810e-test. It worked properly when setting ACLs from Windows XP in our setup. I also tested with Excel and was unable to duplicate the problems we had seen prior to 3.0.25 with files going read-only when opened back and forth between multiple users.
Closing as fixed
*** Bug 5094 has been marked as a duplicate of this bug. ***
The problem is still here. I also tried the patches with no luck. This problem is not fixed in every situation.
First, what version have you tested? the attached patches, the mentioned git patches, latest git, etc.? Second, you'll need to give more details on what part is not working, as the original reported case is working for the reporter. You may need to provide instructions to reproduce, smb.conf, log files...