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;
>>>>
diff --git a/setcifsacl.c b/setcifsacl.c
index 7eeeaa6..0d4b15f 100644
--- a/setcifsacl.c
+++ b/setcifsacl.c
@@ -50,24 +50,33 @@ enum setcifsacl_actions {
static void *plugin_handle;
static bool plugin_loaded;
-static void
+static int
copy_cifs_sid(struct cifs_sid *dst, const struct cifs_sid *src)
{
- int i;
+ int i, size = 0;
dst->revision = src->revision;
+ size += sizeof(uint8_t);
+
dst->num_subauth = src->num_subauth;
+ size += sizeof(uint8_t);
+
for (i = 0; i < NUM_AUTHS; i++)
dst->authority[i] = src->authority[i];
+ size += (sizeof(uint8_t) * NUM_AUTHS);
+
for (i = 0; i < src->num_subauth; i++)
dst->sub_auth[i] = src->sub_auth[i];
+ size += (sizeof(uint32_t) * src->num_subauth);
+
+ return size;
}
static void
copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
- int numaces, int acessize)
+ int numaces, int acessize, ssize_t *bufsize)
{
- int osidsoffset, gsidsoffset, dacloffset;
+ int size, osidsoffset, gsidsoffset, dacloffset;
struct cifs_sid *owner_sid_ptr, *group_sid_ptr;
struct cifs_sid *nowner_sid_ptr, *ngroup_sid_ptr;
struct cifs_ctrl_acl *dacl_ptr, *ndacl_ptr;
@@ -77,28 +86,34 @@ copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
gsidsoffset = le32toh(pntsd->gsidoffset);
dacloffset = le32toh(pntsd->dacloffset);
+ size = sizeof(struct cifs_ntsd);
pnntsd->revision = pntsd->revision;
pnntsd->type = pntsd->type;
pnntsd->osidoffset = pntsd->osidoffset;
pnntsd->gsidoffset = pntsd->gsidoffset;
pnntsd->dacloffset = pntsd->dacloffset;
+ *bufsize = *bufsize + size;
dacl_ptr = (struct cifs_ctrl_acl *)((char *)pntsd + dacloffset);
ndacl_ptr = (struct cifs_ctrl_acl *)((char *)pnntsd + dacloffset);
+ size = acessize + sizeof(struct cifs_ctrl_acl);
ndacl_ptr->revision = dacl_ptr->revision;
- ndacl_ptr->size = htole16(acessize + sizeof(struct cifs_ctrl_acl));
+ ndacl_ptr->size = htole16(size);
ndacl_ptr->num_aces = htole32(numaces);
+ *bufsize = *bufsize + size;
/* copy owner sid */
owner_sid_ptr = (struct cifs_sid *)((char *)pntsd + osidsoffset);
nowner_sid_ptr = (struct cifs_sid *)((char *)pnntsd + osidsoffset);
- copy_cifs_sid(nowner_sid_ptr, owner_sid_ptr);
+ size = copy_cifs_sid(nowner_sid_ptr, owner_sid_ptr);
+ *bufsize = *bufsize + size;
/* copy group sid */
group_sid_ptr = (struct cifs_sid *)((char *)pntsd + gsidsoffset);
ngroup_sid_ptr = (struct cifs_sid *)((char *)pnntsd + gsidsoffset);
- copy_cifs_sid(ngroup_sid_ptr, group_sid_ptr);
+ size = copy_cifs_sid(ngroup_sid_ptr, group_sid_ptr);
+ *bufsize = *bufsize + size;
return;
}
@@ -156,10 +171,10 @@ compare_aces(struct cifs_ace *sace, struct cifs_ace *dace, int compflags)
}
static int
-get_sec_desc_size(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd,
- int aces, ssize_t *bufsize, size_t *acesoffset)
+alloc_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd,
+ int aces, size_t *acesoffset)
{
- unsigned int size, acessize, dacloffset;
+ unsigned int size, acessize, bufsize, dacloffset;
size = sizeof(struct cifs_ntsd) +
2 * sizeof(struct cifs_sid) +
@@ -169,9 +184,9 @@ get_sec_desc_size(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd,
*acesoffset = dacloffset + sizeof(struct cifs_ctrl_acl);
acessize = aces * sizeof(struct cifs_ace);
- *bufsize = size + acessize;
+ bufsize = size + acessize;
- *npntsd = malloc(*bufsize);
+ *npntsd = malloc(bufsize);
if (!*npntsd) {
printf("%s: Memory allocation failure", __func__);
return errno;
@@ -188,7 +203,7 @@ ace_set(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
size_t acesoffset;
char *acesptr;
- rc = get_sec_desc_size(pntsd, npntsd, numcaces, bufsize, &acesoffset);
+ rc = alloc_sec_desc(pntsd, npntsd, numcaces, &acesoffset);
if (rc)
return rc;
@@ -198,9 +213,8 @@ ace_set(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
acessize += size;
acesptr += size;
}
- copy_sec_desc(pntsd, *npntsd, numcaces, acessize);
- acesptr = (char *)*npntsd + acesoffset;
+ copy_sec_desc(pntsd, *npntsd, numcaces, acessize, bufsize);
return 0;
}
@@ -215,7 +229,7 @@ ace_add(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
char *acesptr;
numaces = numfaces + numcaces;
- rc = get_sec_desc_size(pntsd, npntsd, numaces, bufsize, &acesoffset);
+ rc = alloc_sec_desc(pntsd, npntsd, numaces, &acesoffset);
if (rc)
return rc;
@@ -230,7 +244,8 @@ ace_add(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
acesptr += size;
acessize += size;
}
- copy_sec_desc(pntsd, *npntsd, numaces, acessize);
+
+ copy_sec_desc(pntsd, *npntsd, numaces, acessize, bufsize);
return 0;
}
@@ -249,7 +264,7 @@ ace_modify(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
return -1;
}
- rc = get_sec_desc_size(pntsd, npntsd, numfaces, bufsize, &acesoffset);
+ rc = alloc_sec_desc(pntsd, npntsd, numfaces, &acesoffset);
if (rc)
return rc;
@@ -270,7 +285,7 @@ ace_modify(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
acessize += size;
}
- copy_sec_desc(pntsd, *npntsd, numfaces, acessize);
+ copy_sec_desc(pntsd, *npntsd, numfaces, acessize, bufsize);
return 0;
}
@@ -294,7 +309,7 @@ ace_delete(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
return -1;
}
- rc = get_sec_desc_size(pntsd, npntsd, numfaces, bufsize, &acesoffset);
+ rc = alloc_sec_desc(pntsd, npntsd, numfaces, &acesoffset);
if (rc)
return rc;
@@ -317,7 +332,8 @@ ace_delete(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize,
printf("%s: Nothing to delete\n", __func__);
return 1;
}
- copy_sec_desc(pntsd, *npntsd, numaces, acessize);
+
+ copy_sec_desc(pntsd, *npntsd, numaces, acessize, bufsize);
return 0;
}
