When setting a non-default ACL, don't forget to apply masks to SMB_ACL_USER and SMB_ACL_GROUP entries.

[Jeremy Allison]

On Fri, Oct 05, 2012 at 02:36:44PM -0700, Jeremy Allison wrote:
> On Sat, Oct 06, 2012 at 07:27:28AM +1000, Andrew Bartlett wrote:
> > On Fri, 2012-10-05 at 08:44 -0700, Jeremy Allison wrote:
> > > On Fri, Oct 05, 2012 at 06:01:08PM +1000, Andrew Bartlett wrote:
> > > > On Tue, 2012-10-02 at 22:28 +0200, Jeremy Allison wrote:
> > > > > commit 6575d1d34fee45c7a965c7c9641cc52b566a9e7f
> > > > > Author: Jeremy Allison <jra at samba.org>
> > > > > Date:   Tue Oct 2 10:15:54 2012 -0700
> > > > > 
> > > > >     When setting a non-default ACL, don't forget to apply masks to
> > > > > SMB_ACL_USER and SMB_ACL_GROUP entries.
> > > > 
> > > > Jeremy,
> > > > 
> > > > With this change, does this mean we have changed the mapping between
> > > > posix ACLs and NT ACLs?
> > > > 
> > > > If so, I'm concerned that any NT ACLs that have been set with
> > > > vfs_acl_xattr will be invalidated, as the hash won't match up.  
> > > 
> > > Andrew, it *never* matters on set, it only matters on get.
> > > 
> > > On set we will a re-hash, so changing the mapping on sets
> > > doesn't matter.
> > 
> > Sure, but this code seems to be in the get codepath,
> > ensure_canon_entry_valid() is called via canonicalise_acl() from
> > posix_get_nt_acl_common(). 
> 
> static bool ensure_canon_entry_valid(connection_struct *conn,
>                                         canon_ace **pp_ace,
>                                         bool is_default_acl,
>                                         const struct share_params *params,
>                                         const bool is_directory,
>                                         const struct dom_sid *pfile_owner_sid,
>                                         const struct dom_sid *pfile_grp_sid,
>                                         const SMB_STRUCT_STAT *pst,
>                                         bool setting_acl)
> 
> Note the last bool parameter, which is only true when we're
> setting an ACL.

Although as Volker often points out :-), it might be a better
solution to factor this function out into a version for set
and one for get. It would remove some of the entwined logic.

Jeremy.