Reviewer: Sean Turner Review result: Has Issues This draft updates NFSv4.1 to correct handling of the fs_locations and fs_locations_info attributes; NFSv4.1 weighs in at 617 pages and this draft weighs in a relatively spry 106; let’s say I was really, really hoping for some multi-page ASCII art, but also no pretty much all text. The draft does a good job of explaining what’s being replaced and why, but I have to say that I did not check all of section pointers. These instructions will really help when work on a fully revised NFS RFC gets going. Unfortunately for me, I do not have NFSv4.1 in my head so let’s just say that I did my best … Summary: This draft is probably worthy of some type of DISCUSS based on the following: 0) The big issue being addressed in the security considerations is the fact that RPSEC_GSS is must support not must use and the other mechanism referred to by RFC 5661 are: - AUTH_NONE - nada authentication - AUTH_SYS as described in Appendix A is known to be insecure due to the lack of a verifier to permit the credential to be validated. AUTH_SYS SHOULD NOT be used for services that permit clients to modify data. AUTH_SYS MUST NOT be specified as RECOMMENDED or REQUIRED for any Standards Track RPC service. - AUTH_DH as mentioned in Sections 8.2 and 13.4.2 is considered obsolete and insecure; see [RFC2695]. AUTH_DH SHOULD NOT be used for services that permit clients to modify data. AUTH_DH MUST NOT be specified as RECOMMENDED or REQUIRED for any Standards Track RPC service. So what do you do when RFC 5661 says: The fetching of attributes containing file system location information SHOULD be performed using RPCSEC_GSS with integrity protection, And, you don’t now change to MUST use RPSEC_GSS? Well I think at a minimum you should do some 2119 language: 0) s16 (after the bullets): 2119 this should: In light of the above, a server should present file system location entries that correspond to file systems on other servers using a host name. 1) s16 (2nd para after the bullets): 2199 this SHOULD NOT: As a result, the client should not use such unverified location entries as a basis for migration, even though RPCSEC_GSS might be available on the destination. 2) s16 (last bullet): 2119 this should: Where the use of the returned information cannot be avoided, it should be subject to filtering to eliminate the possibility that the client would treat an invalid address as if it were a NFSv4 server. And, I guess since AUTH_SYS is insecure can I suggest a friendly amendment to the following sentence: OLD: The use of requests issued without RPCSEC_GSS (i.e. using AUTH_SYS), while undesirable, may not be avoidable in all cases. NEW: The use of requests issued without RPCSEC_GSS (i.e. using the known to be insecure AUTH_SYS), while undesirable and Insecure, may not be avoidable in all cases. It’s probably also worth discussing whether s14.3 should drop support for includes id-sha1. These are the nits: 0) Update to use new 2119 paragraph: The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all capitals, as shown here. 1) s3.1 (wordy): r/since the ports to be used for various types of connections might be required to be different/since the ports to be used for various types of connections might be different. 2) s3.1 (missing period): r/Two such addresses support the use of clientid ID trunking, as described in [RFC5661]/Two such addresses support the use of clientid ID trunking, as described in [RFC5661]. 3) s4.2 (missing word): r/which may accessed/which may be accessed 4) s4.3 (1st sentence): Not sure the 2119-RECOMMENDED is needed. 5) s4.3 (penultimate para): Seems like r/should/SHOULD. 6) s4.5: r/present , is/present, is 7) s4.5: Note sure you need the parenthetical in the following: a server may (but is not required to) 8) s4.5.4: r/In the event that server failures,/In the event that server fails, or r/In the event that server failures,/In the event of server failures, 9) s12.2.3: replace ietf.org with example.org (not supposed to use real domains even if it is “ours”). 10) s16 (3rd bullet): Why is requirement is caps?
