Reviewer: Dale Worley Review result: Ready with Nits 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] -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
