[nfsv4] WGLC review of draft-ietf-nfsv4-umask-02

[David Noveck <[e-mail]>]

*General Comments*

*Overall Evaluation*

>From a technical point of view, this document has no issues that would
prevent it from going forward fairly quickly.  Any technical issues are
quite minor and should be easy to address.

Unfortunately, there are two significant issues regarding the presentation
of this extension that need to be addressed:


   - Right now this document makes no reference to the NFSv4 extension
   framework.  It proposes something which, while sensible, is wholly at
   variance with the text of RFC5661 and with the entire tenor of previous
   standards-track treatment NFSv4 protocol extension.  More on this issue
   below within *Presentation as Extension.*
   - With regard to one critical reference it runs afoul of the
   consequences of the working group's ambivalence about whether NFSv4.0 and
   NFSv4.1 are best treated as two distinct protocols or as one. For the
   specific reference, see *2. Problem Statement* which also discusses how
   this issue might be addressed solely within the document under review.  For
   other possibilities, see the discussion within *Potential Need for
   Clarification Regarding post-NFSv4.0 Protocols*.

*Presentation as Extension*

The authors of the current document have chosen to avoid explicit reference
to NFSv4 extension issues.  They seem to believe that because what they are
proposing is sensible, addresses serious  protocol deficiencies, and can be
implemented interoperably with existing clients and servers, the IESG is
going to be OK with it.  I think this is too optimistic but feel that if
the authors want to take this approach, I shouldn't stand in their way.  It
may be that others in the working group will object to the document going
forward in this form..

In some of the per-section comments I have offered suggestions on how the
document could be modified to be compatible with the protocol extension
model defined in draft-ietf-nfsv4-versioning.  Nevertheless, these are
suggestions and the authors are free to decide on their own path in this
respect.

*Potential Need for Clarification Regarding post-NFSv4.0 Protocols*

I feel that the working group has to eventually deal with some of
the consequences of the fact that many necessary corrections/clarifications
to NFSv4 were only applied/specified in RFC7530, and that an RFC5661bis
could not be done in any reasonable time frame.  In the context of the
current document these only concern  the discussion of mode/acl interaction
 but eventually the working group is going to have to address the general
issue.

In *2. Problem Statement*, I try to see what we can do to proceed on the
current document without addressing the larger issue.  Whether it is
appropriate to present that work to the IESG without addressing the larger
issue is a matter for the authors and ultimately for the working group to
decide.

To summarize  the longer-term issue, there are a number of areas where we
have a clarification/correction that is appropriate and relevant to NFSv4.1
that needed to be made and we have made the appropriate changes only
with regard to NFSv4.0, in RFC7530.

Only the issue concerning the interactions of the various security-related
attributes has relevance to this document.  I'll address the others that I
know about later.

Since an rfc5661bis does not seem feasible in any reasonable period time,
the best approach would be to produce a document updating RFC5661.  It
could be based on chapter 6 of RFC7530, with appropriate changes to reflect
the addition of the sacl, dacl, and set_mode_masked attributes.  The
document could state that it supersedes chapter 6 of RFC5661.

*Comments by Section  *

*Header*

Note that this document, while it proposes an extension to NFSv4.2 and
additions to the XDR for NFSv4.2, is not marked as updating RFCs 7862 and
7863.  The IESG is almost sure to ask about this.  You need to be prepared
for this question.  My preference would be to build upon the versioning
document but that's your choice.  Whatever you do, the question will be
asked, perhaps repeatedly.

*Title*

I suggest capitalizing the words "inheritable", "override", and "umask".

*Abstract*

In the first sentence, the phrase "In some environments" seems too weak,
given that this issue arises within Unix environments in general. Suggest
"In many important environments" as a replacement.

Suggest the following replacement for the second sentence:

This issue can be addressed by transmitting the umask and create mode as
separate pieces of data. Doing so via the protocol extension discussed in
this document would allow servers to make intelligent decisions about the
mode to be assigned to the new file.

*2. Problem Statement*

In the first sentence of the second paragraph, suggest replacing "currently
does not" by "does not currently".

In the third sentence of the second paragraph, suggest replacing "on the
safe side" by "on the the side of caution".

In the fourth sentence of the second paragraph, suggest replacing "Thus the
mode" by "As a result, the mode attrbute".

The reference in the first sentence of the third paragraph may create
difficulties since this document is directed to an extension within NFSv4.2
and minor versions beyond NFSv4.0 essentially constitute a new protocol,
making the RECOMMENDATIONS within RFC7530, officially speaking,
non-normative. Unfortunately, when I looked for a parallel text in RFC5661,
I didn't find one and am presuming that none exists.

In my suggestions below, I am going to attempt to dance around this issue
as well as I can.  For other possibilities, see *Potential Need for
Clarification Regarding post-NFSv4.0 Protocols*.

Suggest the following replacement for the third paragraph:

When applying the mode, section 6.4.1.1 of [RFC7530] RECOMMENDS that
servers restrict permissions granted to any user or group named in the
ACL so as to be no more extensive than the permissions granted by the
MODE4_RGRP, MODE4_WGRP, and MODE4_XGRP bits.  While this
RECOMMENDATION is not, strictly speaking, applicable to post-NFSv4.0
protocols, NFSv4.1 server implementations have followed it.
Apparently this is because sthe clarification within [RFC7530] applies
to NFSv4.1 as well, since it also contains both mode and acl
attributes which need to be coordinated. Also, servers aiming to
provide clients with Unix-like chmod behavior may have been motivated
by similar requirements in [SUSv4].  (See the discussion of additional
and alternate access control mechanisms in Section 4.4 of that
document, entitled "File Permissions".)

Suggest replacing the last sentence of the fourth paragraph by the
following:

As a result, inherited ACEs describing the permissions to be granted
to named users and groups are often ignored.

Suggest replacing the fifth and sixth paragraphs by the following:

This issue leads to file permissions which are more restrictive than
they should be in common cases.  It appears that permission
inheritance, included within NFSv4 as part ACL model, cannot be used
effectively in many important environments and needs to be corrected.

To provide an appropriate correction, the server needs to apply the
umask since depending on the client to do so has proved unworkable.

*Protocol Extension Considerations [Suggested new section]*

This document presents an extension to the NFSv4 protocol as described in
[nfsv4-versioning].  It describes a new OPTIONAL feature and the extension
is such as allow it to be implemented so that existing NFSv4.2 clients and
servers, implemented without knowledge of this extensions will operate with
severs and clients that are aware of this extension, including clients that
use and do not use this extension and servers that support and do not
support this extension.

Note that [RFC7862] does not define NFSv4.2 as non-extensible, so that it
is considered by [nfsv4-versioning] to be an extensible minor version.  As
a result, upon publication of this document as a Proposed Standard, the
extension described herein will effectively be part of NFSv4.2, even though
this document does not update [RFC7862] or [RFC7863].

*3. mode_umask Attribute*

Proposing the following as a new first paragraph for this section:

In order to allow the server to apply the umask, the new attribute
mode_umask is used when creating objects.  This allows the server to
apply the umask only when there are no inheritable permissions.

I'm not sure what you need to do with regard to XDR although I am fairly
sure that what is there is not adequate. One possible approach is to follow
the model used in draft-ietf-nfsv4-xattrs-03 but I can see why you might
not want to do all of that, given that your XDR additions are so limited.

Nevertheless, I think you need to do the following:

   - Address the issue of code components licensing somehow.
   - Describe how one can produce an XDR file for the extended protocol.
   - Insert "<CODE BEGINS>" and <CODE ENDS> appropriately.
   - Include XDR for the attribute code and typedef.

In the first sentence of the current first paragraph, the phrase "open
mode" is potentially confusing.  I believe the reference is to the
optional third parameter of the POSIX open call.  However, others
might interpret "open mode" as referring to whether a file is opened
for read or for write.  Suggest "mode value specified at open time" as
a possible replacement.

With regard to the current second paragraph, the following issues need
to be addressed:

   - I don't see any reason why this is a "SHOULD" (rather than a
"MUST").  What else might a server do and how would interoperability
be affected?
   - Besides indicating that this be rejected, there should be an
indication of how.  I suggest handling this like reading a write-only
attribute and returning NFS4ERR_INVAL.

In the last bulleted paragraph (i.e. the current seventh paragraph),
suggest replacing "the mode to use for creating the object" by "the
mode to be assigned to the object being created"

*4.  Security Considerations*

In the second sentence of the first paragraph, suggest replacing "the
recommendation that they SHOULD" by "the RECOMMENDATION that they".

Suggest replacing the second paragraph by the following:

The practice of ignoring the umask when there are inheritable
permissions in the form of a "POSIX" default ACL is of long standing
and has not given rise to security issues. The "POSIX" default ACL
mechanism and the mechanism for permission inheiritance in NFSv4 are
equivalent from a security perspective.

*IANA Considerations [Missing Section]*

Although it really doesn't make sense, you need something here.  Suggest
adding the section with the following text:

This document does not require any actions by IANA.

*5.  Normative References*

Regarding POSIX-1003.1e, I don't see how a *withdrawn *POSIX draft can
be consiered normative.  I think this should go in an "Informative
References" section.

I note there is no explicit reference in the text to LEGAL but expect
that once the XDR issues are resolved there will have to be,

Regarding RFCs 5661 and 5662, I note that neither is actually
referenced in this document.  The details will depend on how you want
to address the extension issue but I offer the following suggestions
regaring version definition documents for post-v4.0 minor versions:


   - Given that this document is to apply to the to the NFSv4.1 minor
version group, it makes sense  to include this reference to RFC5661,
even without an explicit reference in the text.  I'd leave this in,
unless it becomes an issue with idnits.
   - Even though this is an extension to NFSv4.2, I don't see a need
to explicitly reference RFC7862, since none of the NFSV4.2 extensions
has any relevance to this issue.
   - Although this will depend on what you do with the XDR stuff (see
*3. mode_umask Attribute*), I don;t see a need to reference RFC5662.
You may wind up referencing RC7863.

Although this will depend on how you decide to address the extension
issue, I believe a normative reference to draft-ietf-nfsv4-versioning
is necessary.