I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

Summary: In my opinion, this I-D needs more work.

This is a huge draft. It's on track to become the 3rd biggest RFC behind the security glossary and the NFS 4.1 RFC (5661). I've spent more time on this than my previous SECDIR reviews combined, and I still don't think I've done a thorough enough job. This revision splits RFC 3530 in two: this document, and the XDR document. This is the third revision of the NFSv4 spec (after 3530 and 3010). RFC 3010 was published on December 2000, over 12 years ago. It may have been a good idea back then to write the spec as a comparison to previous specifications, but I don't think this makes sense 12 years later. As an example, we need look no further than the Abstract:

   The Network File System (NFS) version 4 is a distributed filesystem
   protocol which owes heritage to NFS protocol version 2, RFC 1094, and
   version 3, RFC 1813.  Unlike earlier versions, the NFS version 4
   protocol supports traditional file access while integrating support
   for file locking and the mount protocol.

And it goes on throughout the document - always a diff from NFS 3. That kind of voice makes sense when addressing a community with significant NFS 3 experience, and little or no NFS 4 experience. Is this still the case? 

The Introduction section does not talk at all about what NFS is and what it could possibly be used for. It's just a series of sections detailing differences from version 3, from 3530, and from 3010. Nowhere does it say why we need a revision of version 4, when version 4.1 is already published. Much of the text is taken from RFC 3530, and in some places it gets weird:
 - Section 11 talks about requirements for future minor versions. Except that unlike when 3530 was published, now a minor version already exists. But the text ignores it, and talks in the future tense.
 - Section 18 instructs IANA to set up a  "NFSv4 Named Attribute Definitions Registry". That registry has been up since 2009

OK. On to security things.  Section 3.2.1 talks about security mechanisms. There's this table on page 27 that lists some algorithms, and have DES and MD5. This looks really bad for something published in 2013. I understand that this relates to the deployed base of Kerberos, and the IETF mailing list has an active thread about this: 


http://www.ietf.org/mail-archive/web/ietf/current/msg78389.html



Other security issues, such as discussions of ACLs, locks, and the handling of edge conditions (9.6.3.4) seem OK.

The Security Considerations section again begins with a discussion of previous versions. Beyond that, it can be summarized as these bullet points:
 - Authentication is end-to-end
 - Translation from NFS identity to local identity needs to be secure (but it doesn't say secure against what. What is the "integrity of the translation"?)
 - These security mechanisms are optional, but SECINFO and GETATTR should (no RFC 2119 language) be secured.
 - For SETCLIENTID/SETCLIENTID_CONFIRM, it's imperative (again, no RFC 2119 language) to check the principal against that of previous requests.

Not being familiar with operational NFS environments, I don't see anything missing from this.


NITS:
 - There's this weird thing in section 1.5.1, where it says "As with previous versions of NFS, XDR and RPC mechanisms used for NFSv4 are those defined in RFC5531 and RFC4506". NFSv4 was first defined in RFC 3010, so it probably uses some mechanism from earlier versions of those documents.
 - In section 6.4.1.1, "Access mask bits other those listed above". There's a missing "than" between the "other" and the "those".
 - Section 9.1.1 discusses the Client ID. The id is generated by the client and has to be unique. Several methods of generating this ID are discussed. Finally, on page 107 a true random number is suggested (might be better to say "pseudo-random number") , and then the text says:
                                However since this number ought to be
         the same between client incarnations, this shares the same
         problem as that of the using the timestamp of the software
         installation.
Where "between client incarnations" means "across client reboots". But this requirement ("ought to") is never explained. I get that if the id changes, all the locks will be broken, but does any system expect locks to remain across client reboots? If so, I think it should be explicitly said.

Yoav