Thanks for your detailed (as always!) review, Elwyn. I definitely agree that there should be a note about non-reuse of the random number.
Jari On 18 Jan 2016, at 00:15, Elwyn Davies <[email protected]> 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 wait for direction from your > document shepherd or AD before posting a new version of the draft. > > For more information, please see the FAQ at > > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Document: draft-ietf-nfsv4-minorversion2-40.txt > Reviewer: Elwyn Davies > Review Date: 2016/01/16 > IETF LC End Date: 2015/12/09 > IESG Telechat date: 2016/01/21 > > Summary: Almost ready. Thank you for addressing almost all the issues that I > raised in my last call review. A couple of additional points have arisen as > documented below. Also I missed the usage 'we' phraseology on the first pass > and there are a couple of typos that appeared in the modified text of -40. > > Update: I forgot that the last call discussion covered the (non-)reuse of the > shared secret random number used in the structured privilege data structures. > A comment in this has not made it into -40. See below. > > Major issues: > > Minor issues: > s4.10.1.1.1, bullet #2: A late-breaking issue with RPCSEC_GSS v3 was raised > just prior to the last IESG meeting (see email [1] quoted below). I think > the requirement to use QoP rpc_gss_svc_privacy for at least the privileges > copy_from_auth and copy_to_auth for other reasons (the shared secret being > carried) effectively mitigates the problem identified which relates to > multi-principal authorization. However I am not clear if the problem would > apply to the third privilege defined in this document > (copy_confirm_auth_priv). If it does then presumably extending the use of > the privacy QoP to all the privileges would mitigate the problem. As I > understand it there is ongoing discussion of the appropriate changes needed > in the RPCSEC_GSS v3 draft and there is a possibility that fixes applied > there might have a knock-on effect in this draft: Please liaise with the > authors of draft-ietf-nfscv4-gssv3. > > s4.10.1.1.1, bullet #2 (again): The LC review asked about the generation and > reuse of the random number used as a shared secret. I understood that the > author's view was that it should not be reused for more than one copy. I > expected that a note to this effect would be included but I don't currently > see one. > > ==================================== > > Nits/editorial comments: > General: I also missed a number of instances (17, I think) of the "we <do > something>" construction familiar from scientific papers. 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/ > > Genera: I realized that there is no general terminology section in this > document. Clearly most of it is taken over from either or both of RFC 7530 > (s1.5) and RFC 5661 (s1.6). What triggered this was the point that stateid > isn't actually defined in this doc. A reference to one or both of these > and/or possibly some copies of definitions would be helpful. > > s2, last para: s/metadata sever/metadata server/ > > s3.3: s/E.g., as per Section 16.2.3 of [RFC5661],/For example, as per Section > 16.2.3 of [RFC5661],/ > > s4.1: Removing the s4.1 header would be in keeping with usual style as you > have already done for other sections. > > s4.2, para 2: s/intra-sever/intra-server/ > > s4.4.2, para 1: > OLD: > Other operations are OPTIONAL in the context of a particular feature Section > 13, > NEW: > Other operations are OPTIONAL in the context of a particular feature (see > Table 6 in Section 13), > > s4.9, last para: > I was supposed to be letting you know if some extra explanation of why seqid > being zero is ambiguous.... so, yes, I do think a bit extra is needed. Here > goes: > >> s15.8.3 notes that there can be multiple file copies associated with a >> single file going on at the same time. This is only implicit up to >> that point I think. It would be helpful to add a note about this >> possibility and the availability of asynchronous copy in general to >> the intro of section 4. >> >> In the following I may not have exactly grokked what the copy offload >> stateid represents... if so please adjust the words >> >> Add to intro (was in s4.1, s/b in s4) as new last para: >> ADD: >> The copy feature allows the server to perform the copying either >> synchronously or asynchronously. The client can request synchronous >> copying but the server may not be able to honor this request. If the >> server intends to perform asynchronous copying, it supplies the client >> with a request identifier that the client can use to monitor the >> progress of the copying and, if appropriate, cancel a request in >> progress. The request identifier is a stateid representing the >> internal locks held by the server while the copying is performed. >> Multiple asynchronous copies of all or part of a file may be in >> progress in parallel on a server; the stateid request identifier >> allows monitoring and canceling to be applied to the correct request. >> END >> >> Then modify the last para of s4.9: >> OLD: >> A copy offload stateid's seqid MUST NOT be zero. In the context of a >> copy offload operation, it is ambiguous to indicate the most recent >> copy offload operation using a stateid with seqid of zero. Therefore >> a copy offload stateid with seqid of zero MUST be considered invalid. >> NEW: >> A copy offload stateid's seqid MUST NOT be zero. In the context of a >> copy offload operation, it is inappropriate to indicate "the most >> recent >> copy offload operation" using a stateid with seqid of zero (see >> Section 8.2.2 >> of [RFC5661] for the meaning of a seqid of zero). It is inappropriate >> because the stateid refers to internal state in the server and >> there may >> be several asynchronous copy operations being performed in parallel >> on the same file by the server. Therefore >> a copy offload stateid with seqid of zero MUST be considered invalid. >> END > > s4.10, para 2: Is it essential that every server implements all three > structured privileges? As I understand the specification, a server that only > acted as a source would only need copy_from_auth whereas a server that only > acted as a destination would only need copy_to_auth and copy_confirm_auth > privileges. Presumably this could alternatively be covered by appropriate > policies in a server that implemented all three.. I am not sure whether the > error responses would be clearer if the implementation was missing or the > policy was used. Is this worthy of a comment? > > s4.10.1.1, para 3: s/This features allow/This feature allows/ > > s4.10.1.1: Some explanatory text has been added to the specification of > structured privileges in draft-ietf-nfsv4-rpcsec-gssv3-15. I suggest that > some minor updates to s4.10.1.1 should be made to tie in with this > specification. In particular minorversion2 needs to specify how the data > structure is encoded as specified in the GSS draft - RPCSEC_GSSv3 doesn't > know or care since it is treated as opaque data at the GSS level. Clearly, > for NFSv4.2, 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 > > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
