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
_______________________________________________
Ietf@xxxxxxxx
https://www.ietf.org/mailman/listinfo/ietf