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

Rick Macklem <rick.macklem@gmail.com> Mon, 12 August 2024 00:10 UTC

Return-Path: <rick.macklem@gmail.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5C409C14F5FB for <nfsv4@ietfa.amsl.com>; Sun, 11 Aug 2024 17:10:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.108
X-Spam-Level:
X-Spam-Status: No, score=-2.108 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xw2avQ5Ko53S for <nfsv4@ietfa.amsl.com>; Sun, 11 Aug 2024 17:09:58 -0700 (PDT)
Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (P-256) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6343BC14E513 for <nfsv4@ietf.org>; Sun, 11 Aug 2024 17:09:58 -0700 (PDT)
Received: by mail-pf1-x429.google.com with SMTP id d2e1a72fcca58-70d199fb3dfso3074879b3a.3 for <nfsv4@ietf.org>; Sun, 11 Aug 2024 17:09:58 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723421398; x=1724026198; darn=ietf.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=sqtyP16knIe/LnRSVJXNOfcdu7GhrHfSk1gPDzQDXSA=; b=VEvEZn2r+J23nzDbbID8BKSucvN2oLcRcDw3MwBuYaZ3Jm65uo8c08EMulnrYhynXp 6HKd7hehxSxXDvIUX6RUEltWidxbL7pTAGWIWG0NzlpkVfwqrNDCV9nO5pmNwRK9gyno 0AZRcRBHDgWXi0KIplXHOZrNWu5AUtEj7Q+i5o70Ysek5Wq0Acbhq49nLiSWCaXVpYlV Xdj+lD4OeQI2IJ1H4kVrUcC7Y9EfBTqaJ6Tm5ahY4KfW+ZRtdsFgMKWtziGlevGa5xSG NRKe+xnXxjjC8ay3FxPq691z0OnwFPYLbXSis+g6Li5DElPacVJcjU7hr4ibOzfwAcWw kFpA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723421398; x=1724026198; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=sqtyP16knIe/LnRSVJXNOfcdu7GhrHfSk1gPDzQDXSA=; b=LhO1aLnnyfxAq9Z/21kj1bZgA5lwK1AgprTYZ2XIoo5MyEe5UWc+eguDk9llYVOUze +Zo7/384PHcS33LyOVsVSdRfRSWgfCQcGSg1bat+D21GiYsZwcf0VeZO8Y80nG9DB3nM uh1nhPgPf+ri4NIcqQ2QFfWbcnVquV+ds+XedXejJF0OYysMhEt8tYlQsXFeq3qkRUmg zFEYjSHjcBo14bkHQYvJgMfTshvH9k122lu5U+BooDMFPm0qS3aQrNFnmI/nPl2z3Eox sWWcexLnTLNnNs50BaFT8dZ+66K/FPm3AkMlvD4BNJsQRn1tOzC31IQirsA7H5t6fesq YXag==
X-Gm-Message-State: AOJu0YzYMBqBQDOmRCtO92hxkXWaUsQYPaXGglTIQf7HOIb7B0nU3GA/ wJFkd6ggDjKWGa54S+dSi3dsPICrgeg3ly93xHVm224KuWHo8WYveTsSR1GO+OBxjqvaMDA+k5h 9p2okT5JbVdHYdSQBvqsGazwdzsKV
X-Google-Smtp-Source: AGHT+IF8OWfYbahk16dI/tM82q/alARnrWxLu8btwJDFmz1J4ygbQp1iHtqfNeEz76aopvKf0+sSJrDMg/pNzXuWV/k=
X-Received: by 2002:a05:6a20:6f8c:b0:1c3:b16d:9ebf with SMTP id adf61e73a8af0-1c89fe9749bmr11591810637.15.1723421397403; Sun, 11 Aug 2024 17:09:57 -0700 (PDT)
MIME-Version: 1.0
References: <20240810003154.xw4gnos6fqr2yw54@pali> <CAM5tNy4fo=8wzbM8qbRwtiPycSLTKPd7sVaBQZk72zZDJyq93Q@mail.gmail.com> <20240810223629.dukpymniqn3uy2s6@pali> <CAM5tNy6+XHApuCu=N6RKLTgWToU7JQpp4QTxV9T2EE_dJQcGsQ@mail.gmail.com> <20240811101105.iuiyk7ygelhs7crv@pali>
In-Reply-To: <20240811101105.iuiyk7ygelhs7crv@pali>
From: Rick Macklem <rick.macklem@gmail.com>
Date: Sun, 11 Aug 2024 17:09:44 -0700
Message-ID: <CAM5tNy412=y44uFNLet0_Q6gZRKu4bOKF_Q0dGfO0v7sqf-U1A@mail.gmail.com>
To: Pali Rohár <pali-ietf-nfsv4@ietf.pali.im>
Content-Type: multipart/alternative; boundary="00000000000030d052061f7150ea"
Message-ID-Hash: Q6VYFKNNO64AYNXDOWWXLQSN33RSS4GF
X-Message-ID-Hash: Q6VYFKNNO64AYNXDOWWXLQSN33RSS4GF
X-MailFrom: rick.macklem@gmail.com
X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-nfsv4.ietf.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header
CC: nfsv4@ietf.org
X-Mailman-Version: 3.3.9rc4
Precedence: list
Subject: [nfsv4] Re: Comments for draft-rmacklem-nfsv4-posix-acls-04
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/e_R9ECBwHSrUr9HXc42KJxifZ80>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nfsv4>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Owner: <mailto:nfsv4-owner@ietf.org>
List-Post: <mailto:nfsv4@ietf.org>
List-Subscribe: <mailto:nfsv4-join@ietf.org>
List-Unsubscribe: <mailto:nfsv4-leave@ietf.org>

On Sun, Aug 11, 2024 at 3:11 AM Pali Rohár <pali-ietf-nfsv4@ietf.pali.im>
wrote:

> On Saturday 10 August 2024 16:54:39 Rick Macklem wrote:
> > On Sat, Aug 10, 2024 at 3:36 PM Pali Rohár <pali-ietf-nfsv4@ietf.pali.im
> >
> > 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 <
> pali-ietf-nfsv4@ietf.pali.im>
> > > > 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
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.)

Now, remount using NFSv4.1/4.2
- setfacl fails with an error.
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?

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

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.



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


> 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".
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.
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 -- nfsv4@ietf.org
> > To unsubscribe send an email to nfsv4-leave@ietf.org
>
>