[Samba] Issue with acl_xattr:ignore system acls in 4.5rc2

[Ralph Böhme]

On Thu, Aug 18, 2016 at 12:21:37PM +0300, Uri Simchoni wrote:
> On 08/18/2016 08:28 AM, Ralph Böhme wrote:
> > Hi Eric,
> > 
> > On Wed, Aug 17, 2016 at 05:56:51PM -0600, Eric Eastman via samba wrote:
> >> I was testing Samba 4.5rc2 with an existing smb.conf file that I have been
> >> using since Samba 4.1.x and found that I cannot access data in the share on
> >> Windows 2012 (my AD server), a Windows 2008 client or on a Ubuntu 16.04
> >> client.  I built a new version of Samba 4.4.5 using the same procedure as I
> >> did for building 4.5rc2 and the 4.4.5 Samba version worked with my smb.conf
> >> file.  I then created a very basic smb.conf file and slowly built it up,
> >> and by doing testing between 4.4.5 and 4.5rc2, I finally found the line
> >> that caused the problem. Here is my cut down smb.conf file:
> >>
> >> [global]
> >> security = ads
> >> realm = ERIC.LOCAL
> >> workgroup = ERIC
> >> netbios name = C1-GW03-T5
> >> idmap uid = 500-10000000
> >> idmap gid = 500-10000000
> >> winbind use default domain = Yes
> >> winbind nested groups = Yes
> >> map acl inherit = yes
> >> vfs objects = acl_xattr streams_xattr
> >> acl_xattr:ignore system acls = yes
> >> load printers = no
> >> printcap name = /dev/null
> >>
> >> [zzz]
> >> path = /zzz
> >> writeable = yes
> >> browseable = yes
> >>
> >>
> >> The line causing the problem with 4.5rc2 is:
> >>   acl_xattr:ignore system acls = yes
> > 
> > this change was introduced in
> > <https://bugzilla.samba.org/show_bug.cgi?id=12028>
> > 
> > Before explaining the gory details, one question: why are you setting
> > this option?
> > 
> > With the patches from the bugreport the behaviour of vfs_acl_xattr
> > changed when "acl_xattr:ignore system acls" is set to yes for the case
> > a file or directory lacks the NT ACL xattr (eg when creating files and
> > directories on the server).
> > 
> > Before the change, we would query the system POSIX permissions and map
> > them to a NT ACL.
> > 
> > After the change we sythesize a default NT ACL with just
> > 
> >   ACL:$uid:ALLOWED/0x0/$mapped_mode
> >   ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
> > 
> > As this severly impacts existing setups, we have three options to
> > address this:
> > 
> > 1. Revert it,
> > 2. Document it, or
> > 3. Do it differently
> > 
> > 1. Revert it
> > 
> > Brings back the original problem: not behaving as a Windows server and
> > in certain situations unexpectedly exposing system POSIX permissions
> > as described in the above bug.
> > 
> > 2. Document it
> > 
> > One could argue that this works as designed, so just add a big note to
> > the release notes so people are aware of this change. As everybody
> > reads release notes, there'll be no surprise. :)
> > 
> > When acl_xattr:ignore system acls is set to yes, we now completely
> > ignore the system POSIX permissions. Since bug 11806 we already ignore
> > the the system POSIX permissions when setting an NT ACL, we now ignore
> > them as well when querying them. So this can be seen as improvement
> > because previously our behaviour was inconsistent.
> > 
> > In some sense "acl_xattr:ignore system acls = yes" works like a
> > fictional option "acl_xattr:ignore system permissions = yes".
> > 
> > Any filesytem object without NT ACL xattr will get default permissions
> > matching Windows:
> > 
> >   ACL:$uid:ALLOWED/0x0/$mapped_mode
> >   ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
> > 
> > This affects directories created on the server, like in Eric'c case,
> > or when an SMB client creates a file or directory where the parent
> > directory has no inheritable ACEs, as described in bug 12028.
> > 
> > 3. Do it differently
> > 
> > Implement the fictional option from 2 "acl_xattr:ignore system
> > permissions" and only ignore system POSIX permissions when querying
> > permissions if this is set. Sing: one more option before you go, to
> > the valley below.
> > 
> > *scratching head* Thoughts?
> 
> I think the current behavior is correct (also tested it against Windows
> for comparison).

thanks for confirming.

> However, if it breaks things for users, we need a way out for them, and
> the only way out that's guaranteed to work in all cases is a
> compatibility switch - generate the "old" DACL as default, wrong or
> surprising as it may be, to maintain backward compatibility. So that's
> an additional "acl_xattr:legacy default acl = yes" param IMHO.

well, if we conclude we need a new option that

a) preserves existing behaviour, and

b) allows enabling full Windows compatibility by completely ignoring
   system POSIX permissions,

then my suggestion would be

o restore the previous behaviour of "acl_xattr:ignore system acls =
  yes", in order not to break exisiting setups, and

o add a new option that would tell us to completely ignore system
  POSIX permissions when querying and setting, defaulting to off:

  acl_xattr:ignore system permissions = yes | no (default: no)

  "acl_xattr:ignore system permissions = yes" implicitly sets
  "acl_xattr:ignore system acls = yes"

The name "ignore system permissions" also better reflects the fact
that this option is not only about ACLs, but about system permissions
including POSIX mode.

We should then mark "acl_xattr:ignore system acls" as legacy and
explain the broken behaviour (system permisions ignored when setting,
but sometimes (when the filesystem object lacks an NT ACL xattr) still
used when querying).

*still scratching head* :) Thoughts?

Cheerio!
-slow