2018-08-07 21:53:30

by J. Bruce Fields

[permalink] [raw]
Subject: nfs4-acl-tools 0.3.4

nfs4-acl-tools doesn't get a lot of attention, but it does get a *few*
commits, so it seems a little ridiculous to go 10 years without a
release. And it was getting annoying for Steve Dickson, who packages
the Fedora version.

So I've tagged 0.3.4 in the git tree at

git://linux-nfs.org/~bfields/nfs4-acl-tools.git

and posted a tarball at:

http://linux-nfs.org/~bfields/nfs4-acl-tools/nfs4-acl-tools-0.3.4.tar.gz

Mostly bugfixes; aside from those, one of the only new things you're
likely to notice is recursive nfs4_getfacl support (thanks to Kenneth
Dsouza).

--b.


2018-08-21 20:12:25

by J. Bruce Fields

[permalink] [raw]
Subject: nfs4-acl-tools 0.3.5

One more nfs4-acl-tools release. This removes the GUI, which appears to
have been unused and unmaintained. I'm happy to revive it from the git
history if someone thinks it's worth taking over. I also did some minor
tampering with the autotools stuff.

git://linux-nfs.org/~bfields/nfs4-acl-tools.git

http://linux-nfs.org/~bfields/nfs4-acl-tools/nfs4-acl-tools-0.3.5.tar.gz

--b.

On Tue, Aug 07, 2018 at 03:37:36PM -0400, bfields wrote:
> nfs4-acl-tools doesn't get a lot of attention, but it does get a *few*
> commits, so it seems a little ridiculous to go 10 years without a
> release. And it was getting annoying for Steve Dickson, who packages
> the Fedora version.
>
> So I've tagged 0.3.4 in the git tree at
>
> git://linux-nfs.org/~bfields/nfs4-acl-tools.git
>
> and posted a tarball at:
>
> http://linux-nfs.org/~bfields/nfs4-acl-tools/nfs4-acl-tools-0.3.4.tar.gz
>
> Mostly bugfixes; aside from those, one of the only new things you're
> likely to notice is recursive nfs4_getfacl support (thanks to Kenneth
> Dsouza).
>
> --b.

2018-08-22 03:07:09

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/21/2018 9:51 AM, J. Bruce Fields wrote:
> One more nfs4-acl-tools release. This removes the GUI, which appears to

I had recently posted an inquiry regarding the nfs4-acl-tools; I don't
believe I saw a response but if you are maintaining them perhaps you
might know?

I'm currently working on implementing support for the native NFSv4 ACL
in the ZFS on Linux port, using nfs4-acl-tools as the mechanism for
viewing and updating them. I noticed a discrepancy between the
definitions in the ZFS NFSv4 ACL include file versus the ones in the
nfs4-acl-tools headers.

Specifically, in zfs:

#define ACE_INHERITED_ACE 0x0080
#define ACE_OWNER 0x1000
#define ACE_GROUP 0x2000
#define ACE_EVERYONE 0x4000

Whereas in nfs4-acl-tools:

#define NFS4_ACE_OWNER 0x00000080
#define NFS4_ACE_GROUP 0x00000100
#define NFS4_ACE_EVERYONE 0x00000200

I'm not sure where these values came from, I don't see them in the NFSv4
RFC. Do you happen to have any idea where the values used in the
nfs4-acl-tools came from and why they might be different than those used
by zfs?

Thanks much…

2018-08-22 03:55:25

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Tue, Aug 21, 2018 at 04:44:38PM -0700, Paul B. Henson wrote:
> On 8/21/2018 9:51 AM, J. Bruce Fields wrote:
> >One more nfs4-acl-tools release. This removes the GUI, which appears to
>
> I had recently posted an inquiry regarding the nfs4-acl-tools; I
> don't believe I saw a response but if you are maintaining them
> perhaps you might know?

Sorry, yes, I saw the question but hadn't gotten a chance to look at it.

In the wire protocol there's no need for such constants. An ACE can
have one of the special strings OWNER@, GROUP@, or EVERYONE@ in the
owner field, and that's all you need. Without actually looking at the
code, I'm guessing that both the Linux and Solaris ACL tools wanted a
quick way to test whether an ACE had one of those special owners without
doing a strcmp(), so defined these extra bits in the flag field. Which
seems like a premature optimization to me, but, whatever.

The only use of those constants is probably internal to the ACL tools,
so they don't have to agree with any standards.

But that's just a guess, check the code to be sure.

--b.

>
> I'm currently working on implementing support for the native NFSv4
> ACL in the ZFS on Linux port, using nfs4-acl-tools as the mechanism
> for viewing and updating them. I noticed a discrepancy between the
> definitions in the ZFS NFSv4 ACL include file versus the ones in the
> nfs4-acl-tools headers.
>
> Specifically, in zfs:
>
> #define ACE_INHERITED_ACE 0x0080
> #define ACE_OWNER 0x1000
> #define ACE_GROUP 0x2000
> #define ACE_EVERYONE 0x4000
>
> Whereas in nfs4-acl-tools:
>
> #define NFS4_ACE_OWNER 0x00000080
> #define NFS4_ACE_GROUP 0x00000100
> #define NFS4_ACE_EVERYONE 0x00000200
>
> I'm not sure where these values came from, I don't see them in the
> NFSv4 RFC. Do you happen to have any idea where the values used in
> the nfs4-acl-tools came from and why they might be different than
> those used by zfs?
>
> Thanks much…

2018-08-22 04:41:34

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/21/2018 5:33 PM, J. Bruce Fields wrote:

> In the wire protocol there's no need for such constants. An ACE can
> have one of the special strings OWNER@, GROUP@, or EVERYONE@ in the
> owner field, and that's all you need.

Ah, so an NFSv4 client is supposed to figure out whether or not it's a
special ACE just by the who_string... That probably explains why I
didn't see the same issue using the nfs4-acl-tools on a Linux client
mounting an illumos ZFS share via NFS.

> The only use of those constants is probably internal to the ACL tools,
> so they don't have to agree with any standards.

Ok, I found the illumos NFSv4 server code where it converts the local
ZFS ACLs to/from xdr format, and you're right, those flags are not
included in the outbound mapping, it converts them to the special entry
strings, and it does string comparisons to determine whether or not to
add them for the inbound mapping.

However, in the nfs4-acl-tools code it seems to expect those bits off
the wire to decode and is willing to send them on the wire?

For example, in libnfs4acl/nfs4_get_ace_flags.c

if (flags & NFS4_ACE_OWNER)
*buf++ = FLAG_OWNER_AT;

If it sees that bit set in the flags, it adds 'O' to the string
representation, and correspondingly in libnfs4acl/nfs4_ace_from_string.c:

case FLAG_OWNER_AT:
flags |= NFS4_ACE_OWNER;
break;

If you include O in your ACL specification, it will add that flag and
include it when it sends it? The same for the NFS4_ACE_GROUP and
NFS4_ACE_EVERYONE flags.

I'm confused why the nfs4-acl-tools would need these local defines. On
the ZFS side, the on-disk ACL format doesn't include strings, just flags
and uids/gids, so the extra flag bits are presumably needed so it can
tell which entries are special. However, the tools presumably are only
intended to consume NFSv4 xdr, and generate it? So why did they need
these flags given that the NFSv4 xdr format doesn't include them?

Thanks for the input, much appreciated…

2018-08-22 18:37:30

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Tue, Aug 21, 2018 at 06:18:49PM -0700, Paul B. Henson wrote:
> On 8/21/2018 5:33 PM, J. Bruce Fields wrote:
>
> >In the wire protocol there's no need for such constants. An ACE can
> >have one of the special strings OWNER@, GROUP@, or EVERYONE@ in the
> >owner field, and that's all you need.
>
> Ah, so an NFSv4 client is supposed to figure out whether or not it's
> a special ACE just by the who_string... That probably explains why I
> didn't see the same issue using the nfs4-acl-tools on a Linux client
> mounting an illumos ZFS share via NFS.
>
> >The only use of those constants is probably internal to the ACL tools,
> >so they don't have to agree with any standards.
>
> Ok, I found the illumos NFSv4 server code where it converts the
> local ZFS ACLs to/from xdr format, and you're right, those flags are
> not included in the outbound mapping, it converts them to the
> special entry strings, and it does string comparisons to determine
> whether or not to add them for the inbound mapping.
>
> However, in the nfs4-acl-tools code it seems to expect those bits
> off the wire to decode and is willing to send them on the wire?
>
> For example, in libnfs4acl/nfs4_get_ace_flags.c
>
> if (flags & NFS4_ACE_OWNER)
> *buf++ = FLAG_OWNER_AT;
>
> If it sees that bit set in the flags, it adds 'O' to the string
> representation, and correspondingly in
> libnfs4acl/nfs4_ace_from_string.c:
>
> case FLAG_OWNER_AT:
> flags |= NFS4_ACE_OWNER;
> break;

Huh. Yeah, that does look like a bug. But I'm surprised the utilities
would work at all in that case--I'd expect both the Solaris and Linux
knfsd to reject an ACL with those extra bits, and certainly neither
would set them on read. So I can't help thinking I'm overlooking some
sanitization of the flags field somewhere.....

Time to test and look at the wire traffic in wireshark, I guess.

> If you include O in your ACL specification, it will add that flag
> and include it when it sends it? The same for the NFS4_ACE_GROUP and
> NFS4_ACE_EVERYONE flags.
>
> I'm confused why the nfs4-acl-tools would need these local defines.
> On the ZFS side, the on-disk ACL format doesn't include strings,
> just flags and uids/gids, so the extra flag bits are presumably
> needed so it can tell which entries are special. However, the tools
> presumably are only intended to consume NFSv4 xdr, and generate it?
> So why did they need these flags given that the NFSv4 xdr format
> doesn't include them?

I suspect they're unnecessary, and I'd happily take a patch to remove
them once we figure out what's going on.

--b.

2018-08-22 22:54:40

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/22/2018 8:12 AM, J. Bruce Fields wrote:
>
> Huh. Yeah, that does look like a bug. But I'm surprised the utilities
> would work at all in that case--I'd expect both the Solaris and Linux
> knfsd to reject an ACL with those extra bits, and certainly neither

Hmm, you're right; trying to include any of them when updating an ACL on
an illumos server from the Linux client results in "Failed setxattr
operation: Invalid argument".

It looks like those extra flags aren't even documented. Given a server
will never actually return them such that a user sees them displayed,
and they are not documented, it's most likely no one has ever actually
tried to use them 8-/. From a historical perspective, it would be
interesting to know why they are there, but that is perhaps lost to
antiquity ;).

> I suspect they're unnecessary, and I'd happily take a patch to remove
> them once we figure out what's going on.

I'll strip them out and send you a patch soon; hopefully it won't take
10 more years to show up in a release :).

A side question while I have your attention; once I get the ZFS NFSv4
acl support committed upstream, what is it going to take to have the
linux NFSv4 server use them and provide them to clients?

Thanks much…

2018-08-22 23:12:33

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Wed, Aug 22, 2018 at 12:28:13PM -0700, Paul B. Henson wrote:
> On 8/22/2018 8:12 AM, J. Bruce Fields wrote:
> >
> >Huh. Yeah, that does look like a bug. But I'm surprised the utilities
> >would work at all in that case--I'd expect both the Solaris and Linux
> >knfsd to reject an ACL with those extra bits, and certainly neither
>
> Hmm, you're right; trying to include any of them when updating an
> ACL on an illumos server from the Linux client results in "Failed
> setxattr operation: Invalid argument".
>
> It looks like those extra flags aren't even documented. Given a
> server will never actually return them such that a user sees them
> displayed, and they are not documented, it's most likely no one has
> ever actually tried to use them 8-/. From a historical perspective,
> it would be interesting to know why they are there, but that is
> perhaps lost to antiquity ;).

Oh, I see now, I was assuming they were automatically set on owner ACEs
or something, but they're only set if you happen to know this bit of
undocumented UI implemented in get_ace_flags/ace_from_string. So, yeah,
we should just remove that code.

> >I suspect they're unnecessary, and I'd happily take a patch to remove
> >them once we figure out what's going on.
>
> I'll strip them out and send you a patch soon; hopefully it won't
> take 10 more years to show up in a release :).

OK, great, thanks.

> A side question while I have your attention; once I get the ZFS
> NFSv4 acl support committed upstream, what is it going to take to
> have the linux NFSv4 server use them and provide them to clients?

I can't take knfsd patches to support something that only an out-of-tree
filesystem cares about.

Looks like ZFS has pretty much no chance of going upstream.

You could try to port them to some other filesystem (xfs, ext4). But
the attempt to implement rich ACLs (which are pretty similar) seemed to
get vetoed for reasons I don't completely understand, and I don't see
why this would fare any better.

So I think you'd be stuck carrying your own out-of-tree patches for it.

Ganesha probably could (may already) do this, for what it's worth.

--b.

2018-08-23 04:38:46

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/22/2018 12:46 PM, J. Bruce Fields wrote:
>
>> I'll strip them out and send you a patch soon; hopefully it won't
>> take 10 more years to show up in a release :).
>
> OK, great, thanks.

Patch sent; it was pretty trivial, hopefully it will suffice.

> I can't take knfsd patches to support something that only an
> out-of-tree filesystem cares about.

Hmm, I hadn't considered that, but I understand the position.

> Looks like ZFS has pretty much no chance of going upstream.

Yah, GPL, CDDL, they don't get along :(.

> You could try to port them to some other filesystem (xfs, ext4).

The ZFS on Linux port already supports the standard POSIX ACL. I'm
curious, do those work with the NFS server? Does something specifically
need to be done individually for each file system, or if it supports the
standard extended attribute does any file system (including an out of
tree file system) automatically function?

> But the attempt to implement rich ACLs (which are pretty similar)
> seemed to get vetoed for reasons I don't completely understand, and I
> don't see why this would fare any better.

While they have different implementation details, NFSv4 ACLs and rich
ACLs seem to have compatible expression formats. If they ever do get
implemented, I'd be able to switch from the somewhat inefficient
system.nfs4 extended attribute interface between the kernel and user
space for ZFS ACLs to a rich ACL API... And if the NFS server would
simply work with any file system that exported a standard rich ACL,
maybe ZFS would work then.

> So I think you'd be stuck carrying your own out-of-tree patches for
> it.

Hmm, that would greatly reduce the size of the user base; most
distributions now make ZFS available via their packaging systems, but I
don't know how many would be willing to include an extra ZFS specific
kernel patch for NFS service.

Thanks again…

2018-08-23 18:08:33

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Wed, Aug 22, 2018 at 06:11:20PM -0700, Paul B. Henson wrote:
> >You could try to port them to some other filesystem (xfs, ext4).
>
> The ZFS on Linux port already supports the standard POSIX ACL. I'm
> curious, do those work with the NFS server?

Yes. Sort of. There's a sideband protocol for NFSv3 that supports them
pretty much as they are. NFSv4 only has NFSv4 ACLs, so we support them
by translating between "posix" and NFSv4 ACLs on the fly, and the
result can be a pain to use.

> Does something specifically need to be done individually for each file
> system, or if it supports the standard extended attribute does any
> file system (including an out of tree file system) automatically
> function?

Nothing special's required, it should be automatic.

--b.

2018-08-23 23:12:53

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/23/2018 7:38 AM, J. Bruce Fields wrote:

>> Does something specifically need to be done individually for each
>> file system, or if it supports the standard extended attribute does
>> any file system (including an out of tree file system)
>> automatically function?
>
> Nothing special's required, it should be automatic.

So if, hypothetically, the NFSv4 server was enhanced to look for and
understand the standard linux system.nfs4_acl extended attribute, any
file system, whether in kernel or out of tree, would support exposing
NFSv4 ACLs via NFS? Even though there's nothing ZFS specific about it,
that general functionality would not be acceptable for inclusion in the
mainstream kernel?

That seems a bit of a chicken and egg problem, do you add a feature for
a subsystem to use so said subsystem could be updated to use it, or you
update a subsystem to use a feature that doesn't exist yet :)?

2018-08-23 23:13:19

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/23/2018 7:38 AM, J. Bruce Fields wrote:

>> Does something specifically need to be done individually for each
>> file system, or if it supports the standard extended attribute does
>> any file system (including an out of tree file system)
>> automatically function?
>
> Nothing special's required, it should be automatic.

So if, hypothetically, the NFSv4 server was enhanced to look for and
understand the standard linux system.nfs4_acl extended attribute, any
file system, whether in kernel or out of tree, would support exposing
NFSv4 ACLs via NFS? Even though there's nothing ZFS specific about it,
that general functionality would not be acceptable for inclusion in the
mainstream kernel?

That seems a bit of a chicken and egg problem, do you add a feature for
a subsystem to use so said subsystem could be updated to use it, or you
update a subsystem to use a feature that doesn't exist yet :)?

2018-08-24 00:28:29

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Thu, Aug 23, 2018 at 12:41:49PM -0700, Paul B. Henson wrote:
> On 8/23/2018 7:38 AM, J. Bruce Fields wrote:
>
> >>Does something specifically need to be done individually for each
> >>file system, or if it supports the standard extended attribute does
> >>any file system (including an out of tree file system)
> >>automatically function?
> >
> >Nothing special's required, it should be automatic.
>
> So if, hypothetically, the NFSv4 server was enhanced to look for and
> understand the standard linux system.nfs4_acl extended attribute,
> any file system, whether in kernel or out of tree, would support
> exposing NFSv4 ACLs via NFS? Even though there's nothing ZFS
> specific about it, that general functionality would not be
> acceptable for inclusion in the mainstream kernel?

Right, it's against kernel policy, and even if it weren't, I don't want
to be in the position of maintaining code, even simple code, that's
really only needed for third-party modules without any path to upstream.

> That seems a bit of a chicken and egg problem, do you add a feature
> for a subsystem to use so said subsystem could be updated to use it,
> or you update a subsystem to use a feature that doesn't exist yet
> :)?

Honestly the system.nfs4_acl extended attribute interface, which just
exposes the raw xdr of the ACL to userspace, is kind of a kludge. It
could be made to work for other filesystems but I was hoping that other
filesystems would adopt something designed for them from scratch (like
richacls).

That said, there *is* already an in-kernel filesystem that supports
system.nfs4_acl: knfsd does actually allow limited re-export of NFS. So
knfsd code that used system.nfs4_acl when available might actually have
some use, I don't really know. I'm a little skeptical of the idea, to
be honest.

--b.

2018-08-24 04:22:53

by Paul B. Henson

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On 8/23/2018 1:57 PM, J. Bruce Fields wrote:

> Honestly the system.nfs4_acl extended attribute interface, which
> just exposes the raw xdr of the ACL to userspace, is kind of a
> kludge. It could be made to work for other filesystems but I was
> hoping that other filesystems would adopt something designed for them
> from scratch (like richacls).

I agree; but it's what I have to work with :). And from a pragmatic
perspective I'd rather have something that works even if not perfect
than perfect vaporware I can't use ;). If something like richacls comes
along at some point in the future it should be possible to migrate to it.

> That said, there *is* already an in-kernel filesystem that supports
> system.nfs4_acl: knfsd does actually allow limited re-export of NFS.
> So knfsd code that used system.nfs4_acl when available might actually
> have some use, I don't really know. I'm a little skeptical of the
> idea, to be honest.

Hmm, the door is open a crack :). When I get a chance to put something
together I'll be back…

From a design perspective, would you want this to just take the
verbatim xdr encoded acl from the file system and shove it over the
wire, or would you want the NFS server to decode the acl received from
the extended attribute, process or sanity check as necessary, and then
re-encode it to send over the wire? The same I guess for ones received
over the network, pass as is to fs xattr call or decode/re-encode.

Thanks…

2018-08-24 09:24:13

by Christoph Hellwig

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Thu, Aug 23, 2018 at 12:41:23PM -0700, Paul B. Henson wrote:
> On 8/23/2018 7:38 AM, J. Bruce Fields wrote:
>
> > > Does something specifically need to be done individually for each
> > > file system, or if it supports the standard extended attribute does
> > > any file system (including an out of tree file system)
> > > automatically function?
> >
> > Nothing special's required, it should be automatic.
>
> So if, hypothetically, the NFSv4 server was enhanced to look for and
> understand the standard linux system.nfs4_acl extended attribute, any file
> system, whether in kernel or out of tree, would support exposing NFSv4 ACLs
> via NFS? Even though there's nothing ZFS specific about it, that general
> functionality would not be acceptable for inclusion in the mainstream
> kernel?

Hell no. We're not going to support anything out of tree in general,
and not something that violates our licensing terms for that matter.

2018-08-24 19:01:13

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfs4-acl-tools 0.3.5

On Thu, Aug 23, 2018 at 05:50:22PM -0700, Paul B. Henson wrote:
> Hmm, the door is open a crack :). When I get a chance to put
> something together I'll be back…

OK. I can't promise anything. It all depends on whether it would make
any sense for the NFS reexport case, and I just don't know.

> From a design perspective, would you want this to just take the
> verbatim xdr encoded acl from the file system and shove it over the
> wire, or would you want the NFS server to decode the acl received
> from the extended attribute, process or sanity check as necessary,
> and then re-encode it to send over the wire? The same I guess for
> ones received over the network, pass as is to fs xattr call or
> decode/re-encode.

It'd be simplest to pass it straight through, but the complicated issue
is names. See the logic in fs/nfsd/idmap.c which defaults to string
names in the krb5 case (where we assume the necessary infrastructure to
get that right is all set up), but ascii-encoded numeric id's in the
auth_sys case (when legacy numeric ID's are the simpler default).

--b.