Re: [secdir] Secdir early review of draft-ietf-nfsv4-rpc-tls-03

[Chuck Lever <[e-mail]>]

Derrell, thank you for your comments.


> On Oct 22, 2019, at 3:54 PM, Derrell Piper via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Derrell Piper
> Review result: Not Ready
> 
> Reviewer: Derrell Piper
> Review result: Not Ready
> 
> I reviewed this document as part of the security directorate's ongoing
> effort to review all IETF documents entering the IESG.  These comments
> are directed at the security area director(s).  Document editors and WG
> chairs should treat these comments like any other last call comments.
> 
> This draft is entitled, "Remote Procedure Call Encryption by Default",
> except it does not define this.

The document defines a mechanism to achieve this purpose. The goal is
that any server administrator can deploy a certificate (or other
authentication material) on her NFS server, and client implementations
that comply with this document will be able to employ encryption with
no additional encryption material needed. If client implementation
policy requires encryption (which over time, clients should do) then
encryption will be available everywhere without the need to provide
special configuration on each client (unlike GSS/Kerberos).

I can retitle this document "Towards Remote Procedure Call Encryption
by Default". Please let me know if you have a preferred title.


>  It instead discusses opportunistic RPC
> encryption using TLS (DTLS), encryption that might be used if the sun,
> moon, and stars align, and likely only if you're running one of two NFS
> implementations mentioned in this draft, which exclude most existing NFS
> servers on the Internet today and is incompatible with Linux (pp. 13).
> 
> To some extent, this draft simply defines a new enum in ONC RPC, named
> AUTH_TLS.  It completely handwaves all code changes in RPC and NFS to
> support TLS, PKI, GSS-API, or DNSSEC.  It contains no pseudocode, just
> RFC2119 MUST, MAYs, and SHOULDs.

I refer you to other work products of the nfsv4 working group. In them
you will find no pseudo-code (except for XDR), only MUST, MAY, and
SHOULD. As a protocol specification, it assumes that support for TLS,
PKI, GSS-API, and/or DNSSEC is already available in an implementation
where RPC-on-TLS might be added. Should the document state that?


> pp. 5, Discovery
> 
> "The mechanism described in this document interoperates fully with RPC
> implementations that do not support TLS. The use of TLS is automatically
> disabled in these cases."
> 
> Hence, Not Ready.
> 
> I have great sympathy for wanting to try to make it possible to use TLS
> by default in new NFS servers that support it, however even then, I
> think this draft falls short.  It seems self-contradictory at times, and
> seems to continue to default to off, not on, which is exactly what
> RFC7258 says we ought not be doing anymore.

This mechanism is the first step towards making RPC encryption
the default everywhere. And, except for policy changes in NFS
implementations, the only necessary coding change is support
for sending and recognizing RPC_AUTH_TLS, and then performing
a TLS handshake before allowing further RPC traffic on a
connection.


> Or maybe it doesn't intend to say this, since Token binding and TLSA are
> mentioned in Security Considerations, but optional, so it kinda does.
> So, defer to the ADs.
> 
> pp. 6, Discovery
> 
> "Once the TLS handshake is complete, the RPC client and server will have
> established a secure channel for communicating.  The client MUST switch
> to a security flavor other than AUTH_TLS within that channel, presumably
> after negotiating down redundant RPCSEC_GSS privacy and integrity
> services and applying channel binding."
> 
> What are the code changes?  GSS-API is subtle, please explain.  Are
> there really no TLS or DTLS changes for any of this?

An ALPN number is allocated. No other code changes to TLS are
required. What kind of code changes do you expect, other than
implementation-specific logic to interpose the TLS handshake
at the proper moment? Would it be appropriate to detail those
changes in a document that is suppose to be implementation-
agnostic?


> pp. 6, Discovery, STARTTLS discussion
> 
> ONC RPC person needed.  The AUTH_NONE and NULL RPC text is of concern.

Can you be more specific?


> pp. 7, Authentication
> 
> "If authentication of either peer fails, or if authorization based on
> those identities blocks access to the server, the client association
> SHOULD be rejected."
> 
> MUST be rejected.
> 
> pp. 7, Authentication
> 
> "Once a TLS session is established, the server MUST NOT utilize the
> client peer's TLS identity for the purpose of authorizing individual RPC
> requests."
> 
> That's a curious statement to end a section on Authentication with.  At
> least justify that statement.

I can change this to a "MUST NOT substitute RPC_AUTH_TLS or the
peer identity used for TLS authentication, for the existing forms
of per-request RPC authentication specified by RFC 5531".


> pp. 8, TLS Requirements
> 
> "Support for TLS-PSK mutual authentication [RFC4279] is OPTIONAL."
> 
> Considering this is retrofit security for a legacy UNIX UID/GID
> protocol, making PSKs OPTIONAL almost seems quaint here, but okay.

What is your preferred alternative?


> pp. 8, TLS Requirements
> 
> "Support for and negotiation of compression is OPTIONAL."  Noted.
> 
> pp. 9, Operation on Other Transports
> 
> "Transports that provide intrinsic TLS-level security (e.g. QUIC) would
> need to be accomodated separatey from the current document.  In such
> cases, use of TLS might not be opportunistic as it is for TCP or UDP."
> 
> "opportunitic" is misspelled, and I don't know what to make of this
> sentence, but I have very partisan views on QUIC, so defer to the ADs.
> 
> I assume Section 5.1.3 is there for RDMA but it doesn't say anything.
> 
> pp. 11, X.509 Certificates Using Fingerprints
> 
> "Implementations MUST support SHA-1 as the hash algorithm for the
> fingerprint.  To prevent attacks based on hash collisions, support for a
> more contemporary hash function, such as SHA-256, is RECOMMENDED."
> 
> SHA-1's deprecated, right?  So we shoudn't be mandating SHA-1 in new
> RFCs, right?  Defer to AD.

I can change this to SHA-256, or whatever is the current
preference.


> pp. 11 Pre-Shared Keys
> 
> "should be exposed" -> "SHOULD be exposed"
> 
> pp. 12, DESY, Hammerspace, and Linux
> 
> Why are these two implementations called out?  This section does not
> give me confidence that this all interoperates, is it supposed to?

My understanding of the Implementation Status section is to
show that there are at least two implementations of the protocol
specified in the document. I was not aware that this section was
supposed to make any statement about interoperability with
implementations not mentioned here. In fact, that would seem to
be difficult to do in a document that is fixed at publication
time -- it can't refer to future implementations, of course.

In any event, a Linux kernel implementation is under way, and
I believe there is also interest among Solaris and FreeBSD
implementers. My plan is to introduce those implementations to
the list in this section as they become publicly available. I
am not at liberty to announce products that are not public.


> pp. 13, Security Considerations
> 
> Since absolute compatibilty with fielded insecure NFS is stated as a
> requirement, the obvious downgrade attack is not only permitted, but
> required.  Again, RFC7258 says that's no longer acceptable, doesn't it?
> nAgain, defer to the ADs.
> 
> "Implementations must take care to accurately represent to all RPC
> consumers the level of security that is actually in effect."  How?

I can clarify this.


> pp. 14, Security Considerations for AUTH_SYS on TLS
> 
> "In light of the above, it is RECOMMENDED that when AUTH_SYS is used,
> RPC clients present authentication material necessary for RPC servers
> they contact to have a degree of trust that the clients are acting
> responsibly."
> 
> Hence, "if the sun, moon, and stars align."  Again, this is not in the
> spirit of RFC7258.

The point of this language is to suggest a policy of requiring
TLS, rather than leaving it as opportunistic. This is good
implementation guidance.


> And just to remind, AUTH_SYS doesn't make sense on
> non-UNIX operating systems, but that perhaps is my partisan viewpoint.

AUTH_SYS is the most widely deployed RPC authentication flavor
by far. Addressing the security shortcomings for this flavor
is a prime motivation for enabling encryption via TLS.


> In closing, there's two broad questions to consider:
> 
> 1) Does this do no harm?

What exactly would demonstrate harmlessness?


> 2) Does it improve security on the Internet in the spirit of RFC7258?
> 
> This is going to have to be much more detailed to convince me that it
> does either of these things for any implementation other than DESY or
> Hammerspace, and without other reference implemenatation in the BSDs or
> a least some flavors of Linux, you can't say this broadly interoperable
> either.

Simple analysis of the discovery protocol specified here shows
that the mechanism can indeed detect when its peer does not
support RPC_AUTH_TLS, and then not use it.

We host two NFS interoperability events a year and can easily
demonstrate that we have achieved reasonable interop, if that
is required by the IESG.


> Distributed file systems are never easy, hence DCE/AFS, so
> granted it's not an easy problem, but this is not ready to advance on
> Standards Track, unless merely being interoperable with legacy code is
> all we aspire to, and I sincerely hope it's not.
> 
> Perhaps this needle can be threaded and with appropriate configuration
> by enterprising people, TLS can be configured with DNSSEC and GSS-API in
> RPC and NFS and it will do something reasonable and secure, but I would
> like to see at least some more comments from implementation experience
> before I could recommend this advance.
> 
> Derrell
> 
> 

--
Chuck Lever