Last Call Review of draft-ietf-nfsv4-rpc-tls-07
review-ietf-nfsv4-rpc-tls-07-genart-lc-worley-2020-05-24-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document:  draft-ietf-nfsv4-rpc-tls-07
Reviewer:  Dale R. Worley
Review Date:  2020-05-24
IETF LC End Date:  2020-05-27
IESG Telechat date:  unknown

Summary:

Note that I am not familiar with the operations of TLS, so I have not
reviewed issues that are specific to security implementations.  I
assume the SECDIR review has adequately covered those.

This draft is quite solid and nearly ready for publication, but has
nits that should be fixed before publication.

Nits:

4.1.  Discovering Server-side TLS Support

Somewhere in this section you need to specify the semi-obvious:

   In principle, RPC is transport-agnostic.  In the context of
   RPC-Over-TLS, the initial request MUST be sent using either TCP or
   UDP.  If an initial TCP request is successful, a TLS connection is
   established.  If an initial UDP request is successful, a DTLS
   association is established.

--

   If the Reply's reply_stat is MSG_ACCEPTED but the verifier does not
   contain the "STARTTLS" token, or if the Reply's reply_stat is
   MSG_DENIED, the RPC client MUST NOT send a "ClientHello" message.

Strictly, this should list the three tests required by the preceding
paragraph:

   If the Reply's reply_stat is not MSG_ACCEPTED, if its verifier is
   not an AUTH_NONE, or if its verifier's contents are not "STARTTLS",
   the RPC client MUST NOT send a "ClientHello" message.

5.  TLS Requirements

   If the server responds incorrectly, the client MUST NOT establish a
   TLS session for use with RPC on this connection.

I think the condition can be made more specific as "If the "ServerHello"
message does not conform to the above requirement, ...".

5.1.1.  Protected Operation on TCP

   The server does not attempt to establish a TLS session
   on a TCP connection for backchannel operation.

I think "... does not attempt to establish a separate TLS session ..."
is clearer.

I can't find any discussion of "backchannel operation" in RFC 5531.
Might this need an additional reference?

5.2.1.  X.509 Certificates Using PKIX trust

   o  For services accessed by their network identifiers (netids) and
      universal network addresses (uaddr), the iPAddress subjectAltName
      SHOULD be present in the certificate and must exactly match the
      address represented by universal address.

I suspect that "iPAddress" is not capitalized correctly.

"universal address" probably should be either "universal network
address" or "uaddr".

5.2.2.  X.509 Certificates Using Fingerprints

   Implementations MUST support SHA-256
   [FIPS.180-4] or stronger as the hash algorithm for the fingerprint.

Perhaps there is a usage in security, but it seems to me that
"stronger" is not well-defined.  I expect that it means that the hash
algorithm must produce output of at least 256 bits, but that criterion
makes no allowances that over time algorithms might be deprecated.  Is
there a list of accepted "strong" hash algorithms that can be
referenced here to make this unambiguous and track best practice over
time?

Also -- here I'm assuming that one peer presents a certificate to the
other, and the second peer first hashes the certificate to create a
fingerprint and then checks the fingerprint against a list -- this
sentence does not require the peer to *use* an algorithm that meets
this criterion.  The text needs to be something like

   Implementations MUST use SHA-256 [FIPS.180-4] or stronger as the
   hash algorithm for the fingerprint.

7.1.  Limitations of an Opportunistic Approach

   Implementations of
   the mechanism described in the current document must take care to
   accurately represent to all RPC consumers the level of security that
   is actually in effect, ...

I think you want s/must/MUST/.  There's also an unstated requirement
that the RPC consumers have some way of accessing this information.

7.3.  Security Considerations for AUTH_SYS on TLS

   Using a TLS-protected transport when the AUTH_SYS authentication
   flavor is in use addresses several longstanding weaknesses (as
   detailed in Appendix A).

For clarity "... several longstanding weaknesses in AUTH_SYS (detailed
in Appendix A)."

   TLS guards against the
   insertion or deletion of messages, thus also ensuring the integrity
   of the message stream between RPC client and server.

This statement needs to be weakened somewhat for DTLS.

   o  When using RPCSEC_GSS, GSS/Kerberos provides adequate host
      authentication and a policy that requires GSS mutual
      authentication and rejection of a connection when host
      authentication fails.  GSS integrity and privacy services,
      therefore, can be disabled in favor of TLS encryption with peer
      authentication.

This paragraph is not constructed correctly.  The first sentence says
that GSS provides certain facilities, and the second sentence says
that "therefore" GSS (or certain parts of GSS) can be disabled.  There
are a number of possibilities for the form the argument should take,
but I don't know enough about security to know what was intended.

8.1.  RPC Authentication Flavor

   Description:  Signals the use of TLS to protect RPC messages on
      socket-based transports

Better would be "Used in probes for support of RPC-Over-TLS."

9.3.  URIs

Note that all references to URIs in this section are to be removed by
the RFC Editor, so this section should be removed also.

[END]