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
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf