Hi Tom, thanks for your review. The authors (Tom H. in particular) will be addressing your comments shortly.
Cheers, Gonzalo On 16/06/2014 4:02 AM, Tom Taylor 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
