Re: [PATCH] overlayfs: ignore empty NFSv4 ACLs in ext4 upperdir

[J. Bruce Fields]

On Fri, May 03, 2019 at 01:26:01PM -0400, Amir Goldstein wrote:
> On Fri, May 3, 2019 at 12:03 PM J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> >
> > On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> > > On Tue, Dec 06 2016, J. Bruce Fields wrote:
> > >
> > > > On Tue, Dec 06, 2016 at 02:18:31PM +0100, Andreas Gruenbacher wrote:
> > > >> On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> > > >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas GrÃnbacher
> > > >> > <andreas.gruenbacher@xxxxxxxxx> wrote:
> > > >> >> 2016-12-06 0:19 GMT+01:00 Andreas GrÃnbacher <andreas.gruenbacher@xxxxxxxxx>:
> > > >> >
> > > >> >>> It's not hard to come up with a heuristic that determines if a
> > > >> >>> system.nfs4_acl value is equivalent to a file mode, and to ignore the
> > > >> >>> attribute in that case. (The file mode is transmitted in its own
> > > >> >>> attribute already, so actually converting .) That way, overlayfs could
> > > >> >>> still fail copying up files that have an actual ACL. It's still an
> > > >> >>> ugly hack ...
> > > >> >>
> > > >> >> Actually, that kind of heuristic would make sense in the NFS client
> > > >> >> which could then hide the "system.nfs4_acl" attribute.
> > > >> >
> > > >> > Even simpler would be if knfsd didn't send the attribute if not
> > > >> > necessary.  Looks like there's code actively creating the nfs4_acl on
> > > >> > the wire even if the filesystem had none:
> > > >> >
> > > >> >     pacl = get_acl(inode, ACL_TYPE_ACCESS);
> > > >> >     if (!pacl)
> > > >> >         pacl = posix_acl_from_mode(inode->i_mode, GFP_KERNEL);
> > > >> >
> > > >> > What's the point?
> > > >>
> > > >> That's how the protocol is specified.
> > > >
> > > > Yep, even if we could make that change to nfsd it wouldn't help the
> > > > client with the large number of other servers that are out there
> > > > (including older knfsd's).
> > > >
> > > > --b.
> > > >
> > > >> (I'm not saying that that's very helpful.)
> > > >>
> > > >> Andreas
> > >
> > > Hi everyone.....
> > >  I have a customer facing this problem, and so stumbled onto the email
> > >  thread.
> > >  Unfortunately it didn't resolve anything.  Maybe I can help kick things
> > >  along???
> > >
> > >  The core problem here is that NFSv4 and ext4 use different and largely
> > >  incompatible ACL implementations.  There is no way to accurately
> > >  translate from one to the other in general (common specific examples
> > >  can be converted).
> > >
> > >  This means that either:
> > >    1/ overlayfs cannot use ext4 for upper and NFS for lower (or vice
> > >       versa) or
> > >    2/ overlayfs need to accept that sometimes it cannot copy ACLs, and
> > >       that is OK.
> > >
> > >  Silently not copying the ACLs is probably not a good idea as it might
> > >  result in inappropriate permissions being given away.  So if the
> > >  sysadmin wants this (and some clearly do), they need a way to
> > >  explicitly say "I accept the risk".
> >
> > So, I feel like silently copying ACLs up *also* carries a risk, if that
> > means switching from server-enforcement to client-enforcement of those
> > permissions.
> >
> > Sorry, I know we had another thread recently about permissions in this
> > situation, and I've forgotten the conclusion.
> >
> > Out of curiosity, what's done with selinux labels?
> >
> 
> overlayfs calls security_inode_copy_up_xattr(name) which
> can fail (<0) allow (0) or skip(1).
> 
> selinux_inode_copy_up_xattr() as well as smack_inode_copy_up_xattr()
> skip their own xattr on copy up and fail any other xattr copy up.

If it's OK to silently skip copying up security labels, maybe it's OK to
silently skip NFSv4 ACLs too?

--b.