Re: [rohc] Review of the RoHC over IPsec drafts

Hi, Yoav,

Thanks for your comments and review of our drafts.  Please find our
responses below.

> 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.

Thanks!

> *	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 reason for this additional integrity-check is to mitigate scenarios
where intentional (severe) packet reordering or packet loss in the
unprotected network can cause the decompressor to invalidly decompress
packet headers (incorrectly reconstruct fields of a packet header), and
forward them into the protected domain.  

To mitigate this, we add the additional integrity algorithm to verify
that the decompression operation is successful.  

Note that since we are adding an additional ICV, the overall efficiency
gain of header compression is reduced.  It may be desired not to
negotiate this additional RoHC integrity algorithm for the RoHC-enabled
SA, but this comes with the risk that compressed headers may be
invalidly decompressed and forwarded into the protected domain.

> 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.

I think that Pasi responded to this in his email.  We want to be able to
ensure (potentially to the level of the cryptographic strength
negotiated for IPsec) the integrity of all bits in the decompressed
header.  

Note, however, that the values negotiated for the integrity-check are
based on Integrity Algorithms defined in the Transform Type 3 registry.
Therefore, an implementation could negotiate the value of "0" (no
integrity-algorithm), and as you mentioned, rely on the built-in RoHC
CRC.  

> *	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.

We'll clarify the introduction based on your comment.

What we intended to indicate was that for RoHCoIPsec, there may be
multiple physical hops between compressor and decompressor (even though
it looks like a single hop from the IPsec perspective).   Therefore,
RoHCv2 profiles that are tolerant to packet reordering/loss should be
used. 

Initial profiles of RoHC did not necessarily account for significant
packet reordering scenarios, since RoHC was initially designed to
operate over a single physical hop.

> *	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.

It's about the problem in general.  The second sentence was intended to
indicate that an ESP tunnel mode SA provides partial traffic flow
confidentiality, but that tunneling incurs additional packet overhead.

I'm inclined to leave the text as-is; let me know if you still think it
needs to be modified.

> *	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?

Fair comment.  We'll delete this paragraph.

> *	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.

This is more of a question of policy rather than functionality.  For
example, the middlebox may have a policy to drop ESP-NULL encrypted
packets whose inner-headers cannot be parsed.  On the other hand, the
middlebox could have a policy that says inspect all ESP-NULL encrypted
packets, drop packets whose inner headers cannot be parsed, except for
those packets whose the NextHeader field is set to "RoHC".

> *	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

Agreed.  Later in the paragraph, we indicate:

"  Note that the requirement
   for two SAs aligns with the operation of IKE, which creates SAs in
   pairs by default."

We'll modify the last sentence of the paragraph to indicate that the
decompressor associated with an inbound SA will relay feedback to the
compressor associated with the corresponding outbound SA.

> 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.

Sure, we can add a figure illustrating the Notify payload.  We actually
had this illustration in the -02 version of the draft, and then removed
it since it was identical to Figure 16 of RFC 4306.  But, we'll add it
back. 

> *	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.

Agreed--we have updated the draft to indicate that this bit if off.  We
also removed the definition of the content of the reserved field.  

> *	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.

Agreed--this is fixed.

Best Regards,
Emre 
_______________________________________________
Rohc mailing list
Rohc@ietf.org
https://www.ietf.org/mailman/listinfo/rohc