Bug 15126 - acl_xattr VFS module may unintentionally use filesystem permissions instead of ACL from xattr
Summary: acl_xattr VFS module may unintentionally use filesystem permissions instead o...
Status: ASSIGNED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 critical (vote)
Target Milestone: ---
Assignee: Ralph Böhme
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-19 07:54 UTC by Ralph Böhme
Modified: 2022-07-29 14:00 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Ralph Böhme 2022-07-19 07:57:48 UTC
I guess while at this, we should also change the default of "acl_xattr:default acl style" to "windows" if "acl_xattr:ignore system acls" is enabled.
Comment 2 Jeremy Allison 2022-07-19 18:17:54 UTC
Couldn't we just check for fsp->base_fsp != NULL and use that when calling into FGETXATTR ?
Comment 3 Ralph Böhme 2022-07-29 13:59:12 UTC
Luckily this is not a security issue...

When using vfs_streams_xattr, for a pathref handle of a stream the system fd will be a fake fd (creater by pipe() in vfs_fake_fd().

The code path is

SMB_VFS_CREATE_FILE(..., "file:stream", ...)
=> open_file():
   if (open_fd):
   -> taking the else branch:
   -> smbd_check_access_rights_fsp(stream_fsp)
      -> SMB_VFS_FGET_NT_ACL(stream_fsp)

Calling SMB_VFS_FGET_NT_ACL(stream_fsp) on a handle of a named stream will try to retrieve the stored ACL when using vfs_acl_xattr, as it ends up calling             
fgetxattr(fake-fd) which returns EBADF. The vfs_acl_xattr code ignores the specific error and handles this as if there was no ACL stored and then runs the code to    
synthesize a default ACL accoding to the setting of "acl:default acl style".

As the correct access check for streams has already been carried out by calling check_base_file_access() from create_file_unixpath(), the above problem is not a      
security issue: it can only lead to "decreased" permissions resulting in unexpected ACCESS_DENIED errors.