Subject: [secdir] SecDir review of draft-ietf-nfsv4-rfc3530bis-25

From: Yoav Nir <[email-masked]>
Date: Mon, 8 Apr 2013 10:35:36 +0000

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

---

From: "Haynes, Tom" <[email-masked]>
Date: Mon, 8 Apr 2013 16:55:27 +0000

Yoav,

Thanks for the comments. Rather than address them all, I want to
talk about the main ones driving the document.

On Apr 8, 2013, at 3:35 AM, Yoav Nir <[email-masked]<mailto:[email-masked]>>
 wrote:

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).

Yes, and the WG has taken pains to make sure we do not do that again.
See for example draft-ietf-nfsv4-minorversion2-19<http://datatracker.ietf.org/doc/draft-ietf-nfsv4-minorversion2/> which weighs in at
under 100 pages.

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:

While some implementors may go straight to NFSv4.1, the path is still NFSv3
to NFSv4.0.

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.

Valid point and the main one I want to address here.

We have a lot of implementation experience in NFSv4.0 that made its way into NFSv4.1. We still
have many NFSv4 implementations that do not have corresponding NFSv4.1 implementations.

We want to drive back those lessons that do not require protocol changes into the base
document to improve interoperability.

I'll look at how to add that back to the document.

Tom

---

From: Yoav Nir <[email-masked]>
Date: Mon, 8 Apr 2013 20:35:33 +0000

On Apr 8, 2013, at 7:55 PM, "Haynes, Tom" <[email-masked]<mailto:[email-masked]>> wrote:

Yoav,

That's good to know.

Yes. My question is whether that path has already been traversed. Looking at some vendor sites and Internet forums, it looks like all the major operating systems (Windows, Linux, Mac OS, even IBM's exotic systems) already have NFSv4. I believe the same can be said for storage products such as NetApp. So who are those developers who have existing NFSv3 implementations, and have not yet implemented NFSv4?

OK. Thanks

Yoav

---

From: "Haynes, Tom" <[email-masked]>
Date: Mon, 8 Apr 2013 22:11:44 +0000

On Apr 8, 2013, at 1:35 PM, Yoav Nir <[email-masked]<mailto:[email-masked]>> wrote:

Size doesn't matter here - the developers who would write a NFSv4
implementation from scratch would come from a NFSv3 background.

NFSv3 -> NFSv4 is a major release, it deserves some transition.

NFSv4.1 -> NFSv4.2 is a minor release, it has almost no text wasted on transition.

---

From: "Haynes, Tom" <[email-masked]>
Date: Wed, 17 Apr 2013 00:13:00 +0000

Yoav,

Thank you for the effort of reviewing this document.

(top-posting because of inline comments)

Tom

> 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…

An editorial decision was to try to not reference RFC 5661 as much as possible and to not cause the reader to go to RFC 3530 for clarification. I.e., the IANA section is verbatim in order to allow the reader to get it all in one place.

I've modified the Intro to push the first two sections to the end. That starts off with the push to the NFSv2 and NFSv3 documents.

> Much of the text is taken from RFC 3530, and in some places it gets weird:

I did add a sentence about how the registry already exists.

> - 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 define…

So "previous versions" means NFSv2 and NFSv3. 

> - In section 6.4.1.1, "Access mask bits other those listed above". There's a missing "than" between the "other" and the "those".

Fixed

> 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 sys…

Besides client reboots, there is an issue of network partitioning. In that case, the client may believe that the server rebooted and the
server may believe that the client rebooted. But actually, neither did. In such a scenario, if the client can identify itself properly, then
lock state can quickly be recovered.