Using Samba version 3.6 a file system without posix acl support Samba would manipulate the traditional unix permissions instead. When trying the same operation with samba 4.1.5 the operation fails with an "Access is Denied" dialog box. The specific test I ran was the following: using a Linux server with Samba 3.6.10 installed, I created a ext4 file system and mounted it with the noacl mount option. I created a world writable dir for the windows test system to write into. $ lvcreate --name srv --size 4G foo $ mkfs.ext4 /dev/mapper/foo-srv $ mount /dev/mapper/foo-srv /srv/ -o noatime,noacl $ mkdir /srv/abc $ chmod ugo+rwx /srv/abc I set up samba with the following config: [global] workgroup = ENGWIN2K8 realm = ENGWIN2K8.FOOBAR.NET server string = SMB Server security = ADS allow trusted domains = No syslog = 0 server max protocol = SMB2 min receivefile size = 131072 load printers = No printcap name = /dev/null disable spoolss = Yes show add printer wizard = No winbind enum users = Yes winbind enum groups = Yes idmap config *: read only = yes idmap config *: range = 200000-210000 idmap config ENGWIN2K8: read only = no idmap config ENGWIN2K8: backend = rid idmap config ENGWIN2K8: range = 400000-8000000 idmap config * : backend = tdb force unknown acl user = Yes [testsrv] path = /srv/ read only = No inherit permissions = Yes inherit acls = Yes map acl inherit = Yes hide unreadable = Yes veto files = /.snapshot/ store dos attributes = Yes delete readonly = Yes dos filemode = Yes Using a Windows 2008 Server system as a client I connected to the testsrv share and created file one.txt. Following that I changed the permissions of the group and then both the group and other (everyone) using the windows security tab. $ getfacl /srv/abc/one.txt getfacl: Removing leading '/' from absolute path names # file: srv/abc/one.txt # owner: ENGWIN2K8\134johnm # group: ENGWIN2K8\134domain\040users user::rwx group::rw- other::rw- $ ls -l /srv/abc/one.txt -rwxrw-rw- 1 ENGWIN2K8\johnm ENGWIN2K8\domain users 0 Mar 7 15:14 /srv/abc/one.txt $ getfacl /srv/abc/one.txt getfacl: Removing leading '/' from absolute path names # file: srv/abc/one.txt # owner: ENGWIN2K8\134johnm # group: ENGWIN2K8\134domain\040users user::rwx group::r-- other::rw- $ ls -l /srv/abc/one.txt -rwxr--rw- 1 ENGWIN2K8\johnm ENGWIN2K8\domain users 0 Mar 7 15:14 /srv/abc/one.txt $ getfacl /srv/abc/one.txt getfacl: Removing leading '/' from absolute path names # file: srv/abc/one.txt # owner: ENGWIN2K8\134johnm # group: ENGWIN2K8\134domain\040users user::rwx group::--- other::--- $ ls -l /srv/abc/one.txt -rwx------ 1 ENGWIN2K8\johnm ENGWIN2K8\domain users 0 Mar 7 15:14 /srv/abc/one.txt After verifying that Samba 3.6 worked as expected I upgraded to Samba 4.1.5. Once upgraded and restarted I tried changing the permissions of the group again (same client). Now the permissions change fails and the following appears in the samba log: [2014/03/07 15:35:19.306498, 2] ../source3/smbd/open.c:972(open_file) ENGWIN2K8\johnm opened file abc/one.txt read=No write=No (numopen=2) [2014/03/07 15:35:19.307430, 2] ../source3/smbd/posix_acls.c:3014(set_canon_ace_list) set_canon_ace_list: sys_acl_set_file type file failed for file abc/one.txt (Operation not supported). [2014/03/07 15:35:19.307576, 3] ../source3/smbd/posix_acls.c:3099(convert_canon_ace_to_posix_perms) convert_canon_ace_to_posix_perms: Too many ACE entries for file abc/one.txt to convert to posix perms. [2014/03/07 15:35:19.307831, 3] ../source3/smbd/posix_acls.c:3923(set_nt_acl) set_nt_acl: failed to convert file acl to posix permissions for file abc/one.txt. [2014/03/07 15:35:19.308384, 2] ../source3/smbd/close.c:780(close_normal_file) ENGWIN2K8\johnm closed file abc/one.txt (numopen=1) NT_STATUS_OK [2014/03/07 15:35:20.759663, 3] ../source3/smbd/service.c:1122(close_cnum) If I enable posix acls for the file system and try again Windows saves the acls without error and the posix acls are updated on the file system. $ mount /srv/ -o remount,noatime,acl $ getfacl /srv/abc/one.txt getfacl: Removing leading '/' from absolute path names # file: srv/abc/one.txt # owner: ENGWIN2K8\134johnm # group: ENGWIN2K8\134domain\040users user::rwx user:ENGWIN2K8\134domain\040users:r-- group::r-- group:ENGWIN2K8\134domain\040users:r-- group:ENGWIN2K8\134johnm:rwx mask::rwx other::--- I did search samba bugzilla and did not see this specific issue already reported. I apologize if I missed something and this has been previously reported. Please let me know if there is any additional information I can provide. I am attaching logs from both logs from the 3.6 and 4.1 tests.
Created attachment 9759 [details] samba 3.6 log
Created attachment 9760 [details] Samba 4.1 log
More info needed. A wireshark capture showing the ACL sent by the client would help I think. Jeremy.
Ignore the previous comment, it was meant for a different bug (#10072). I'll take a look at this, but to be honest it's going to be a low priority as making smbd work without any underlying ACLs on the file system is getting to be a very minor use-case. Jeremy.
Hello there. I think I ran across this same but in Samba 4.2.8 and think I have a patch. The bug is in source3/smbd/posix_acls.c. Trying to change permissions call set_nt_acl. From what I can gather the code calls unpack_canon_ace, then ensure_canon_entry_valid_on_set. That code creates an ACL list from the unix permissions. It creates 3 ACEs based on the owner, group & other permssions. But the as the comments at line 1505 says: /* Ensure when setting a POSIX ACL, that the uid for a SMB_ACL_USER_OBJ ACE (the owner ACE entry) has a duplicate permission entry as an SMB_ACL_USER, and a gid for a SMB_ACL_GROUP_OBJ ACE (the primary group ACE entry) also has a duplicate permission entry as an SMB_ACL_GROUP. If not, then if the ownership or group ownership of this file or directory gets changed, the user or group can lose their access. */ So then there are 5 ACEs. They get passed back and eventually the code calls convert_canon_ace_to_posix_perms and it fails at line 3088: if (ace_count != 3) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " "entries for file %s to convert to posix perms.\n", fsp_str_dbg(fsp))); return False; } So basically it appears that if you don't have real ACLs and just use unix permissions it will always fail since the code will always create 5 ACEs in the ACL and then fail since it only wants 3. I'll attach a proposed patch. Please contact me if you have questions
Actually the patch is so small I'll just put it here. *** source3/smbd/posix_acls.c.orig Mon Feb 8 07:45:38 2016 --- source3/smbd/posix_acls.c Mon Feb 8 07:45:49 2016 *************** *** 3085,3091 **** canon_ace *group_ace = NULL; canon_ace *other_ace = NULL; ! if (ace_count != 3) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " "entries for file %s to convert to posix perms.\n", fsp_str_dbg(fsp))); --- 3085,3091 ---- canon_ace *group_ace = NULL; canon_ace *other_ace = NULL; ! if (ace_count != 5) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " "entries for file %s to convert to posix perms.\n", fsp_str_dbg(fsp)));
Created attachment 11822 [details] git-am fix for master. Can you test this fix ? I'm afraid your fix is a little too simple to catch all cases, and I think this might do the trick. If you can confirm it works, I'll push to master and get it back-ported to all release branches. Thanks a *lot* for your help on this ! Jeremy.
Thanks Jeremy. Unfortunately that didn't quite work. The log has: [2016/02/09 14:20:33.256915, 3, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:3141(convert_canon_ace_to_posix_perms) Invalid type 1, uid 20 in ACE for file tim test. I went to loglevel 10 so it would print the ACL. [2016/02/09 14:20:33.256370, 10, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:2935(set_canon_ace_list) canon_ace index 0. Type = allow SID = S-1-5-21-1062074158-754817265-4250498807-513 gid 20 (users) SMB_ACL_GROUP ace_flags = 0x0 perms r-x [2016/02/09 14:20:33.256473, 10, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:2935(set_canon_ace_list) canon_ace index 1. Type = allow SID = S-1-5-21-1062074158-754817265-4250498807-1330 uid 165 (temp15) SMB_ACL_USER ace_flags = 0x0 perms rwx [2016/02/09 14:20:33.256585, 10, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:2935(set_canon_ace_list) canon_ace index 2. Type = allow SID = S-1-5-21-1062074158-754817265-4250498807-1330 uid 165 (temp15) SMB_ACL_USER_OBJ ace_flags =0x0 perms rwx [2016/02/09 14:20:33.256699, 10, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:2935(set_canon_ace_list) canon_ace index 3. Type = allow SID = S-1-5-21-1062074158-754817265-4250498807-513 gid 20 (users) SMB_ACL_GROUP_OBJ ace_flags = 0x0 perms r-x [2016/02/09 14:20:33.256802, 10, pid=55888, effective(165, 20), real(0, 0), class=acls] ../source3/smbd/posix_acls.c:2935(set_canon_ace_list) canon_ace index 4. Type = allow SID = S-1-1-0 other SMB_ACL_OTHER ace_flags = 0x0 perms rwx I think you need to include the last bit about the WORLD_ACE in an else. Otherwise you will always hit it even if ace_p->owner_type is UID_ACE or GID_ACE. Here's the updated patch. It seems to work for me. *** source3/smbd/posix_acls.c.orig Tue Jul 14 03:54:24 2015 --- source3/smbd/posix_acls.c Tue Feb 9 14:59:10 2016 *************** *** 3085,3091 **** canon_ace *group_ace = NULL; canon_ace *other_ace = NULL; ! if (ace_count != 3) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " "entries for file %s to convert to posix perms.\n", fsp_str_dbg(fsp))); --- 3085,3091 ---- canon_ace *group_ace = NULL; canon_ace *other_ace = NULL; ! if (ace_count != 5) { DEBUG(3,("convert_canon_ace_to_posix_perms: Too many ACE " "entries for file %s to convert to posix perms.\n", fsp_str_dbg(fsp))); *************** *** 3107,3112 **** --- 3107,3149 ---- return False; } + /* + * Ensure all ACE entries are owner, group or other. + * We can't set if there are any other SIDs. + */ + for (ace_p = file_ace_list; ace_p; ace_p = ace_p->next) { + if (ace_p == owner_ace || ace_p == group_ace || + ace_p == other_ace) { + continue; + } + if (ace_p->owner_type == UID_ACE) { + if (ace_p->unix_ug.id != owner_ace->unix_ug.id) { + DEBUG(3,("Invalid uid %u in ACE for file %s.\n", + (unsigned int)ace_p->unix_ug.id, + fsp_str_dbg(fsp))); + return false; + } + } else if (ace_p->owner_type == GID_ACE) { + if (ace_p->unix_ug.id != group_ace->unix_ug.id) { + DEBUG(3,("Invalid gid %u in ACE for file %s.\n", + (unsigned int)ace_p->unix_ug.id, + fsp_str_dbg(fsp))); + return false; + } + } else { + + /* + * There should be no duplicate WORLD_ACE entries. + */ + + DEBUG(3,("Invalid type %u, uid %u in ACE for file %s.\n", + (unsigned int)ace_p->owner_type, + (unsigned int)ace_p->unix_ug.id, + fsp_str_dbg(fsp))); + return false; + } + } + *posix_perms = (mode_t)0; *posix_perms |= owner_ace->perms;
Created attachment 11823 [details] git-am fix for master. Doh ! Of course - great catch - thanks ! Can you confirm this, then I'll push to master ?
Hi Jeremy - that patch seemed to do the trick. Thanks for the fix and all your work on Samba over the years. We've been using it ~20 years now and love it.
Created attachment 11835 [details] git-am fix for 4.4.0, 4.3.x, 4.2.x Cherry-pick from master. Applies cleanly to 4.4.0, 4.3.next, 4.2.next.
Assigning to Karolin for inclusion in 4.4.0, 4.3.next, 4.2.next.
(In reply to Uri Simchoni from comment #12) Pushed to autobuild-v4-[4|3|2]-test.
(In reply to Karolin Seeger from comment #13) Pushed to all branches. Closing out bug report. Thanks!