Subject: RE: Last call comments for ROHCoIPsec: draft-ietf-rohc-hcoipsec, draft-ietf-rohc-ikev2-extensions-hcoipsec, draft-ietf-rohc-ipsec-extensions-hcoipsec

From: Emre Ertekin <[email-masked]>
Date: Sun, 20 Sep 2009 20:13:42 -0700

Hi, Pasi,

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

> 1) None of the drafts really describe the reason why the ROHC ICV is
> included. It was not present in the early drafts, and was added after
> long and complex discussions. I would strongly encourage summarizing
> those discussions in one of the drafts -- otherwise, the reader cannot
> really figure out why the ROHC ICV is included (since the packets are
> already protected by the AH/ESP ICV), and what risks omitting the ROHC
> ICV might have.

Sure, we can add some description on this.  How much detail are you looking for?

Perhaps we can add the following text in a separate section under
Section 6.1 of the framework draft, entitled "Motivation for the ROHC
ICV":

	Although ROHC was designed to tolerate packet loss and reordering,
      the algorithm does not guarantee that packets reconstructed at the
      decompressor are identical to those constructed at the compressor.  As
      stated in Section 5.2 of [RFC 4224], the consequences of packet
      reordering between ROHC peers may include undetected
decompression failures,
      where erroneous packets are constructed and forwarded to upper layers.

	When using IPsec integrity protection, a packet received
      at the egress of an IPsec tunnel is identical to the packet that was
      processed at the ingress (given that the key is not compromised, etc.).

	When ROHC is integrated into the IPsec processing framework,
      the ROHC processed packet is protected by the AH/ESP ICV.  However,
      bits in the original IP header are not protected by this ICV.  Therefore,
      under certain circumstances, erroneous packets may be constructed and
      forwarded into the protected domain.

	To ensure the integrity of the original IP header within the
      ROHCoIPsec-processing model, an additional integrity check may be applied
      before the packet is compressed.  This integrity check will ensure that
      erroneous packets are not forwarded into the protected domain.  The
      specifics of this integrity check are documented in Section 3.2
of [IPSEC-ROHC].

> 2) ipsec-extensions, Section 3.3, doesn't really correctly describe
> the MTU-related processing in RFC 4301. The three cases refer to IPsec
> implementations that both process unprotected ICMP messages and
> actually receive them (they're often filtered in the network), and
> thus have a Path MTU estimate value.  But an IPsec implementation that
> doesn't process (or receive) unprotected ICMP messages does not even
> have a Path MTU estimate value...

This is a good point.  I would assume that if the IPsec implementation
does not have a Path MTU estimate value, then it SHOULD NOT attempt to
segment packet headers, as it does not have full insight into the MTU
in the unprotected domain.  Is this in line with your thoughts?

> 3) According to RFC 4224, ROHC segmentation does not work over
> reordering channels. Thus, it seems suggesting that ROHC segmentation
> could be used instead of pre-encryption fragmentation (e.g.
> ipsec-extensions, Section 3.3) -- and in general, allowing
> segmentation at all -- seems misguided (it's unnecessary complexity
> that would be IMHO best left out).

Although I agree that ROHC segmentation is not a good idea, it is an
optional functionality in the spec.  The implementer can decide
whether or not they want to code it.

If a vendor chooses not to implement the functionality, they can
hardwire the MRRU channel parameter to zero.

> 4) None of the drafts have any RFC 2119 keywords (MUST/SHOULD/etc).
> They SHOULD use those to make it less ambiguous what is the required
> behavior (and what is optional) to claim compliance with these drafts.

OK, we will take a run through the IKEv2 and IPsec extensions drafts
to account for these keywords.  Not the framework draft though, since
the draft is intended to be informational.

> 5) In ikev2-extensions, Section 2.1.2 it is not very clear which of
> the attributes are just one-way notifications (the sender of the
> attribute tells the other end what it can handle), and which are
> negotiated (the initiator tells the other end what it supports; the
> responder then selects one, and tells what it selected).
>
> I would suggest adding text like "Note that different ATTRIBUTE_NAME
> value can be used in each direction" for those attributes that are
> just one-way (I think at least MAX_CID, ROHC_PROFILE -- for MRRU and
> ROHC_ICV_LEN, I don't know which way they're supposed to work).

OK, sounds good.  We will add this clarification to the parameters.

FYI, we intended to have the ROHC_INTEG algorithm as the only
negotiated parameter.  All other parameters are one-way negotiations.
Please let us know if you see any issues with this.

> 6) ikev2-extensions, Section 2.1.2, says "The key for this Integrity
> Algorithm is computed using the same method as is used to compute
> IPsec's Integrity Algorithm key ([IKEV2], Section 2.17)."  I don't
> think this is sufficient to get interoperable implementations; more
> details are needed.

Could you clarify why this is not sufficient?

> In addition, there's couple of places that probably need some
> clarifications and/or minor fixes:
>
> 7) ikev2-extensions, Section 2.1.2, ROHC_ICV_LEN: The text talks about
> "announcing their preference"; how is the actual ICV length determined
> from these preferences?

Since this parameter is one-way, "preference" is a bad word choice.
Perhaps we can change the statement to "Both the initiator and
responder announce a single value...".

> 8) ikev2-extensions, Section 2.1.2, ROHC_INTEG: Should also describe
> what happens if the responder doesn't accept any of the algorithms
> proposed by the initiator (not doing ROHC at all would be one obvious
> alternative; NO_PROPOSAL_CHOSEN another).

Since this is a negotiated parameter, I'm leaning towards adding text
that indicates that ROHC is not done at all.

> 9) ikev2-extensions, Section 2.1.1, says "The most significant bit in
> the
> field is the Attribute Format (AF) bit." No, according to Figure 2
> "AF" is a separate field, not part of the "ROHC Attribute Type" field.

Thank you for catching this oversight.  We will update the text accordingly.

> 10) ipsec-extensions, Section 3.2, says "The authentication data must
> not be included in the calculation of the ICV." This is very vague,
> considering that we have several different authentication data/ICVs
> here. Is the intent to say "The ROHC ICV field is not included in the
> calculation of the ROHC ICV", or something else?

Yes, this is what we intended.  We will update the text with your
proposed wording.

> 11) ikev2-extensions, Section 4: "6-65536 Unassigned" should be
> "6-32767 Unassigned".

Thank you for catching this oversight.  We will update the text accordingly.

Best Regards,
Emre

> Best regards,
> Pasi

---

From: <[email-masked]>
Date: Tue, 22 Sep 2009 12:35:34 +0200

Emre Ertekin wrote:[mailto:[email-masked]]

> > 1) None of the drafts really describe the reason why the ROHC ICV
> > is included. It was not present in the early drafts, and was added
> > after long and complex discussions. I would strongly encourage
> > summarizing those discussions in one of the drafts -- otherwise,
> > the reader cannot really figure out why the ROHC ICV is included
> > (since the packets are already protected by the AH/ESP ICV), and
> > what risks omitting the ROHC ICV might have.
> 
> Section 6.1 of the framework draft, entitled "Motivation for the
> ROHC ICV":
> 
> 	Although ROHC was designed to tolerate packet loss and
> 	reordering, the algorithm does not guarantee that packets
> 	reconstructed at the decompressor are identical to those
> 	constructed at the compressor.  As stated in Section 5.2 of
> 	[RFC 4224], the consequences of packet reordering between ROHC
> 	peers may include undetected decompression failures, where
> 	erroneous packets are constructed and forwarded to upper
> 	layers.
> 
>       When using IPsec integrity protection, a packet received at
>       the egress of an IPsec tunnel is identical to the packet that
>       was processed at the ingress (given that the key is not
>       compromised, etc.).
> 
>       the ROHC processed packet is protected by the AH/ESP ICV.
>       However, bits in the original IP header are not protected by
>       this ICV.  Therefore, under certain circumstances, erroneous
>       packets may be constructed and forwarded into the protected
>       domain.
> 
>       ROHCoIPsec-processing model, an additional integrity check may
>       be applied before the packet is compressed.  This integrity
>       check will ensure that erroneous packets are not forwarded
>       into the protected domain.  The specifics of this integrity
>       check are documented in Section 3.2 of [IPSEC-ROHC].

Looks very good!
 
> > 2) ipsec-extensions, Section 3.3, doesn't really correctly
> > describe the MTU-related processing in RFC 4301. The three cases
> > refer to IPsec implementations that both process unprotected ICMP
> > messages and actually receive them (they're often filtered in the
> > network), and thus have a Path MTU estimate value.  But an IPsec
> > implementation that doesn't process (or receive) unprotected ICMP
> > messages does not even have a Path MTU estimate value...
> 

Yes.
 
> > reordering channels. Thus, it seems suggesting that ROHC
> > segmentation could be used instead of pre-encryption fragmentation
> > (e.g.  ipsec-extensions, Section 3.3) -- and in general, allowing
> > segmentation at all -- seems misguided (it's unnecessary
> > complexity that would be IMHO best left out).
> 

OK, let me phrase my question in another way: why does the spec contain 
a feature that does not really work? (Even as optional feature?)
 
> > 4) None of the drafts have any RFC 2119 keywords
> > (MUST/SHOULD/etc).  They SHOULD use those to make it less
> > ambiguous what is the required behavior (and what is optional) to
> > claim compliance with these drafts.
> 

Being "Informational" (vs. Proposed Standard) RFC has nothing to do with
the question -- many Informational RFCs do use RFC 2119 keywords, and
there's nothing special about that.

To me, it looks like the framework draft has normative statements
(things implementations are required or recommended to do in order
to get interoperability), too, so 2119 keywords would be appropriate 
(and actually, it could be Standards Track, too).

> negotiated parameter.  All other parameters are one-way
> negotiations.  Please let us know if you see any issues with this.

Seems OK to me.

> Could you clarify why this is not sufficient?

If it's computed using exactly the same method as the IPsec Integrity
Algorithm Key, it would be the *same* key, and that's certainly not
the intent here.

Perhaps something like "The keys (one for each direction) for this
Integrity Algorithm are derived from the IKEv2 KEYMAT (see [IKEV2],
Section 2.17). For the purposes of this key derivation, ROHC is
considered to be an IPsec protocol."?

Best regards,
Pasi

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 22 Sep 2009 12:52:19 +0200

> a feature that does not really work? (Even as optional feature?)

Well, it actually does work.

RFC 4224, section 5.2.1 tells us that when a channel is reordering and  
you don't notice, packets will be lost.
Now IPsec is a strange animal: It is reordering, but the order can be  
reconstructed from the sequence number.
So a reassembler could (here really: SHOULD) use that to avoid  
stitching together packets in the wrong order and then discarding them.

Is ROHC segmentation "better" than IP fragmentation?
It certainly has fewer of the problems of IP fragmentation.
It does require throwing some CPU at the data (it uses a CRC).

Because header compression creates a variable-MTU channel, being able  
to segment in a pinch is a boon.

Since the ROHC framework has the functionality, and it is not hard to  
make it work on IPsec, I would favor retaining it.

Gruesse, Carsten

---

From: <[email-masked]>
Date: Tue, 22 Sep 2009 14:03:07 +0200

Carsten Bormann wrote:

> > OK, let me phrase my question in another way: why does the spec
> > contain a feature that does not really work? (Even as optional
> > feature?)
> 
> RFC 4224, section 5.2.1 tells us that when a channel is reordering
> and you don't notice, packets will be lost.  Now IPsec is a strange
> animal: It is reordering, but the order can be reconstructed from
> the sequence number.  So a reassembler could (here really: SHOULD)
> use that to avoid stitching together packets in the wrong order and
> then discarding them.

Well, the current drafts don't specify any such behavior, so the
feature as it's currently written does not work. (Also, using the
ESP/AH sequence number this way would be very unusual -- there
might be some complications...)

> Since the ROHC framework has the functionality, and it is not hard
> to make it work on IPsec, I would favor retaining it.

Making it work for IPsec might not be impossible, but it does
require additional work beyond what's in the current drafts.

Best regards,
Psai

---

From: <[email-masked]>
Date: Fri, 25 Sep 2009 18:04:24 +0200

Emre Ertekin wrote:

> If we include text that states this behavior, does this address your
> concern?

Depends on the text (certainly more than 1-2 sentences are needed
here)... but it's possible it could address my concern.

> FWIW, I could also think of deployment scenarios where packet
> reordering is minimal (e.g., ROHCOIPsec is instantiated over a single
> [bandwidth constrained] link).  Such scenarios may not even require
> the use of the ESP/AH sequence number to reconstruct ROHC segments.

Perhaps; but in general, IPsec runs over IP, and doesn't know about
the properties of the underlying links (even if it's only one hop,
not all links preserve order always).

Best regards,
Pasi

---

From: Emre Ertekin <[email-masked]>
Date: Fri, 25 Sep 2009 06:25:52 -0700

Hi Pasi,

> (and actually, it could be Standards Track, too).

OK.  I just meant that the framework draft was intended to be guidance
for ROHCOIPsec implementers.  However, since you think there are
benefits to including these keywords, I'll update the draft to include
them.

> considered to be an IPsec protocol."?

Sounds good to me.

BR,
Emre

---

From: Emre Ertekin <[email-masked]>
Date: Fri, 25 Sep 2009 06:28:14 -0700

Hi Pasi,

> might be some complications...)

If we include text that states this behavior, does this address your concern?

Best Regards,
Emre

> Best regards,
> Psai
>
> _______________________________________________
> Rohc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rohc