Thanks for the detailed review, Tom T! From what I can see, your questions are valid. Tom H, any update regarding changes and/or other responses?
Jari On 16 Jun 2014, at 03:02, Tom Taylor <[email protected]> wrote: > I am the assigned Gen-ART reviewer for this draft. For background on > Gen-ART, please see the FAQ at > > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Please resolve these comments along with any other Last Call comments > you may receive. > > Document: draft-ietf-hip-rfc5201-bis-14 > Reviewer: Tom Taylor > Review Date: 2014-06-12 > IETF LC End Date: 2014-06-11 > IESG Telechat date: 2014-06-26 > > Summary: The draft is mostly ready, some minor issues and a number of > editorial improvements identified to this point. This is the final and > complete version of the review. Lines have been drawn below to indicate where > the first version left off. > > Major issues: > ============ > > Minor issues: > ============ > > Section 4.3 (Error Processing) final case: if the receiving host does not > send some sort of response (e.g., ICMP) to the sender, the sender may have no > way of knowing that the HIP session has failed. The state transitions from > ESTABLISHED in Table 6 time out on no packet "sent/received" for a given > period of time. If the sending application doesn't expect a response, it > could be sending packets that are ignored at the other end for an > indefinitely long time. At the least, this point should be brought out in the > text of that error case, and possibly the ICMP response should be RECOMMENDED. > > Section 5.2.7: when the host supports more than one D-H Group, is each group > specified in a separate instance of the Diffie-Hellman parameter? The text > does not say. > > Section 5.2.18: given the strict ordering of HIP parameters, the initial > plaintext for the Encrypted content (type and length of initial parameter) > may be fairly easily guessed. This opens up the minor possibility of a known > plaintext attack. [Comment to be reviewed after further examination.] [Upon > review: I1 packets have fixed type but variable length due to varying lengths > of DH-GROUP-LISTS. The set of such lengths is limited, however, so it is > worth considering whether known plain-text attacks offer any real threat.] > > Section 5.3, last paragraph: forbids fragmentation of the HIP packet. Doesn't > this contradict Section 5.1.3? > > -------- Issues below here added in second version > > > Nits/editorial comments: > ======================= > > Idnits generated 13 warnings. Most are spurious, but there are a few outdated > references. I didn't check out the IPv6 addresses. > > Inconsistent capitalization: "Base Exchange" (in the opening sections) vs. > "base exchange" (most of the document) > > Sometimes the phrase is "HIP session", sometimes "HIP association". The RFC > Editor will probably want you to choose which one to use consistently. Noted > that "HIP association" is defined in Section 2.3. > > Section 1, second paragraph is a little brief on the additional specification > beyond the base exchange provided in this document. The sentence beginning > "It also defines ..." should include " and terminating the association". > > Section 1, last paragraph: missing period. > "... later discovered to be vulnerable This update" > ^ > > It would be nice to have a definition of the term "HIP packet" in Section > 2.3. If I understand correctly, after the base exchange, most of the packets > flowing in a HIP association are ordinary user data packets and not HIP > packets, and this point should be made. In some places the term "HIP control > packets" is used. Choose one or the other term for consistency. > > Section 3.1, third line from end: s/a hosts/a host/ > > Section 4.1.4: perhaps the last and second-last paragraphs should be > interchanged. I got lost for a moment on the association of multiple R1s with > the Initiator rather than the Responder when reading the second-last > paragraph. The last paragraph makes the situation clear. > > Section 4.1.5, first paragraph, second sentence: assumes that the host is > willing to carry out a base exchange with the original Initiator, albeit on > its own terms. Should something like "and policy allows the establishment of > a HIP association with the original Initiator" be added in front of the comma > in that second sentence? That leads nicely to discussion of that case in the > next paragraph. > > State diagram, Figure 2, in Section 4.4.4: unlabelled line from CLOSING to > I1-SENT. > > Last line of Section 4.5.3: s/transform/transport format/ > > Section 5.1.1 IPv4 case refers back to Section 4 for the HIP protocol number. > The proper reference should be to Section 5.1. > > Section 5.1.3 first paragraph last sentence: probably should rephrase so the > host doesn't get presented as a person, e.g., "If it is expected that a host > will send HIP packets that are larger than the minimum IPv6 MTU, the > implementation MUST include IPv6 fragmentation." > > Section 5.2.5, missing space at the right hand end of the line containing > Opaque in the figure. > > Section 5.2.6, immediately beneath the figure: > - DH GROUP ID explanation: > -- s/defines/identifies/ on first line. This comment applies to a > number of other parameter definitions, please review. > -- The third "sentence" is missing a verb. > > Second paragraph below list of defined group IDs: > s/symmstric/symmetric/ > > Section 5.2.10, first paragraph, last sentence: > Should "which source HITs" be "which source HIT suites"? If the text is > correct as it stands, perhaps text is needed to say what a source HIT is and > which peer it identifies. > > Section 5.2.11: > > Change "transform" and "transport form" to "transport format" for consistency. > > Middle paragraph: "... parameters and their use are defined ..." > ^^^ > > Last paragraph: > - In the first line, if I understand correctly, add "TRANSPORT_FORMAT_LIST" > before "parameter". > - s/repective/respective/. > - s/TRANSPORT_FORM_LIST/TRANSPORT_FORMAT_LIST/ > - The last sentence is receiver behaviour. Suggest it be rewritten as" > "The receiver MUST ignore ..." > > Section 5.2.13, first paragraph, first sentence might read better as: "The > HIP_MAC_2 is a MAC of the packet and the HI of the sender in the form of a > HOST_ID parameter when that parameter is not actually included in the packet." > > Section 5.2.19: > > Paragraph above the list of Notify message types: perhaps clearer if "with > status Notify Message Types" is rewritten as: "where the Notify Message Type > is in the status range". > > INVALID_SYNTAX: have "denial- of-service". Typically in XML2RFC the unwanted > blank comes when your XML editor has split the phrase at a hyphen, so > "denial-" comes at the end of a line and the remainder on the next line. > Solution is to move "denial-" to the next line too. > > Section 5.2.20, end of second-last sentence: s/parameter/parameters/ > > -------- Material below here added in second version > > Sections 5.3.x: UPDATE implementation is specified to be "MANDATORY", NOTIFY > implementation is specified to be "optional" (sic, lower-case), and nothing > is specified for the other packet types. You probably need to specify > MANDATORY implementation for each packet type except NOTIFY, and that one > should be OPTIONAL. > > Section 5.3.5: > Syntax: should "updated parameters" be added after "ACK,"? > Second-last paragraph: s/forgo/forego/ > > Section 6.1: in processing step 3, do I understand correctly that the packet > to be queued is an user data packet, not a HIP packet? The text should make > this clear. > > Section 6.6 step 4: would "trial counter" be a more accurate term than > "timeout counter", given that the counter is incremented before any timeout > occurs? > > Section 9 (IANA) Hit Suite, start of last paragraph: "For the time being" is > more usual than "At the time being". > > Appendix C.1 (IPv6 example): the correct documentation prefix is > 2001:DB8::/32, not 2001:D88:0. > > C.1 through C.3: The new prefix for ORCHID as defined in rfc4843-bis is > 2001:0000::/23 and no longer 2001:10::/28. > > Appendix E, opening sentence: ORCHID prefix is now 23 bits, not 28 bits. > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
