On 09/23/2012 11:35 AM, René Hummen wrote:
Hi all,

I had a close look at draft-ietf-hip-rfc5201-bis-09 and found some
technical as well as editorial issues that I consider worth discussing
and fixing. Please note that especially the technical issues regarding
the used packet space may not be considered problematic in case of
everyday Internet connections. However, resource-constrained
environments as targeted by RPL, 6LowPAN, CoAP, and HIP DEX have hard
payload limits. Here, a few bytes of additional packet size can
determine whether packets need to be fragmented or not. Therefore, I
think, we should also consider these scenarios in the basic HIP
specification that is shared by all HIP variants.

I structured my feedback into different topics highlight by ###.


Technical issues:
------------------------

### R1 Counter ###

4.1.4.  HIP Replay Protection
"The R1 counter SHOULD NOT roll over."

No explanation is given what implementors should do when a roll over
occurs. I suggest to add the following text:

"If a roll over is detected, the associated HIs SHOULD be removed and
new ones should be generated."

Note, however, that such a change would also invalidate ACLs and HI-IP
lookup information based on the original HIs.

Well it would take a lot of HIP packets to roll over, but I guess I can see it on a big server. So thought for implementing. I can see the recommended behaviour.



### HIP Checksum ###

5.1.1.  Checksum

What is the reason for using the IP-based pseudo-headers here? If I am
not overlooking something, non-HIP-aware NATs on the communication
path effectively break the HIP checksum. I know that this is the way
how TCP/IP pseudo-headers are defined. Still, I suggest to only
checksum the HIP header and the HIP parameters to avoid problems in
the future and to maintain layer separation. What happens if HIP is
used in non-IP environments?

Others can best comment on this.



### Decreasing the per-packet overhead ###

This is a great proposal, but we would have to net out the actual impact of these changes.

First for BEX: Work with ECDSA p-256 HIs and ECDH p-256. Need to use (x,y) coordinate formats for both. What other parameters do we need to 'set' to get to actual packet size. Then give the packet size and see what we save.

For DEX it is a bit simpler, but need to go through the same exercise.

At this point I am reluctant to make a general change for BEX. Perhaps a specific variant of each of these for DEX (as I have already have had to make other changes).


4.1.2.  Puzzle Exchange
"Using the Opaque data field in the PUZZLE (see Section 5.2.4), in an
ECHO_REQUEST_SIGNED (see Section 5.2.20) or in an
ECHO_REQUEST_UNSIGNED parameter (see Section 5.2.21), the Responder
can include some data in R1 that the Initiator MUST copy unmodified in
the corresponding I2 packet."

5.2.4.  PUZZLE
"The Opaque and Random #I field are not covered by the HIP_SIGNATURE_2
parameter."

Especially in resource-constrained environments such as targeted by
HIP DEX, each byte that needs to be transmitted is valuable (from a
power perspective) and may lead to fragmentation at the lower layers.
Hence, I suggest removing the 2 bytes of opaque value from the PUZZLE
parameter and to add +2 to the type number (resulting in 259) for the
new PUZZLE type. If the Responder has to transfer state in an unsigned
fashion, we already provide the ECHO_REQUEST_UNSIGNED parameter for
this purpose. Furthermore, the opaque value does not seem to be
necessary, as HIPL currently does not use it.


5.2.5.  SOLUTION

Here, I suggest removing 1 byte of reserved space and the 2 byte,
echoed opaque value for the same reason presented above. The type
number could be moved to 323 (SOLUTION + 2). If we need to modify the
puzzle parameter for some reason in the future, I suggest to design a
new parameter instead of using the reserved field. That is how new
semantics are handled for any other parameter as well (see MAC_X).


5.2.3.  R1_COUNTER
"It SHOULD be included in the R1 (in which case, it is covered by the
signature), and if present in the R1, it MUST be echoed (including the
Reserved field verbatim) by the Initiator in the I2 packet."

Similar to the SOLUTION parameter, I suggest removing the 4 bytes of
reserved space from the R1 counter parameter and to add +2 to the type
number (resulting in 131) for the new R1_COUNTER type.


I understand that these changes break parameter compatibility with
existing v1 implementations. However, I do not consider a new
parameter layout problematic as the semantics won't change
considerably allowing for code re-use in most cases.


Editorial changes:
------------------------
thanks for these, I will work with my co-authors on them.


1.3.  Memo Structure
"Finally, Sections 7, 8, and 9 discuss policy, security, and IANA
considerations, respectively."

8 and 9 lack internal references to the respective sections in the document.


4.1.7.  HIP Downgrade Protection
"In contrast to the first version of HIP [RFC5201],the version 2 of [...]"

There is a space missing after the comma:
"In contrast to the first version of HIP [RFC5201], the version 2 of [...]"


### HIP Header Length ###

5.1.  Payload Format
"The Header Length field contains the combined length of the HIP
Header and HIP parameters in 8-byte units, excluding the first 8
bytes."

5.2.21.  ECHO_REQUEST_UNSIGNED
"The creator of the ECHO_REQUEST_UNSIGNED (end-host or middlebox) has
to create the Opaque field so that it can later identify and remove
the corresponding ECHO_RESPONSE_UNSIGNED parameter."

5.3.2.  R1 - the HIP Responder Packet
"The signature is calculated over the whole HIP packet, after setting
the Initiator's HIT, header checksum, as well as the Opaque field and
the Random #I in the PUZZLE parameter temporarily to zero, and
excluding any parameters that follow the signature, as described in
Section 5.2.15."

These three sections indicate the following problem with the HIP
header length field:
The sender generates the packet, sets the header length, and
signs/MACs the packet. A middlebox afterwards adds an
ECHO_REQUEST_UNSIGNED parameter to the packet. However, this
invalidates the HIP header length information resulting in a broken
packet. If the middlebox modifies the HIP header length value, it
invalidates the signature/MAC.

However, following the references to:
     6.4.1.  HMAC Calculation
     6.4.2.  Signature Calculation
... clarifies the issue I just described:
"In the HIP header, the Header Length field value is calculated to the
beginning of the [HIP_SIGNATURE | HIP_MAC] parameter."

Hence, I suggest the following text in Section 5.3.2 in order to
prevent misconceptions by implementors:
"The signature is calculated over the HIP packet as described in
Section 5.2.15."

Again thank you and will review with co-authors.



### References ###

12.  References

References to HIP-related documents refer to old revisions (e.g.,
I-D.ietf-hip-cert).

oops.  Thanks.  will fix. (and there might be a -bis on that, we shall see).


_______________________________________________
Hipsec mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/hipsec

Reply via email to