Hi Shirish, I've tested the patch, and as far as I can see it works perfectly. The extra zero bytes are no longer in the request and the server now accepts them. I've tested on a number of directories, with a number of different ACEs, adding and deleting, and could not find an issue so far. I've not tested the smb2 patches for setting acl's yet, as I do not have an environment ready to go for recompiling the cifs kernel module. The server we are using is a Hitachi HNAS: https://www.hds.com/en-us/products-solutions/storage/network-attached-storage-platform.html Thanks, Paul On Wed, Jul 5, 2017 at 7:08 AM, Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > Can you try this patch, I did preliminary testing... > > > > On Tue, Jul 4, 2017 at 4:17 PM, Shirish Pargaonkar > <shirishpargaonkar@xxxxxxxxx> wrote: >> Hi Paul, >> >> So something like this works with smb 2.1 with the patchset I posted >> on the mailing list >> to make set acl work over smb 2 onwards... >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> $ sudo setcifsacl -S >> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x1f01b9,ACL:\Everyone:ALLOWED/0x0/0x1f01b9" >> /mnt/file1.txt >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwxr-xr-x 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> I then changed it back to 700 using smb1 set cifs acl and it worked too. >> >> $ sudo setcifsacl -S >> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x120088,ACL:\Everyone:ALLOWED/0x0/0x120088" >> /mnt/file1.txt >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> What type of Windows server is this? These requests succeed but I do >> see extra bytes >> at the end of the security descriptor in both the requests, trying to >> figure out why is it allocated in the first place... >> >> >> On Tue, Jun 27, 2017 at 5:43 AM, Paul van Schayck <polleke@xxxxxxxxx> wrote: >>> Hi Shirish, >>> >>> Thank you for your answer. >>> >>> Attached you can find three pcap files. >>> >>> 1. linux-fail.pcap: Using the setcifsacl 6.7 version. >>> 2. linux-succes.pcap: Using the modified setcifsacl with the patch >>> above (note, I had to change to len + 4 in trim_request()) >>> 3. windows-succes-trimmed.pcap: Using the dialog in Windows. Note that >>> this is over smbv2. The file was trimmed down, because windows was >>> noisy. >>> >>> The command I used was >>> >>> setcifsacl -S "ACL:S-1-5-21-1572361299-1184395705-1606240830-247332:ALLOWED/OI|CI/CHANGE,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/I/FULL,ACL:S-1-3-0:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-512:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-549266:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-18:ALLOWED/OI|CI|I/FULL" >>> cifs-test/ >>> >>> In Windows I used the dialog to set the same ACEs. Between the >>> linux-success and linux-fail file you can see the difference in the >>> number of zero bytes after the DACL. >>> >>> If this test case is still a bit noisy for you, I can try to build a >>> more simple ACE list. But I have to setup a new share for this. >>> >>> Thanks, >>> >>> Paul van Schayck >>> >>> On Tue, Jun 27, 2017 at 6:40 AM, Shirish Pargaonkar >>> <shirishpargaonkar@xxxxxxxxx> wrote: >>>> if you can provide tcpdumpas well as the command you are using of/for >>>> both Windows and Linux way, that would be helpful >>>> >>>> On Mon, Jun 26, 2017 at 8:04 AM, Paul van Schayck via samba-technical >>>> <samba-technical@xxxxxxxxxxxxxxx> wrote: >>>>> Dear samba-technical, >>>>> >>>>> If this list is inappropriate for this question then please redirect >>>>> me to a better one. >>>>> >>>>> I'm working with a HDS HNAS (Hitachi NAS) which has its own >>>>> implementation of the CIFS protocol. Mounting, and interactions all go >>>>> fine using linux-cifs and related tools. >>>>> >>>>> However, when sending a `setcifsacl -a ACE /dir` the response is >>>>> "main: setxattr error: Input/output error". Upon further digging using >>>>> tcpdump/wireshark it became clear that the HNAS is responding with >>>>> >>>>> NT Status: STATUS_INVALID_SECURITY_DESCR (0xc0000079) >>>>> >>>>> Setting the same ACE using a Windows client did work. So further >>>>> examining of the request being sent showed that the only difference >>>>> between the Windows client and setcifsacl was the addition of a lot of >>>>> padding zero bytes at the end of the security descriptor. >>>>> >>>>> Examining the code of setcifsacl.c and the request more it seems like >>>>> the request was not properly trimmed down to the number of aces being >>>>> sent. More buffer was allocated that the number of aces being sent. So >>>>> I made the attached patch to trim down the request. This fixes some of >>>>> the problems with setcifsacl, but for example breaks deleting aces. I >>>>> also think it's most likely an ugly fix, and the problem needs to be >>>>> solved elsewhere most likely. >>>>> >>>>> So my question is if someone can point me at the correct way to fix >>>>> this in setcifsacl, or help me fix it properly. If necessary I can >>>>> provide tcpdump's of Windows and Linux clients performing the request. >>>>> >>>>> Thanks, >>>>> >>>>> Paul van Schayck >>>>> >>>>> diff --git a/setcifsacl.c b/setcifsacl.c >>>>> index 7eeeaa6..4ada3c8 100644 >>>>> --- a/setcifsacl.c >>>>> +++ b/setcifsacl.c >>>>> @@ -761,6 +761,20 @@ setacl_action(struct cifs_ntsd *pntsd, struct >>>>> cifs_ntsd **npntsd, >>>>> return rc; >>>>> } >>>>> >>>>> +ssize_t >>>>> +trim_request(ssize_t bufsize, struct cifs_ntsd *npntsd) { >>>>> + int i, len = 0; >>>>> + char *a = (char *) npntsd; >>>>> + >>>>> + for( i = 0; i < bufsize; i++) { >>>>> + if ( a[i] != 0 ) >>>>> + len = i; >>>>> + } >>>>> + >>>>> + return (ssize_t) len + 2; >>>>> +} >>>>> + >>>>> + >>>>> static void >>>>> setcifsacl_usage(const char *prog) >>>>> { >>>>> @@ -902,7 +916,7 @@ cifsacl: >>>>> if (rc) >>>>> goto setcifsacl_action_ret; >>>>> >>>>> - attrlen = setxattr(filename, ATTRNAME, ntsdptr, bufsize, 0); >>>>> + attrlen = setxattr(filename, ATTRNAME, ntsdptr, >>>>> trim_request(bufsize, ntsdptr), 0); >>>>> if (attrlen == -1) { >>>>> printf("%s: setxattr error: %s\n", __func__, strerror(errno)); >>>>> goto setcifsacl_facenum_ret; >>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
