Last Call Review of draft-ietf-hip-rfc5201-bis-14
review-ietf-hip-rfc5201-bis-14-genart-lc-taylor-2014-06-16-00

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.