[nfsv4] Re: New Version Notification for draft-rmacklem-nfsv4-posix-acls-00.txt

[David Noveck <[e-mail]>]

On Wed, Jul 31, 2024 at 8:22 PM Rick Macklem <[e-mail]> wrote:

> I've put a first draft of a POSIX ACLs document in the datatracker.
> Please take a look when you have time.
>
> It currently specifies "per-file system" for the attribute that
> reports what ACL model is being stored in the server.
>

That scope does not address cases that are important to me.


> --> This could be changed to "per file".
>

Works for me,


> or "per server".
>

That would make things much worse.


>     Opinions?
>

Lots of them.  See *Document Review *below.


>
> Thanks, rick
>
>
> On Wed, Jul 31, 2024 at 5:16 PM <[e-mail]> wrote:
>
>> CAUTION: This email originated from outside of the University of Guelph.
>> Do not click links or open attachments unless you recognize the sender and
>> know the content is safe. If in doubt, forward suspicious emails to
>> [e-mail].
>>
>>
>> A new version of Internet-Draft draft-rmacklem-nfsv4-posix-acls-00.txt has
>> been successfully submitted by Rick Macklem and posted to the
>> IETF repository.
>>
>> Name:     draft-rmacklem-nfsv4-posix-acls
>> Revision: 00
>> Title:    POSIX Draft ACL support for Network File System Version 4,
>> Minor Version 2 (NFSv4.2)
>> Date:     2024-07-31
>> Group:    Individual Submission
>> Pages:    9
>> URL:
>> https://www.ietf.org/archive/id/draft-rmacklem-nfsv4-posix-acls-00.txt
>> Status:
>> https://datatracker.ietf.org/doc/draft-rmacklem-nfsv4-posix-acls/
>> HTML:
>> https://www.ietf.org/archive/id/draft-rmacklem-nfsv4-posix-acls-00.html
>> HTMLized:
>> https://datatracker.ietf.org/doc/html/draft-rmacklem-nfsv4-posix-acls
>>
>>
>> Abstract:
>>
>>    This document describes the addition of three new attributes that are
>>    used by servers to support POSIX ACLs.  The term POSIX ACLs refers to
>>    the ACL component of the Portable Operating System Interface (POSIX)
>>    1003.1e draft 17 [IEEE] document for which sponsorship was withdrawn
>>    in January 1998.  Although the draft POSIX standard that describes
>>    these ACLs was never ratified, several POSIX based operating systems,
>>    such as Linux, Solaris and FreeBSD include support for them.  The NFS
>>    Version 4 ACLs described in [RFC8881] henseforth referred to as NFSv4
>>    ACLs, use a different model and attempts to map between the ACLs of
>>    these two models has not been completely successful.  As such, this
>>    document proposes three new attributes that may optionally be used by
>>    an NFSv4.2 server to support ACLs that conform the afor mentioned
>>    POSIX 1003.1e draft 17.
>>
>>
>>
>> The IETF Secretariat
>>
>>
>> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]


*Document Review *

*Overall Evaluation*
This is a good start on addressing an important security issue.  I feel
the use of the new attribute currently called "server_acl_model" is an
excellent choice.

There are a number of major issues that need to be addressed before
document adoption.  Most of these are discussed in *General Comments*.   For
the few within *Per-section Comments*, that status is specially noted below
if necessary.

*General Comments *

*Needed Changes in Direction*

The changes I am referring to all derive, in some  way, from the need to
provide better support for the case in which support for both NFSv4 ACLs
and POSIX ACLs are available.

While this is partially addressed by shifting server_acl_model to be a
per-fs-object attribute as discussed in *7.1 Attribute NN: server_acl_model*,
this makes necessary other  changes specifying how other actions affect the
value of this attribute.  Some of those are discussed in *Per-section
Comments* while others are mentioned in *Things That are Missing.*

I'd appreciate your response to comments in this section as soon as
possible and in advance of a next draft.   Right now, I am fairly far along
in draftng acl-05 and beliee that if we can come to agreement on these
matters, I can shift and produce a different acl-05 that fits together with
your v4.2 extensons.

*Things That are Missing.*

I think the following thing needs to be done:

   - I think you have to look at the issues related to XDR extraction and
   how the updated v4.2 XDR is to be arrived a .  I think you should look at
   how this is addressed in other v4.2 extensions.
   - I think you need a section describing the differences between the two
   ACL models.   This is to address the part of the audience who will be
   adding your extensions to an implementation supporting the NFSv4 ACL
   model.  The current document is oriented to those that do not have an such
   an existing.   While this is a large part of the audience, it is not all of
   it.


*Per-section Comments*
*Abstract*
Suggest replacing "POSIX based" by "POSIX-based".

Suggest replacing "henseforth" by "henceforth".

Suggest replacing "has not been completely successful" by "have not been
completely successful"

Suggest replacing "as such" by "In order to adequately  support these ACLs,"

Suggest replacing "the afor mentioned" by "to the aforementioned".

*1, Introduction*

The current first sentence assumes that there needs to be mapping without
giving the reader any basis for that feeling, which seems to come from out
of nowhere.  Suggest the following replacement for the
existing first sentence:

Because of the semantic differences between the two ACL models involved,
attempts have been made to map between these two sorts of ACLs.
However, implementation
experience with mapping between NFSv4 and POSIX ACLs has not been
completely successful.

The last sentence of the paragraph, as currently written may provoke
unhelpful may-vs-MAY discusssion.  Suggest starting "A server has the
option of choosing".

In the second paragraph suggest replacing "as such" by "In order to provide
better support for POSIX ACLs.".  In the second sentence of the paragraph,
suggest replacing "these attributes" by "any of these attributes".

The current third paragraph leaves me mystified.   Is the following more
like what you want?

The semantics relevant  to the implementation of  POSIX ACLs by the
server, are  described in [Grünbacher].  In addition, issues related
to the over-the-wire format of POSIX ACLs and the interactions among
the various new attributes are dealt with in this document.

 *  4.  XDR definitions for new attributes*

If the attribute returning aclmodel4 is, as I believe it should be, of
per-fIle scope, you are missing an enum value to cover the case in which a
file has a neither an NFSv4 ACL nor a POSIX ACL.   ACL_MODEL_NEITHER?

*5.  POSIX ACL Considerations*

As far as I can see from Andreas' paper, the first clause of the first
sentence of the third paragraph is not true.

Regarding the second sentence of that paragraph:

   - I can't see how "Therefore" is justfied.
   - I don't think it is possible to use set_mode_masked in this case.

Regarding the third sentence of that paragraph, I think it is relevant that
RFC8811 does address both these being set for the NFSv4 ACL model,
casting doubt on the implication that the choices you made were somehow
forced on you.  I don't think your choices are bad ones,  but you cannot
claim here that you had no other choice.

In the fourth paragraph, the reference to "POSIX system call" is
inapproprIate.  I think you need to refer to the NFSv4 operation.

*7.1.  Attribute NN: server_acl_model*

I'd like to suggest a change in this name to something like "acl_model".

In my view, this is not particularly server-specific although it is
determined on the server based on server attributes supported and
client action.

In any case, we don't do this for other attributes.  For example, fh
is chosen by the server but is not called "server_fh.

While I have previous said that this should be per-object, you need to
make additional changes with that for the whole thing to make sense,
Suggest the following:

This attribute is a read-only attribute that indicates the ACL model
of the ACL used to regulate access authorization for the current file
object .  Distinct values indicate whether:


   - This is controlled by a draft POSIX ACL.
   - This is controlled by ALLOW and DENY ACEs as part of an ACL set
by the acl or dacl attributes or by ACL inheritance within NFSv4 ACLs.
Note that this is not affected by the presence of a sacl attribute
value or of AUDIT and ALARM ACEs within an acl atribute value.
   - This is not controlled by an acl but only by the mode attribute.

It is a per-file system object attribute.

*7.1.1 Rationale*

The current first sentence ignores the problem that doing this as suggested
cost one extra client-sever round ttrip.

The current second sentence is kind of misleading in that the real
reason your proposal does no mapping is that it specifes you do no
mapping

It appears that you have decided that mapping is  impossible to do correctly.

In any case, I would suggest deleting this section and adding the
following to 7.1:

This attribute provides a convenient low-overhead means of determining
the acl model in use without the effort of obtaining the acl itself.

Often, clients are not prepared to interpret acls (of eiter model)
that are fetched and rely on ACCESS to determine authorization
decisions.

*7.2.1.  Rationale*

I suggest replacing this by something like the following either in
this section or the preceding section:

Without this attribute the client would have  no way to set or
interrogate the default acl, as provided for by the structure of the
POSIX ACL model.  While it is theoretically possible to somehow map
NFSv4 ACLs to POSIX ACLs, important semantic differences make this a
poor choice,  In addition, the fundamentally different approaches
taken to ACL inheritance compounds the associated difficulties, and no
provision to do such mapping has been provided in this specification.

*7.3.  Attribute NN: posix_access_acl*

I suggest adding a paragraph like the following:

When this attribute is set to to a contain an ACL of length greater
than zero, it has the effect of changing the value of server_acl_model
to ACL_MODEL_POSIX.

*7.3.1.  Rationale*

I suggest replacing this by something like the following either in
this section or the preceding section:

Without this attribute the client would have  no way to set or
interrogate the access ACL, as provided for by the structure of the
POSIX ACL model.  While it is theoretically possible to somehow map
NFSv4 ACLs to POSIX ACLs, important semantic differences make this a
poor choice,

*9.  References*

We have an unfortunate difference of opinion between David B. and
Chris.  So far, the only conclusion I can draw is that at least one of
them is wrong.

It is true that there is a lot of material that characterizes the
distinction between normative and informative references between as
David says they are to be distinguished.

Nevertheless the terms "Normative" and "Informative" are used and it
is hard to ignore  Chris's point that these also should have the role.

I have considered the possibility of three reference subdivisions for


   - "normative references"
   - "essential non-prescriptive references"
   - "other informative references"

but  could see how you might not want to do that.

Nevertheless, regardless of the IETF's possible attachment to this
Procrustean bucket, it needs to be made clear that the knowledge in
thisdocument

cannot be dispensed with.  While you can kinda sorta understand this
document without it, if an implementer did not get this knowledge
somehow, the

resulting implementation is not of any value.

In any case, although you might not want to say this explcitly,
reading this document is more important than reading most of the
current normative references.

I know this is heretical, but, so far I know, the IETF has never
burned anyone at the stake.