SOLVED SMB ACL Inheritance: Parent permissions overriding child permissions at access level

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
Hi,
I was quiet happy with globally setting ACL permissions on my Shares until I needed to manually set a group permissions to read only for a single file.
In the root of one of my shares called "partage" there is a ".share.online" file used by backup sync operations to check if the share is properly mounted.

1- On default inherited settings from the share permissions, here are the ACL permissions of the file:
Code:
getfacl .share.online
# file: .share.online
# owner: someuser
# group: smb_nobody
            owner@:rwxpDdaARWc--s:------I:allow
            group@:r-x---a-R-c---:------I:allow
   group:smb_admin:rwxpDdaARWcCos:------I:allow
group:smb_read_only:r-x---a-R-c---:------I:allow
  group:smb_modify:rwxpDdaARWc--s:------I:allow
group:smb_partage_ro:r-x---a-R-c---:------I:allow
 group:smb_partage:rwxpDdaARWc--s:------I:allow
    group:smb_dlna:r-x---a-R-c---:------I:allow
         everyone@:--------------:------I:allow


2- I set the read only attributes on the file for the group "smb_partage"
Code:
setfacl -m group:smb_partage:read_set:allow .share.online

getfacl .share.online
# file: .share.online
# owner: someuser
# group: smb_nobody
            owner@:rwxpDdaARWc--s:------I:allow
            group@:r-x---a-R-c---:------I:allow
   group:smb_admin:rwxpDdaARWcCos:------I:allow
group:smb_read_only:r-x---a-R-c---:------I:allow
  group:smb_modify:rwxpDdaARWc--s:------I:allow
group:smb_partage_ro:r-x---a-R-c---:------I:allow
 group:smb_partage:r-----a-R-c---:-------:allow
    group:smb_dlna:r-x---a-R-c---:------I:allow
         everyone@:--------------:------I:allow


3- Now, in any client, Android or Windows, any user in the group "smb_partage" can still delete the .share.online file !

4- In windows, I get the warning that permissions are not properly ordered


Capture2.PNG


5- If I accept the prompt to order them, the "smb_partage" group is duplicated with the inherited permissions assigned to the new group
Capture3.PNG


Code:
getfacl .share.online
# file: .share.online
# owner: someuser
# group: smb_nobody
 group:smb_partage:r-----a-R-c---:-------:allow
            owner@:rwxpDdaARWc---:------I:allow
            group@:r-x---a-R-c---:------I:allow
   group:smb_admin:rwxpDdaARWcCo-:------I:allow
group:smb_read_only:r-x---a-R-c---:------I:allow
  group:smb_modify:rwxpDdaARWc---:------I:allow
group:smb_partage_ro:r-x---a-R-c---:------I:allow
 group:smb_partage:rwxpDdaARWc---:------I:allow
    group:smb_dlna:r-x---a-R-c---:------I:allow



6- I now set the read only attribute on the file: this causes the file to become hidden from listing to any user in the "smb_partage" group
Code:
setfacl -m group:smb_partage:r:allow .share.online

# relevant code only:
getfacl .share.online
    group:smb_partage:r-------------:-------:allow


Why applying a different than the inherited ACL to a file causes this issue ? Mainly, it seems that the inherited ACLs always and forcefully apply to created files when using setfacl

Thank you for your help
 
Last edited:

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
I'll take a look at this in a second, but in general if you discover what you believe is a security issue please file a JIRA ticket in our bugtracker or send an email to our security team.
 
Last edited:

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
Initial observation:
(1) the inherited ACL you posted indicates that the smb_partage group has DELETE_CHILD permissions on the parent directory. This permission is sufficient to grant members of the group the ability to delete contents of the directory. This explains item (3).

(4) - the misordered permissions are because you chose to edit the ACL from the CLI. setfacl does attempt to canonicalize the ACL order. So this is expected.
(5) - this is Windows client fixing your ACL ordering and re-applying ACL entries that should be present because you haven't disabled auto-inheritance. So this is expected.
(6) - you removed ability to "read attributes", and so the users in that group no longer can stat() the file, which means there's no way to tell (among other things) whether this is a file or a directory. You need to grant at a minimum `raRc` permissions set. So this is expected.

Using setfacl is an unsupported and advanced operation. It works, but you have to understand the ins-and-outs of how permissions work.
 

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
There are also various situations where a user may be able to override the ACL on a file. For instance:
- he is the file owner.
- he is a member of BUILTIN\Administrators group
- he has been explicitly granted ability to override DAC.

But I don't see anything out of the ordinary here. That said, I'd like to look at the debug / communicate through PM in order to make sure I'm not missing anything.
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
Initial observation:
(1) the inherited ACL you posted indicates that the smb_partage group has DELETE_CHILD permissions on the parent directory. This permission is sufficient to grant members of the group the ability to delete contents of the directory. This explains item (3).

About this part of your answer. Am I missing something in step (2) ?
I read these docs:

In below code, in step (2), it seems like I properly unset inheritance for the smb_partage group on that specific already existing file. From the getfacl output below, I don't see any entry allowing smb_partage members to delete the ".share.online" file, yet they are able to do it

- I thought the inherited ACL entries from parent are there to be applied at file creation on child files/dirs. They should not apply at child file modification or to grant access !
- I did not see anywhere in the GUI where the inheritance was set to start with ? Mainly, in the oracle doc it is written that by default, inheritance is disabled, but I guess in TrueNAS it is forced by default when creating an ACL SMB enabled share !

Code:
setfacl -m group:smb_partage:read_set:allow .share.online

getfacl .share.online
# file: .share.online
# owner: someuser
# group: smb_nobody
            owner@:rwxpDdaARWc--s:------I:allow
            group@:r-x---a-R-c---:------I:allow
   group:smb_admin:rwxpDdaARWcCos:------I:allow
group:smb_read_only:r-x---a-R-c---:------I:allow
  group:smb_modify:rwxpDdaARWc--s:------I:allow
group:smb_partage_ro:r-x---a-R-c---:------I:allow
 group:smb_partage:r-----a-R-c---:-------:allow
    group:smb_dlna:r-x---a-R-c---:------I:allow
         everyone@:--------------:------I:allow


And just for the info, even if you don't need it because of the Inheritance showing the parent ACLs:
Code:
getfacl partage

# file: partage
# owner: someowner
# group: smb_nobody
            owner@:rwxpDdaARWc--s:fd-----:allow
            group@:r-x---a-R-c---:fd-----:allow
   group:smb_admin:rwxpDdaARWcCos:fd-----:allow
group:smb_read_only:r-x---a-R-c---:fd-----:allow
  group:smb_modify:rwxpDdaARWc--s:fd-----:allow
group:smb_partage_ro:r-x---a-R-c---:fd-----:allow
 group:smb_partage:rwxpDdaARWc--s:fd-----:allow
    group:smb_dlna:r-x---a-R-c---:fd-----:allow
         everyone@:--------------:fd-----:allow



Give me sometime to sanitize a minimum the debug log because of some sensitive info in file names please. I will gladly take the time to do it it if you really think there is a bug in what I posted and that you cannot reproduce it

Thank you again for all the support on the SMB code
 
Last edited:

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
TrueNAS isn't solaris. Solaris documentation is not expected to be correct. The moment you choose to change the ACL through an SMB client, conceptually all bets are off WRT inheritance. Windows may force certain behaviors to "fix" an issue, but that is happening through the SMB client and not the FreeBSD kernel / ZFS.

We have ACL inheritance enabled because that's what is needed on SMB shares (and in general). Our ACLs act basically the same as NTFS ACLs in windows.

FYI, inheritance flags are set per-ACE in NFSv4 and NTFS ACLs.
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
TrueNAS isn't solaris. Solaris documentation is not expected to be correct. The moment you choose to change the ACL through an SMB client, conceptually all bets are off WRT inheritance. Windows may force certain behaviors to "fix" an issue, but that is happening through the SMB client and not the FreeBSD kernel / ZFS.

We have ACL inheritance enabled because that's what is needed on SMB shares (and in general). Our ACLs act basically the same as NTFS ACLs in windows.

FYI, inheritance flags are set per-ACE in NFSv4 and NTFS ACLs.

Ok, I dropped setfacl and I tried on windows 10 enterprise to alter the ".share.online" file access permissions and remove inheritance on all entries:

The end result is still the same and seems unexpected !

I disable inheritance on the file to be able to edit the permissions, then I properly set the permissions for smb_partage group to read only:

Capture4.PNG

Capture5.PNG



After applying, I verify the permissions were properly set in TrueNAS:
Code:
getfacl .share.online

# file: .share.online
# owner: someowner
# group: smb_nobody
            owner@:rwxpDdaARWc---:-------:allow
            group@:r-x---a-R-c---:-------:allow
   group:smb_admin:rwxpDdaARWcCo-:-------:allow
group:smb_read_only:r-x---a-R-c---:-------:allow
  group:smb_modify:rwxpDdaARWcCo-:-------:allow
group:smb_partage_ro:r-x---a-R-c---:-------:allow
 group:smb_partage:r-----a-R-c---:-------:allow
    group:smb_dlna:r-x---a-R-c---:-------:allow


Still, any user in smb_partage group, obviously not owning the file and not member of any other group allowing write/delete permissions, is capable of deleting the ".share.online" file

This is fine for my specific use because I only apply changes from TrueNAS usually. However, it still seems, unless I am really missing something, that any modification outside TrueNAS UI, even from Windows or the CLI in TrueNAS, will completely break the ACLs.
What's surprising is that the file level permissions seem overridden by the parent permissions. I can understand it on a new file creation, but not at the file access level !

Are you sure there is not a bug allowing the inheritance to apply when accessing child entries, always bypassing the child own permissions ?

Let me know to send the debug logs if it is clearly a bug that you cannot reproduce. However, I tried now on different shares, on a new dataset and new share, and on two other windows machines, still same result
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
To add to my above proof of concept, here's the Help tip in TrueNAS GUI from the "per ACE" inheritance flag I found after your post:
Flags / Inherit:
How this ACE is applied to newly created directories and files within the dataset. Basic flags enable or disable ACE inheritance. Advanced flags allow further control of how the ACE is applied to files and directories in the dataset.

As I read, it clearly should only apply "to newly created directories and files", not on existing ones and not bypassing child specific permissions on access !

Should I submit a Jira ticket ?
 

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
Without seeing paths so that I know which is a parent directory and which is the file in question and user tokens I can't give an absolute answer, but

Still, any user in smb_partage group, obviously not owning the file and not member of any other group allowing write/delete permissions, is capable of deleting the ".share.online" file
Is expected if user has "delete child" permissions or "write data" (depending on context) on the parent directory. See RFC here: https://datatracker.ietf.org/doc/html/rfc5661#section-6.2.1.3.2
 

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
Example:
Code:
root@homenas[/mnt/dozer/ISO]# getfacl /mnt/dozer/ISO/unlinktest
# file: /mnt/dozer/ISO/unlinktest
# owner: smbuser
# group: wheel
      user:isouser:r-x---a-R-c---:fd-----:allow
   group:SMBADMINS:rwxpDdaARWcCos:fd----I:allow
            owner@:rwxpDdaARWcCos:fd----I:allow
            group@:rwxpDdaARWcCos:fd----I:allow
root@homenas[/mnt/dozer/ISO]# getfacl /mnt/dozer/ISO/unlinktest/filetodelete
# file: /mnt/dozer/ISO/unlinktest/filetodelete
# owner: root
# group: wheel
      user:isouser:r-x---a-R-c---:------I:allow
   group:SMBADMINS:rwxpDdaARWcCos:------I:allow
            owner@:rwxpDdaARWcCos:------I:allow
            group@:rwxpDdaARWcCos:------I:allow
root@homenas[/mnt/dozer/ISO]# smbclient //127.0.0.1/iso_share -U isouser -c 'rm unlinktest\filetodelete'
Enter WORKGROUP\isouser's password:
NT_STATUS_ACCESS_DENIED deleting remote file \unlinktest\filetodelete

^^^ unlink fails

If I try to edit ACL with READ permissions:
Code:
root@homenas[/mnt/dozer/ISO]# smbcacls //127.0.0.1/iso_share unlinktest -U isouser
Enter WORKGROUP\isouser's password:
REVISION:1
CONTROL:SR|DI|DP
OWNER:HOMENAS\smbuser
GROUP:Unix Group\wheel
ACL:HOMENAS\isouser:ALLOWED/OI|CI/READ
ACL:HOMENAS\SMBADMINS:ALLOWED/OI|CI|I/FULL
ACL:HOMENAS\smbuser:ALLOWED/I/FULL
ACL:Creator Owner:ALLOWED/OI|CI|IO|I/FULL
ACL:Unix Group\wheel:ALLOWED/I/FULL
ACL:Creator Group:ALLOWED/OI|CI|IO|I/FULL
root@homenas[/mnt/dozer/ISO]# smbcacls //127.0.0.1/iso_share unlinktest -U isouser -a 'ACL:HOMENAS\isouser:ALLOWED/OI|CI/FULL'
Enter WORKGROUP\isouser's password:
Failed to open \unlinktest: NT_STATUS_ACCESS_DENIED


If I grant full permissions to parent dir, I can delete:
Code:
root@homenas[/mnt/dozer/ISO]# setfacl -m u:isouser:full_set:fd:allow /mnt/dozer/ISO/unlinktest
root@homenas[/mnt/dozer/ISO]# smbclient //127.0.0.1/iso_share -U isouser -c 'rm unlinktest\filetodelete'
Enter WORKGROUP\isouser's password:
root@homenas[/mnt/dozer/ISO]#


this is all expected behavior.
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
Without seeing paths so that I know which is a parent directory and which is the file in question and user tokens I can't give an absolute answer, but


Is expected if user has "delete child" permissions or "write data" (depending on context) on the parent directory. See RFC here: https://datatracker.ietf.org/doc/html/rfc5661#section-6.2.1.3.2

Well, reading that RFC entry, you are right, sorry. But it was really strange since I never expected ACLs of the parent when inheriting, to apply on child at access level, in addition to at new entry creation.

All the docs I read clearly specify that the inherit flag applies at "new file/directory creation", never specifying that it overrides any specific permission on access level.

In my case, yes, the parent dataset/share has the below permissions as I posted previously:
Code:
getfacl partage

# file: partage
# owner: someuser
# group: smb_nobody
            owner@:rwxpDdaARWc--s:fd-----:allow
            group@:r-x---a-R-c---:fd-----:allow
   group:smb_admin:rwxpDdaARWcCos:fd-----:allow
group:smb_read_only:r-x---a-R-c---:fd-----:allow
  group:smb_modify:rwxpDdaARWc--s:fd-----:allow
group:smb_partage_ro:r-x---a-R-c---:fd-----:allow
 group:smb_partage:rwxpDdaARWc--s:fd-----:allow
    group:smb_dlna:r-x---a-R-c---:fd-----:allow
         everyone@:--------------:fd-----:allow


The inherit flag is enabled for every single ACE in the "partage" dataset

So, if I understand it well, whatever permissions are set on "partage/.share.online" file, they will be simply ignored if a similar ACE is set on the parent. The assumption that the flag only applies at new files/dirs creation is not complete and lets some interpretation.

Thank you again as I really was not aware of such a thing
 
Last edited:

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
Or well, I should just consider them like Network ACE / ACL rules...
Thank you again for the support and clarifications
 
Last edited:

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
I edited the thread title to remove any doubt about an ongoing vulnerability. Hope it can help others and maybe improve the Help tip of the inherit ACE flag in future releases
 

anodos

Sambassador
iXsystems
Joined
Mar 6, 2014
Messages
9,554
ACL is evaluated any time internally in ZFS any time zfs_zaccess() is called. This means that many / most VFS ops in FreeBSD / Linux will face an ACL check. During creation of new object (dir / file), ZFS will calculate an inherited ACL and apply it to the new object. This is the primary place where the ACE inheritance flags play a role. Depending on the context, userspace utilities (or SMB clients) may have convenience features for administering ACLs. In SCALE I've implemented NFSv41 ACL auto-inheritance flags -- see RFC5661 (but haven't exposed yet through middleware / GUI).
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
ACL is evaluated any time internally in ZFS any time zfs_zaccess() is called. This means that many / most VFS ops in FreeBSD / Linux will face an ACL check. During creation of new object (dir / file), ZFS will calculate an inherited ACL and apply it to the new object. This is the primary place where the ACE inheritance flags play a role. Depending on the context, userspace utilities (or SMB clients) may have convenience features for administering ACLs. In SCALE I've implemented NFSv41 ACL auto-inheritance flags -- see RFC5661 (but haven't exposed yet through middleware / GUI).
I read through it and the auto inherit implementation looks great
However, your reference to this section "ACE4_DELETE vs. ACE4_DELETE_CHILD" is the most relevant for my issue
Thank you again
 

Phil1295

Explorer
Joined
Sep 20, 2020
Messages
79
Without seeing paths so that I know which is a parent directory and which is the file in question and user tokens I can't give an absolute answer, but


Is expected if user has "delete child" permissions or "write data" (depending on context) on the parent directory. See RFC here: https://datatracker.ietf.org/doc/html/rfc5661#section-6.2.1.3.2
I can confirm the following behavior:
- permissions are properly read on access. Parent permissions properly do not override child permissions
- on the parent share "partage", I set the ACE for group "smb_partage" to advanced, I allow read/write, but deny "take ownership", "write ACL" and DELETE_CHILD. I enable basic inherit flag.
- I manually set the "partage/testfile" ACE for group "smb_partage" to read only

=> Now, members of smb_partage, effectively cannot delete "partage/testfile", unless they own it obviously
=> They still can delete other files under "partage/" since the files inherit the delete flag, unless it is manually overridden

So it works as in the RFC you mention, and after all similarly to UNIX unlink permission on directories contents

Thank you for your precious guidance. I can now fix this specific issue I had with this share open permissions
 
Last edited:
Top