Hi- Below are proposals for addressing the Gen-ART comments. > On May 24, 2020, at 4:50 PM, Dale Worley via Datatracker <noreply@xxxxxxxx> wrote: > > 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. Original: When an RPC client is ready to begin a TLS session, it sends a NULL RPC procedure with an auth_flavor of AUTH_TLS. The value of AUTH_TLS is defined in Section 8.1. The NULL request is made to the same port as if TLS were not in use. Replacement: In general, RPC is transport-agnostic. However, RPC-on-TLS supports only operation on TCP (with TLS) and UDP (with DTLS). For further details, consult Section 5.1. When an RPC client is ready to begin a TLS session, it opens a TCP connection to an RPC server and sends a NULL RPC procedure with an auth_flavor of AUTH_TLS on that connection. To begin a DTLS association, it sends the server a UDP datagram containing a NULL RPC procedure with an auth_flavor of AUTH_TLS. The value of AUTH_TLS is defined in Section 8.1. The NULL request is made to the same port as if TLS were not in use. > 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. Original: 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. RPC operation can continue, however it will be without any confidentiality, integrity or authentication protection from (D)TLS. Replacement: Conversely, if the Reply's reply_stat is not MSG_ACCEPTED, if it's verifier flavor is not AUTH_NONE, or if it's verifier does not contain the "STARTTLS" token, the RPC client MUST NOT send a "ClientHello" message. RPC operation can continue, however it will be without any confidentiality, integrity or authentication protection from (D)TLS. > 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, ...". Original: If the server responds incorrectly, the client MUST NOT establish a TLS session for use with RPC on this connection. See [RFC7301] for further details about how to form these messages properly. Replacement: If the server responds incorrectly (for instance, if the "ServerHello" message does not conform to the above requirement), the client MUST NOT establish a TLS session for use with RPC on this connection. See [RFC7301] for further details about how to form these messages properly. > 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? Original: Backchannel operation occurs only on connected transports such as TCP. To protect backchannel operations, an RPC server uses the existing TLS session on that connection to send backchannel operations. The server does not attempt to establish a TLS session on a TCP connection for backchannel operation. Replacement: Reverse-direction operation occurs only on connected transports such as TCP (see Section 2 of [RFC8166]). To protect reverse-direction RPC operations, the RPC server does not establish a separate TLS session on the TCP connection, but instead uses the existing TLS session on that connection to protect these operations. > 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. We believe that iPAddress is the correct spelling of this attribute's name. No replacement proposed. > "universal address" probably should be either "universal network > address" or "uaddr". Original: 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. Replacement: 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 the universal network address. > 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. Our proposal is to remove the SHA-256 requirement: • There is no standard specification for computing certificate fingerprints. • I have found no authority that claims SHA-1 (which is most common in fingerprinting tools) is inadequate. • Certificate fingerprints are constructed by system components other than the RPC implementation, thus a normative requirement here is very likely beyond the scope of this document. Original: This mechanism is OPTIONAL to implement. In this mode, the fingerprint of the presented certificate uniquely identifies the RPC peer. Implementations SHOULD allow the configuration of a list of trusted certificates, identified via fingerprint of the DER-encoded certificate octets. Implementations MUST support SHA-256 [FIPS.180-4] or stronger as the hash algorithm for the fingerprint. Replacement: This mechanism is OPTIONAL to implement. In this mode, the fingerprint of a certificate uniquely identifies an RPC peer. A "fingerprint" is typically defined as a cryptographic digest of the Distinguished Encoding Rules (DER) form [X.690] of an X.509v3 certificate [X.509]. Implementations SHOULD allow the configuration of a list of trusted certificates that is indexed by 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. As discussed earlier, we believe that "must" is correct in this context. No replacement proposed. > 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)." Original: Using a TLS-protected transport when the AUTH_SYS authentication flavor is in use addresses several longstanding weaknesses (as detailed in Appendix A). Replacement: Using a TLS-protected transport when the AUTH_SYS authentication flavor is in use addresses several longstanding weaknesses in AUTH_SYS (as 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. Original: TLS augments AUTH_SYS by providing both integrity protection and confidentiality that AUTH_SYS lacks. TLS protects data payloads, RPC headers, and user identities against monitoring and alteration while in transit. TLS guards against the insertion or deletion of messages, thus also ensuring the integrity of the message stream between RPC client and server. Lastly, transport layer encryption plus peer authentication protects receiving XDR decoders from deserializing untrusted data, a common coding vulnerability. Replacement: TLS augments AUTH_SYS by providing both integrity protection and confidentiality that AUTH_SYS lacks. TLS protects data payloads, RPC headers, and user identities against monitoring and alteration while in transit. TLS guards against in-transit insertion and deletion of RPC messages, thus ensuring the integrity of the message stream between RPC client and server. DTLS does not provide full message stream protection, but it does enable receivers to reject non-participant messages. In particular, transport layer encryption plus peer authentication protects receiving XDR decoders from deserializing untrusted data, a common coding vulnerability. > 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. Original: 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. Replacement: o RCPSEC_GSS provides integrity and privacy services which are redundant when TLS is in use. These services should be disabled in that case. > 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." Original: Description: Signals the use of TLS to protect RPC messages on socket-based transports Replacement: Description: Indicates support for RPC-on-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. Section 9.3 is generated by xml2rfc. It is our understanding that the RFC Editor invokes xml2rfc for the final publication in a mode where this section is not generated and thus it will automatically be removed. -- Chuck Lever -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
