Subject: [IPsec] FW: Last call comments for ROHCoIPsec: draft-ietf-rohc-hcoipsec, draft-ietf-rohc-ikev2-extensions-hcoipsec, draft-ietf-rohc-ipsec-extensions-hcoipsec

From: <[email-masked]>
Date: Tue, 15 Sep 2009 20:03:54 +0200

(Forwarding to IPSECME list, too. I would encourage other WG
members to review these drafts and comment, too. -Pasi)

> -----Original Message-----
> From: Eronen Pasi (Nokia-NRC/Helsinki)
> Sent: 14 September, 2009 12:13
> To: [email-masked]; [email-masked]'
> Cc: [email-masked]'
> Subject: Last call comments for ROHCoIPsec: draft-ietf-rohc-hcoipsec,
> draft-ietf-rohc-ikev2-extensions-hcoipsec, draft-ietf-rohc-ipsec-
> extensions-hcoipsec
> 
> I've reviewed the ROHCoIPsec draft set (not wearing any hats), and
> have couple of comments. Most important first:
> 
> 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.
> 
> 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...
> 
> 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).
> 
> 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.
> 
> 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).
> 
> 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.
> 
> 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?
> 
> 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).
> 
> 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.
> 
> 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?
> 
> 11) ikev2-extensions, Section 4: "6-65536 Unassigned" should be
> "6-32767 Unassigned".
> 
> Best regards,
> Pasi