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

[J. Bruce Fields]

On Wed, Sep 18, 2019 at 11:07:31AM +0200, Miklos Szeredi wrote:
> On Fri, May 10, 2019 at 04:09:41PM -0400, J. Bruce Fields wrote:
> > On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> > > Interesting perspective  though doesn't NFSv4 explicitly allow
> > > client-side ACL enforcement in the case of delegations?
> > 
> > Not really.  What you're probably thinking of is the single ACE that the
> > server can return on granting a delegation, that tells the client it can
> > skip the ACCESS check for users matching that ACE.  It's unclear how
> > useful that is.  It's currently unused by the Linux client and server.
> > 
> > > Not sure how relevant that is
> > > 
> > > It seems to me we have two options:
> > >  1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> > > recommend people use NFSv3, or
> > >  2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
> > >  2a/ always - and ignore all other acls and probably all system. xattrs,
> > >  or
> > >  2b/ based on a mount option that might be
> > >   2bi/ general "noacl" or might be
> > >   2bii/ explicit "noxattr=system.nfs4acl"
> > >  
> > > I think that continuing to discuss the miniature of the options isn't
> > > going to help.  No solution is perfect - we just need to clearly
> > > document the implications of whatever we come up with.
> > > 
> > > I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> > > me.
> > 
> > I guess I'd also lean towards 2a.
> > 
> > I don't think it applies to posix acls, as overlayfs is capable of
> > copying those up and evaluating them on its own.
> 
> POSIX acls are evaluated and copied up.
> 
> I guess same goes for "security.*" attributes, that are evaluated on MAC 
> checks.
> 
> I think it would be safe to ignore failure to copy up anything else.  That 
> seems
> a bit saner than just blacklisting nfs4_acl...
> 
> Something like the following untested patch.

It seems at least simple to implement and explain.

>  fs/overlayfs/copy_up.c |   16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -36,6 +36,13 @@ static int ovl_ccup_get(char *buf, const
>  module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644);
>  MODULE_PARM_DESC(check_copy_up, "Obsolete; does nothing");
>  
> +static bool ovl_must_copy_xattr(const char *name)
> +{
> + return !strcmp(name, XATTR_POSIX_ACL_ACCESS) ||
> +!strcmp(name, XATTR_POSIX_ACL_DEFAULT) ||
> +!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN);
> +}
> +
>  int ovl_copy_xattr(struct dentry *old, struct dentry *new)
>  {
>   ssize_t list_size, size, value_size = 0;
> @@ -107,8 +114,13 @@ int ovl_copy_xattr(struct dentry *old, s
>   continue; /* Discard */
>   }
>   error = vfs_setxattr(new, name, value, size, 0);
> - if (error)
> - break;
> + if (error) {

Can we check for EOPNOTSUPP instead of any error?

Maybe we're copying up a user xattr to a filesystem that's perfectly
capable of supporting those.  And maybe there's a disk error or we run
out of disk space or something.  Then I'd rather get EIO or ENOSPC than
silently fail to copy some xattrs.

--b.

> + if (ovl_must_copy_xattr(name))
> + break;
> +
> + /* Ignore failure to copy unknown xattrs */
> + error = 0;
> + }
>   }
>   kfree(value);
>  out:

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

[Miklos Szeredi]

On Fri, May 10, 2019 at 04:09:41PM -0400, J. Bruce Fields wrote:
> On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> > Interesting perspective  though doesn't NFSv4 explicitly allow
> > client-side ACL enforcement in the case of delegations?
> 
> Not really.  What you're probably thinking of is the single ACE that the
> server can return on granting a delegation, that tells the client it can
> skip the ACCESS check for users matching that ACE.  It's unclear how
> useful that is.  It's currently unused by the Linux client and server.
> 
> > Not sure how relevant that is
> > 
> > It seems to me we have two options:
> >  1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> > recommend people use NFSv3, or
> >  2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
> >  2a/ always - and ignore all other acls and probably all system. xattrs,
> >  or
> >  2b/ based on a mount option that might be
> >   2bi/ general "noacl" or might be
> >   2bii/ explicit "noxattr=system.nfs4acl"
> >  
> > I think that continuing to discuss the miniature of the options isn't
> > going to help.  No solution is perfect - we just need to clearly
> > document the implications of whatever we come up with.
> > 
> > I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> > me.
> 
> I guess I'd also lean towards 2a.
> 
> I don't think it applies to posix acls, as overlayfs is capable of
> copying those up and evaluating them on its own.

POSIX acls are evaluated and copied up.

I guess same goes for "security.*" attributes, that are evaluated on MAC checks.

I think it would be safe to ignore failure to copy up anything else.  That seems
a bit saner than just blacklisting nfs4_acl...

Something like the following untested patch.

Thanks,
Miklos

---
 fs/overlayfs/copy_up.c |   16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -36,6 +36,13 @@ static int ovl_ccup_get(char *buf, const
 module_param_call(check_copy_up, ovl_ccup_set, ovl_ccup_get, NULL, 0644);
 MODULE_PARM_DESC(check_copy_up, "Obsolete; does nothing");
 
+static bool ovl_must_copy_xattr(const char *name)
+{
+   return !strcmp(name, XATTR_POSIX_ACL_ACCESS) ||
+  !strcmp(name, XATTR_POSIX_ACL_DEFAULT) ||
+  !strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN);
+}
+
 int ovl_copy_xattr(struct dentry *old, struct dentry *new)
 {
ssize_t list_size, size, value_size = 0;
@@ -107,8 +114,13 @@ int ovl_copy_xattr(struct dentry *old, s
continue; /* Discard */
}
error = vfs_setxattr(new, name, value, size, 0);
-   if (error)
-   break;
+   if (error) {
+   if (ovl_must_copy_xattr(name))
+   break;
+
+   /* Ignore failure to copy unknown xattrs */
+   error = 0;
+   }
}
kfree(value);
 out:

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

[J. Bruce Fields]

On Tue, May 07, 2019 at 10:24:58AM +1000, NeilBrown wrote:
> Interesting perspective  though doesn't NFSv4 explicitly allow
> client-side ACL enforcement in the case of delegations?

Not really.  What you're probably thinking of is the single ACE that the
server can return on granting a delegation, that tells the client it can
skip the ACCESS check for users matching that ACE.  It's unclear how
useful that is.  It's currently unused by the Linux client and server.

> Not sure how relevant that is
> 
> It seems to me we have two options:
>  1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
> recommend people use NFSv3, or
>  2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
>  2a/ always - and ignore all other acls and probably all system. xattrs,
>  or
>  2b/ based on a mount option that might be
>   2bi/ general "noacl" or might be
>   2bii/ explicit "noxattr=system.nfs4acl"
>  
> I think that continuing to discuss the miniature of the options isn't
> going to help.  No solution is perfect - we just need to clearly
> document the implications of whatever we come up with.
> 
> I lean towards 2a, but I be happy with with any '2' and '1' won't kill
> me.

I guess I'd also lean towards 2a.

I don't think it applies to posix acls, as overlayfs is capable of
copying those up and evaluating them on its own.

--b.

> 
> Do we have a vote?  Or does someone make an executive decision??
> 
> NeilBrown

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

[J. Bruce Fields]

On Tue, May 07, 2019 at 04:07:21AM -0400, Miklos Szeredi wrote:
> On Fri, May 3, 2019 at 11:35 AM J. Bruce Fields  wrote:
> >
> > On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:
> 
> > >  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.
> 
> That's not correct: permissions are checked on the overlay layer,
> regardless of where the actual file resides.  For filesystems using a
> server enforced permission model that means possibly different
> permissions for accesses through overlayfs than for accesses without
> overlayfs.  Apparently this is missing from the documentation and
> definitely needs to be added.

Well, we did have a thread on this pretty recently, I think, and I'm
just not remembering the conclusion.  Yes, it'd be nice to have this
documented.

In the case of NFSv4 ACLs, we not only lack storage for them, we don't
even have code to evaluate them.

--b.

> So I think it's perfectly fine to allow copying up ACLs, as long as
> the ACL is representable on the upper fs.  If that cannot be ensured,
> then the only sane thing to do is to disable ACL checking across the
> overlay ("noacl" option).

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

[Miklos Szeredi]

On Fri, May 3, 2019 at 11:35 AM J. Bruce Fields  wrote:
>
> On Thu, May 02, 2019 at 12:02:33PM +1000, NeilBrown wrote:

> >  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.

That's not correct: permissions are checked on the overlay layer,
regardless of where the actual file resides.  For filesystems using a
server enforced permission model that means possibly different
permissions for accesses through overlayfs than for accesses without
overlayfs.  Apparently this is missing from the documentation and
definitely needs to be added.

So I think it's perfectly fine to allow copying up ACLs, as long as
the ACL is representable on the upper fs.  If that cannot be ensured,
then the only sane thing to do is to disable ACL checking across the
overlay ("noacl" option).

Thanks,
Miklos

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

[NeilBrown]

On Fri, May 03 2019, J. Bruce Fields 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  wrote:
>> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>> >> >  wrote:
>> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
>> >> >> :
>> >> >
>> >> >>> 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.

Interesting perspective  though doesn't NFSv4 explicitly allow
client-side ACL enforcement in the case of delegations?
Not sure how relevant that is

It seems to me we have two options:
 1/ declare the NFSv4 doesn't work as a lower layer for overlayfs and
recommend people use NFSv3, or
 2/ Modify overlayfs to work with NFSv4 by ignoring nfsv4 ACLs either
 2a/ always - and ignore all other acls and probably all system. xattrs,
 or
 2b/ based on a mount option that might be
  2bi/ general "noacl" or might be
  2bii/ explicit "noxattr=system.nfs4acl"
 
I think that continuing to discuss the miniature of the options isn't
going to help.  No solution is perfect - we just need to clearly
document the implications of whatever we come up with.

I lean towards 2a, but I be happy with with any '2' and '1' won't kill
me.

Do we have a vote?  Or does someone make an executive decision??

NeilBrown


signature.asc
Description: PGP signature

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

[Vivek Goyal]

On Fri, May 03, 2019 at 01:41:25PM -0400, Amir Goldstein wrote:
> On Fri, May 3, 2019 at 1:31 PM J. Bruce Fields  wrote:
> >
> > 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  
> > > 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 
> > > > > >>  wrote:
> > > > > >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > > > > >> >  wrote:
> > > > > >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > > > > >> >> :
> > > > > >> >
> > > > > >> >>> 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?
> >
> 
> I think overlayfs inode security context is taken from overlayfs
> mount parameters (i.e. per container context) and therefore
> the lower security. xattr are ignored (CC Vivek).

If mount was done with "context=" option, then it is used otherwise
selinux security context comes from real inode xattr (lower/upper).

Thanks
Vivek

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

[Amir Goldstein]

On Fri, May 3, 2019 at 1:31 PM J. Bruce Fields  wrote:
>
> 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  
> > 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  
> > > > >> wrote:
> > > > >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > > > >> >  wrote:
> > > > >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > > > >> >> :
> > > > >> >
> > > > >> >>> 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?
>

I think overlayfs inode security context is taken from overlayfs
mount parameters (i.e. per container context) and therefore
the lower security. xattr are ignored (CC Vivek).

Thanks,
Amir.

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

[Goetz, Patrick G]

On 5/3/19 10:27 AM, J. Bruce Fields wrote:
> Christoph also had some objections to the implementation which I think
> were addressed, but I could be wrong.


I'm certainly no expert, but yes, the objections to the RichACL patches 
were addressed and not really counter challenged.  It seems like a case 
of Windows ACLs are yucky and complicated and not needed in all linux 
environments (which, by the way, I've learned isn't entirely true).


-
Christoph Hellwig:
 > For one I still see no reason to merge this broken ACL model at all.
 > It provides our actualy Linux users no benefit at all, while breaking
 > a lot of assumptions, especially by adding allow and deny ACE at the
 > same sime.

From: Andreas Gruenbacher:
This permission model is useful in mixed environments that involve
UNIX and Windows machines. Think of NAS boxes with Linux and Windows
clients, for example. It also fits the NFSv4 ACL model very well. If
you're not a user dealing with such environments, then the model
likely won't provide any benefits to *you*, and you're better off with
a less complicated permission model. That doesn't say anything about
other users, though.
-

and here:

-
Christoph Hellwig:
 > It also doesn't help with the issue that the main thing it's trying
 > to be compatible with (Windows) actually uses a fundamentally
 > different identifier to apply the ACLs to - as long as you're
 > still limited to users and groups and not guids we'll still
 > have that mapping problem anyway.

From: Andreas Gruenbacher:
Samba has been dealing with mapping between SIDs and UIDs/GIDs for a
long time, and it's working acceptably well.

We could store SIDs in ACEs, but that wouldn't make the actual
problems go away: Files on Linux have an owner and an owning group
which are identitifed by UID/GID, whereas a file is owned by a SID
which can be either a user or a group in a SID world. Also, processes
on Linux have an owner and a list of groups which are identified by
UID/GID, so any SIDs stored in filesystems would never match a
process, anyway.

(NFSv4 refers to users and groups as opposed to SIDs, and so it
doesn't have this problem.)
-

Further, the counterarguments seem weak, at best:

 > People have long learned that we only have 'alloc' permissions.  Any
 > model that mixes allow and deny ACE is a mistake.

As someone pointed out elsewhere in the thread, linux people are 
hopefully used to learning new tricks.

Who these days has the luxury of not working in a mixed environment?  I 
agree that Windows ACLs are kind of gross and overly complicated (and as 
a result, many people don't use them at all and end up with low security 
permission environments), but the professional Windows admins I know do 
use them, and we've had a number of use cases already where permissions 
problems are simply solved using Windows ACLs, impossible to get just 
right with mode bits and POSIX ACLs.  Not having RichACLs just makes it 
really easy for the Windows storage people to win every argument.

The SID <--> UID thing is manageable; we're doing it now.  Dealing with 
groups is a bit harder.  What I've been doing is continuing to use 
/etc/group, but populating the entries with the usernames associated 
with SIDs (which in our case are mapped directly to UIDs).  For files 
owned by a group SID, the simple solution is just to treat this SID as a 
dummy UID.  I haven't had to use this yet, but it seems like it should 
work. sssd has made much of this more manageable.  The missing killer 
feature is a somewhat unified authorization model.

The fact that Windows assigns SIDs to machines is actually a major 
technological advantage over the UNIX/linux model (this sure would 
simplify NFS, for example), but then they had the advantages of 
hindsight.  And that's just an aside anyway.

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  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  
> > > >> wrote:
> > > >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > > >> >  wrote:
> > > >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > > >> >> :
> > > >> >
> > > >> >>> 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.

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

[Amir Goldstein]

On Fri, May 3, 2019 at 12:03 PM J. Bruce Fields  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  
> > >> wrote:
> > >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > >> >  wrote:
> > >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > >> >> :
> > >> >
> > >> >>> 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.

Thanks,
Amir.

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

[J. Bruce Fields]

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  wrote:
> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >> >  wrote:
> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> >> :
> >> >
> >> >>> 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?

--b.

> If only standard Unix permissions
>  are used, there is no risk, so this seems reasonable.
> 
>  So I would like to propose a new option for overlayfs
> nocopyupacl:   when overlayfs is copying a file (or directory etc)
> from the lower filesystem to the upper filesystem, it does not
> copy extended attributes with the "system." prefix.  These are
> used for storing ACL information and this is sometimes not
> compatible between different filesystem types (e.g. ext4 and
> NFSv4).  Standard Unix ownership permission flags (rwx) *are*
> copied so this option does not risk giving away inappropriate
> permissions unless the lowerfs uses unusual ACLs.
> 
> 
>  Miklos: would you find that acceptable?
> 
> Thanks,
> NeilBrown
> 
>

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

[J. Bruce Fields]

On Thu, May 02, 2019 at 05:51:12PM +, Goetz, Patrick G wrote:
> 
> 
> On 5/2/19 12:44 PM, Andreas Gruenbacher wrote:
> > On Thu, 2 May 2019 at 19:27, Goetz, Patrick G  
> > wrote:
> >> On 5/1/19 10:57 PM, NeilBrown wrote:
> >>> Support some day support for nfs4 acls were added to ext4 (not a totally
> >>> ridiculous suggestion).  We would then want NFS to allow it's ACLs to be
> >>> copied up.
> >>
> >> Is there some reason why there hasn't been a greater effort to add NFSv4
> >> ACL support to the mainstream linux filesystems?  I have to support a
> >> hybrid linux/windows environment and not having these ACLs on ext4 is a
> >> daily headache for me.
> > 
> > The patches for implementing that have been rejected over and over
> > again, and nobody is working on them anymore.
> > 
> > Andreas
> 
> That's the part I don't understand -- why are the RichACL patches being 
> rejected?

Looking back through old mail:

http://lkml.kernel.org/r/20160311140134.ga14...@infradead.org

For one I still see no reason to merge this broken ACL model at
all.  It provides our actualy Linux users no benefit at all,
while breaking a lot of assumptions, especially by adding allow
and deny ACE at the same sime.

It also doesn't help with the issue that the main thing it's
trying to be compatible with (Windows) actually uses a
fundamentally different identifier to apply the ACLs to - as
long as you're still limited to users and groups and not guids
we'll still have that mapping problem anyway.

Christoph also had some objections to the implementation which I think
were addressed, but I could be wrong.

--b.

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

[Andreas Grünbacher]

Am Fr., 3. Mai 2019 um 01:24 Uhr schrieb NeilBrown :
> On Thu, May 02 2019, Andreas Gruenbacher wrote:
> > On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
> >> On Wed, May 01 2019, Amir Goldstein wrote:
> >> > On Wed, May 1, 2019 at 10:03 PM 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  
> >> >> >> wrote:
> >> >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >> >> >> >  wrote:
> >> >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> >> >> >> :
> >> >> >> >
> >> >> >> >>> 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.
> >
> > I still think the nfs client could make this problem mostly go away by
> > not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
> > the file mode.
>
> Maybe ... but this feels a bit like "sweeping it under the carpet".
> What happens if some file on the lower layer does have a more complex
> ACL?
> Do we just fail any attempt to modify that object?  Doesn't that violate
> the law of least surprise?

It would at least expose that there is a problem only if there is an
actual problem.

> Maybe if the lower-layer has an i_op->permission method, then overlayfs
> should *always* call that for permission checking - unless a
> chmod/chown/etc has happened on the file.  That way, we wouldn't need to
> copy-up the ACL, but would still get correct ACL testing.

No, the permissions need to stick with the object. Otherwise, what
would you do on rename or when the lower layer changes?

Andreas

> Thanks,
> NeilBrown
>
>
> > The richacl patches contain a workable abgorithm for
> > that. The problem would remain for files that have an actual NFS4 ACL,
> > which just cannot be mapped to a file mode or to POSIX ACLs in the
> > general case, as well as for files that have a POSIX ACL. Mapping NFS4
> > ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
> > in many cases as well, but the code would be quite messy. A better way
> > seems to be to using a filesystem that doesn't support POSIX ACLs in
> > the first place. Unfortunately, xfs doesn't allow turning off POSIX
> > ACLs, for example.
> >
> > Andreas
> >
> >> >> >> > 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.
> >> >
> >> > For example? permissions given away to do what?
> >> > Note that ovl_permission() only check permissions of *mounter*
> >> > to read the lower NFS file and ovl_open()/ovl_read_iter() access
> >> > the lower file with *mounter* credentials.
> >> >
> >> > I might be wrong, but seems to me that once admin mounted
> >> > overlayfs with lower NFS, NFS ACLs are not being enforced at all
> >> > even 

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

[NeilBrown]

On Thu, May 02 2019, Andreas Gruenbacher wrote:

> On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
>> On Wed, May 01 2019, Amir Goldstein wrote:
>> > On Wed, May 1, 2019 at 10:03 PM 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  
>> >> >> wrote:
>> >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>> >> >> >  wrote:
>> >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
>> >> >> >> :
>> >> >> >
>> >> >> >>> 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.
>
> I still think the nfs client could make this problem mostly go away by
> not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
> the file mode.

Maybe ... but this feels a bit like "sweeping it under the carpet".
What happens if some file on the lower layer does have a more complex
ACL?
Do we just fail any attempt to modify that object?  Doesn't that violate
the law of least surprise?

Maybe if the lower-layer has an i_op->permission method, then overlayfs
should *always* call that for permission checking - unless a
chmod/chown/etc has happened on the file.  That way, we wouldn't need to
copy-up the ACL, but would still get correct ACL testing.

Thanks,
NeilBrown


> The richacl patches contain a workable abgorithm for
> that. The problem would remain for files that have an actual NFS4 ACL,
> which just cannot be mapped to a file mode or to POSIX ACLs in the
> general case, as well as for files that have a POSIX ACL. Mapping NFS4
> ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
> in many cases as well, but the code would be quite messy. A better way
> seems to be to using a filesystem that doesn't support POSIX ACLs in
> the first place. Unfortunately, xfs doesn't allow turning off POSIX
> ACLs, for example.
>
> Andreas
>
>> >> >> > 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.
>> >
>> > For example? permissions given away to do what?
>> > Note that ovl_permission() only check permissions of *mounter*
>> > to read the lower NFS file and ovl_open()/ovl_read_iter() access
>> > the lower file with *mounter* credentials.
>> >
>> > I might be wrong, but seems to me that once admin mounted
>> > overlayfs with lower NFS, NFS ACLs are not being enforced at all
>> > even before copy up.
>>
>> I guess it is just as well that copy-up fails then - if the lower-level
>> permission check is being ignored.
>>
>> >
>> >> So if the
>> >>  sysadmin wants this (and some clearly do), they need a way to
>> >>  explicitly say "I accept the risk".  If only standard Unix permissions
>> >>  are used, there is no risk, so this seems reasonable.
>> >>
>> >>  So I would like to propose a new option for overlayfs
>> >> nocopyupacl:   when overlayfs is copying a file (or directory etc)
>> >>

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

[NeilBrown]

On Thu, May 02 2019, Miklos Szeredi wrote:

> On Thu, May 2, 2019 at 10:05 AM Andreas Gruenbacher  
> wrote:
>>
>> On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
>> > On Wed, May 01 2019, Amir Goldstein wrote:
>> > > On Wed, May 1, 2019 at 10:03 PM 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  
>> > >> >> wrote:
>> > >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>> > >> >> >  wrote:
>> > >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
>> > >> >> >> :
>> > >> >> >
>> > >> >> >>> 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.
>>
>> I still think the nfs client could make this problem mostly go away by
>> not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
>> the file mode. The richacl patches contain a workable abgorithm for
>> that. The problem would remain for files that have an actual NFS4 ACL,
>> which just cannot be mapped to a file mode or to POSIX ACLs in the
>> general case, as well as for files that have a POSIX ACL. Mapping NFS4
>> ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
>> in many cases as well, but the code would be quite messy. A better way
>> seems to be to using a filesystem that doesn't support POSIX ACLs in
>> the first place. Unfortunately, xfs doesn't allow turning off POSIX
>> ACLs, for example.
>
> How about mounting NFSv4 with noacl?  That should fix this issue, right?

No.
"noacl" only affect NFSv3 (and maybe v2) and it disables use of the
NFSACL side-protocol.
"noacl" has no effect on an NFSv4 mount.

NeilBrown


signature.asc
Description: PGP signature

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

[Goetz, Patrick G]

On 5/2/19 12:44 PM, Andreas Gruenbacher wrote:
> On Thu, 2 May 2019 at 19:27, Goetz, Patrick G  wrote:
>> On 5/1/19 10:57 PM, NeilBrown wrote:
>>> Support some day support for nfs4 acls were added to ext4 (not a totally
>>> ridiculous suggestion).  We would then want NFS to allow it's ACLs to be
>>> copied up.
>>
>> Is there some reason why there hasn't been a greater effort to add NFSv4
>> ACL support to the mainstream linux filesystems?  I have to support a
>> hybrid linux/windows environment and not having these ACLs on ext4 is a
>> daily headache for me.
> 
> The patches for implementing that have been rejected over and over
> again, and nobody is working on them anymore.
> 
> Andreas
> 


That's the part I don't understand -- why are the RichACL patches being 
rejected?

Everyone loves the simplicity of mode bits (including me) until you run 
into things like the need to automatically create home directories on an 
NFS-mounted filesystem or security situations where, for example, you 
want users to be able to edit but not delete files, and then you're kind 
of stuck listening to your Windows colleagues propose a Storage Spaces 
solution.

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

[Andreas Gruenbacher]

On Thu, 2 May 2019 at 19:16, J. Bruce Fields  wrote:
> On Thu, May 02, 2019 at 05:08:14PM +0200, Andreas Grünbacher wrote:
> > You'll still see permissions that differ from what the filesystem
> > enforces, and copy-up would change that behavior.
>
> That's always true, and this issue isn't really specific to NFSv4 ACLs
> (or ACLs at all), it already exists with just mode bits.  The client
> doesn't know how principals may be mapped on the server, doesn't know
> group membership, etc.
>
> That's the usual model, anyway.  Permissions are almost entirely the
> server's responsibility, and we just provide a few attributes to set/get
> those server-side permissions.

Sure, if the client and server don't share the same user and group
databases, ACLs can get a very different meaning.

Andreas

> The overlayfs/NFS case is different, I think: the nfs filesystem may be
> just a static read-only template for a filesystem that's only ever used
> by clients, and for all I know maybe permissions should only be
> interpreted on the client side in that case.
>
> --b.

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

[Andreas Gruenbacher]

On Thu, 2 May 2019 at 19:27, Goetz, Patrick G  wrote:
> On 5/1/19 10:57 PM, NeilBrown wrote:
> > Support some day support for nfs4 acls were added to ext4 (not a totally
> > ridiculous suggestion).  We would then want NFS to allow it's ACLs to be
> > copied up.
>
> Is there some reason why there hasn't been a greater effort to add NFSv4
> ACL support to the mainstream linux filesystems?  I have to support a
> hybrid linux/windows environment and not having these ACLs on ext4 is a
> daily headache for me.

The patches for implementing that have been rejected over and over
again, and nobody is working on them anymore.

Andreas

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

[Goetz, Patrick G]

On 5/1/19 10:57 PM, NeilBrown wrote:
> Support some day support for nfs4 acls were added to ext4 (not a totally
> ridiculous suggestion).  We would then want NFS to allow it's ACLs to be
> copied up.


Is there some reason why there hasn't been a greater effort to add NFSv4 
ACL support to the mainstream linux filesystems?  I have to support a 
hybrid linux/windows environment and not having these ACLs on ext4 is a 
daily headache for me.

Also, it doesn't take much need for security granularity to realize that 
POSIX ACLs (not ever even formally standardized!) are fairly inadequate, 
but more importantly, don't play nicely with their Windows friends.

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

[J. Bruce Fields]

On Thu, May 02, 2019 at 05:08:14PM +0200, Andreas Grünbacher wrote:
> You'll still see permissions that differ from what the filesystem
> enforces, and copy-up would change that behavior.

That's always true, and this issue isn't really specific to NFSv4 ACLs
(or ACLs at all), it already exists with just mode bits.  The client
doesn't know how principals may be mapped on the server, doesn't know
group membership, etc.

That's the usual model, anyway.  Permissions are almost entirely the
server's responsibility, and we just provide a few attributes to set/get
those server-side permissions.

The overlayfs/NFS case is different, I think: the nfs filesystem may be
just a static read-only template for a filesystem that's only ever used
by clients, and for all I know maybe permissions should only be
interpreted on the client side in that case.

--b.

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

[Andreas Grünbacher]

Am Do., 2. Mai 2019 um 16:28 Uhr schrieb Miklos Szeredi :
> On Thu, May 2, 2019 at 10:05 AM Andreas Gruenbacher  
> wrote:
> > On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
> > > On Wed, May 01 2019, Amir Goldstein wrote:
> > > > On Wed, May 1, 2019 at 10:03 PM 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  
> > > >> >> wrote:
> > > >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > > >> >> >  wrote:
> > > >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > > >> >> >> :
> > > >> >> >
> > > >> >> >>> 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.
> >
> > I still think the nfs client could make this problem mostly go away by
> > not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
> > the file mode. The richacl patches contain a workable abgorithm for
> > that. The problem would remain for files that have an actual NFS4 ACL,
> > which just cannot be mapped to a file mode or to POSIX ACLs in the
> > general case, as well as for files that have a POSIX ACL. Mapping NFS4
> > ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
> > in many cases as well, but the code would be quite messy. A better way
> > seems to be to using a filesystem that doesn't support POSIX ACLs in
> > the first place. Unfortunately, xfs doesn't allow turning off POSIX
> > ACLs, for example.
>
> How about mounting NFSv4 with noacl?  That should fix this issue, right?

You'll still see permissions that differ from what the filesystem
enforces, and copy-up would change that behavior.

Andreas

> Thanks,
> Miklos
>
>
>
> >
> > Andreas
> >
> > > >> >> > 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.
> > > >
> > > > For example? permissions given away to do what?
> > > > Note that ovl_permission() only check permissions of *mounter*
> > > > to read the lower NFS file and ovl_open()/ovl_read_iter() access
> > > > the lower file with *mounter* credentials.
> > > >
> > > > I might be wrong, but seems to me that once admin mounted
> > > > overlayfs with lower NFS, NFS ACLs are not being enforced at all
> > > > even before copy up.
> > >
> > > I guess it is just as well that copy-up fails then - if the lower-level
> > > permission check is being ignored.
> > >
> > > >
> > > >> So if the
> > > >>  sysadmin wants this (and some clearly do), they need a way to
> > > >>  explicitly say "I accept the risk".  If only standard Unix permissions
> > > >>  are used, there is no risk, so this seems reasonable.
> > > >>
> > > >>  So I would like to propose a new 

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

[Miklos Szeredi]

On Thu, May 2, 2019 at 10:05 AM Andreas Gruenbacher  wrote:
>
> On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
> > On Wed, May 01 2019, Amir Goldstein wrote:
> > > On Wed, May 1, 2019 at 10:03 PM 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  
> > >> >> wrote:
> > >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> > >> >> >  wrote:
> > >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> > >> >> >> :
> > >> >> >
> > >> >> >>> 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.
>
> I still think the nfs client could make this problem mostly go away by
> not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
> the file mode. The richacl patches contain a workable abgorithm for
> that. The problem would remain for files that have an actual NFS4 ACL,
> which just cannot be mapped to a file mode or to POSIX ACLs in the
> general case, as well as for files that have a POSIX ACL. Mapping NFS4
> ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
> in many cases as well, but the code would be quite messy. A better way
> seems to be to using a filesystem that doesn't support POSIX ACLs in
> the first place. Unfortunately, xfs doesn't allow turning off POSIX
> ACLs, for example.

How about mounting NFSv4 with noacl?  That should fix this issue, right?

Thanks,
Miklos



>
> Andreas
>
> > >> >> > 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.
> > >
> > > For example? permissions given away to do what?
> > > Note that ovl_permission() only check permissions of *mounter*
> > > to read the lower NFS file and ovl_open()/ovl_read_iter() access
> > > the lower file with *mounter* credentials.
> > >
> > > I might be wrong, but seems to me that once admin mounted
> > > overlayfs with lower NFS, NFS ACLs are not being enforced at all
> > > even before copy up.
> >
> > I guess it is just as well that copy-up fails then - if the lower-level
> > permission check is being ignored.
> >
> > >
> > >> So if the
> > >>  sysadmin wants this (and some clearly do), they need a way to
> > >>  explicitly say "I accept the risk".  If only standard Unix permissions
> > >>  are used, there is no risk, so this seems reasonable.
> > >>
> > >>  So I would like to propose a new option for overlayfs
> > >> nocopyupacl:   when overlayfs is copying a file (or directory etc)
> > >> from the lower filesystem to the upper filesystem, it does not
> > >> copy extended attributes with the "system." prefix.  These are
> > >> used for storing ACL information and this is sometimes not
> > >> compatible between different filesystem types (e.g. ext4 and
> > >> NFSv4).  Standard 

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

[Andreas Gruenbacher]

On Thu, 2 May 2019 at 05:57, NeilBrown  wrote:
> On Wed, May 01 2019, Amir Goldstein wrote:
> > On Wed, May 1, 2019 at 10:03 PM 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  
> >> >> wrote:
> >> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >> >> >  wrote:
> >> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> >> >> :
> >> >> >
> >> >> >>> 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.

I still think the nfs client could make this problem mostly go away by
not exposing "system.nfs4_acl" xattrs when the acl is equivalent to
the file mode. The richacl patches contain a workable abgorithm for
that. The problem would remain for files that have an actual NFS4 ACL,
which just cannot be mapped to a file mode or to POSIX ACLs in the
general case, as well as for files that have a POSIX ACL. Mapping NFS4
ACL that used to be a POSIX ACL back to POSIX ACLs could be achieved
in many cases as well, but the code would be quite messy. A better way
seems to be to using a filesystem that doesn't support POSIX ACLs in
the first place. Unfortunately, xfs doesn't allow turning off POSIX
ACLs, for example.

Andreas

> >> >> > 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.
> >
> > For example? permissions given away to do what?
> > Note that ovl_permission() only check permissions of *mounter*
> > to read the lower NFS file and ovl_open()/ovl_read_iter() access
> > the lower file with *mounter* credentials.
> >
> > I might be wrong, but seems to me that once admin mounted
> > overlayfs with lower NFS, NFS ACLs are not being enforced at all
> > even before copy up.
>
> I guess it is just as well that copy-up fails then - if the lower-level
> permission check is being ignored.
>
> >
> >> So if the
> >>  sysadmin wants this (and some clearly do), they need a way to
> >>  explicitly say "I accept the risk".  If only standard Unix permissions
> >>  are used, there is no risk, so this seems reasonable.
> >>
> >>  So I would like to propose a new option for overlayfs
> >> nocopyupacl:   when overlayfs is copying a file (or directory etc)
> >> from the lower filesystem to the upper filesystem, it does not
> >> copy extended attributes with the "system." prefix.  These are
> >> used for storing ACL information and this is sometimes not
> >> compatible between different filesystem types (e.g. ext4 and
> >> NFSv4).  Standard Unix ownership permission flags (rwx) *are*
> >> copied so this option does not risk giving away inappropriate
> >> permissions unless the lowerfs uses unusual ACLs.
> >>
> >>
> >
> > I am wondering if it would make more sense for nfs to register a
> > security_inode_copy_up_xattr() hook.
> > That is the mechanism that prevents copying up other security.*
> > 

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

[NeilBrown]

On Wed, May 01 2019, Amir Goldstein wrote:

> On Wed, May 1, 2019 at 10:03 PM 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  wrote:
>> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>> >> >  wrote:
>> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
>> >> >> :
>> >> >
>> >> >>> 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.
>
> For example? permissions given away to do what?
> Note that ovl_permission() only check permissions of *mounter*
> to read the lower NFS file and ovl_open()/ovl_read_iter() access
> the lower file with *mounter* credentials.
>
> I might be wrong, but seems to me that once admin mounted
> overlayfs with lower NFS, NFS ACLs are not being enforced at all
> even before copy up.

I guess it is just as well that copy-up fails then - if the lower-level
permission check is being ignored.

>
>> So if the
>>  sysadmin wants this (and some clearly do), they need a way to
>>  explicitly say "I accept the risk".  If only standard Unix permissions
>>  are used, there is no risk, so this seems reasonable.
>>
>>  So I would like to propose a new option for overlayfs
>> nocopyupacl:   when overlayfs is copying a file (or directory etc)
>> from the lower filesystem to the upper filesystem, it does not
>> copy extended attributes with the "system." prefix.  These are
>> used for storing ACL information and this is sometimes not
>> compatible between different filesystem types (e.g. ext4 and
>> NFSv4).  Standard Unix ownership permission flags (rwx) *are*
>> copied so this option does not risk giving away inappropriate
>> permissions unless the lowerfs uses unusual ACLs.
>>
>>
>
> I am wondering if it would make more sense for nfs to register a
> security_inode_copy_up_xattr() hook.
> That is the mechanism that prevents copying up other security.*
> xattrs?

No, I don't think that would make sense.
Support some day support for nfs4 acls were added to ext4 (not a totally
ridiculous suggestion).  We would then want NFS to allow it's ACLs to be
copied up.

Thanks,
NeilBrown


>
> Thanks,
> Amir.


signature.asc
Description: PGP signature

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

[Amir Goldstein]

On Wed, May 1, 2019 at 10:03 PM 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  wrote:
> >> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >> >  wrote:
> >> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> >> :
> >> >
> >> >>> 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.

For example? permissions given away to do what?
Note that ovl_permission() only check permissions of *mounter*
to read the lower NFS file and ovl_open()/ovl_read_iter() access
the lower file with *mounter* credentials.

I might be wrong, but seems to me that once admin mounted
overlayfs with lower NFS, NFS ACLs are not being enforced at all
even before copy up.

> So if the
>  sysadmin wants this (and some clearly do), they need a way to
>  explicitly say "I accept the risk".  If only standard Unix permissions
>  are used, there is no risk, so this seems reasonable.
>
>  So I would like to propose a new option for overlayfs
> nocopyupacl:   when overlayfs is copying a file (or directory etc)
> from the lower filesystem to the upper filesystem, it does not
> copy extended attributes with the "system." prefix.  These are
> used for storing ACL information and this is sometimes not
> compatible between different filesystem types (e.g. ext4 and
> NFSv4).  Standard Unix ownership permission flags (rwx) *are*
> copied so this option does not risk giving away inappropriate
> permissions unless the lowerfs uses unusual ACLs.
>
>

I am wondering if it would make more sense for nfs to register a
security_inode_copy_up_xattr() hook.
That is the mechanism that prevents copying up other security.*
xattrs?

Thanks,
Amir.

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

[NeilBrown]

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  wrote:
>> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>> >  wrote:
>> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
>> >> :
>> >
>> >>> 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".  If only standard Unix permissions
 are used, there is no risk, so this seems reasonable.

 So I would like to propose a new option for overlayfs
nocopyupacl:   when overlayfs is copying a file (or directory etc)
from the lower filesystem to the upper filesystem, it does not
copy extended attributes with the "system." prefix.  These are
used for storing ACL information and this is sometimes not
compatible between different filesystem types (e.g. ext4 and
NFSv4).  Standard Unix ownership permission flags (rwx) *are*
copied so this option does not risk giving away inappropriate
permissions unless the lowerfs uses unusual ACLs.


 Miklos: would you find that acceptable?

Thanks,
NeilBrown

   


signature.asc
Description: PGP signature

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

[J. Bruce Fields]

On Tue, Dec 06, 2016 at 02:18:31PM +0100, Andreas Gruenbacher wrote:
> On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi  wrote:
> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >  wrote:
> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> :
> >
> >>> 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

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

[J. Bruce Fields]

On Tue, Dec 06, 2016 at 02:18:31PM +0100, Andreas Gruenbacher wrote:
> On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi  wrote:
> > On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
> >  wrote:
> >> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher 
> >> :
> >
> >>> 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

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

[Andreas Gruenbacher]

On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi  wrote:
> On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>  wrote:
>> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :
>
>>> 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. (I'm not saying that that's very helpful.)

Andreas

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

[Andreas Gruenbacher]

On Tue, Dec 6, 2016 at 11:08 AM, Miklos Szeredi  wrote:
> On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
>  wrote:
>> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :
>
>>> 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. (I'm not saying that that's very helpful.)

Andreas

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

[Miklos Szeredi]

On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
 wrote:
> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :

>> 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?

Thanks,
Miklos

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

[Miklos Szeredi]

On Tue, Dec 6, 2016 at 12:24 AM, Andreas Grünbacher
 wrote:
> 2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :

>> 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?

Thanks,
Miklos

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

[Andreas Grünbacher]

2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :
> 2016-12-05 23:58 GMT+01:00 Patrick Plagwitz :
>> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
 On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
> wrote:
>>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>>
>> This is the interface the NFS client provides for applications to modify
>> NFSv4 ACLs on servers that support them.
>
> Fine, but why are we seeing this xattr on exports where no xattrs are
> set on the exported fs?

 I don't know.  I took another look at the original patch and don't see
 any details on the server setup: which server is it (knfsd, ganesha,
 netapp, ...)?  How is it configured?

>>> What can overlayfs do if it's a non-empty ACL?
>>
>> As little as possible.  You can't copy it up, can you?  So any attempt
>> to support it is going to be incomplete.
>
> Right.
>
>>
>>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>>> back.  Should we do a generic POSIX<->NFS acl translator?
>>
>> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>
> This does explain the nfs4_acl xattr on the client.  Question: if it's
> empty, why have it at all?

 I'm honestly not sure what's going on there.  I'd be curious to see a
 network trace if possible.
>>>
>>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>>> attribute is missing. The attribute shouldn't be empty though; when
>>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>>> permissions. The "system.nfs4_acl" attribute exposes the information
>>> on the wire; there is no resonable way to translate that into an ACL
>>> on another filesystem, really.
>>>
>>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>>
>> getfattr -m - -d nfs/folder -e text gives
>>
>> # file: nfs/folder/
>> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
>> EVERYONE@\000\000"
>>
>> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
>> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
>> empty after all and checking *value == 0 does not tell if there are actually
>> ACLs present or not, sorry for the confusion.
>>
>> You are right, when I mount the exported fs with noacl the problem goes away.
>> You already helped me there, thanks.
>>
>> Still, I think there should be a way to copy up files that actually have no
>> ACLs since acl is often the default for ext4 mounts and giving an "Operation
>> not supported" for random open(2)s is not a very good way to convey what's
>> going on.
>
> 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.

Andreas

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

[Andreas Grünbacher]

2016-12-06 0:19 GMT+01:00 Andreas Grünbacher :
> 2016-12-05 23:58 GMT+01:00 Patrick Plagwitz :
>> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
 On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
> wrote:
>>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>>
>> This is the interface the NFS client provides for applications to modify
>> NFSv4 ACLs on servers that support them.
>
> Fine, but why are we seeing this xattr on exports where no xattrs are
> set on the exported fs?

 I don't know.  I took another look at the original patch and don't see
 any details on the server setup: which server is it (knfsd, ganesha,
 netapp, ...)?  How is it configured?

>>> What can overlayfs do if it's a non-empty ACL?
>>
>> As little as possible.  You can't copy it up, can you?  So any attempt
>> to support it is going to be incomplete.
>
> Right.
>
>>
>>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>>> back.  Should we do a generic POSIX<->NFS acl translator?
>>
>> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>
> This does explain the nfs4_acl xattr on the client.  Question: if it's
> empty, why have it at all?

 I'm honestly not sure what's going on there.  I'd be curious to see a
 network trace if possible.
>>>
>>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>>> attribute is missing. The attribute shouldn't be empty though; when
>>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>>> permissions. The "system.nfs4_acl" attribute exposes the information
>>> on the wire; there is no resonable way to translate that into an ACL
>>> on another filesystem, really.
>>>
>>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>>
>> getfattr -m - -d nfs/folder -e text gives
>>
>> # file: nfs/folder/
>> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
>> EVERYONE@\000\000"
>>
>> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
>> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
>> empty after all and checking *value == 0 does not tell if there are actually
>> ACLs present or not, sorry for the confusion.
>>
>> You are right, when I mount the exported fs with noacl the problem goes away.
>> You already helped me there, thanks.
>>
>> Still, I think there should be a way to copy up files that actually have no
>> ACLs since acl is often the default for ext4 mounts and giving an "Operation
>> not supported" for random open(2)s is not a very good way to convey what's
>> going on.
>
> 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.

Andreas

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

[Andreas Grünbacher]

2016-12-05 23:58 GMT+01:00 Patrick Plagwitz :
> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
>>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
 On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
 wrote:
>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>
> This is the interface the NFS client provides for applications to modify
> NFSv4 ACLs on servers that support them.

 Fine, but why are we seeing this xattr on exports where no xattrs are
 set on the exported fs?
>>>
>>> I don't know.  I took another look at the original patch and don't see
>>> any details on the server setup: which server is it (knfsd, ganesha,
>>> netapp, ...)?  How is it configured?
>>>
>> What can overlayfs do if it's a non-empty ACL?
>
> As little as possible.  You can't copy it up, can you?  So any attempt
> to support it is going to be incomplete.

 Right.

>
>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> back.  Should we do a generic POSIX<->NFS acl translator?
>
> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated

 This does explain the nfs4_acl xattr on the client.  Question: if it's
 empty, why have it at all?
>>>
>>> I'm honestly not sure what's going on there.  I'd be curious to see a
>>> network trace if possible.
>>
>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>> attribute is missing. The attribute shouldn't be empty though; when
>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>> permissions. The "system.nfs4_acl" attribute exposes the information
>> on the wire; there is no resonable way to translate that into an ACL
>> on another filesystem, really.
>>
>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>
> getfattr -m - -d nfs/folder -e text gives
>
> # file: nfs/folder/
> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
> EVERYONE@\000\000"
>
> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
> empty after all and checking *value == 0 does not tell if there are actually
> ACLs present or not, sorry for the confusion.
>
> You are right, when I mount the exported fs with noacl the problem goes away.
> You already helped me there, thanks.
>
> Still, I think there should be a way to copy up files that actually have no
> ACLs since acl is often the default for ext4 mounts and giving an "Operation
> not supported" for random open(2)s is not a very good way to convey what's
> going on.

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 ...

Andreas

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

[Andreas Grünbacher]

2016-12-05 23:58 GMT+01:00 Patrick Plagwitz :
> On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
>> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
>>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
 On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
 wrote:
>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>
> This is the interface the NFS client provides for applications to modify
> NFSv4 ACLs on servers that support them.

 Fine, but why are we seeing this xattr on exports where no xattrs are
 set on the exported fs?
>>>
>>> I don't know.  I took another look at the original patch and don't see
>>> any details on the server setup: which server is it (knfsd, ganesha,
>>> netapp, ...)?  How is it configured?
>>>
>> What can overlayfs do if it's a non-empty ACL?
>
> As little as possible.  You can't copy it up, can you?  So any attempt
> to support it is going to be incomplete.

 Right.

>
>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> back.  Should we do a generic POSIX<->NFS acl translator?
>
> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated

 This does explain the nfs4_acl xattr on the client.  Question: if it's
 empty, why have it at all?
>>>
>>> I'm honestly not sure what's going on there.  I'd be curious to see a
>>> network trace if possible.
>>
>> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
>> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
>> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
>> attribute is missing. The attribute shouldn't be empty though; when
>> the file has no real ACL, "system.nfs4_acl" represents the file mode
>> permissions. The "system.nfs4_acl" attribute exposes the information
>> on the wire; there is no resonable way to translate that into an ACL
>> on another filesystem, really.
>>
>> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
>>
> getfattr -m - -d nfs/folder -e text gives
>
> # file: nfs/folder/
> system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
> EVERYONE@\000\000"
>
> Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
> It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
> empty after all and checking *value == 0 does not tell if there are actually
> ACLs present or not, sorry for the confusion.
>
> You are right, when I mount the exported fs with noacl the problem goes away.
> You already helped me there, thanks.
>
> Still, I think there should be a way to copy up files that actually have no
> ACLs since acl is often the default for ext4 mounts and giving an "Operation
> not supported" for random open(2)s is not a very good way to convey what's
> going on.

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 ...

Andreas

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

[Patrick Plagwitz]

On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
>>> wrote:
> Can NFS people comment on this?  Where does the nfs4_acl come from?

 This is the interface the NFS client provides for applications to modify
 NFSv4 ACLs on servers that support them.
>>>
>>> Fine, but why are we seeing this xattr on exports where no xattrs are
>>> set on the exported fs?
>>
>> I don't know.  I took another look at the original patch and don't see
>> any details on the server setup: which server is it (knfsd, ganesha,
>> netapp, ...)?  How is it configured?
>>
> What can overlayfs do if it's a non-empty ACL?

 As little as possible.  You can't copy it up, can you?  So any attempt
 to support it is going to be incomplete.
>>>
>>> Right.
>>>

> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> back.  Should we do a generic POSIX<->NFS acl translator?

 knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>>
>>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>>> empty, why have it at all?
>>
>> I'm honestly not sure what's going on there.  I'd be curious to see a
>> network trace if possible.
> 
> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
> attribute is missing. The attribute shouldn't be empty though; when
> the file has no real ACL, "system.nfs4_acl" represents the file mode
> permissions. The "system.nfs4_acl" attribute exposes the information
> on the wire; there is no resonable way to translate that into an ACL
> on another filesystem, really.
> 
> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
> 
getfattr -m - -d nfs/folder -e text gives

# file: nfs/folder/
system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
EVERYONE@\000\000"

Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
empty after all and checking *value == 0 does not tell if there are actually
ACLs present or not, sorry for the confusion.

You are right, when I mount the exported fs with noacl the problem goes away.
You already helped me there, thanks.

Still, I think there should be a way to copy up files that actually have no
ACLs since acl is often the default for ext4 mounts and giving an "Operation
not supported" for random open(2)s is not a very good way to convey what's
going on.

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

[Patrick Plagwitz]

On 12/05/2016 08:37 PM, Andreas Grünbacher wrote:
> 2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
>> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  
>>> wrote:
> Can NFS people comment on this?  Where does the nfs4_acl come from?

 This is the interface the NFS client provides for applications to modify
 NFSv4 ACLs on servers that support them.
>>>
>>> Fine, but why are we seeing this xattr on exports where no xattrs are
>>> set on the exported fs?
>>
>> I don't know.  I took another look at the original patch and don't see
>> any details on the server setup: which server is it (knfsd, ganesha,
>> netapp, ...)?  How is it configured?
>>
> What can overlayfs do if it's a non-empty ACL?

 As little as possible.  You can't copy it up, can you?  So any attempt
 to support it is going to be incomplete.
>>>
>>> Right.
>>>

> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> back.  Should we do a generic POSIX<->NFS acl translator?

 knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>>
>>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>>> empty, why have it at all?
>>
>> I'm honestly not sure what's going on there.  I'd be curious to see a
>> network trace if possible.
> 
> I do see "system.nfs4_acl" attributes on knfsd exported filesystems
> that support POSIX ACLs (for ext4: "mount -o acl"). For exported
> filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
> attribute is missing. The attribute shouldn't be empty though; when
> the file has no real ACL, "system.nfs4_acl" represents the file mode
> permissions. The "system.nfs4_acl" attribute exposes the information
> on the wire; there is no resonable way to translate that into an ACL
> on another filesystem, really.
> 
> Patrick, what does 'getfattr -m- -d /nfs/file' give you?
> 
getfattr -m - -d nfs/folder -e text gives

# file: nfs/folder/
system.nfs4_acl="\000\000\000^C\000\000\000\000\000\000\000\000\000^V^A\000\000\000^FOWNER@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000^FGROUP@\000\000\000\000\000\000\000\000\000\000\000^R\000\000\000\000
EVERYONE@\000\000"

Those are 80 bytes. I checked again and vfs_getxattr indeed returns size=80.
It just looked empty because the first byte is 0... Ok, so nfs4_acl is not
empty after all and checking *value == 0 does not tell if there are actually
ACLs present or not, sorry for the confusion.

You are right, when I mount the exported fs with noacl the problem goes away.
You already helped me there, thanks.

Still, I think there should be a way to copy up files that actually have no
ACLs since acl is often the default for ext4 mounts and giving an "Operation
not supported" for random open(2)s is not a very good way to convey what's
going on.

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

[Andreas Grünbacher]

2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
>> >> Can NFS people comment on this?  Where does the nfs4_acl come from?
>> >
>> > This is the interface the NFS client provides for applications to modify
>> > NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
>
> I don't know.  I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)?  How is it configured?
>
>> >> What can overlayfs do if it's a non-empty ACL?
>> >
>> > As little as possible.  You can't copy it up, can you?  So any attempt
>> > to support it is going to be incomplete.
>>
>> Right.
>>
>> >
>> >> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> >> back.  Should we do a generic POSIX<->NFS acl translator?
>> >
>> > knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>> empty, why have it at all?
>
> I'm honestly not sure what's going on there.  I'd be curious to see a
> network trace if possible.

I do see "system.nfs4_acl" attributes on knfsd exported filesystems
that support POSIX ACLs (for ext4: "mount -o acl"). For exported
filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
attribute is missing. The attribute shouldn't be empty though; when
the file has no real ACL, "system.nfs4_acl" represents the file mode
permissions. The "system.nfs4_acl" attribute exposes the information
on the wire; there is no resonable way to translate that into an ACL
on another filesystem, really.

Patrick, what does 'getfattr -m- -d /nfs/file' give you?

>> > algorithm, and lossy (in the NFSv4->POSIX direction).  The client
>> > developers have been understandably reluctant to have anything to do
>> > with it.
>> >
>> > So, I think listxattr should omit system.nfs4_acl, and attempts to
>> > set/get the attribute should error out.  The same should apply to any
>> > "system." attribute not supported by both filesystems, I think?
>>
>> Basically that's what happens now.  The problem is that nfsv4 mounts
>> seem always have these xattrs, even when the exported fs doesn't have
>> anything.
>
> I said "both", that's a logical "and".  Whether or not nfs claims
> support would then be irrelevant in this case, since ext4 doesn't
> support system.nfs4_acl.
>
>> We could do the copy up even if the NFS4->POSIX translation was
>> possible (which is the case with POSIX ACL translated by knfsd).  We'd
>> just get back the original ACL, so that's OK.
>
> Note that knfsd is an exception, most NFSv4-acl-supporting servers
> aren't translating from POSIX ACLs.
>
>> NFS is only supported as lower (read-only) layer, so we don't care
>> about changing the ACL on the server.
>
> Out of curiosity, how do you check permissions after copy up?
>
> The client doesn't do much permissions-checking normally, because it's
> hard to get right--even in the absence of ACLs, it may not understand
> the server's owners and groups completely.
>
> I guess that's fine, you may be happy to let people write to the file
> without permissions to the lower file, since the writes aren't going
> there anyway.
>
> So, I don't know what want here.
>
> You're not going to want to use the ACL for actual permission-checking,
> and you can't modify it, so it doesn't seem very useful.

IMO the only thing overlayfs can do is hide those attributes from the
user and ignore them when copying up. Still, the attributes shouldn't
be empty.

Andreas

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

[Andreas Grünbacher]

2016-12-05 17:25 GMT+01:00 J. Bruce Fields :
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
>> >> Can NFS people comment on this?  Where does the nfs4_acl come from?
>> >
>> > This is the interface the NFS client provides for applications to modify
>> > NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
>
> I don't know.  I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)?  How is it configured?
>
>> >> What can overlayfs do if it's a non-empty ACL?
>> >
>> > As little as possible.  You can't copy it up, can you?  So any attempt
>> > to support it is going to be incomplete.
>>
>> Right.
>>
>> >
>> >> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> >> back.  Should we do a generic POSIX<->NFS acl translator?
>> >
>> > knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>> empty, why have it at all?
>
> I'm honestly not sure what's going on there.  I'd be curious to see a
> network trace if possible.

I do see "system.nfs4_acl" attributes on knfsd exported filesystems
that support POSIX ACLs (for ext4: "mount -o acl"). For exported
filesystem that don't support POSIX ACLs (ext4: mount -o noacl), that
attribute is missing. The attribute shouldn't be empty though; when
the file has no real ACL, "system.nfs4_acl" represents the file mode
permissions. The "system.nfs4_acl" attribute exposes the information
on the wire; there is no resonable way to translate that into an ACL
on another filesystem, really.

Patrick, what does 'getfattr -m- -d /nfs/file' give you?

>> > algorithm, and lossy (in the NFSv4->POSIX direction).  The client
>> > developers have been understandably reluctant to have anything to do
>> > with it.
>> >
>> > So, I think listxattr should omit system.nfs4_acl, and attempts to
>> > set/get the attribute should error out.  The same should apply to any
>> > "system." attribute not supported by both filesystems, I think?
>>
>> Basically that's what happens now.  The problem is that nfsv4 mounts
>> seem always have these xattrs, even when the exported fs doesn't have
>> anything.
>
> I said "both", that's a logical "and".  Whether or not nfs claims
> support would then be irrelevant in this case, since ext4 doesn't
> support system.nfs4_acl.
>
>> We could do the copy up even if the NFS4->POSIX translation was
>> possible (which is the case with POSIX ACL translated by knfsd).  We'd
>> just get back the original ACL, so that's OK.
>
> Note that knfsd is an exception, most NFSv4-acl-supporting servers
> aren't translating from POSIX ACLs.
>
>> NFS is only supported as lower (read-only) layer, so we don't care
>> about changing the ACL on the server.
>
> Out of curiosity, how do you check permissions after copy up?
>
> The client doesn't do much permissions-checking normally, because it's
> hard to get right--even in the absence of ACLs, it may not understand
> the server's owners and groups completely.
>
> I guess that's fine, you may be happy to let people write to the file
> without permissions to the lower file, since the writes aren't going
> there anyway.
>
> So, I don't know what want here.
>
> You're not going to want to use the ACL for actual permission-checking,
> and you can't modify it, so it doesn't seem very useful.

IMO the only thing overlayfs can do is hide those attributes from the
user and ignore them when copying up. Still, the attributes shouldn't
be empty.

Andreas

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

[Patrick Plagwitz]

On 12/05/2016 05:25 PM, J. Bruce Fields wrote:
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
 Can NFS people comment on this?  Where does the nfs4_acl come from?
>>>
>>> This is the interface the NFS client provides for applications to modify
>>> NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
> 
> I don't know.  I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)?  How is it configured?
> 
The server is the one in the kernel (knfsd, I assume), with completely default
configuration. /etc/exports is

/srv/nfsv4 localhost(fsid=root)

Adding rw, or various other options, does not change the observations in the
original mail. As far as I know, there are no options for controlling ACLs.

 What can overlayfs do if it's a non-empty ACL?
>>>
>>> As little as possible.  You can't copy it up, can you?  So any attempt
>>> to support it is going to be incomplete.
>>
>> Right.
>>
>>>
 Does knfsd translate posix ACL into NFS acl?  If so, we can translate
 back.  Should we do a generic POSIX<->NFS acl translator?
>>>
>>> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>> empty, why have it at all?
> 
> I'm honestly not sure what's going on there.  I'd be curious to see a
> network trace if possible.
> 
Just make folders like in the original mail, start the NFS server with the
exports from above and touch merged/folder/file to reproduce the problem.
I don't know anything about the NFS protocol but here you have a tshark
packet summary as well as the details from the folder/ lookup reply (51).
As far as I can tell, no ACL information is in there.

 22 NFS 286 V4 Call CREATE_SESSION
 23 NFS 214 V4 Reply (Call In 22) CREATE_SESSION
 24 NFS 214 V4 Call RECLAIM_COMPLETE
 26 NFS 178 V4 Reply (Call In 24) RECLAIM_COMPLETE
 27 NFS 218 V4 Call SECINFO_NO_NAME
 29 NFS 194 V4 Reply (Call In 27) SECINFO_NO_NAME
 30 NFS 230 V4 Call PUTROOTFH | GETATTR
 31 NFS 350 V4 Reply (Call In 30) PUTROOTFH | GETATTR
 32 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 33 NFS 254 V4 Reply (Call In 32) GETATTR
 34 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 35 NFS 254 V4 Reply (Call In 34) GETATTR
 36 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 37 NFS 254 V4 Reply (Call In 36) GETATTR
 38 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 39 NFS 254 V4 Reply (Call In 38) GETATTR
 40 NFS 234 V4 Call GETATTR FH: 0x62d40c52
 41 NFS 206 V4 Reply (Call In 40) GETATTR
 42 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 43 NFS 254 V4 Reply (Call In 42) GETATTR
 44 NFS 238 V4 Call GETATTR FH: 0x62d40c52
 45 NFS 330 V4 Reply (Call In 44) GETATTR
 46 NFS 246 V4 Call ACCESS FH: 0x62d40c52, [Check: RD LU MD XT DL]
 47 NFS 258 V4 Reply (Call In 46) ACCESS, [Access Denied: MD XT DL], [Allowed: 
RD LU]
 48 NFS 238 V4 Call GETATTR FH: 0x62d40c52
 49 NFS 250 V4 Reply (Call In 48) GETATTR
 50 NFS 258 V4 Call LOOKUP DH: 0x62d40c52/folder
 51 NFS 366 V4 Reply (Call In 50) LOOKUP
 52 NFS 254 V4 Call ACCESS FH: 0x96d0c04a, [Check: RD LU MD XT DL]
 53 NFS 258 V4 Reply (Call In 52) ACCESS, [Access Denied: MD XT DL], [Allowed: 
RD LU]
 54 NFS 262 V4 Call LOOKUP DH: 0x96d0c04a/file
 55 NFS 186 V4 Reply (Call In 54) LOOKUP Status: NFS4ERR_NOENT
 56 NFS 246 V4 Call GETATTR FH: 0x96d0c04a
 57 NFS 278 V4 Reply (Call In 56) GETATTR

Network File System, Ops(5): SEQUENCE PUTFH LOOKUP GETFH GETATTR
[Program Version: 4]
[V4 Procedure: COMPOUND (1)]
Status: NFS4_OK (0)
Tag: 
length: 0
contents: 
Operations (count: 5)
Opcode: SEQUENCE (53)
Status: NFS4_OK (0)
sessionid: 50a245584a819c9a0a00
seqid: 0x000d
slot id: 0
high slot id: 30
target high slot id: 30
status flags: 0x
       ...0 = 
SEQ4_STATUS_CB_PATH_DOWN: Not set
       ..0. = 
SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRING: Not set
       .0.. = 
SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRED: Not set
       0... = 
SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED: Not set
      ...0  = 
SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED: Not set
      ..0.  = 
SEQ4_STATUS_ADMIN_STATE_REVOKED: Not set
      .0..  = 
SEQ4_STATUS_RECALLABLE_STATE_REVOKED: Not set
      0...  = 
SEQ4_STATUS_LEASE_MOVED: Not set
     ...0   = 
SEQ4_STATUS_RESTART_RECLAIM_NEEDED: Not set
  

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

[Patrick Plagwitz]

On 12/05/2016 05:25 PM, J. Bruce Fields wrote:
> On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
>> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
 Can NFS people comment on this?  Where does the nfs4_acl come from?
>>>
>>> This is the interface the NFS client provides for applications to modify
>>> NFSv4 ACLs on servers that support them.
>>
>> Fine, but why are we seeing this xattr on exports where no xattrs are
>> set on the exported fs?
> 
> I don't know.  I took another look at the original patch and don't see
> any details on the server setup: which server is it (knfsd, ganesha,
> netapp, ...)?  How is it configured?
> 
The server is the one in the kernel (knfsd, I assume), with completely default
configuration. /etc/exports is

/srv/nfsv4 localhost(fsid=root)

Adding rw, or various other options, does not change the observations in the
original mail. As far as I know, there are no options for controlling ACLs.

 What can overlayfs do if it's a non-empty ACL?
>>>
>>> As little as possible.  You can't copy it up, can you?  So any attempt
>>> to support it is going to be incomplete.
>>
>> Right.
>>
>>>
 Does knfsd translate posix ACL into NFS acl?  If so, we can translate
 back.  Should we do a generic POSIX<->NFS acl translator?
>>>
>>> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
>>
>> This does explain the nfs4_acl xattr on the client.  Question: if it's
>> empty, why have it at all?
> 
> I'm honestly not sure what's going on there.  I'd be curious to see a
> network trace if possible.
> 
Just make folders like in the original mail, start the NFS server with the
exports from above and touch merged/folder/file to reproduce the problem.
I don't know anything about the NFS protocol but here you have a tshark
packet summary as well as the details from the folder/ lookup reply (51).
As far as I can tell, no ACL information is in there.

 22 NFS 286 V4 Call CREATE_SESSION
 23 NFS 214 V4 Reply (Call In 22) CREATE_SESSION
 24 NFS 214 V4 Call RECLAIM_COMPLETE
 26 NFS 178 V4 Reply (Call In 24) RECLAIM_COMPLETE
 27 NFS 218 V4 Call SECINFO_NO_NAME
 29 NFS 194 V4 Reply (Call In 27) SECINFO_NO_NAME
 30 NFS 230 V4 Call PUTROOTFH | GETATTR
 31 NFS 350 V4 Reply (Call In 30) PUTROOTFH | GETATTR
 32 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 33 NFS 254 V4 Reply (Call In 32) GETATTR
 34 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 35 NFS 254 V4 Reply (Call In 34) GETATTR
 36 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 37 NFS 254 V4 Reply (Call In 36) GETATTR
 38 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 39 NFS 254 V4 Reply (Call In 38) GETATTR
 40 NFS 234 V4 Call GETATTR FH: 0x62d40c52
 41 NFS 206 V4 Reply (Call In 40) GETATTR
 42 NFS 242 V4 Call GETATTR FH: 0x62d40c52
 43 NFS 254 V4 Reply (Call In 42) GETATTR
 44 NFS 238 V4 Call GETATTR FH: 0x62d40c52
 45 NFS 330 V4 Reply (Call In 44) GETATTR
 46 NFS 246 V4 Call ACCESS FH: 0x62d40c52, [Check: RD LU MD XT DL]
 47 NFS 258 V4 Reply (Call In 46) ACCESS, [Access Denied: MD XT DL], [Allowed: 
RD LU]
 48 NFS 238 V4 Call GETATTR FH: 0x62d40c52
 49 NFS 250 V4 Reply (Call In 48) GETATTR
 50 NFS 258 V4 Call LOOKUP DH: 0x62d40c52/folder
 51 NFS 366 V4 Reply (Call In 50) LOOKUP
 52 NFS 254 V4 Call ACCESS FH: 0x96d0c04a, [Check: RD LU MD XT DL]
 53 NFS 258 V4 Reply (Call In 52) ACCESS, [Access Denied: MD XT DL], [Allowed: 
RD LU]
 54 NFS 262 V4 Call LOOKUP DH: 0x96d0c04a/file
 55 NFS 186 V4 Reply (Call In 54) LOOKUP Status: NFS4ERR_NOENT
 56 NFS 246 V4 Call GETATTR FH: 0x96d0c04a
 57 NFS 278 V4 Reply (Call In 56) GETATTR

Network File System, Ops(5): SEQUENCE PUTFH LOOKUP GETFH GETATTR
[Program Version: 4]
[V4 Procedure: COMPOUND (1)]
Status: NFS4_OK (0)
Tag: 
length: 0
contents: 
Operations (count: 5)
Opcode: SEQUENCE (53)
Status: NFS4_OK (0)
sessionid: 50a245584a819c9a0a00
seqid: 0x000d
slot id: 0
high slot id: 30
target high slot id: 30
status flags: 0x
       ...0 = 
SEQ4_STATUS_CB_PATH_DOWN: Not set
       ..0. = 
SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRING: Not set
       .0.. = 
SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRED: Not set
       0... = 
SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED: Not set
      ...0  = 
SEQ4_STATUS_EXPIRED_SOME_STATE_REVOKED: Not set
      ..0.  = 
SEQ4_STATUS_ADMIN_STATE_REVOKED: Not set
      .0..  = 
SEQ4_STATUS_RECALLABLE_STATE_REVOKED: Not set
      0...  = 
SEQ4_STATUS_LEASE_MOVED: Not set
     ...0   = 
SEQ4_STATUS_RESTART_RECLAIM_NEEDED: Not set
  

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

[J. Bruce Fields]

On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
> >> Can NFS people comment on this?  Where does the nfs4_acl come from?
> >
> > This is the interface the NFS client provides for applications to modify
> > NFSv4 ACLs on servers that support them.
> 
> Fine, but why are we seeing this xattr on exports where no xattrs are
> set on the exported fs?

I don't know.  I took another look at the original patch and don't see
any details on the server setup: which server is it (knfsd, ganesha,
netapp, ...)?  How is it configured?

> >> What can overlayfs do if it's a non-empty ACL?
> >
> > As little as possible.  You can't copy it up, can you?  So any attempt
> > to support it is going to be incomplete.
> 
> Right.
> 
> >
> >> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> >> back.  Should we do a generic POSIX<->NFS acl translator?
> >
> > knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
> 
> This does explain the nfs4_acl xattr on the client.  Question: if it's
> empty, why have it at all?

I'm honestly not sure what's going on there.  I'd be curious to see a
network trace if possible.

> > algorithm, and lossy (in the NFSv4->POSIX direction).  The client
> > developers have been understandably reluctant to have anything to do
> > with it.
> >
> > So, I think listxattr should omit system.nfs4_acl, and attempts to
> > set/get the attribute should error out.  The same should apply to any
> > "system." attribute not supported by both filesystems, I think?
> 
> Basically that's what happens now.  The problem is that nfsv4 mounts
> seem always have these xattrs, even when the exported fs doesn't have
> anything.

I said "both", that's a logical "and".  Whether or not nfs claims
support would then be irrelevant in this case, since ext4 doesn't
support system.nfs4_acl.

> We could do the copy up even if the NFS4->POSIX translation was
> possible (which is the case with POSIX ACL translated by knfsd).  We'd
> just get back the original ACL, so that's OK.

Note that knfsd is an exception, most NFSv4-acl-supporting servers
aren't translating from POSIX ACLs.

> NFS is only supported as lower (read-only) layer, so we don't care
> about changing the ACL on the server.

Out of curiosity, how do you check permissions after copy up?

The client doesn't do much permissions-checking normally, because it's
hard to get right--even in the absence of ACLs, it may not understand
the server's owners and groups completely.

I guess that's fine, you may be happy to let people write to the file
without permissions to the lower file, since the writes aren't going
there anyway.

So, I don't know what want here.

You're not going to want to use the ACL for actual permission-checking,
and you can't modify it, so it doesn't seem very useful.

--b.

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

[J. Bruce Fields]

On Mon, Dec 05, 2016 at 04:36:03PM +0100, Miklos Szeredi wrote:
> On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
> >> Can NFS people comment on this?  Where does the nfs4_acl come from?
> >
> > This is the interface the NFS client provides for applications to modify
> > NFSv4 ACLs on servers that support them.
> 
> Fine, but why are we seeing this xattr on exports where no xattrs are
> set on the exported fs?

I don't know.  I took another look at the original patch and don't see
any details on the server setup: which server is it (knfsd, ganesha,
netapp, ...)?  How is it configured?

> >> What can overlayfs do if it's a non-empty ACL?
> >
> > As little as possible.  You can't copy it up, can you?  So any attempt
> > to support it is going to be incomplete.
> 
> Right.
> 
> >
> >> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> >> back.  Should we do a generic POSIX<->NFS acl translator?
> >
> > knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
> 
> This does explain the nfs4_acl xattr on the client.  Question: if it's
> empty, why have it at all?

I'm honestly not sure what's going on there.  I'd be curious to see a
network trace if possible.

> > algorithm, and lossy (in the NFSv4->POSIX direction).  The client
> > developers have been understandably reluctant to have anything to do
> > with it.
> >
> > So, I think listxattr should omit system.nfs4_acl, and attempts to
> > set/get the attribute should error out.  The same should apply to any
> > "system." attribute not supported by both filesystems, I think?
> 
> Basically that's what happens now.  The problem is that nfsv4 mounts
> seem always have these xattrs, even when the exported fs doesn't have
> anything.

I said "both", that's a logical "and".  Whether or not nfs claims
support would then be irrelevant in this case, since ext4 doesn't
support system.nfs4_acl.

> We could do the copy up even if the NFS4->POSIX translation was
> possible (which is the case with POSIX ACL translated by knfsd).  We'd
> just get back the original ACL, so that's OK.

Note that knfsd is an exception, most NFSv4-acl-supporting servers
aren't translating from POSIX ACLs.

> NFS is only supported as lower (read-only) layer, so we don't care
> about changing the ACL on the server.

Out of curiosity, how do you check permissions after copy up?

The client doesn't do much permissions-checking normally, because it's
hard to get right--even in the absence of ACLs, it may not understand
the server's owners and groups completely.

I guess that's fine, you may be happy to let people write to the file
without permissions to the lower file, since the writes aren't going
there anyway.

So, I don't know what want here.

You're not going to want to use the ACL for actual permission-checking,
and you can't modify it, so it doesn't seem very useful.

--b.

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

[Miklos Szeredi]

On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>
> This is the interface the NFS client provides for applications to modify
> NFSv4 ACLs on servers that support them.

Fine, but why are we seeing this xattr on exports where no xattrs are
set on the exported fs?

>> What can overlayfs do if it's a non-empty ACL?
>
> As little as possible.  You can't copy it up, can you?  So any attempt
> to support it is going to be incomplete.

Right.

>
>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> back.  Should we do a generic POSIX<->NFS acl translator?
>
> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated

This does explain the nfs4_acl xattr on the client.  Question: if it's
empty, why have it at all?

> algorithm, and lossy (in the NFSv4->POSIX direction).  The client
> developers have been understandably reluctant to have anything to do
> with it.
>
> So, I think listxattr should omit system.nfs4_acl, and attempts to
> set/get the attribute should error out.  The same should apply to any
> "system." attribute not supported by both filesystems, I think?

Basically that's what happens now.  The problem is that nfsv4 mounts
seem always have these xattrs, even when the exported fs doesn't have
anything.

We could do the copy up even if the NFS4->POSIX translation was
possible (which is the case with POSIX ACL translated by knfsd).  We'd
just get back the original ACL, so that's OK.  NFS is only supported
as lower (read-only) layer, so we don't care about changing the ACL on
the server.

Thanks,
Miklos

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

[Miklos Szeredi]

On Mon, Dec 5, 2016 at 4:19 PM, J. Bruce Fields  wrote:
>> Can NFS people comment on this?  Where does the nfs4_acl come from?
>
> This is the interface the NFS client provides for applications to modify
> NFSv4 ACLs on servers that support them.

Fine, but why are we seeing this xattr on exports where no xattrs are
set on the exported fs?

>> What can overlayfs do if it's a non-empty ACL?
>
> As little as possible.  You can't copy it up, can you?  So any attempt
> to support it is going to be incomplete.

Right.

>
>> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
>> back.  Should we do a generic POSIX<->NFS acl translator?
>
> knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated

This does explain the nfs4_acl xattr on the client.  Question: if it's
empty, why have it at all?

> algorithm, and lossy (in the NFSv4->POSIX direction).  The client
> developers have been understandably reluctant to have anything to do
> with it.
>
> So, I think listxattr should omit system.nfs4_acl, and attempts to
> set/get the attribute should error out.  The same should apply to any
> "system." attribute not supported by both filesystems, I think?

Basically that's what happens now.  The problem is that nfsv4 mounts
seem always have these xattrs, even when the exported fs doesn't have
anything.

We could do the copy up even if the NFS4->POSIX translation was
possible (which is the case with POSIX ACL translated by knfsd).  We'd
just get back the original ACL, so that's OK.  NFS is only supported
as lower (read-only) layer, so we don't care about changing the ACL on
the server.

Thanks,
Miklos

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

[J. Bruce Fields]

On Mon, Dec 05, 2016 at 10:28:18AM +0100, Miklos Szeredi wrote:
> [Added a few more CCs]
> 
> On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
>  wrote:
> > Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes 
> > copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to 
> > fail with EOPNOTSUPP.
> > For example, having the following folders:
> >
> > |- nfs <- NFSv4 is mounted here
> > |--|- folder
> > |- root <- ext4 is mounted here
> > |- work <- also ext4
> > |- merged <- overlay is mounted here with
> >  lowerdir=nfs,upperdir=root,workdir=work
> >
> > And calling
> > # touch merged/folder/file
> > will print
> > touch: cannot touch 'merged/folder/file': Operation not supported
> >
> > This is because NFS returns the xattr system.nfs4_acl with an empty value 
> > even if no NFS ACLs are in use in the lower filesystem. Trying to set this 
> > xattr in the upperdir
> > fails because ext4 does not support it.
> >
> > Fix this by explicitly checking for the name of the xattr and an empty 
> > value and ignoring EOPNOTSUPP if both things match.
> >
> > Signed-off-by: Patrick Plagwitz 
> > ---
> > Maybe NFS could be changed to not return empty system.nfs4_acl values, I 
> > don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, 
> > returning the error code from
> > vfs_setxattr with this xattr name must be avoided as long as the value is 
> > empty.
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 36795ee..505b86e 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry 
> > *new)
> > continue; /* Discard */
> > }
> > error = vfs_setxattr(new, name, value, size, 0);
> > +   if (error == -EOPNOTSUPP && *value == '\0' &&
> > +   strcmp(name, "system.nfs4_acl") == 0)
> > +   error = 0;
> > if (error)
> > break;
> > }
> 
> I agree that this should be fixed, but adding such exceptions for
> certain filesystems or xattrs is not the proper way, IMO.
> 
> Can NFS people comment on this?  Where does the nfs4_acl come from?

This is the interface the NFS client provides for applications to modify
NFSv4 ACLs on servers that support them.

> What can overlayfs do if it's a non-empty ACL?

As little as possible.  You can't copy it up, can you?  So any attempt
to support it is going to be incomplete.

> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> back.  Should we do a generic POSIX<->NFS acl translator?

knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
algorithm, and lossy (in the NFSv4->POSIX direction).  The client
developers have been understandably reluctant to have anything to do
with it.

So, I think listxattr should omit system.nfs4_acl, and attempts to
set/get the attribute should error out.  The same should apply to any
"system." attribute not supported by both filesystems, I think?

I don't understand overlayfs very well, though.

--b.

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

[J. Bruce Fields]

On Mon, Dec 05, 2016 at 10:28:18AM +0100, Miklos Szeredi wrote:
> [Added a few more CCs]
> 
> On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
>  wrote:
> > Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes 
> > copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to 
> > fail with EOPNOTSUPP.
> > For example, having the following folders:
> >
> > |- nfs <- NFSv4 is mounted here
> > |--|- folder
> > |- root <- ext4 is mounted here
> > |- work <- also ext4
> > |- merged <- overlay is mounted here with
> >  lowerdir=nfs,upperdir=root,workdir=work
> >
> > And calling
> > # touch merged/folder/file
> > will print
> > touch: cannot touch 'merged/folder/file': Operation not supported
> >
> > This is because NFS returns the xattr system.nfs4_acl with an empty value 
> > even if no NFS ACLs are in use in the lower filesystem. Trying to set this 
> > xattr in the upperdir
> > fails because ext4 does not support it.
> >
> > Fix this by explicitly checking for the name of the xattr and an empty 
> > value and ignoring EOPNOTSUPP if both things match.
> >
> > Signed-off-by: Patrick Plagwitz 
> > ---
> > Maybe NFS could be changed to not return empty system.nfs4_acl values, I 
> > don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, 
> > returning the error code from
> > vfs_setxattr with this xattr name must be avoided as long as the value is 
> > empty.
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 36795ee..505b86e 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry 
> > *new)
> > continue; /* Discard */
> > }
> > error = vfs_setxattr(new, name, value, size, 0);
> > +   if (error == -EOPNOTSUPP && *value == '\0' &&
> > +   strcmp(name, "system.nfs4_acl") == 0)
> > +   error = 0;
> > if (error)
> > break;
> > }
> 
> I agree that this should be fixed, but adding such exceptions for
> certain filesystems or xattrs is not the proper way, IMO.
> 
> Can NFS people comment on this?  Where does the nfs4_acl come from?

This is the interface the NFS client provides for applications to modify
NFSv4 ACLs on servers that support them.

> What can overlayfs do if it's a non-empty ACL?

As little as possible.  You can't copy it up, can you?  So any attempt
to support it is going to be incomplete.

> Does knfsd translate posix ACL into NFS acl?  If so, we can translate
> back.  Should we do a generic POSIX<->NFS acl translator?

knsd does translate between POSIX and NFSv4 ACLs.  It's a complicated
algorithm, and lossy (in the NFSv4->POSIX direction).  The client
developers have been understandably reluctant to have anything to do
with it.

So, I think listxattr should omit system.nfs4_acl, and attempts to
set/get the attribute should error out.  The same should apply to any
"system." attribute not supported by both filesystems, I think?

I don't understand overlayfs very well, though.

--b.

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

[Miklos Szeredi]

[Added a few more CCs]

On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
 wrote:
> Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes 
> copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to 
> fail with EOPNOTSUPP.
> For example, having the following folders:
>
> |- nfs <- NFSv4 is mounted here
> |--|- folder
> |- root <- ext4 is mounted here
> |- work <- also ext4
> |- merged <- overlay is mounted here with
>  lowerdir=nfs,upperdir=root,workdir=work
>
> And calling
> # touch merged/folder/file
> will print
> touch: cannot touch 'merged/folder/file': Operation not supported
>
> This is because NFS returns the xattr system.nfs4_acl with an empty value 
> even if no NFS ACLs are in use in the lower filesystem. Trying to set this 
> xattr in the upperdir
> fails because ext4 does not support it.
>
> Fix this by explicitly checking for the name of the xattr and an empty value 
> and ignoring EOPNOTSUPP if both things match.
>
> Signed-off-by: Patrick Plagwitz 
> ---
> Maybe NFS could be changed to not return empty system.nfs4_acl values, I 
> don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, returning 
> the error code from
> vfs_setxattr with this xattr name must be avoided as long as the value is 
> empty.
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 36795ee..505b86e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
> continue; /* Discard */
> }
> error = vfs_setxattr(new, name, value, size, 0);
> +   if (error == -EOPNOTSUPP && *value == '\0' &&
> +   strcmp(name, "system.nfs4_acl") == 0)
> +   error = 0;
> if (error)
> break;
> }

I agree that this should be fixed, but adding such exceptions for
certain filesystems or xattrs is not the proper way, IMO.

Can NFS people comment on this?  Where does the nfs4_acl come from?

What can overlayfs do if it's a non-empty ACL?

Does knfsd translate posix ACL into NFS acl?  If so, we can translate
back.  Should we do a generic POSIX<->NFS acl translator?

Thanks,
Miklos

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

[Miklos Szeredi]

[Added a few more CCs]

On Mon, Dec 5, 2016 at 1:51 AM, Patrick Plagwitz
 wrote:
> Mounting an overlayfs with an NFSv4 lowerdir and an ext4 upperdir causes 
> copy_up operations, specifically the function copy_up.c:ovl_copy_xattr, to 
> fail with EOPNOTSUPP.
> For example, having the following folders:
>
> |- nfs <- NFSv4 is mounted here
> |--|- folder
> |- root <- ext4 is mounted here
> |- work <- also ext4
> |- merged <- overlay is mounted here with
>  lowerdir=nfs,upperdir=root,workdir=work
>
> And calling
> # touch merged/folder/file
> will print
> touch: cannot touch 'merged/folder/file': Operation not supported
>
> This is because NFS returns the xattr system.nfs4_acl with an empty value 
> even if no NFS ACLs are in use in the lower filesystem. Trying to set this 
> xattr in the upperdir
> fails because ext4 does not support it.
>
> Fix this by explicitly checking for the name of the xattr and an empty value 
> and ignoring EOPNOTSUPP if both things match.
>
> Signed-off-by: Patrick Plagwitz 
> ---
> Maybe NFS could be changed to not return empty system.nfs4_acl values, I 
> don't know. In any case, to support upperdir ext4 + lowerdir NFSv4, returning 
> the error code from
> vfs_setxattr with this xattr name must be avoided as long as the value is 
> empty.
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 36795ee..505b86e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -123,6 +123,9 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new)
> continue; /* Discard */
> }
> error = vfs_setxattr(new, name, value, size, 0);
> +   if (error == -EOPNOTSUPP && *value == '\0' &&
> +   strcmp(name, "system.nfs4_acl") == 0)
> +   error = 0;
> if (error)
> break;
> }

I agree that this should be fixed, but adding such exceptions for
certain filesystems or xattrs is not the proper way, IMO.

Can NFS people comment on this?  Where does the nfs4_acl come from?

What can overlayfs do if it's a non-empty ACL?

Does knfsd translate posix ACL into NFS acl?  If so, we can translate
back.  Should we do a generic POSIX<->NFS acl translator?

Thanks,
Miklos