Dale, thanks for your reviews. Marc, thanks for your response. I have entered a No Objection ballot.
Alissa > On Apr 16, 2018, at 5:49 PM, Marc Petit-Huguenin <[email protected]> > wrote: > > Thanks again for the review. Comments inline. > > On 03/30/2018 04:45 AM, Dale Worley 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 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. > > I still do not have text about that but, as this is blocking this response > since 2 weeks now, I am releasing it as is and will come back to that after I > process the other reviews that accumulated during my time traveling around > Europe. > >> >> 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. > > Updated. > >> >> 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 > > Updated using the former. > >> >> 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/. > > Already fixed following a previous review. > >> >> 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. > > Fixed. > >> >> 14.4. USERHASH >> >> userhash = SHA-256(Opaque(username) ":" Opaque(realm)) >> >> I believe that this should be >> >> userhash = SHA-256(OpaqueString(username) ":" OpaqueString(realm)) > > Fixed. > >> >> 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. > > Already fixed following a previous review. > >> >> 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. > > Applied. > >> >> 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 > > Hmm, that was not the intent, and not how I read the text. Even in Figure 2 > less significant bits are on the right. So the intent is real tat > bit0=0x001, bit1 = > 0x002, and so on. All we do is about interoperability, so I added some text > that makes that less ambiguous. > > > -- > Marc Petit-Huguenin > Email: [email protected] <mailto:[email protected]> > Blog: https://marc.petit-huguenin.org <https://marc.petit-huguenin.org/> > Profile: https://www.linkedin.com/in/petithug > <https://www.linkedin.com/in/petithug> > > _______________________________________________ > tram mailing list > [email protected] <mailto:[email protected]> > https://www.ietf.org/mailman/listinfo/tram > <https://www.ietf.org/mailman/listinfo/tram>
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
