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 wait for direction from your document shepherd or AD before posting a new version of the draft. For more information, please see the FAQ at <https://wiki.tools.ietf.org/area/gen/wiki/GenArtfaq>. Document: draft-ietf-tram-stunbis-16 Reviewer: Dale R. Worley Review Date: 2018-03-29 IETF LC End Date: 2018-02-20 IESG Telechat date: 2018-04-19 Summary: This draft is basically ready for publication, but has nits that should be fixed before publication. The only interesting item concerns section 17.1, where the assignment of meanings to bits in the "security feature set" value is different from the assignment in -16. This is either non-upward-compatible with -16, or there is an error in either -16 or -17. ---------------------------------------------------------------------- There is an issue that shows up in several places: The NAT may forward the request using an IP family that is different from the IP family that it received the request using. This means that the "source IP family of the request" may depend on whether one is speaking of the client or the server. The draft is cognizant of this, and mentions its consequences in sections 6.3.3 and 12. But this also has consequences for ALTERNATE-SERVER: Section 14.15 says "The IP address family MUST be identical to that of the source IP address of the request." even though that family might not be usable by the client. The draft doesn't seem to explicitly say that this comes from address-switching by the NAT. It would help if there was a higher-level discussion of this matter, pointing to the various consequences. 3. Terminology This section needs to be updated to reference RFC 8174, including updating the paragraph (especially the final two lines) to read as specified in RFC 8174: The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all capitals, as shown here. 6.2.2. Sending over TCP or TLS-over-TCP o if multiplexing other application protocols over that port, has finished using that other protocol, and The two clauses don't match in grammatical number. This should be either o if multiplexing other application protocols over that port, has finished using those other protocols, and or o if multiplexing another application protocol over that port, has finished using that other protocol, and 9.2.4. Receiving a Request * If the request contains neither PASSWORD-ALGORITHMS nor PASSWORD- ALGORITHM, then the request is processed as though PASSWORD- ALGORITHM were MD5 (Note that if the original PASSWORD-ALGORITHMS attribute did not contain MD5, this will result in a 400 Bad Request in a later step below). There are two places where s/PASSWORD- ALGORITHM/PASSWORD-ALGORITHM/. 14.3. USERNAME The value of USERNAME is a variable-length value containing the authentication username. It MUST contain a UTF-8 [RFC3629] encoded sequence of less than 509 bytes, and MUST have been processed using the OpaqueString profile [RFC8265]. A compliant implementation MUST be able to parse UTF-8 encoded sequence of less than 763. The final "763" is grammatically incorrect. I think you intend s/763/763 bytes/, to parallel other items. 14.4. USERHASH userhash = SHA-256(Opaque(username) ":" Opaque(realm)) I believe that this should be userhash = SHA-256(OpaqueString(username) ":" OpaqueString(realm)) 14.5. MESSAGE-INTEGRITY Petit-Huguenin, et al. Expires September 6, 2018 [Page 40] Internet-Draft Session Traversal Utilities for NAT (STUN) March 2018 This bit of text appears as body text in the middle of page 40. 14.6. MESSAGE-INTEGRITY-SHA256 The MESSAGE-INTEGRITY-SHA256 attribute contains an HMAC-SHA256 [RFC2104] of the STUN message. The MESSAGE-INTEGRITY-SHA256 attribute can be present in any STUN message type. The MESSAGE- INTEGRITY-SHA256 attribute contains an initial portion of the HMAC- SHA-256 [RFC2104] of the STUN message. The value will be at most 32 bytes and MUST be a positive multiple of 4 bytes. The HMAC MUST NOT be truncated below a minimum size of 16 bytes. The value must be the full 32 bytes unless the STUN Usage explicitly specifies that truncation is allowed. STUN Usages may specify a minimum length longer than 4 bytes. This seems to be less clear than it could be. (And my apologies, some of the redundancy was my suggestion!) Perhaps this is an improvement: The value will be at most 32 bytes, MUST be at least 16 bytes, and MUST be a multiple of 4 bytes. The value must be the full 32 bytes unless the STUN Usage explicitly specifies that truncation is allowed. STUN Usages may specify a minimum length longer than 16 bytes. 17.1. STUN Security Features Registry A STUN Security Feature set defines 24 bit as flags. IANA is requested to create a new registry containing the STUN Security Features that are protected by the bid down attack prevention mechanism described in section Section 9.2.1. The initial STUN Security Features are: Bit 0: Password algorithms Bit 1: Username anonymity Bit 2-23: Unassigned Beware that the assignment of features to bits in the security feature value has changed! Bit numbers are assigned from the left/high-order end -- see figure 2 in the draft. So the *values* for these bits are: 0x400000 Bit 0: Password algorithms 0x200000 Bit 1: Username anonymity But the values assigned in -15 were: 0x000001: Password algorithms 0x000002: Username anonymity [END]