[nfsv4] Re: Comments for draft-rmacklem-nfsv4-posix-acls-04

[David Noveck <[e-mail]>]

On Sat, Aug 17, 2024, 6:28 PM Rick Macklem <[e-mail]> wrote:

>
>
> On Sat, Aug 17, 2024 at 12:47 PM Pali Rohár <[e-mail]>
> wrote:
>
>> On Monday 12 August 2024 18:03:09 Rick Macklem wrote:
>> > I am only going to make a few generic statements...
>>
>> Ok, if you have a time, try to look at my comments from previous
>> message. I would like to know if my ideas at least makes some sense or
>> how hard would be to design this NFS4 POSIX ACL extension compatible
>> with NFS4 servers with any possible RFC 8881 compatible meaning of
>> "mode" attribute.
>>
>> Or if it is not clear, I could try to show it on more / other examples.
>>
>> > Before I started writing this draft, I prototyped
>> > the posix_access_acl/posix_default_acl in FreeBSD.
>> > Both client and server changes took about 3hrs.
>> > Why was it so easy?
>> > Because all of the stuff related to POSIX draft ACL
>> > semantics was already there. Handling of mode vs POSIX
>> > draft ACL was already there.
>> > --> All I had to do was translate between the XDR format
>> >     and FreeBSD's internal format for a POSIX draft ACL.
>> > By using a file system configured to use POSIX draft ACLs
>> > for testing, that was all there was to it.
>> >
>> > What I cannot easily do is change any of the POSIX draft ACL
>> > semantics, including how mode vs ACL are handled. Those are in
>> > functions written by others at least 15years ago and trying
>> > to change the semantics in any way would get serious pushback.
>> >
>> > I am not conversant with the Linux sources, but I suspect
>> > that the situation is similar.
>> >
>> > I have trouble understanding your concerns w.r.t. mode for
>> > a couple of reasons:
>> > 1 - A NFSv4 client just sets/gets the mode bits. How setting
>> >     them affects ACLs and vice versa, are handled internally
>> >     by the server.
>>
>> Yes, it is handled by server and server is relatively free to choose one
>> of the option what to do with it. But IEEE POSIX ACL does not allow to
>> choose more options how to handle it.
>>
>> > 2 - Right now, there may some words in RFC8881 related to
>> >     NFSv4 ACLs vs mode bits, but in the end, it is whatever
>> >     the server implementation chooses to do.
>>
>> Yes, this is the primary problem related to mode as I detailed
>> described. Servers can chose and existing servers have already chosen
>> one of the option how to handle it. And if server implementation has
>> chosen option incompatible with IEEE POSIX ACL draft then such server
>> cannot implement this NFS4 POSIX ACL extension if backward compatibility
>> is priority of the server.
>>
> When I first started working on this draft, I assumed that a file system
> would choose to do POSIX draft ACLs or it would choose to do NFSv4 ACLs.
> As such, the file system would maintain mode based on which it chose.
> (Or, put another way, the mode vs ACL algorithms will be different for
> the different true form ACLs.)
>
> Then David Noveck indicated that he thought a "per-file object" scope
> was useful (and Frank Filz noted that IBM's GPFS could do this already).
> I would like to know how IBM's GPFS handles mode when the true form
> ACL (lets say NFSv4) is replaced by a POSIX draft ACL?
> (I'd guess that the mode gets set to whatever the POSIX draft ACL requires,
>

Yes.

essentially replacing both mode and ACL at the same time, but I do not
> know.)
>

"at the same time" suggests atomicity.  I'm not sure the draft POSIX ACLs
requires that, bit if it does, you have to do the same.

>
> Now, if you meant the mode would be replaced when you talked about "two
> modes"
> then, yes, I would agree.
>
> David, do you have an opinion w.r.t. what should happen to mode when a ACL
> of one true form is replaced by an ACL of the other true form for a file
> object?
>

Yes.  The mode should be that required by the new true to reflect the new
ACL.  I don't see how you could do anything else.


> rick
>
>
>> If server implementation already chosen option fully compatible with
>> IEEE POSIX ALC draft (which I guess is the case of Linux and FreeBSD
>> servers) then implementation of NFS4 POSIX ACL extension is
>> straightforward.
>>
>> >     For OpenZFS, there is a knob (they call them properties)
>> >     called "aclmode". It controls what happens when either
>> >     a NFSv4 ACL is set or the 9 bits of mode is set.
>> >     The knob currently has 4 possible settings.
>> >     Does any of these four settings conform to the
>> >     vague requirements in RFC8881? I am not sure, but again,
>> >     changing those semantics now would not be easy. (Maybe
>> >     yet another knob setting?)
>> >     (If you google for "man zfsprops", you can read what they are.)
>>
>> I looked at it. I think that all four options are "compatible" with NFS4
>> server according to RFC 8881. Just because RFC 8881 allows server to do
>> lot of options.
>>
>
🤢


>> On the other hand, IEEE POSIX ACL draft is exact and strict how chmod
>> modifies (POSIX) ACL.
>>
>> > The case my prototype does not implement is the per file object scope
>> > case (ACL_SCOPE_FILE_OBJECT), because no file system on FreeBSD can
>> > do NFSv4 and POSIX draft ACLs on the same file system.
>> >
>> > Btw, if there is a server implementor that wants to implement
>> > posix_access_acl/posix_default_acl and the system they are working
>> > on does not have a POSIX draft ACL implementation, there are at
>> > least 3 open source implementations (with different licenses) they
>> > can work from, plus the Grunbacher paper.
>>
>> I guess that this would apply for industry environment where is mainly
>> used Windows-style/NFS4 ACLs.
>>
>> > rick
>> >
>> >
>> > On Mon, Aug 12, 2024 at 11:17 AM Pali Rohár <
>> [e-mail]>
>> > wrote:
>> >
>> > > On Sunday 11 August 2024 17:09:44 Rick Macklem wrote:
>> > > > On Sun, Aug 11, 2024 at 3:11 AM Pali Rohár <
>> [e-mail]
>> > > >
>> > > > wrote:
>> > > >
>> > > > > On Saturday 10 August 2024 16:54:39 Rick Macklem wrote:
>> > > > > > On Sat, Aug 10, 2024 at 3:36 PM Pali Rohár <
>> > > [e-mail]
>> > > > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > On Saturday 10 August 2024 13:55:36 Rick Macklem wrote:
>> > > > > > > > I have updated the draft to -05. I think I have
>> > > > > > > > addressed your comments.
>> > > > > > > >
>> > > > > > > > A few inline responses below.
>> > > > > > > >
>> > > > > > > > On Fri, Aug 9, 2024 at 5:31 PM Pali Rohár <
>> > > > > [e-mail]>
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > Hello Rick,
>> > > > > > > > >
>> > > > > > > > > I have read your new version -04 of extension document
>> and just
>> > > > > now I
>> > > > > > > > > have figured out some new things there.
>> > > > > > > > >
>> > > > > > > > > For better readability I have one small suggestion for
>> section
>> > > 9.
>> > > > > > > > > In description of each attribute could be list of all
>> possible
>> > > > > values.
>> > > > > > > > > Currently the possible values are defined only in XDR
>> section
>> > > 4.
>> > > > > And it
>> > > > > > > > > is quite hard for me to read this section 9, because I
>> need to
>> > > > > scroll
>> > > > > > > up
>> > > > > > > > > (to read list of possible values) and then back down to
>> the
>> > > > > > > description.
>> > > > > > > > > When I first time read this section in -02 I was quite
>> lost
>> > > and I
>> > > > > had
>> > > > > > > to
>> > > > > > > > > read it more times.
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > My comment from -02 which still applies for -04 is
>> version is
>> > > > > ambiguity
>> > > > > > > > > of NFS4 mode attribute which does not match POSIX mode as
>> > > required
>> > > > > by
>> > > > > > > > > POSIX ACL.
>> > > > > > > > >
>> > > > > > > > I think I have addressed this now. It notes that mode is
>> > > sychronized
>> > > > > > > > with the true form ACL for the file object.
>> > > > > > >
>> > > > > > > I think that there is still an issue. Meaning of the mode
>> > > attribute is
>> > > > > > > defined differently in RFC 8881.
>> > > > > > >
>> > > > > > > RFC 8881 6.2.4. Attribute 33: mode says: "Bits MODE4_RGRP,
>> > > MODE4_WGRP,
>> > > > > > > and MODE4_XGRP apply to principals identified in the
>> owner_group
>> > > > > > > attribute but who are not identified in the owner attribute."
>> > > > > > >
>> > > > > > > RFC 8881 6.3.2.1. Discussion says: "Some server
>> implementations
>> > > also
>> > > > > add
>> > > > > > > bits permitted to named users and groups to the group bits
>> > > (MODE4_RGRP,
>> > > > > > > MODE4_WGRP, and MODE4_XGRP). Implementations are discouraged
>> from
>> > > doing
>> > > > > > > this."
>> > > > > > >
>> > > > > > > So according to meaning of mode attribute, MODE4_RGRP,
>> MODE4_WGRP,
>> > > and
>> > > > > > > MODE4_XGRP bits matches POSIXACE4_TAG_GROUP_OBJ. Which is not
>> > > > > compatible
>> > > > > > > with IEEE POSIX draft.
>> > > > > > >
>> > > > > > > I see that you have added following requirement into
>> description.
>> > > > > > >
>> > > > > > >   "If the true form is ACL_MODEL_POSIX_DRAFT, synchronization
>> is
>> > > > > > >   described in [Grünbacher]."
>> > > > > > >
>> > > > > > > Which sounds like that it wants to override what is defined
>> in the
>> > > > > > > "RFC 8881 6.2.4. Attribute 33: mode", and change meaning of
>> the
>> > > NFS4
>> > > > > > > mode attribute.
>> > > > > > >
>> > > > > > > I do not think that it is a good idea if some optional
>> extension is
>> > > > > > > going to change meaning or wants that server should implement
>> > > something
>> > > > > > > which RFC 8881 6.3.2.1. says that "Implementations are
>> discouraged
>> > > from
>> > > > > > > doing this". It it contradiction.
>> > > > > > >
>> > > > > > > Example scenario why it is a bad idea.
>> > > > > > >
>> > > > > > > Some existing NFS4 server implements mode attribute according
>> to
>> > > > > > > RFC 8881 and always synchronize group bits with owner_group
>> (who
>> > > is not
>> > > > > > > owner). So group bits are never POSIX mask. And also
>> implements
>> > > NFS4
>> > > > > > > ACL support.
>> > > > > > >
>> > > > > > > There is some NFS4 client which is using that NFS4 server,
>> and this
>> > > > > > > client uses only mode attribute (does not touch
>> acl/dacl/sacl),
>> > > and is
>> > > > > > > using chmod with group bits to set access for owner_group
>> (who is
>> > > not
>> > > > > > > owner).
>> > > > > > >
>> > > > > > > Everything is working fine up to here.
>> > > > > > >
>> > > > > > > Now that NFS4 server wants to implement per-file scope POSIX
>> ACL
>> > > > > > > support. The only requirement is not break that existing NFS4
>> > > client.
>> > > > > > > As the NFS4 client does not look or touch any ACL, POSIX ACL
>> > > support on
>> > > > > > > server filesystem should be harmless.
>> > > > > > >
>> > > > > > > But problem is that such server cannot implement POSIX ACL
>> support
>> > > > > > > according to this extension because of "mode" compatibility
>> issues
>> > > with
>> > > > > > > that NFS4 client. For compatibility reasons for every file,
>> server
>> > > has
>> > > > > > > to return MODE4_RGRP/MODE4_WGRP/MODE4_XGRP bits from
>> owner_group
>> > > (like
>> > > > > > > before). But this POSIX ACL extension requires that mode
>> group bits
>> > > > > > > would be synchronized with POSIX MASK, so chmod of group bits
>> stops
>> > > > > > > working and breaks that NFS4 mode-only client.
>> > > > > > >
>> > > > > > That is a choice the server implementer makes. It is no
>> different
>> > > > > > than local use of POSIX draft ACLs on systems such as Linux.
>> > > > > > The user chooses to set a POSIX draft ACL on the local file
>> object
>> > > > > > and in doing so, gets POSIX draft ACL semantics. If they do not
>> set
>> > > > > > a POSIX draft ACL on the object, they get "mode only" semantics.
>> > > > >
>> > > > > By implementing this extension there is no choice for server in
>> above
>> > > > > example to allow it to return GETATTR(mode) attribute according to
>> > > > > preferred way as in RFC 8881.
>> > > > >
>> > > > > It is not same as on local Linux system, because on local Linux
>> system,
>> > > > > all get-mode operations (stat or statx, ...) can already return
>> MASK in
>> > > > > mode group bits.
>> > > > >
>> > > > > RFC 8881 say that "Implementations are discouraged from doing
>> this."
>> > > > > Local Linux systems do not say anything like this.
>> > > > >
>> > > > > So it is different situation.
>> > > > >
>> > > > I am aware from home until Friday, so I cannot do any actual
>> > > > test on Linux until then. However, I expect to see the following:
>> > > > - Given a typical server running knfsd, where the server's file
>> > > >   system uses POSIX draft as its true form.
>> > > > Locally on the server: AC
>> > > > - create a file in a directory without any default ACL.
>> > > >   ls -l <-- shows the default mode
>> > > >   setfacl of an extended POSIX ACL
>> > > >   ls -l <-- shows the mode has changed, as expected
>> > > > - Mount this file system via NFSv3 and do the same as above.
>> > > >   - create a file in a directory without any default ACL.
>> > > >   ls -l <-- shows the default mode
>> > > >   setfacl of an extended POSIX ACL
>> > > >   ls -l <-- shows the mode has changed, as expected
>> > > > Bottom line, Since both the Linux client and knfsd server
>> > > > support the undocumented NFSACL sideband protocol, I would
>> > >
>> > > I'm not sure if NFS3 ACL is really undocumented, year ago when I was
>> > > looking for it I found some documentation, XDR definitions and also
>> some
>> > > Sun or Oracle descriptions. But I do not have references in hands.
>> > >
>> > > > expect the outcome to look identical to what happens when
>> > > > the same commands are done locally on the file system.
>> > > > (As I said, I cannot actually try this until I get home
>> > > > at the end of the week and will post if do not see the above.)
>> > >
>> > > Yes, IIRC last time when I checked it, over NFS3 network protocol,
>> this
>> > > behavior of Linux NFS3 client and Linux knfsd NFS3 server was same as
>> on
>> > > the local Linux ext4 filesystem. Matches what you described above.
>> > >
>> > > > Now, remount using NFSv4.1/4.2
>> > > > - setfacl fails with an error.
>> > >
>> > > Yes, this is truth, as Linux NFS4 client does not allow to access
>> > > Linux NFS4 server's ACLs via setfacl, even that Linux NFS4 knfsd
>> server
>> > > provides mapping in "acl" attribute.
>> > >
>> > > > The user has to go back to NFSv3 to do the setfacl.
>> > > > Now, wouldn't it be nice if NFSv4 worked exactly the
>> > > > same way as NFSv3 and local file system access?
>> > >
>> > > Yes, it would be really nice and I hope that this your extension would
>> > > allow to implement it.
>> > >
>> > > > The object of this extension is to permit the semantics for
>> > > > NFSv4 to be identical to NFSv3 and locally on the server file
>> > > > system. That is what this extension is all about.
>> > >
>> > > Yes, I agree.
>> > >
>> > > > Until the Linux folk do an implementation, I cannot be sure what
>> > > > will be needed, but I am confident that the Linux developers
>> > > > will figure it out.
>> > >
>> > > The important is that _both_ Linux NFS4 client and Linux NFS4 knfsd
>> > > server have to be modified to support this new extension. Just one
>> part
>> > > (e.g. only client or only server) is not enough. So it would be
>> required
>> > > to upgrade all clients and server.
>> > >
>> > > > Since [Grunbacher] describes how this is done on Linux, that
>> > > > is the "standard" for handling mode when a server chooses to
>> > > > support posix_access_acl for a true form of POSIX draft ACL.
>> > >
>> > > Here I would say that it (mode) is _not_ standard way of handling. I
>> > > would say that standard way of mode is only what is descried in
>> released
>> > > RFCs, therefore in RFC 8881. More NFS4 servers do not handle NFS4 mode
>> > > attribute in this way as Linux NFS4 server (which is allowed by RFC
>> 8881
>> > > but not preferred).
>> > >
>> > > But Linux NFS4 knfsd server is handling it in this way as you
>> described
>> > > So for combination of Linux NFS4 client and Linux knfsd NFS4 server
>> > > would work it fine.
>> > >
>> > > > Maybe the draft needs a sentence clarifying this.
>> > > > Something like:
>> > > > For a server file system that supports the posix_access_acl
>> > > > attribute and file objects where acl_trueform is
>> ACL_MODEL_POSIX_DRAFT,
>> > > > if there is a discrepancy between the semantics for mode handling as
>> > > > described
>> > > > in [RFC8881] vs the semantics for mode handling in {Grunbacher], the
>> > > > semantics
>> > > > specified in [Grunbacher] MUST be used.
>> > >
>> > > I still do not think it is a good idea. It will work perfectly fine
>> for
>> > > Linux knfsd server and Linux client. But would not work for
>> combination
>> > > of Linux client with some non-Linux NFS4 server which cannot (for
>> > > existing backward compatibility) provide existing NFS4 mode attribute
>> > > according to [Grunbacher] mapping, even when some optional extension
>> > > will be implemented.
>> > >
>> > > I'm trying to show that this way is not universal for any NFS4 server.
>> > > I understand that it will work fine for Linux and FreeBSD NFS4 servers
>> > > and this ecosystem around but as I described it would not work for
>> some
>> > > other servers.
>> > >
>> > > I'm looking at this from other perspective: to be usable for more
>> > > servers and ecosystems, not just Linux<-->Linux or FreeBSD<-->Linux
>> > > but also for industry<-->Linux.
>> > >
>> > > And I would like to see this POSIX extension to be implementable also
>> > > for other NFS4 server which do not provide current NFS4 mode attribute
>> > > according to [Grunbacher] but provide it according to [RFC8881].
>> > >
>> > > Changing mode mapping in industrial NFS software, just for
>> implementing
>> > > some optional extension, is a huge risk and can be a big stop gap to
>> not
>> > > implement this optional extension at all. Changing behavior of
>> anything
>> > > (does not matter what it is) is always a problem. So if some
>> industrial
>> > > NFS server returns in MODE4_*GRP bits permissions only and only for
>> > > owner_group then this server would have to do it in any future
>> versions
>> > > under any conditions, and changing this behavior does not have to be
>> > > acceptable. Industrial software is lot of times "certificated" or
>> > > "tested" for behavior to ensure forward and backward compatibility
>> under
>> > > any condition (for this case, "any condition" means also any value of
>> > > acl_trueform setting).
>> > >
>> > > That is why I'm trying to say that it is better to not change meaning
>> of
>> > > NFS4 mode attribute, let it as is in RFC 8881 (and discrepancy handle
>> as
>> > > defined in RFC 8881) and instead for POSIX ACL extension purposes
>> > > provide a way for client to retrieve the real POSIX mode from some
>> other
>> > > attribute which will be according to POSIX ACL specification. I
>> proposed
>> > > two alternatives how it could be done.
>> > >
>> > > This should work with any NFS4 server, any NFS4 client, including
>> Linux,
>> > > and also allows NFS4 servers which needs to have existing NFS4 mode
>> > > attribute to stay according to RFC 8881, to implement optional
>> per-file
>> > > POSIX ACL support.
>> > >
>> > > >
>> > > >
>> > > > > > If a server chooses to implement the extension proposed in this
>> > > draft,
>> > > > > > then there is no NFSv4 ACL and the behaviour looks exactly the
>> same
>> > > > > > as for local uses with/without POSIX draft ACLs on file objects
>> on
>> > > the
>> > > > > > system.
>> > > > >
>> > > > > In above example there no usage of NFS4 ACL. So this issue is not
>> > > > > related to NFS4 ACL. It is related to NFS4 mode.
>> > > > >
>> > > > > I do not see a choice how could that NFS4 server in above example
>> > > > > implement this POSIX extension without violating some MUST parts
>> of
>> > > > > description.
>> > > > >
>> > > > > >
>> > > > > > > So how could this NFS4 server implements POSIX ACL support for
>> > > Linux or
>> > > > > > > FreeBSD client without breaking that one mentioned NFS4
>> client?
>> > > > > >
>> > > > > >
>> > > > > > > I think that it is needed to define a new posix_mode
>> attribute. I
>> > > do
>> > > > > not
>> > > > > > > see right now any option how to not break backward
>> compatibility
>> > > for
>> > > > > > > existing mode-only clients (which wants behavior of mode group
>> > > bits as
>> > > > > > > written in RFC 8881 / 6.2.4) and at the same time support
>> POSIX ACL
>> > > > > > > which requires that mode group bits contains either MASK (or
>> > > group_obj
>> > > > > > > if MASK is not defined).
>> > > > > > >
>> > > > > > > Or another option, putting content of POSIX-correct "mode"
>> > > attribute
>> > > > > > > into the "posix_access_acl" attribute (e.g. at beginning of
>> the
>> > > > > > > structure) as it is used only when POSIX ACL mode is active.
>> > > > > > >
>> > > > > > I do not think a separate mode is a good idea, just like I do
>> not
>> > > > > > think having both POSIX draft ACL(s) and a NFSv4 ACL
>> stored/used for
>> > > > > > permission checking on the same file
>> > > > > > object at a given time is a good idea (the true form).
>> > > > > >
>> > > > > > The simple summary for all this is the following:
>> > > > > > RFC8881 was written for only one kind of ACL (NFSv4), so any
>> > > discussions
>> > > > > > in it w.r.t. the relationship between ACLs and mode are
>> implicitly
>> > > for
>> > > > > > NFSv4 ACLs only and, understandably, did not take the POSIX
>> draft
>> > > ACLs
>> > > > > > into account.
>> > > > > > --> This optional extension changes all the above, so that
>> server
>> > > > > > implementers
>> > > > > >     can choose to implement POSIX draft ACLs and use POSIX draft
>> > > rules
>> > > > > > related
>> > > > > >     to ACLs and mode, if they choose to do so.
>> > > > >
>> > > > > What is in RFC 8881 written:
>> > > > >
>> > > > > "Bits MODE4_RGRP, MODE4_WGRP, and MODE4_XGRP apply to principals
>> > > > > identified in the owner_group attribute but who are not
>> identified in
>> > > > > the owner attribute."
>> > > > >
>> > > > > This does not imply NFS4 ACL usage. At least this is how I
>> understand
>> > > it.
>> > > > >
>> > > > > It if the extension is suppose to change all this above then this
>> > > > > extension is not backward compatible with existing RFC 8881
>> > > > > implementations and NFS4 servers for which is backward
>> compatibility
>> > > > > crucial part, cannot implement this extension at all.
>> > > > >
>> > > > > In my opinion, backward compatible extension should not change
>> meaning
>> > > > > of some parts or require servers to implements something which is
>> > > marked
>> > > > > as "discourage do it".
>> > > > >
>> > > > > For me this is really too restrictive.
>> > > > >
>> > > > > From one direction I'm seeing it quite inconsistent if POSIX ACL
>> is
>> > > > > stored in separate attribute than NFS4 ACL, but POSIX mode is
>> stored in
>> > > > > same attribute as NFS4 mode. For an engineer who is going to
>> implement
>> > > > > this extension it looks to be a mess.
>> > > > >
>> > > > >
>> > > > > I have just another option without need for a new posix_mode
>> attribute
>> > > > > (if you think that new separate attribute is not a good idea) how
>> to
>> > > > > make the extension backward compatible and allow NFS4 server in
>> above
>> > > > > example implement it without braking that NFS4 client:
>> > > > >
>> > > > > - Explicitly mention that NFS4 mode attribute is exactly
>> according to
>> > > > >   RFC 8881 description, so MODE4_RGRP, MODE4_WGRP, and MODE4_XGRP
>> bits
>> > > > >   on existing server implementations could refer to POSIX
>> group_obj
>> > > > >   (but server implementations may refer to also POSIX mask, but
>> it is
>> > > > >   discourage do it). All this is just explicit backward
>> compatibility.
>> > > > >
>> > > > > - Describe that NFS4 client, which is implementing this
>> extension, and
>> > > > >   wants to call CHMOD on ACL_MODEL_POSIX_DRAFT file with MASK
>> ACE, it
>> > > > >   has to use SETATTR(posix_access_acl) instead of SETATTR(mode).
>> Also
>> > > > >   if client wants to receive POSIX MODE, it has to use
>> > > > >   GETATTR(posix_access_acl) instead of GETATTR(mode). This is for
>> > > making
>> > > > >   compatibility with existing NFS4 server which follows RFC 8881.
>> > > > >   NFS4 client implementing this extension can calculate correct
>> POSIX
>> > > > >   mode from posix_access_acl attribute (checking if there is a
>> MASK and
>> > > > >   based on this put content of MODE_xGRP bits).
>> > > > >
>> > > > > >
>> > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > > New comments for each section are below.
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Section 5. POSIX ACL Considerations
>> > > > > > > > >
>> > > > > > > > > > in a directory that has a POSIX default ACL, the
>> low-order
>> > > nine
>> > > > > bits
>> > > > > > > > > > of the mode MUST be specified by mode_umask in the
>> setable
>> > > > > attributes
>> > > > > > > > > > for the operation.
>> > > > > > > > >
>> > > > > > > > > I think that here is missing an important fact that mode
>> is
>> > > > > specified
>> > > > > > > in
>> > > > > > > > > "mu_mode member of mode_umask" attribute and that the
>> "mu_umask
>> > > > > member
>> > > > > > > > > of mode_umask" is being ignored in this case (when POSIX
>> > > default
>> > > > > ACL is
>> > > > > > > > > being used).
>> > > > > > > > >
>> > > > > > > > > Because from current description is not clear how
>> mode_umask is
>> > > > > being
>> > > > > > > > > used as this attribute contains two members ("mu_mode" and
>> > > > > "mu_umask").
>> > > > > > > > >
>> > > > > > > > I added a "See RFC8275..." for this.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Section 6. POSIX ACL vs NFSv4 ACL Considerations
>> > > > > > > > >
>> > > > > > > > > > Using SETATTR to set the dacl attribute to an array of
>> > > non-zero
>> > > > > > > length
>> > > > > > > > > > ... will result in the object's acl model being set to
>> > > > > > > ACL_MODEL_NFS4.
>> > > > > > > > >
>> > > > > > > > > Why setting NFS v4.1 dacl attribute to zero length is
>> excluded
>> > > > > here?
>> > > > > > > > > I think it is perfectly fine to set dacl with
>> ACL4_PROTECTED
>> > > flag
>> > > > > and
>> > > > > > > > > with zero length (empty list of) ACEs. This is fully
>> valid NFS4
>> > > > > model
>> > > > > > > > > ACL which overwrites automatic inheritance to ACL with no
>> > > access.
>> > > > > > > > >
>> > > > > > > > > In my opinion using SETATTR on dacl attribute should
>> always
>> > > change
>> > > > > acl
>> > > > > > > > > model to ACL_MODEL_NFS4. I'm expecting that as a user I'm
>> > > setting
>> > > > > NFS4
>> > > > > > > > > dacl attribute which refers to NFS4 model, so I'm
>> > > unconditionally
>> > > > > > > > > setting model to NFS4.
>> > > > > > > > >
>> > > > > > > > I compromised by saying any setting of dacl does this and
>> any
>> > > setting
>> > > > > > > > of acl, if it has ALLOW/DENY ACEs in it does this. (I don't
>> know
>> > > if
>> > > > > a 0
>> > > > > > > > length
>> > > > > > > > setting for acl means set the ACL model to NFSv4 or get rid
>> of
>> > > all
>> > > > > > > > ALARM/AUDIT ACEs or ???)
>> > > > > > >
>> > > > > > > If acl attribute for some object contains only one ACE with
>> > > > > > > "AUDIT SUCCESS|FAILURE EVERYONE@ FULL_ACCESS" then server
>> should
>> > > send
>> > > > > > > into audit log any attempt to access that file.
>> > > > > > >
>> > > > > > > If I as admin want to remove auditing on that file then I
>> need to
>> > > > > remove
>> > > > > > > this one ACE from acl attribute. So it means that I have to
>> set acl
>> > > > > > > attribute to empty.
>> > > > > > >
>> > > > > > > So setting zero length acl attribute has to get rid of all
>> ACEs,
>> > > ALLOW,
>> > > > > > > DENY and also AUDIT and ALARM. Otherwise admin would not be
>> able to
>> > > > > stop
>> > > > > > > auditing on file which has only AUDITing rules.
>> > > > > > >
>> > > > > > > > I hope that David Noveck can resolve what a 0 length ACL
>> actually
>> > > > > means
>> > > > > > > > in his draft. As you have noted, SMB considers one of these
>> > > "normal"
>> > > > > and,
>> > > > > > > > as such, I see the argument that it should be that way for
>> NFSv4
>> > > > > ACLs as
>> > > > > > > > well.
>> > > > > > > > However, I see that the FreeBSD port of OpenZFS (which uses
>> > > > > > > ACL_MODEL_NFS4
>> > > > > > > > as
>> > > > > > > > its true form) returns an error if a setting of a zero
>> length
>> > > ACL is
>> > > > > > > > attempted.
>> > > > > > >
>> > > > > > > I thought that it is clear, but seems that there are
>> confusions
>> > > about
>> > > > > > > zero length ACL. So proper clarification in new ACL draft
>> would be
>> > > > > > > really helpful.
>> > > > > > >
>> > > > > > > > I cannot tell if this was done by the FreeBSD guy that
>> worked on
>> > > > > ACLs or
>> > > > > > > was
>> > > > > > > > cribbed from OpenSolaris?
>> > > > > > >
>> > > > > > > I think it could be possible to check what the Oracle's
>> Solaris is
>> > > > > > > doing.
>> > > > > > >
>> > > > > > > > I think it would be nice to somehow "deprecate" acl in
>> favour of
>> > > > > > > dacl/sacl.
>> > > > > > >
>> > > > > > > I'm supporting the idea of deprecating "acl" attribute in
>> favour of
>> > > > > > > separated dacl and sacl attributes.
>> > > > > > >
>> > > > > > > But for backward compatibility with NFS v4.1 (and v4.2) it is
>> > > > > > > impossible. Breaking existing NFS v4.1 clients which supports
>> only
>> > > > > "acl"
>> > > > > > > attribute should be really avoided. So some future NFS v4.3
>> is the
>> > > > > > > candidate for removal of this "acl" attribute.
>> > > > > > >
>> > > > > > > >
>> > > > > > > > > > In addition, if the object's acl model had been
>> > > > > > > ACL_MODEL_POSIX_DRAFT,
>> > > > > > > > > > the existing default and access ACLs are to be deleted.
>> > > > > > > > >
>> > > > > > > > > I think that in this document it is a good idea to always
>> > > > > explicitly
>> > > > > > > > > mention ACL type. Because sometimes it can be really
>> > > confusing. For
>> > > > > > > > > example: "existing POSIX default and access ACLs" make it
>> > > clear.
>> > > > > > > > >
>> > > > > > > > Sorry, but if ACL_MODEL_POSIX_DRAFT does not obviously
>> refer to
>> > > POSIX
>> > > > > > > draft
>> > > > > > > > ACLs,
>> > > > > > > > I don't know what does. I left it as is.
>> > > > > > >
>> > > > > > > I was suggesting this from the point of view of somebody new
>> this
>> > > area.
>> > > > > > > For people familiar with POSIX ACL it is obvious. For
>> somebody who
>> > > is
>> > > > > > > new to ACL it can be hard to figure out what is obvious and
>> what
>> > > not.
>> > > > > > >
>> > > > > > > But if you think it is OK then let it as is.
>> > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Section 7. GETATTR/SETATTR Atomicity should handle VERIFY
>> and
>> > > > > NVERIFY
>> > > > > > > > > atomically in the same way as GETATTR.
>> > > > > > > > >
>> > > > > > > > > For example, NFS4 client may want to check if POSIX ACL
>> is set
>> > > to
>> > > > > some
>> > > > > > > > > specific structure, and VERIFY with both acl_trueform and
>> > > > > > > > > posix_access_acl attributes is the appropriate operation
>> for
>> > > it.
>> > > > > > > > >
>> > > > > > > > > Maybe VERIFY/NVERIFY should be covered also in other
>> sections
>> > > which
>> > > > > > > > > refers to GETATTR/READDIR?
>> > > > > > > > >
>> > > > > > > > Good catch. I think this is fixed now in the draft.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Section 9.3. Attribute 90: posix_default_acl
>> > > > > > > > >
>> > > > > > > > > > Since a POSIX default ACL only applies to directories, a
>> > > SETATTR
>> > > > > of
>> > > > > > > > > > the posix_default_acl for a non-directory object MUST
>> reply
>> > > > > > > > > > NFS4ERR_INVAL.
>> > > > > > > > >
>> > > > > > > > > I guess that this MUST applies also for OPEN/OPEN4_CREATE
>> and
>> > > > > > > > > CREATE/NON-NF4DIR operations.
>> > > > > > > > >
>> > > > > > > > Again, good catch. I think it is fixed now.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Section 9.4. Attribute 91: posix_access_acl
>> > > > > > > > >
>> > > > > > > > > > a successful setting of this attribute sets the value of
>> > > > > acl_trueform
>> > > > > > > > > > to ACL_MODEL_POSIX_DRAFT. In addition, if the object's
>> acl
>> > > model
>> > > > > had
>> > > > > > > > > > been ACL_MODEL_NFS4, all ALLOW and DENY ACEs will be
>> deleted
>> > > so
>> > > > > that
>> > > > > > > > > > the value of the dacl attribute is a zero-length array.
>> > > > > > > > >
>> > > > > > > > > In other email I already wrote similar comment which
>> applies
>> > > here
>> > > > > too.
>> > > > > > > > > I think that this contradicts RFC 8881. Why?
>> > > > > > > > >
>> > > > > > > > > * Per section 1. Introduction: "If the server chooses to
>> > > support
>> > > > > > > > >   posix_default_acl and posix_access_acl for a file
>> system, it
>> > > MUST
>> > > > > > > > >   support the mode/mode_umask attributes for the file
>> system."
>> > > > > > > > >
>> > > > > > > > > * It is not explicitly mentioned but ACL_MODEL_NFS4 should
>> > > imply
>> > > > > that
>> > > > > > > > >   NFS4 server has to support dacl (or acl) attribute. (It
>> > > would be
>> > > > > nice
>> > > > > > > > >   to explicitly mention it.)
>> > > > > > > > >
>> > > > > > > > > * Per RFC 8881 section 6.4. Requirements: "The server that
>> > > supports
>> > > > > > > both
>> > > > > > > > >   mode and ACL must take care to synchronize the
>> MODE4_*USR,
>> > > > > > > MODE4_*GRP,
>> > > > > > > > >   and MODE4_*OTH bits with the ACEs that have respective
>> who
>> > > > > fields of
>> > > > > > > > >   "OWNER@", "GROUP@", and "EVERYONE@". This way, the
>> client
>> > > can
>> > > > > see if
>> > > > > > > > >   semantically equivalent access permissions exist
>> whether the
>> > > > > client
>> > > > > > > > >   asks for the owner, owner_group, and mode attributes or
>> for
>> > > just
>> > > > > the
>> > > > > > > > >   ACL."
>> > > > > > > > >
>> > > > > > > > > So we are in situation when mode, acl/dacl and
>> posix_access_acl
>> > > > > > > > > attributes are supported.
>> > > > > > > > >
>> > > > > > > > > And successful setting of posix_access_acl attribute
>> results in
>> > > > > > > > > zero-length array of dacl attribute. This removal of dacl
>> ACEs
>> > > > > results
>> > > > > > > > > in synchronization of dacl to mode attribute. As there is
>> no
>> > > > > "OWNER@",
>> > > > > > > > > "GROUP@", and "EVERYONE@" ACE entry in dacl anymore
>> (because
>> > > dacl
>> > > > > is
>> > > > > > > > > empty), this will result in synchronization of mode to
>> empty
>> > > access
>> > > > > > > 000.
>> > > > > > > > >
>> > > > > > > > > Per POSIX ACL, mode is being synchronized to POSIX ACL
>> entities
>> > > > > > > > > user_obj, group_obj/mask, other.
>> > > > > > > > >
>> > > > > > > > > If my understanding and steps above are correct then
>> successful
>> > > > > setting
>> > > > > > > > > of posix_access_acl attribute to any value results in
>> > > immediate no
>> > > > > > > > > access for user_obj, group_obj/mask and other POSIX
>> entities.
>> > > And I
>> > > > > > > > > guess that this is not intended behavior.
>> > > > > > > > >
>> > > > > > > > I added a short paragraph that states synchronization of
>> mode
>> > > with
>> > > > > > > > the true form ACL SHOULD be done. Using either RFC8881 or
>> > > Grunbacher
>> > > > > > > > as references.
>> > > > > > >
>> > > > > > > But the problem is still there. The extension document still
>> says
>> > > > > > > "if the object's acl model had been ACL_MODEL_NFS4, all ALLOW
>> and
>> > > DENY
>> > > > > > > ACEs will be deleted so that the value of the dacl attribute
>> is a
>> > > > > > > zero-length array." which as I described above, according to
>> RFC
>> > > 8881
>> > > > > it
>> > > > > > > can be interpreted as removal of all access for owner, group
>> and
>> > > > > others.
>> > > > > > >
>> > > > > > > It is not clear if the synchronization of the mode with true
>> form
>> > > ACL
>> > > > > > > should be done before or after deleting of ALLOW/DENY rules.
>> > > > > > >
>> > > > > > > The problem is that SETATTR(posix_access_acl) is doing lot of
>> > > things,
>> > > > > > > including:
>> > > > > > >
>> > > > > > > * changing acl_trueform_scope to ACL_MODEL_POSIX_DRAFT
>> > > > > > > * changing posix_access_acl to value specified in SETATTR
>> > > > > > > * changing mode to value from posix_access_acl
>> > > > > > > * changing acl and dacl values by removing all ALLOW and DENY
>> rules
>> > > > > > > * changing acl and dacl values to value from mode (RFC 8881
>> 6.4)
>> > > > > > > (Have I forgot something?)
>> > > > > > >
>> > > > > > Although it is hard to write in a format that works for this
>> draft,
>> > > > > > in summary, it is very simple:
>> > > > > > - Set a POSIX draft ACL on the file object and delete any dacl.
>> > > > > > Unfortunately, there is no well defined way to say the dacl has
>> been
>> > > > > > deleted.
>> > > > > > (I thought a zero length ACL worked for this, since OpenZFS
>> > > > > > does not allow setting of a zero length ACL, but you have
>> pointed out
>> > > > > > that that is not the case.)
>> > > > > > --> So, until there is (if there ever is) a better way to say
>> "dacl
>> > > has
>> > > > > >     been deleted", it ends up as "make the dacl a zero length
>> ACL
>> > > and get
>> > > > > > rid
>> > > > > >     of all allow/deny ACEs in acl.
>> > > > >
>> > > > > Just to note that zero length dacl is not accepted by NFS4 XDR
>> because
>> > > > > dacl attribute is a struct which contains flags and list of ACEs.
>> > > > >
>> > > > By aero length dacl, it means that the ACE cnt is zero, not that
>> > > > the entire dacl attribute is of zero length. I will clarify that
>> > > > in the next draft.
>> > >
>> > > Ok, this is a good idea to clarify it. Maybe it would be better to
>> avoid
>> > > "zero length" wording at all as it is confusing. I can imagine even
>> more
>> > > meanings of "zero length" in this RPC / XDR / NFS4 / ACL context. What
>> > > about explicitly saying that dacl's na41_aces array has zero size and
>> > > dacl's na41_flag is something...
>> > >
>> > > >
>> > > > > So for backward compatibility, NFS4 server cannot send zero-length
>> > > > > structure as a response for GETATTR(dacl) operation.
>> > > > >
>> > > > > Saying "delete dacl" or "remove dacl" is ambiguous. Developers of
>> > > > > different NFS4 servers would understand it differently.
>> > > > >
>> > > > > I think it is needed to properly describe what it means.
>> > > > >
>> > > > As I have said before, I do not currently know the correct
>> > > > syntax for "no NFSv4 ACL associated with this file object".
>> > >
>> > > I think that this is not possible to report. At least for me the ACL
>> > > evaluation algorithm is clear that for empty list of ACEs means DENY.
>> > >
>> > > Anyway, RFC 8881 section 6.2.1 "Attribute 12: acl" says:
>> > >
>> > > "Some server platforms may provide access-control functionality that
>> > > goes beyond the UNIX-style mode attribute, but that is not as rich as
>> > > the NFS ACL model. So that users can take advantage of this more
>> limited
>> > > functionality, the server may support the acl attributes by mapping
>> > > between its ACL model and the NFSv4.1 ACL model."
>> > >
>> > > This matches POSIX ACL model, so returning in GETATTR(dacl) mapped
>> > > POSIX-in-NFS4 ACL according to [Grunbacher] should be fine and better
>> > > than returning empty list of ACEs.
>> > >
>> > > Also needs to take care about combination of NFS4 server with POSIX
>> > > extension on per-file level and NFS4 client without this POSIX
>> > > extension. Practical example can be: FreeBSD server + Windows client.
>> > > Windows client would be unhappy if for some files (under POSIX model)
>> it
>> > > would not receive NFS4 ACLs (at least mapped).
>> > >
>> > > > However, if the GETATTR also acquires the acl_trueform attribute
>> > > > and sees it is not ACL_MODEL_NFS4, then the client knows that
>> > > > the replied dacl is meaningless.
>> > >
>> > > I think it would be better to express that NFS4 client which
>> implements
>> > > this POSIX extension should ignore value of both acl and dacl
>> attributes
>> > > and use only posix_access_acl attribute when acl_trueform is not
>> > > ACL_MODEL_NFS4 (still allow clients to use sacl attribute as POSIX ACL
>> > > does not provide auditing support).
>> > >
>> > > But I would not say that dacl is meaningless when acl_trueform is not
>> > > ACL_MODEL_NFS4. For clients which do not implement this POSIX
>> extension,
>> > > they cannot figure out what is state of acl_trueform. And cannot
>> figure
>> > > out that something other is used for access control. Again, quoted
>> > > RFC 8881 section 6.2.1 above can be really useful for them.
>> > >
>> > > So server should return something meaningful for these NFS4 clients
>> > > which do not implement this POSIX extension. It is needed for all
>> > > already released Linux NFS4 clients which will be there for a very
>> > > long time (via enterprise RHEL, SLED, ...) and on which is used
>> > > nfs4_getfacl tool.
>> > >
>> > > > Maybe that is a better way to express it.
>> > > >
>> > > > rick
>> > > >
>> > > >
>> > > > > >
>> > > > > > > And executing these steps in incorrect order can results in
>> > > something
>> > > > > > > unexpected or something not compatible with IEEE POSIX draft.
>> > > > > > >
>> > > > > > Maybe I should just say "delete the dacl" and let server
>> implementors
>> > > > > > take it from there?
>> > > > > >
>> > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > And now I'm thinking, what should happen with dacl attribute's
>> > > > > na41_flag
>> > > > > > > value? Specially with ACL4_AUTO_INHERIT and ACL4_PROTECTED
>> flags.
>> > > > > > >
>> > > > > > > I would say that ACL4_AUTO_INHERIT needs to be cleared (to
>> prevent
>> > > > > > > automatic inheritance to child objects) and ACL4_PROTECTED to
>> be
>> > > set
>> > > > > (to
>> > > > > > > prevent inheritance from parent objects). As POSIX ACL does
>> not
>> > > have
>> > > > > any
>> > > > > > > kind of automatic inheritance.
>> > > > > > >
>> > > > > > The intent of the above (and I have already noted that this is
>> > > difficult
>> > > > > > to write in a format acceptable for this draft) is "delete the
>> dacl".
>> > > > > > --> It goes away, no longer exists, has no effect on mode or
>> access
>> > > to
>> > > > > >     the file.
>> > > > >
>> > > > > It would be nice to explicitly mention this intent. This is
>> something
>> > > > > which really is not clear.
>> > > > >
>> > > > > Anyway, the question reminds, what should NFS4 server return in
>> > > > > GETATTR(dacl) reply? Part of the dacl XDR structure are flags.
>> > > > >
>> > > > > And NFS4 client implementing automatic inheritance, when changing
>> some
>> > > > > top level dacl attribute, it has to traverse whole filesystem
>> hierarchy
>> > > > > and updates all child dacl attributes according to child's dacl
>> flags.
>> > > > >
>> > > > > So incorrect information returned by dacl flags can totally mess
>> up
>> > > > > either NFS4 client or server's ACLs in whole filesystem.
>> > > > >
>> > > > > Due to this automatic inheritance, I think it is import to define
>> > > > > exactly what has to happen. And as I wrote in previous message, I
>> think
>> > > > > that the correct way is to clear ACL4_AUTO_INHERIT and set
>> > > > > ACL4_PROTECTED, to say NFS4 ACL client that automatic inheritance
>> at
>> > > > > that point stops and POSIX ACL (without automatic inheritance)
>> > > > > continues.
>> > > > >
>> > > > >
>> > > > > Hm... Would not it be easier for understanding to say that "dacl"
>> > > > > attribute for object with acl_trueform ACL_MODEL_POSIX_DRAFT
>> should
>> > > > > return POSIX mapped NFS4 ACL [Eriksen]? (Of course only in case
>> server
>> > > > > implements "dacl" attribute). It will avoid any confusion what
>> that
>> > > zero
>> > > > > length means or what that remove dacl mans.
>> > > > >
>> > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > > > If SETATTR of a POSIX extended ACL does not have a
>> > > > > POSIXACE4_TAG_MASK
>> > > > > > > > > > ACE, the SETATTR of the ACL MUST reply NFS4ERR_INVAL.
>> > > > > > > > >
>> > > > > > > > > I think there should be also mentioned that SETATTR of
>> POSIX
>> > > ACL
>> > > > > (does
>> > > > > > > > > not have to be extended) which does not have
>> > > > > POSIXACE4_TAG_USER_OBJ,
>> > > > > > > > > POSIXACE4_TAG_GROUP_OBJ or POSIXACE4_TAG_OTHER ACE MUST
>> result
>> > > in
>> > > > > > > > > NFS4ERR_INVAL too. (Except zero-length.)
>> > > > > > > > >
>> > > > > > > > I felt the description that noted these three ACEs are
>> always in
>> > > a
>> > > > > POSIX
>> > > > > > > > draft
>> > > > > > > > ACL was enough but, yes, I suppose it should be added.
>> > > > > > > > I missed this one for -05, but will do so for -06.
>> > > > > > >
>> > > > > > > Documentation should explicitly say what should happen = which
>> > > error
>> > > > > > > code to return when invalid POSIX ACL is specified in SETATTR
>> (or
>> > > > > > > OPEN/CREATE) operation.
>> > > > > > >
>> > > > > > > I was somehow surprised that XDR definition allows invalid
>> POSIX
>> > > ACL
>> > > > > > > structure (the one without user_obj/group_obj/other parts).
>> > > Because if
>> > > > > > > it allows invalid structure then it is required to explicitly
>> > > specify
>> > > > > > > how to handle invalid cases.
>> > > > > > >
>> > > > > > I have already added a note that "one ACE for each of
>> > > > > > POSIXACE4_USER_OBJ, POSIXACE4_GROUP_OBJ and POSIXACE4_OTHER is
>> > > > > > required in posix_access_acl and posix_default_acl or
>> NFS4ERR_INVAL
>> > > > > > MUST be replied" in the next draft (I won't be updating the
>> draft
>> > > > > > on the datatracker for at least a week or two).
>> > > > >
>> > > > > Ok, this is good.
>> > > > >
>> > > > > I think it is not need to update draft every day.
>> > > > >
>> > > > > > rick
>> > > > > >
>> > > > > >
>> > > > > > > >
>> > > > > > > > rick
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > >
>> > > > > > > > > Regards,
>> > > > > > > > > Pali
>> > > > > > > > >
>> > > > > > >
>> > > > >
>> > > > > > _______________________________________________
>> > > > > > nfsv4 mailing list -- [e-mail]
>> > > > > > To unsubscribe send an email to [e-mail]
>> > > > >
>> > > > >
>> > >
>>
>> > _______________________________________________
>> > nfsv4 mailing list -- [e-mail]
>> > To unsubscribe send an email to [e-mail]
>>
>> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>