Re: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-minorversion2-39

"Adamson, Andy" <William.Adamson@netapp.com> Tue, 05 January 2016 17:54 UTC

Return-Path: <William.Adamson@netapp.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 556C51ACF5B; Tue, 5 Jan 2016 09:54:32 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -5.711
X-Spam-Level:
X-Spam-Status: No, score=-5.711 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, J_CHICKENPOX_12=0.6, J_CHICKENPOX_29=0.6, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mHAowlV12mpo; Tue, 5 Jan 2016 09:54:24 -0800 (PST)
Received: from mx144.netapp.com (mx144.netapp.com [216.240.21.25]) (using TLSv1.2 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7BC011ACEDC; Tue, 5 Jan 2016 09:54:24 -0800 (PST)
X-IronPort-AV: E=Sophos;i="5.20,525,1444719600"; d="scan'208";a="90296053"
Received: from hioexcmbx01-prd.hq.netapp.com ([10.122.105.34]) by mx144-out.netapp.com with ESMTP; 05 Jan 2016 09:49:23 -0800
Received: from HIOEXCMBX03-PRD.hq.netapp.com (10.122.105.36) by hioexcmbx01-prd.hq.netapp.com (10.122.105.34) with Microsoft SMTP Server (TLS) id 15.0.1130.7; Tue, 5 Jan 2016 09:49:23 -0800
Received: from HIOEXCMBX03-PRD.hq.netapp.com ([::1]) by hioexcmbx03-prd.hq.netapp.com ([fe80::b591:bd48:b520:c1b7%21]) with mapi id 15.00.1130.005; Tue, 5 Jan 2016 09:49:23 -0800
From: "Adamson, Andy" <William.Adamson@netapp.com>
To: Elwyn Davies <elwynd@dial.pipex.com>
Thread-Topic: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-minorversion2-39
Thread-Index: AQHROhW95CLCz+Up80SePlrWjOoEj57XrSsAgAAQFYCAEDrygIAEN/oAgAGflICAAAUCAA==
Date: Tue, 05 Jan 2016 17:49:22 +0000
Message-ID: <82887CDF-B009-4B97-8C87-F0AB012FDD63@netapp.com>
References: <566DC57E.5030307@dial.pipex.com> <5E3C518C-397B-435A-AF23-71AF6B69B71E@primarydata.com> <73522861-AA88-44B2-AC21-0B2B1B7C10E5@netapp.com> <797D56D1-84EE-4754-A952-80C3EA894C1A@netapp.com> <5687175F.5090900@dial.pipex.com> <3AC88D8E-4069-44C5-8679-D2E56B71FF25@netapp.com> <568BFDEF.6080301@dial.pipex.com>
In-Reply-To: <568BFDEF.6080301@dial.pipex.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-mailer: Apple Mail (2.2098)
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.122.56.79]
Content-Type: text/plain; charset="utf-8"
Content-ID: <2BEC95B1C15C2C4B80B7BEF79891A90E@hq.netapp.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/YoYAh3OmBxjeXGLgCcXYYs5eUeE>
Cc: "draft-ietf-nfsv4-minorversion2.all@ietf.org" <draft-ietf-nfsv4-minorversion2.all@ietf.org>, General area reviewing team <gen-art@ietf.org>, "Adamson, Andy" <William.Adamson@netapp.com>, Tom Haynes <thomas.haynes@primarydata.com>
Subject: Re: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-minorversion2-39
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 05 Jan 2016 17:54:32 -0000

> On Jan 5, 2016, at 12:31 PM, Elwyn Davies <elwynd@dial.pipex.com> wrote:
> Hi.
> 
> One oops: The comment for rpcsec-gssv3 should apply to s2.7.1.4 and not s2.3.1.4.

No problem - I noticed that. 
> 
> On 04/01/2016 16:44, Adamson, Andy wrote:
>>> On Jan 1, 2016, at 7:18 PM, Elwyn Davies <elwynd@dial.pipex.com> wrote:
>>> 
>>> Hi.
>>> 
>>> Happy New Year!
>> Happy New Year :)
>>> I trust you have had a good break.  Now back to the action...
>>> 
>>> Regarding the definition of 'structured privilege'... I ascertained that 'structured privilege' entered the story at v05 of rpsec_gssv3 so I trawled through the nfsv4 mail archive around the time of -05's genesis to see how this came about.
>>> 
>>> At the end of a fairly lengthy thread on "Soliciting next steps for RPCSEC_GSSv3", I was amused to see that message
>>> https://mailarchive.ietf.org/arch/msg/nfsv4/zpEIx3aC6bwNWQSQ1rjfxqli8nE
>>> ends with Andy asking exactly the same question as I did :-)))
>>>> Where can I find the definition of "structured privilege"? A quick
>>>> google search just turns up a SAP HANA reference ;)
>>> I had actually turned up the same SAP HANA ref.. Having read all this I think I understand that I was trying to read more into the the term than is intended. My understanding is that it just implies that there is an agreed data structure that is associated with a privilege assertion defined by whatever application intends to use the privilege and known to both client and server that support the relevant application.
>>> 
>>> Assuming this is correct I suggest redrafting s 1.1 (bullet 2) and s2.3.1.4 of rpcsec-gssv3-05 as follows:
>>> For s1.1, bullet 2:
>>> OLD:
>>>   o  Application-specific structured privileges.  For an example see
>>>      server-side copy [NFSv4.2].
>>> NEW:
>>>   o  Application-specific structured privileges.  These allow a RPC
>>>      application client to pass structured information to the
>>>      corresponding application code in a server to control the
>>>      applicability of the privilege and/or the conditions in which
>>>      the privilege may be exercised.  For an example see
>>>      server-side copy [NFSv4.2].
>>> END
>>> 
>>> For s2.3.1.4, para after code:
>>> 
>>> OLD:
>>>   A structured privilege is an RPC application defined privilege.
>>>   RPCSEC_GSSv3 clients MAY assert a structured privilege by binding the
>>>   privilege to the RPCSEC_GSSv3 context handle.  This is done by
>>>   including an assertion of type rgss3_privs in the RPCSEC_GSS_CREATE
>>>   rgss3_create_args rca_assertions call data.  Encoding, server
>>>   verification and any policies for structured privileges are described
>>>   by the RPC application definition.
>>> NEW:
>>>   A structured privilege is a capability defined by a specific RPC
>>>   application.  To support the assertion of this privilege, by a client
>>>   using the application, in a server that also supports the application,
>>>   the application may define a private data structure that is understood
>>>   by clients and servers implementing the RPC application.
>>> 
>>>   RPCSEC_GSSv3 clients MAY assert a structured privilege by binding the
>>>   privilege to the RPCSEC_GSSv3 context handle.  This is done by
>>>   including an assertion of type rgss3_privs in the RPCSEC_GSS_CREATE
>>>   rgss3_create_args rca_assertions call data.
>>> 
>>>   The privilege is identified by the description string that is used by
>>>   RPCSEC_GSSv3 to identify the privilege and communicate the private data
>>>   between the relevant RPC application-specific code without needing to
>>>   be aware of the details of the structure used.  Thus, as far as
>>>   RPCSEC_GSSv3 is concerned, the defined structure is passed between client
>>>   and server as opaque data  encoded in the rpc_gss3_privs rp_privilege field.
>>> 
>>>   Encoding, server verification and any server policies for structured
>>>   privileges are described by the RPC application definition.  The
>>>   rp_name field of rpc_gss3_privs carries the description string used to
>>>   identify and list the privilege.
>>> END
>> 
>> That is quite complete and very clear. Thanks!
> :-) .. and it is fpr s2.7.1.4 as mentioned above.

>>> Assuming the RPCSEC_GSS_CREATE call can carry multiple label specifications and structured privilege assertions, I am not clear how a client would discover which label or which assertion caused an error, if the server rejects the creation.
>> You are partially correct.
>> 
>> For labels: a set of labels is sent to the server. The labels that are accepted by the server MUST be enumerated in the rcr_assertions field of the rgss3_create_res RPCSEC_GSS_CREATE reply. So those that fail will not be enumerated.
>> The RPCSEC_GSS_LABEL_PROBLEM AUTH_ERROR is used only if the server does not support labeling, or that does not support labeling under the LSF. The client can get a list of supported LSF’s with the RPCSEC_GSS_LIST operation.
>> So we do know which label(s) were not accepted.
>> 
>> For structured privileges, the client needs to assert the RPC application defined privilege - a partial success does not seem useful to me. It is true that a rejection of a structured privilege due to verification failure returns the RPCSEC_GSS_PRIVILEGE_PROBLEM AUTH_ERROR, so if there are multiple structured privileges in one RPCSEC_GSS_CREATE payload, you will not know which one failed. I don’t think this is a problem as it should be an all or nothing assertion.
>> 
> Thanks for the explanation.  Can I suggest that this could be made more explicit by slightly modifying the text of the next to last para of s2.7.1.4:
> OLD;
>   If a server receives a structured privilege assertion that it fails
>   to verify according to the requirements of the RPC application
>   defined behavior, the assertion is rejected with a reply status of
>   MSG_DENIED, a reject_status of AUTH_ERROR, and an auth_stat of
>   RPCSEC_GSS_PRIVILEGE_PROBLEM.
> NEW:
>   It is assumed that a client asserting more than one privilege to
>   be bound to a context handle would require all the privilege
>   assertions to succeed.
> 
>   If a server receives an RPCSEC_GSS_CREATE request containing one
>   or more  structured privilege assertions, any of which it fails
>   to verify according to the requirements of the RPC application
>   defined behavior, the request is rejected with a reply status of
>   MSG_DENIED, a reject_status of AUTH_ERROR, and an auth_stat of
>   RPCSEC_GSS_PRIVILEGE_PROBLEM.
> END
>>> 
>>> =======
>>> 
>>> At the other end of this in minorversion2-39,  some minor updates to s4.10.1.1 should be  made to tie in with this definition.  In particular minorversion3 needs to specify how the data structure is encoded - RPCSEC_GSSv3 doesn't know or care since it is treated at as opaque data at the GSS level.  Clearly it is intended that XDR encoding is used but this should be stated explicitly.   I suggest adding a new para after the existing para 3 and making it clear that the string at the beginning of each section is passed in the rp_name field (also alter the "We define" which is not the correct style) :
>>> OLD (para 4):
>>> 
>>>   We define three RPCSEC_GSSv3 structured privilege assertions that
>>>   work in tandem to authorize the copy:
>>> 
>>> NEW:
>>>   For each structured privilege assertion defined by a RPC application
>>>   RPCSEC_GSSv3 requires the application to define a name string and a
>>>   data structure that will be encoded and passed between client and server
>>>   as opaque data.  For NFSv4 the data structures specified below MUST
>>>   be serialized using XDR.
>>> 
>>>   Three RPCSEC_GSSv3 structured privilege assertions that
>>>   work together to authorize the copy are defined here.  For each of
>>>   the assertions the description starts with the name string passed in
>>>   the rp_name field of the rgss3_privs structure defined in
>>>   Section 2.7.1.4 of [rpcsec_gssv3] and specifies the XDR encoding of
>>>   the associated structured data passed via the rp_privilege field of
>>>   the structure.
>>> END

I like it. 


>>> 
>>> A question that occurred to me:  MUST a server support all three assertion types if supports any?
>> Yes. The Security Considerations comments on this issue:
>> 
>> 4.10.  Security Considerations
>> 
>> ….
>> 
>>  If the server-to-
>>    server copy protocol is ONC RPC based, the servers are also REQUIRED
>>    to implement [rpcsec_gssv3] including the RPCSEC_GSSv3 copy_to_auth,
>>    copy_from_auth, and copy_confirm_auth structured privileges.
>> 
>> Is the above clear enough?
> Sorry - I missed that comment.  However, it would appear to be logically possible to have source only and destination only servers.  I think a source server would only need to implement copy_from_auth and copy_confirm_auth whereas the destination server would only need to implement copy_to_auth.  Whether this is something that an implementer would wish to allow is beyond the scope of my knowledge but I could see that it might be something that might be interesting if the roles of servers need to be constrained.  Thus it seems to me that requiring that all three are implemented is not essential.  Alternatively, there could be an option to always implement, but set a policy that always denies privileges not appropriate to the defined role of the server.

Well, I think it wiser to have it all or none - e.g. to always implement, and let the policy define privileges appropriate to the role of the server. Furthermore, once one structured privilege is coded, the others are not that big a deal. So I think the existing text is good. 


>> 
>> Thanks again for the review
> :-)
> Will you be updating again before the IESG review on Thursday?  I have to write a Telechat review...
> 
> Cheers,
> Elwyn
>> 
>> —>Andy
>> 
>> 
>>> I spotted a nit in this area that I missed on the previous pass:
>>> s4.10.1.1, para 3: s/This features allow/This feature allows/
>>> 
>>> I also missed a number of instances (17, I think) of the "we <do something>" construction familiar from scientific papers.
>>> (There was one in the paragraph changed above).  This is not appropriate phraseology for an RFC and needs to be
>>> changed to avoid the "we", e..g.,
>>> s1.4.5: s/We introduce WRITE_SAME (see Section 15.12)/The WRITE_SAME operation (see Section 15.12) is introduced/
>>> 
>>> Regards,
>>> Elwyn
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> On 22/12/2015 16:27, Adamson, Andy wrote:
>>>>> On Dec 22, 2015, at 10:30 AM, Adamson, Andy<William.Adamson@netapp.com>  wrote:
>>>>> 
>>>>>> On Dec 18, 2015, at 11:28 PM, Tom Haynes<thomas.haynes@primarydata.com>  wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Dec 13, 2015, at 11:22 AM, Elwyn Davies<elwynd@dial.pipex.com>  wrote:
>>>>>>> 
>>>>>>> I am the assigned Gen-ART reviewer for this draft. The General Area
>>>>>>> Review Team (Gen-ART) reviews all IETF documents being processed
>>>>>>> by the IESG for the IETF Chair.  Please treat these comments just
>>>>>>> like any other last call comments.
>>>>>>> 
>>>>>>> For more information, please see the FAQ at
>>>>>>> 
>>>>>>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>>>>>>> 
>>>>>>> Document: draft-ietf-nfsv4-minorversion2-39.txt
>>>>>>> Reviewer: Elwyn Davies
>>>>>>> Review Date: 2015-12-13
>>>>>>> IETF LC End Date: 2015-12-09
>>>>>>> IESG Telechat date: (if known) -
>>>>>>> 
>>>>>>> Summary: Almost ready.  There are a fair number of minor nits and editorial suggestions.  The couple of 'minor issues' are predominantly questions that came into my mind that might be interesting to a reader/implementer but are probably unlikely to affect the performance of the protocol.  However, the comments on s11.2, s12.1, s13 and s15.2.3 appear to be actual omissions or errors.
>>>>>>> 
>>>>>>> Major issues:
>>>>>>> None.
>>>>>>> 
>>>>>> Hi Elwyn,
>>>>>> 
>>>>>> Thanks as always for making me think!
>>>>>> 
>>>>>> Replies are inline, I have some questions for you and Andy.
>>>>>> 
>>>>>> Hi Andy,
>>>>>> 
>>>>>> There is a question about Inter-Server Copy Security below!
>>>>>> 
>>>>>> 
>>>>>>> Minor issues:
>>>>>>> s4:  There is no discussion in s4 of the Named Attributes associated with a file and the restriction of server-to-server copy to 'regular files' seems to indicate that Named Attributes cannot be copied by this mechanism.  Is there anything to be said about getting the Named Attributes across?
>>>>>> Actually, from Section 5.8.1.2 of RFC 5661:
>>>>>> 
>>>>>>   o  The phrases "is an ordinary file" and "is a regular file" mean
>>>>>>      that the object's type attribute is NF4REG or NF4NAMEDATTR.
>>>>>> 
>>>>>> (I’ll quote from RFC 5661 and not RFC 7530 because of the pnfs aspects.)
>>>>>> 
>>>>>>> s4/s9:  Does server-to-server copy interact (badly, or otherwise) with Labeled NFS?  Nothing is said, but I wonder whether there are issues around atomicity and MAC between the different servers for inter-server copy.
>>>>>> Good question - I think even though we’ve published the Labeled NFS to allow for heterogeneous MAC deployments, sites are going to run a specific variant.
>>>>>> 
>>>>>> In that model, I would expect the object label to travel with the object and still be applied on the destination until modified by someone with those capabilities.
>>>>>> 
>>>>>> For the other model, the object label would control whether the file could be copied to another system or not. And if that system had a different MAC model, then according to the requirements in Section 3.3 of RFC 7204:
>>>>>> 
>>>>>>   A negotiation scheme SHOULD be provided, allowing systems from
>>>>>>   different Label Formats to agree on how they will interpret or
>>>>>>   translate each other's foreign labels.  Multiple concurrent
>>>>>>   agreements may be current between a server and a client.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> s4.10.1.1.1: Size and reuse for random number used as a shared secret for inter-server copy.
>>>>>>> No advice is given on the desirable size/length of the random number or the risks or otherwise of reusing the same context for multiple file copies.  This maybe a non-issue - I defer to those with more security clue than me.
>>>>>> Back to RFC 7530, 3.2.1.1, when we had a similar discussion in the WG about the required algorithms fodeplying Kerberos:
>>>>>> 
>>>>>>   At the time this document was specified, the Advanced Encryption
>>>>>>   Standard (AES) with HMAC-SHA1 was a required algorithm set for
>>>>>>   Kerberos V5.  In contrast, when NFSv4.0 was first specified in
>>>>>>   [RFC3530], weaker algorithm sets were REQUIRED for Kerberos V5, and
>>>>>>   were REQUIRED in the NFSv4.0 specification, because the Kerberos V5
>>>>>>   specification at the time did not specify stronger algorithms.  The
>>>>>>   NFSv4 specification does not specify required algorithms for Kerberos
>>>>>>   V5, and instead, the implementer is expected to track the evolution
>>>>>>   of the Kerberos V5 standard if and when stronger algorithms are
>>>>>>   specified.
>>>>>> 
>>>>>> 3.2.1.1.1.  Security Considerations for Cryptographic Algorithms in
>>>>>>            Kerberos V5
>>>>>> 
>>>>>>   When deploying NFSv4, the strength of the security achieved depends
>>>>>>   on the existing Kerberos V5 infrastructure.  The algorithms of
>>>>>>   Kerberos V5 are not directly exposed to or selectable by the client
>>>>>>   or server, so there is some due diligence required by the user of
>>>>>>   NFSv4 to ensure that security is acceptable where needed.  Guidance
>>>>>>   is provided in [RFC6649] as to why weak algorithms should be disabled
>>>>>>   by default.
>>>>>> 
>>>>>> I.e., I would prefer to not specify a policy on size and reuse of the random number as a shared secret. Well, strike that, the random number should not be reused and the length is determined by the algorithm used to generate that secret.
>>>>>> 
>>>>>> I can craft text to that effect.
>>>>>> 
>>>>>> 
>>>>>>> Nits/editorial comments:
>>>>>>> General: bytes or octets?
>>>>>> RFC 5661:
>>>>>> 
>>>>>>   Byte:  In this document, a byte is an octet, i.e., a datum exactly 8
>>>>>>      bits in length.
>>>>>> 
>>>>>> So bytes.
>>>>>> 
>>>>>>> Abstract: Probably ought to expand i/O.
>>>>>> done
>>>>>> 
>>>>>>> s1/s1.1: I think you could safely delete the s1.1 header leaving the text as the body of s1 - this is generally considered better practice than leaving s1 itself empty.
>>>>>> done
>>>>>> 
>>>>>> 
>>>>>>> s1.2, 3rd bullet: s/protocols.  I.e.,/protocols, that is/; s/apply/apply only/
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>>> s1.4:  The overview doesn't cover the two LAYOUT related operations and there is is no section describing what their usage might be in with sections 4-10.
>>>>>>> 
>>>>>> 1.3.7.  Layout Enhancements
>>>>>> 
>>>>>>   In the parallel NFS implementations of NFSv4.1 (see Section 12 of
>>>>>>   [RFC5661]), the client cannot communicate back to the metadata server
>>>>>>   any errors or performance characteristics with the storage devices.
>>>>>>   NFSv4.2 provides two new operations to do so respectively: LAYOUTERROR
>>>>>>   (see Section 15.6) and LAYOUTSTATS (see Section 15.7).
>>>>>> 
>>>>>>> s1.4: Similarly, there is noting about the CLONE operation.
>>>>>> 1.3.1.  Server Side Clone and Copy
>>>>>> 
>>>>>>   A traditional file copy of a remotely accessed file, whether from one
>>>>>>   server to another or between locations in the same server, results in
>>>>>>   the data being put on the network twice - source to client and then
>>>>>>   client to destination.  New operations are introduced to allow
>>>>>>   unnecessary traffic to be eliminated:
>>>>>> 
>>>>>>   o  The intra-server clone feature allows the client to request a
>>>>>>      synchronous cloning, perhaps by copy-on-write semantics.
>>>>>> 
>>>>>>   o  The intra-server copy feature allows the client to request the
>>>>>>      server to perform the copy internally, avoiding unnecessary
>>>>>>      network traffic.
>>>>>> 
>>>>>>   o  The inter-server copy feature allows the client to authorize the
>>>>>>      source and destination servers to interact directly.
>>>>>> 
>>>>>>> s1.4.1, para 1: s/remotely accessed/remotely accessed file/; s/location/locations/
>>>>>> done
>>>>>> 
>>>>>>> s1.4.1: (very nitty!) Suggest making the descriptions of the two cases (intra- and inter-server) into bulleted paragraphs to make it clearer that they are separate features.
>>>>>>> 
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> s1.4.2: (as with the Abstract) expand I/O on first occurrence.
>>>>>> Done
>>>>>> 
>>>>>>> s1.4.2: It would be worth putting in the reference for posix_fadvise here.
>>>>>> Done
>>>>>> 
>>>>>>> s1.4.3: If you write the data in the hole, it isn't really a hole ;-) ….
>>>>>> Ha!
>>>>>> 
>>>>>>> OLD:
>>>>>>> Such holes are typically transferred as 0s during I/O.
>>>>>>> NEW:
>>>>>>> Such holes are typically transferred as 0s when read from the file.
>>>>>>> END
>>>>>> Done
>>>>>> 
>>>>>>> s1.5: Suggest s/I.e., READ becomes either/For instance, READ would have to be replaced or supplemented by, say, either/
>>>>>> Done
>>>>>> 
>>>>>>> s2, last bullet: Need to expand pNFS on first use (and maybe remind readers that it is a feature of NFS4.1 - Section 12 of RFC 5661)
>>>>>> Done
>>>>>> 
>>>>>>> s2, last bullet: s/metadata sever/metadata server/ - again a pointer to (say) Sections 1.7.2.2 and 12.2.2 of RFC 5661 would clarify what a metadata server is.
>>>>>>> 
>>>>>> Done
>>>>>> 
>>>>>>> s4.2.1:  The first para reads:
>>>>>>> OLD:
>>>>>>>  COPY_NOTIFY:  Used by the client to notify the source server of a
>>>>>>>     future file copy from a given destination server for the given
>>>>>>>     user.  (Section 15.3)
>>>>>>> 
>>>>>>> I completely misread this on first pass, but I understand that it is correct.  Having checked with s15.3 and thought a bit more about it, it is the 'copy from a given destination' that threw me - I guess it means 'the copy will be made from' rather than 'the file being copied from the destination'... Doh!
>>>>>>>> A client sends this
>>>>>>>>   operation in a COMPOUND request to the source server to authorize a
>>>>>>>>   destination server identified by cna_destination_server to read the
>>>>>>>>   file specified by CURRENT_FH on behalf of the given user.
>>>>>>> I suggest the following might be avoid this brain fade:
>>>>>>> NEW:
>>>>>>>  COPY_NOTIFY:  Used by the client to request the source server to authorize a
>>>>>>>     future file copy that will be made by a given destination server on behalf of the given
>>>>>>>     user.  (Section 15.3)
>>>>>>> END
>>>>>> done
>>>>>> 
>>>>>>> s4.2.2, para 5:  s/support all these operations/support these operations/ ('all' is confusing given that only two are then explicitly mentioned).
>>>>>>> 
>>>>>> done - BTW note that we had some WG revisions made to introduce the CLONE operation here. Those
>>>>>> changes are considered part of this review cycle. In other words, please let me know when you want
>>>>>> a revised draft copy published.
>>>>>> 
>>>>>> 
>>>>>>> s4.3, para 1:  s/Inter-server copy is driven/The specification of inter-server copy is driven/
>>>>>>> 
>>>>>> done
>>>>>> 
>>>>>>> s4.4.1, last para: s/The source MUST equate/The source MUST interpret/
>>>>>> done
>>>>>> 
>>>>>>> s4.6/Figures 4 and 5: Need a 'key' to explain os1 and os2.
>>>>>> Done in that I expanded the names in the figure and also made it clear which wa being released
>>>>>> 
>>>>>>> s4.7.2:  Adding a reference to Section 15.3(.3) would help.
>>>>>> Went with 15.3 - let me know if you’d like 15.3.3 better.
>>>>>> 
>>>>>> 
>>>>>>> s4.7.3, para 2: idnits identifies RFC 2616 as obsoleted by RFC 7230, etc.  A more recent ref is desirable.
>>>>>> Done
>>>>>> 
>>>>>>> s4.8, para after code fragment: LDAP and NIS  need to be expanded (DNS and URL are well-known).
>>>>>> Done
>>>>>> 
>>>>>>> s4.9, para 3: Is it possible to add a little explanation as to *why* seqid = 0 is ambiguous?  My limited knowledge doesn't grok this.
>>>>>>> 
>>>>>> The stateid might be that of a lock, which has the provision that it cannot be 0
>>>>>> 
>>>>>> Section 8.2.2 of RFC 5661:
>>>>>> 
>>>>>>   When such a set of locks is first created, the server returns a
>>>>>>   stateid with seqid value of one.  On subsequent operations that
>>>>>>   modify the set of locks, the server is required to increment the
>>>>>>   "seqid" field by one whenever it returns a stateid for the same
>>>>>>   state-owner/file/type combination and there is some change in the set
>>>>>>   of locks actually designated.  In this case, the server will return a
>>>>>>   stateid with an "other" field the same as previously used for that
>>>>>>   state-owner/file/type combination, with an incremented "seqid" field.
>>>>>>   This pattern continues until the seqid is incremented past
>>>>>>   NFS4_UINT32_MAX, and one (not zero) is the next seqid value.
>>>>>> 
>>>>>> Let me know if you want a citation here (and a little bit of explanatory text).
>>>>>> 
>>>>>> Note, RFC 5661 uses “zero” and not “0”. I’ve changed this text to match.
>>>>>> 
>>>>>>> s4.10: Is it possible to provide an abstract definition of 'structured privilege'?  The rpcsec-gssv3 draft appears to punt on this, pointing to the 'example' in the NFSv4.2 draft.
>>>>>>> I think I get the idea but a succinct definition would help I believe.   I guess the definition ought to be in the rpcsec-gss document and referenced from this doc.
>>>>>>> 
>>>>> The succinct definition is here:
>>>>> 
>>>>> From draft-ietf-nfsv4-rpcsec-gssv3-14 Section 2.7.1.4.  Structured Privilege Assertions first sentence, first paragraph:
>>>>> 
>>>>> "A structured privilege is an RPC application defined privilege.”
>>>>> 
>>>>> and at the end of the first paragraph:
>>>>> 
>>>>> "Encoding, server verification and any policies for structured privileges are described by the RPC application definition.”
>>>>> 
>>>>> NFSv4.2 is the RPC application that defines the inter server to server copy structured privileges.
>>>>> 
>>>>> There is really nothing else to add to the definition.
>>>>> 
>>>>>> ^^ Andy???
>>>>>> 
>>>>>> 
>>>>>>> s4.10.1.1.1, 2nd bullet: Need to expand QOP.
>>>>>> Andy, RFC 2203 uses this term, but does not expand it.
>>>>> Actually, it does in a very round-about manner.
>>>>> 
>>>>> rfc2203:
>>>>> 
>>>>> 5.2.1.  Mechanism and QOP Selection
>>>>> 
>>>>>   There is no facility in the RPCSEC_GSS protocol to negotiate GSS-API
>>>>>   mechanism identifiers or QOP values. At minimum, it is expected that
>>>>>   implementations of the RPCSEC_GSS protocol provide a means to:
>>>>> 
>>>>>   *    specify mechanism identifiers, QOP values, and RPCSEC_GSS
>>>>>        service values on the client side, and to
>>>>> 
>>>>>   *    enforce mechanism identifiers, QOP values, and RPCSEC_GSS
>>>>>        service values on a per-request basis on the server side.
>>>>> 
>>>>>   It is necessary that above capabilities exist so that applications
>>>>>   have the means to conform the required set of required set of
>>>>>   <mechanism, QOP, service> tuples (See the section entitled Set of GSS-API Mechanisms).
>>>>>                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>> 
>>>>> 6.  Set of GSS-API Mechanisms
>>>>> 
>>>>>   RPCSEC_GSS is effectively a "pass-through" to the GSS-API layer, and
>>>>>   as such it is inappropriate for the RPCSEC_GSS specification to
>>>>>   enumerate a minimum set of required security mechanisms and/or
>>>>>   quality of protections.
>>>>> 
>>>>> so QOP is "Quality of Protection"
>>>>> 
>>>>>> And RFC 5403 does not use it.
>>>>>> 
>>>>>> It is a field in one of the structures?
>>>>> Here is are some references:
>>>>> 
>>>>> rfc2743  "Generic Security Service Application Program Interface Version 2, Update 1"
>>>>> 
>>>>> 
>>>>> Section 1.2.4:  Quality of Protection
>>>>> 
>>>>>   Some mech_types provide their users with fine granularity control
>>>>>   over the means used to provide per-message protection, allowing
>>>>>   callers to trade off security processing overhead dynamically against
>>>>>   the protection requirements of particular messages. A per-message
>>>>>   quality-of-protection parameter (analogous to quality-of-service, or
>>>>>   QOS) selects among different QOP options supported by that mechanism.
>>>>>   On context establishment for a multi-QOP mech_type, context-level
>>>>>   data provides the prerequisite data for a range of protection
>>>>>   qualities.
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> rfc4121: "The Kerberos Version 5 Generic Security Service Application Program Interface (GSS-API) Mechanism: Version 2"
>>>>> 
>>>>> Section 3.  Quality of Protection
>>>>> 
>>>>> 
>>>>>   The GSS-API specification [RFC2743] provides Quality of Protection
>>>>>   (QOP) values that can be used by applications to request a certain
>>>>>   type of encryption or signing.  A zero QOP value is used to indicate
>>>>>   the "default" protection; applications that do not use the default
>>>>>   QOP are not guaranteed to be portable across implementations, or even
>>>>>   to inter-operate with different deployment configurations of the same
>>>>>   implementation.  Using a different algorithm than the one for which
>>>>>   the key is defined may not be appropriate.  Therefore, when the new
>>>>>   method in this document is used, the QOP value is ignored.
>>>>> 
>>>>>            ……...
>>>>> 
>>>>> 
>>>>>>> s4.10.1.2, para 2: s/uiniquely/uniquely/
>>>>>> Done
>>>>>> 
>>>>>>> s5, title: s?IO?I/O?
>>>>>> fixed
>>>>>> 
>>>>>>> s6.1: This heading could be deleted turning the text in 6.1 into Section 6 which is then not empty.
>>>>>>> 
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>>> s6.1: The term 'inode' is used four times in this section.  Whilst this is well understood, it is strictly associated with *nix file systems. It would be desirable to find an alternative term or, if you can't think of suitable short generic term (I spent a little time thinking about it and couldn't think of one immediately), some weasel words added to the first occurrence saying that it is intended to cover equivalent structures in other sorts of filing system.
>>>>>> Let me try to weasel!
>>>>>> 
>>>>>> I’ve related it to metadata.
>>>>>> 
>>>>>> 
>>>>>>> s6.1, para 2: s/can thought as holes/can thought of as holes/
>>>>>> can be thought of as holes
>>>>>> 
>>>>>> 
>>>>>>> s6.1, para 4:  s/couple/few/  ('couple hundred' is a USA specific colloquialism - UK would use 'couple of’)
>>>>>> done
>>>>>> 
>>>>>>> s6.1, last para: s/punching.  I.e., an application/punching, where an application/
>>>>>>> 
>>>>>> done
>>>>>> 
>>>>>> 
>>>>>>> s7, para 1: s/One example is the posix_fallocate/One example is the posix_fallocate operation/
>>>>>>> 
>>>>>> Done
>>>>>> 
>>>>>>> s7
>>>>>>> OLD:
>>>>>>>  space_freed  This attribute specifies the space freed when a file is
>>>>>>>     deleted, taking block sharing into consideration.
>>>>>>> NEW:
>>>>>>>  space_freed  This attribute reports the space that would be freed when a file is
>>>>>>>     deleted, taking block sharing into consideration.
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>>> s8, bullet #2:
>>>>>>> I found the 2nd sentence hard to parse.  Suggested alternative below.
>>>>>>> OLD:
>>>>>>>  2.  Fields to describe the state of the ADB and a means to detect
>>>>>>>      block corruption.  For both pieces of data, a useful property is
>>>>>>>      that allowed values be unique in that if passed across the
>>>>>>>      network, corruption due to translation between big and little
>>>>>>>      endian architectures are detectable.  For example, 0xF0DEDEF0 has
>>>>>>>      the same bit pattern in both architectures.
>>>>>>> NEW:
>>>>>>>  2.  Fields to describe the state of the ADB and a means to detect
>>>>>>>      block corruption.  For both pieces of data, a useful property would be
>>>>>>>      that the allowed values are specially selected so that if passed across the
>>>>>>>      network, corruption due to translation between big and little
>>>>>>>      endian architectures is detectable.  For example, 0xF0DEDEF0 has
>>>>>>>      the same (32 wide) bit pattern in both architectures, making it inappropriate.
>>>>>>> END
>>>>>> Done
>>>>>> 
>>>>>>> PS: The example is relevant to 32 bit memory architectures... It has never occurred to me to ask if there are 64 bit big and little endian architectures!  Well the IA-64 is bi-endian...
>>>>>>> 
>>>>>>> s8.3:  The pictures of the memory patterns don't match the specification in that the guard pattern appears to be at octet offset 4 in each ADB - You can't tell where the checksum is from the pictures, but as specified there would be a 4 octet gap between the guard pattern and the checksum - I presume this is intentional. Would it be guaranteed to be zero?
>>>>>>> 
>>>>>> Not on purpose - does this work?
>>>>>> 
>>>>>>   Further, assume the application writes a single ADB at 16k, changing
>>>>>>   the guard pattern to 0xcafedead, we would then have in memory:
>>>>>> 
>>>>>>       0k ->   (4k - 1) : 00 00 00 00 ... fe ed fa ce 00 00 ... 00
>>>>>>       4k ->   (8k - 1) : 00 00 00 01 ... fe ed fa ce 00 00 ... 00
>>>>>>       8k ->  (12k - 1) : 00 00 00 02 ... fe ed fa ce 00 00 ... 00
>>>>>>      12k ->  (16k - 1) : 00 00 00 03 ... fe ed fa ce 00 00 ... 00
>>>>>>      16k ->  (20k - 1) : 00 00 00 04 ... ca fe de ad 00 00 ... 00
>>>>>>      20k ->  (24k - 1) : 00 00 00 05 ... fe ed fa ce 00 00 ... 00
>>>>>>      24k ->  (28k - 1) : 00 00 00 06 ... fe ed fa ce 00 00 ... 00
>>>>>>         ...
>>>>>>     396k -> (400k - 1) : 00 00 00 63 ... fe ed fa ce 00 00 ... 00
>>>>>> 
>>>>>> 
>>>>>>> s9.1:  Again it would be best to delete the title of s9.1 and leave the text as s9.
>>>>>>> 
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>>> s9.1, para 2: Expand ACL please.
>>>>>> Done
>>>>>> 
>>>>>>> s9.1/s9.6.1.3: I am dubious about referring back to the requirements doc [RFC7204] for definitions (rather than indicating what requirements were met/not met) . For the ref in s9.1, I think that the MAC definition in RFC 4949 would suffice.
>>>>>> Done
>>>>>> 
>>>>>> 
>>>>>>> [I noted when reviewing the rpcsec-gssv3 draft a couple of days ago that it makes *normative* reference to RFC 7204 - this is even more undesirable
>>>>> Starting with draft-ietf-nfsv4-rpcsec-gssv3-14, rfc7204 is an informative reference, and each reference includes a section number.
>>>>> 
>>>>>>> - my feeling is that it should be possible to find out what implementers need to know from the NFSv4.2 standard, other standards or the security glossary as there is no certainty that what is said in the requirements was actually put into the standard, which could cause confusion for future implementers.]
>>>>> the information that draft-ietf-nfsv4-rpcsec-gssv3-14 references in rfc7402 is ‘big picture’ info:
>>>> Tom - I’ll review Section 9.6 "MAC Security NFS Modes of Operation" of draft-ietf-nfsv4-minorversion2 and suggest some changes (if needed) so that draft-ietf-nfsv4-rpcsec-gssv3 can simply reference draft-ietf-nfsv4-minorversion2 for MAC security issues and get rid of the RFC7204 reference.
>>>> 
>>>> —>Andy
>>>>> 1.  Introduction and Motivation
>>>>> 
>>>>>   Labeled NFS (see Section 9 of [NFSv4.2]) uses an MLS policy with
>>>>>   Mandatory Access Control (MAC) systems as defined in [RFC4949].
>>>>>   Labeled NFS stores MAC file object labels on the NFS server and
>>>>>   enables client Guest Mode MAC as described in Section 4.3 of
>>>>>   [RFC7204].  RPCSEC_GSSv3 label assertions assert client MAC process
>>>>>   subject labels to enable Full Mode MAC when combined with Labeled NFS
>>>>>   as described in Section 3.3 of [RFC7204].
>>>>> 
>>>>> 
>>>>> 1.1.  Added Functionality
>>>>>           ………..
>>>>>  o  Security labels for Full Mode security type enforcement, and other
>>>>>      labeled security models (See Section 5.5.1 in [RFC7204]).
>>>>> 
>>>>>  An option for enumerating server supported label format specifiers
>>>>>   (LFS) is provided.  See Section 2 and Section 3.3 in [RFC7204] for
>>>>>   detail.
>>>>> 
>>>>> 
>>>>>>>  Is it possible to get everything an rpcsec-gssv3 implementer needs to know into this document?
>>>>>>> My impression is that it is almost all there already.
>>>>> I think WRT this issue, draft-ietf-nfsv4-rpcsec-gssv3-14 does give everything an implementor of rpcsec-gssv3 neeeds to know.
>>>>> 
>>>>> —>Andy
>>>>> 
>>>>>> Andy???
>>>>>> 
>>>>>> 
>>>>>>> s9.2, MLS definition: RFC 2401 has been obsoleted by RFC 4301... can this be referenced instead?
>>>>>> MLS appears in RFC 2401 and not RFC 4301.
>>>>>> 
>>>>>> But I wasn’t aware of RFC 4949 - do you want to use it here as well?
>>>>>> 
>>>>>> 
>>>>>>> s9.4: Expand DS and MDS on first occurrence (these should probably go back in s3 where metadata servers and data servers are referred to but without the abbreviations).
>>>>>> Oh, I don’t like using them at all. I think I was supposed to go back and expand them. :-)
>>>>>> 
>>>>>> 
>>>>>>> s11.2, Table 2: The operation IO_ADVISE is missing from the table. [CAVEAT: I don't claim to have checked the possible error codes!] Aesthetically, this table would be better with horizontal separators between the operation items.
>>>>>> Done on the first part.
>>>>>> 
>>>>>> And the newer versions of xml2rfc has allowed me to add the horizontal separators
>>>>>> 
>>>>>> 
>>>>>>> s12.1, Table 4:  The data types of clone_blksize (length4 vs uint32_t) and space_freed (length4 vs uint64_t) do not match between this draft and the XDR draft.
>>>>>> Fixed as per the email on the XDR draft.
>>>>>> 
>>>>>>> s12.2.3, NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR:
>>>>>>> Monotonic is not really a good name for this I think.  This is because the technical definition is either non-increasing or non-decreasing so that it would theoretically cover situations where the change attribute doesn't actually change! IMO a better name would be NFS4_CHANGE_TYPE_STRICTLY_INCR.  This would imply that the value actually increases whenever the info changes.
>>>>>>> 
>>>>>> Researching this one!  Will get back to you...
>>>>>> 
>>>>>> 
>>>>>>> s12.2.3:  With reference to the previous comment...
>>>>>>> OLD:
>>>>>>>  If either NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR,
>>>>>>>  NFS4_CHANGE_TYPE_IS_VERSION_COUNTER, or
>>>>>>>  NFS4_CHANGE_TYPE_IS_TIME_METADATA are set, then the client knows at
>>>>>>>  the very least that the change attribute is monotonically increasing,
>>>>>>>  which is sufficient to resolve the question of which value is the
>>>>>>>  most recent.
>>>>>>> NEW:
>>>>>>>  If either NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR,
>>>>>>>  NFS4_CHANGE_TYPE_IS_VERSION_COUNTER, or
>>>>>>>  NFS4_CHANGE_TYPE_IS_TIME_METADATA are set, then the client knows at
>>>>>>>  the very least that the change attribute is strictly increasing,
>>>>>>>  which is sufficient to resolve the question of which value is the
>>>>>>>  most recent.
>>>>>>> END
>>>>>>> 
>>>>>>> s13, Operations table:  The abbreviation EOL in the title line does not seem to be expanded anywhere (and isn't used in the table either).  It would be good to have table numbers for the tables.
>>>>>>> 
>>>>>> Yes, I think it was taken out in an earlier draft - striking.
>>>>>> 
>>>>>> Table numbers added
>>>>>> 
>>>>>> 
>>>>>>> s13, Operations table: Does not include IO_ADVISE
>>>>>> Added
>>>>>> 
>>>>>> 
>>>>>>> ( and technically doesn't include ILLEGAL, and CB_ILLEGAL isn't in Callbacks).
>>>>>> Added
>>>>>> 
>>>>>>> s15.2.3:
>>>>>>>>   If it cannot make that determination, it must set to false the one it
>>>>>>>>   can and set to true the other.
>>>>>>> The setting of true and false appears to be the inverse of the meaning implied in the previous part of the section.
>>>>>> Agreed
>>>>>> 
>>>>>>> s15.5.6/s15.5.6.1: Pointers into the appropriate parts of the NFS v4.1 RFC would be helpful in giving the background needed to understand what is going on here.    The discussion of dense and sparse packing in s15.5.6.1 is particularly obscure if you are not very well versed in pNFS lore.
>>>>>> Done - added a reference for the COMMIT semantics and the packing.
>>>>>> 
>>>>>>> ss15.6 and 15.7: An overview of the LAYOUT* operations in the earlier part of the document would be helpful, as mentioned above, plus some pointers to parts of the NFSv4.1 document that ties in with the pNFS layout story would be helpful.
>>>>>>> 
>>>>>> Note that we have been trying to shift from pulling in everything every minor version to just the bare minimum.
>>>>>> 
>>>>>> And I’ll note you are saying I’m not at that bar. :-)
>>>>>> 
>>>>>> Still not sure...
>>>>>> 
>>>>>>> s19.2, [Quigley14]:  This is now RFC 7569.  Also I think this should be normative.
>>>>>> Done
>>>>>> 
>>>>>>> s19.2, [BL73]: Can you point me to a copy of this? I have found some closely related work which was originally published as MITRE Technical Report 2547 Vols I and II  athttp://www.albany.edu/acc/courses/ia/classics/belllapadula1.pdf  (belllapadula2.pdf) and in the Journal of Computer Security  but the original doesn't seem to be visible.
>>>>>>> 
>>>>>>> 
>>>>>> I don’t have a copy either - during the work on RFC 7569, Ran Atkinson provided this citation in the review process. Perhaps you can ask him? (Ask me offline and I’ll provide his email.)
>