[PATCH][WIP] Make vfs_acl_xattr use hash of the posix ACL

[Andrew Bartlett]

On Fri, 2012-10-12 at 13:11 +0200, Christian Ambach wrote:
> Hi Andrew,
> 
> On 10/09/2012 01:58 PM, Andrew Bartlett wrote:
> > This patch (still a work in progress, as I know it fails some tests)
> > is what I've been talking about for months, and which I got VFS
> > changes in (almost - one small change in this patch) into 4.0.0rc1.
> >
> > The idea is simple, have the ACL modules (the posix ACL modules in
> > particular) provide a blob form of the ACL, and then hash that,
> > rather than an NT ACL that might change if our ACL mapping code
> > changes.
> 
> I am not sure I am getting the problem you are trying to solve here.
> What is the benefit of adding another representation of an ACL that is
> just intermediate?
> 
> Can you give some more explanations that go further than the commit
> message in your attachment?

This goes back to a discussion that we have had on and off over a few
months.

What I'm working on is an improved implementation of the hash in
vfs_acl_common.c.  The new hash will be of the 'system' ACL, whatever
that is, rather than the NT ACL it maps to.

By defining this interface, vfs_acl_common does not need to know what
the system ACL is, be it posix or nfsv4 or AFS.  It can (if returned)
just hash the contents of the data_blob and store it.  

At a later time, if the contents matches, then the exact NT ACL that the
windows client set is returned.  If the hash does not match, the the
posix, NFSv4 or AFS ACL must have been changed outside Samba, and an
imperfect mapping to an NT ACL is returned instead.

> > So far I've made it simply fall back in the NFSv4 case, but we can
> > define these in IDL as well if we want to do the same there.
> 
> Depending on the use-case you are attempting to solve, I wouldn't
> necessarily agree here. NFSv4 is almost the same as a Windows SD, but
> not completely and there might be potential changes to the mapping
> algorithm, too. There is some work ongoing in that regard (see Alexander
> Werth's patches he sent around a few weeks back).

I would welcome patches to linearise NFSv4 into NDR in the same way I
did for posix ACLs in smb_acl.idl

It was because of the need to handle NFSv4 ACLs that I was asked to add
the additional VFS interface.

> > The idea is to insulate these on-disk values from any required
> > change we have to make.
> 
> 
> > Any thoughts or comments most appreciated.
> + acl_wrapper.access_acl
> + = talloc_steal(frame,
> + smb_vfs_call_sys_acl_get_file(handle,
> + path_p,
> + SMB_ACL_TYPE_ACCESS));

> For me such code would be easier to read if you would use the upper-case
> variant of VFS calls, such as SMB_VFS_SYS_ACL_GET_FILE().
> 
> This makes it much easier to spot the actual calls into the VFS (and
> that convention is used in the whole codebase I have looked into yet).

The choice is quite deliberate.  The upper case versions call the next,
or top module.  This function calls the current module, which often
implements the sys_acl_get_file_fn, and which we then want to call. 

This allows one set of helper functions to assist all the different
posix ACL modules provide linearised ACLs as blobs. 

I hope this clarifies things.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org