I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
Please wait for direction from your document shepherd
or AD before posting a new version of the draft.
Document: draft-ietf-hip-hiccups-02
Reviewer: Elwyn Davies
Review Date: 15 June 2010
IESG Telechat date: 17 June 2010
Summary:
Almost ready for Experimental standard. A number of minor issues (including
those noted by Lars Eggert) plus a fair number of editorial nits (mostly
non-use of in/definite articles).
Major issues:
None.
Minor issues:
s2: The commentary on the MIC is somewhat difficult to parse. I think the
phrase 'and thus need be protected otherwise against tampering' can be misread
- I presume it means that some additional means are needed to secure the MIC
against tampering. e.g., 'and thus it needs additional means to ensure that it
has not been tampered with during transmission.'
s3.1.2. Length: Specified here as the 'length of the parameter' which could
be confusing. RFC 5201 specified that 'Length' is the length of the Contents -
the length of the parameter is derived from it by the algorithm in s5.2.1. of
RFc 5201. Suggest 'length of contents' instead.
s3.2: the term 'half-stateless' is not used in RFC 5201 and I can't find any
other definition of what it means (I can maybe guess!) I think some additional
explanation is needed to backup the term.
s4: In all the packet format specifications the HOST_ID is marked as optional,
but as I understand it this is the public key for the calculation of the
HIP_SIGNATURE. Later (s5.2, item 3) we discover that the public key can be
supplied out of band as an alternative to being carried in the packet. It
would be helpful to explain in the description why it is optional, and make it
explicit that it is interpreted as a public key.
s4: The packet definition does not cater for the potential multiple PAYLOAD_MIC
items allowed by the forllowing words.
ss4.1-4.4: Is there some logic behind the selection of the proposed Type
numbers? Note that the TRANSACTION_ID parameter is taken out the range reserved
for base protocol extensions whereas the others are taken from the first come,
first served range, but not in sequence. Is there a reason?
ss4.1, 4.2, 4.3: Should note explicitly that these parameters are flagged as
CRITICAL.
ss4.1, 4.2: how is the sequence number encoded? Presumably unsigned integer in
network byte order.
s4.2: Why is the length variable? The content is specified as one 32 bit
sequence number - surely the length is fixed at 4.
ss4.1, 4.2: worth noting that no padding is needed (assuming there really is
only one sequence number in s4.2) - see the comment on s5.1.
s4.3: sorry to be pedantic, but what happens if the payload data is less than 8
bytes/octets in length?
s4 and s4.4: S4 says the TRANSACTION_ID is a number. If so then s4.4 needs to
specify how it is encoded. However the TRANSACTION_ID is really just a bit
pattern - interpretation as a number is not necessarily our business.. but the
octet order needs specifying in any case.
s5.1, para 3: 'One ACK_DATA parameter MUST contain one more sequence numbers of
the HIP DATA packets being ACKed.': I cannot parse this. I think it is trying
to say that one ACK_DATA can ack multiple SEQ_DATA items by adding all their
sequence numbers to the ACK_DATA parameter. However the previous sentence
implies only one SEQ_DATA is acked per ACK_DATA and the specification in s4.2
is ambiguous. Please get your act together here!
s5.3.2: with reference to the previous comment, s5.3.2 (initially) implies only
one sequence number in a given ACK_DATA ('The sequence number reported in an
ACK_DATA..'). but the last sentence seems to indicate that there might have
been several.
s5.2, item 5: Exponential backoff MUST be used.
s10.1: Needs a normative reference for the IPv6 next header registry
(definition).
Nits/editorial comments:
General: consistency between use of bytes and octets (octets preferred).
e.g., s4.4 uses octets.
s1, para 2, sentence 2: s/HIP Data packet has/The HIP Data packet has/
s1, para 2, sentence 4: s/aimed/intended/
s1, para2, sentence 5: s/ESP/the ESP/ - and ESP needs expanding.
s2, para 2: s/MIC does/The Mic does/, s/thus need be/thus needs to be/
s3.1.1, last sentence: s?there is either more extensions header or data?there
are either more extension headers and/or data?
s4 needs a reference to the notation conventions used in RFC 5201 (s2.2 of RFC
5201).
s4, next to last para: s/one or more PAYLOAD_MIC parameter/one or more
PAYLOAD_MIC parameters/, s/Hash algorithm used to generate MIC is same/The hash
algorithm used to generate the MIC is the same/
s4, last para: s/receiver/the receiver/
s4.3, 'Next header': s/The values for are/The values for this field are/. A
specific reference for the registry is needed (from the IPv6 protocol).
s4.3, 'Payload MIC': s/points to/points/, s/used function/function used/
s5.1, para 1: s/contain SEQ_DATA or ACK_DATA parameter if both parameters are
missing then packet MUST be dropped as invalid/contain at least one of a
SEQ_DATA or an ACK_DATA parameter; if both parameters are missing then the
packet MUST be dropped as invalid/
s5.1, para 2: s/one or more PAYLOAD_MIC parameter otherwise packet MUST be
dropped/one or more PAYLOAD_MIC parameters or otherwise the packet MUST be
dropped/
s5.2 items 2 and 3: There are lots of definite and indefinite articles missing.
s5.3, first sentence: s/decision/a decision/
s5.3, end of para 1: s/responsible of/responsible for/
s5.3.1, para 2: s/OK/adequate/
s5.3.1, para 3: s/verification fails/verifications fail/
s5.3.2, para 2: s/an earlier sent/a previously sent/
__________________________
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art