[rohc] Review of the RoHC over IPsec drafts

Hi all

At Yaron's request I've gone over the three drafts related to RoHC  
over IPsec tunnels.

The three drafts are:
draft-ietf-rohc-hcoipsec - integration of RoHC over IPsec SAs
draft-ietf-rohc-ikev2-extensions-hcoipsec - IKEv2 extensions to  
negotiate RoHC over IPsec
draft-ietf-rohc-ipsec-extensions-hcoipsec - IPsec extenstions

Overall the drafts drafts seem good, and in my opinion could offer a  
substantial benefit for IPsec tunnels, especially where the bandwidth  
to one or both endpoints is constrained. Here's my comments.

The IKE & IPsec drafts talk about the use of integrity algorithms to  
verify that decompression was successful. According to section 2.1 of  
the IKEv2 draft, these algorithms are the same algorithms (with  
associated keys) as those used in IPsec. I can't figure out why you  
would want this. The entire packet, compressed header and data  
included, is already integrity protected by IPsec. An attacker can't  
flip any bits because of the ESP ICV, so the RoHC ICV is simply there  
to detect decompression errors. I don't see why we need to exceed the  
recommendation in RFC 4995 to use CRC. Even if you have decided that  
you want to upgrade error-detection ICV to a cryptographically strong  
one, HMAC doesn't offer any benefit that I can see - you could use  
straight MD5 or SHA-1 without any key, but I still don't see why CRC  
is not good enough.
The introduction to draft-ietf-rohc-hcoipsec-08 says that "However,  
since current specifications for RoHC detail its operation on a hop-by- 
hop basis, it may require extensions to enable its operation over  
IPsec SAs.". This is not explained later. An IPsec tunnel is very much  
like a single hop (for the tunneled traffic), so I don't see why  
special extensions would be needed.
Section 4 of draft-ietf-rohc-hcoipsec-08 needs some rewording. Is the  
second sentence about the TFC feature of IPsec? About the padding  
feature of RoHC?  About the problem in general? You might want to add  
discussion about whether it is recommended to use RoHC padding or  
IPsec TFC to avoid the problem.
Section 5.2 of draft-ietf-rohc-hcoipsec (last paragraph) has some  
strange text about ESP-NULL, which I don't understand. Why should ESP- 
NULL be any different than any other ESP method?
There is, however, something to consider about ESP-NULL. Some  
middleboxes may want to inspect traffic that is encrypted with ESP- 
NULL (see draft-grewal-ipsec-traffic-visibility-00). It's possible  
that RoHC will cause those middleboxes to drop all traffic that is  
compressed, because they won't be able to parse the compressed headers.
Section 6.1.2  says "f the RoHC protocol requires bi-directional  
communications, two SAs must be instantiated between the IPsec  
implementations." This is a non-issue, since IKE creates SAs in pairs  
anyways. So I don't see the point in the recommendation to use only  
non-feedback profiles. You may want to mention that the decompressor  
needs to relay feedback to the appropriate compressor

Regarding the IKEv2 draft, I only have several comments about section  
2.1:
It's common for IKEv2 drafts to have at the beginning a figure of the  
entire payload and then explain the various fields.
The explanation of the "Critical" bit is incorrect. It does not refer  
to the content of the payload, only to the type of the payload, in  
this case "Notify". The bit MUST be off for Notify payloads, because  
all IKEv2 implementations must support it. Similarly, there's not  
reason to define the content of the reserved bits, because they're the  
same as for any payload.
Protocol ID field: According to the first paragraph of 2.1, the  
notification is only in CCSA and IKE_AUTH. So there's no case where  
this notification is used outside of the creation of the child SA, and  
there's no need to ever set the protocol ID.