Early Review of draft-cel-nfsv4-rpc-tls-02
review-cel-nfsv4-rpc-tls-02-secdir-early-dekok-2019-03-18-00

I think that the document is fairly good, but could use additional
text.

It would a good idea for the authors to review RFC 6614 (RADIUS over
TLS) and RFC 7360 (RADIUS over DTLS).  Those documents both "patch"
RADIUS to allow for TLS / DTLS transport.  The RADIUS bits are perhaps
uninteresting, but it is useful to learn from previous approaches to
patching legacy protocols.

e.g. Section 1 of RFC 7360 says:

   The DTLS protocol does not add reliable
   or in-order transport to RADIUS.  DTLS also does not support
   fragmentation of application-layer messages, or of the DTLS messages
   themselves.

It may be worth using similar text in this document.  Also, Section
2.1 of RFC 7360 clarifies that the standad does not change anything
existing in the legacy protocol, but adding a DTLS layer may affect PMTU:

   We note that the DTLS encapsulation of RADIUS means that RADIUS
   packets have an additional overhead due to DTLS.  Implementations
   MUST support sending and receiving encapsulated RADIUS packets of
   4096 octets in length, with a corresponding increase in the maximum
   size of the encapsulated DTLS packets.  This larger packet size may
   cause the packet to be larger than the Path MTU (PMTU), where a
   RADIUS/UDP packet may be smaller.  See Section 5.2, below, for more
   discussion.

RFC 7360 also mandates support for particular TLS cipher suites, which is
lacking from this document.  I suggest adding text to address this issue.

There may be other TLS / DTLS issues in the RADIUS documents which apply here.

For this document:

4.3.2

   ... However, once encryption of the
   transport connection is established, the server MUST NOT utilize TLS
   identity for the purpose of authorizing RPC requests.

It may be worth reiterating that the protocols are independent.
i.e. This document does not define the *combination* of TLS and RPC,
so much as RPC carried over TLS.  The underlying RPC protocol is
largely unaware of the encapsulating TLS information.

Section 5:

   ... In circumstances where
   the users on that NFS client belong to multiple distinct security
   domains, it may be necessary to establish separate TLS-protected
   connections that do not share the same encryption parameters.

IMHO that's not a "may be necessary", but a hard requirement.  And the
last bit of that sentence should be clearer.  The users will each have
their own encryption credentials and profiles, I suspect.

Section 5.1:

   Therefore, the RECOMMENDED deployment mode is that both servers and
   clients have certificate material available 

Perhaps "configured and used" is better than "available".  An
certificate which is "available" may, in fact, be unused.