Re: [IPsec] I-D Action:draft-ietf-ipsecme-ikev2-fragmentation-03.txt

["Valery Smyslov" <[e-mail]>]

Hi Tero,

> Valery Smyslov writes:
>> There is no field "id" in IKEv2 Fragmentation Payload - just Fragment 
>> Number
>> and Total Fragments. But Total Fragments field plays a dual role if
>> peer uses PMTU discovery - i.e. tries several fragment sizes.
>> In this case Total Fragments allows to distinguish between fragments from
>> different sets (assuming that trying smaller fragment size results in
>> increasing number of fragments).
>
> This was the thing that confiused me when reading the document. The
> section 2.5 should explain the Total Fragments much better. I.e.
> change the definition something like:
>
>   o  Total Fragments (2 octets) - number of fragments original message
>      was divided into. If original message is resent with different
>      fragmentation boundaries this value MUST be larger than the
>      previously used Total Fragments value, i.e the responder will
>      use this to detect whether the incoming message belongs to the
>      group of packets fragmented in same way. This field MUST NOT be
>      zero.
>
> Or something like that.

OK.

> Also I do not understand why the Fragment Number and Total Fragments
> start from 1. I think it would be more logical to have first fragment
> be 0, second 1, and the Total Fragments would be The Last Fragment#
> field. That way we do not need to have special check that fragment
> needs to be ignored if Fragment Number or Total Fragments are 0.

That's a matter of taste. Actually, I wanted to leave one special value - 
zero,
just in case the protocol needs to be extended in the future.
I have currently no idea how it can be extended (if ever), but anyway
it is a good practice in protocol design to leave this possibility.

> In general I think the sending and receiving rules should be explained
> a bit more.
>
> For example the
>
>   o  Check message validity - in particular, check whether values of
>      Fragment Number and Total Fragments in Encrypted Fragment Payload
>      are valid.  If not - message MUST be silently discarded.
>
> should be changed to say:
>
>   o  Check message validity - in particular, check whether values of
>      Fragment Number (must be <= Total Fragments) and Total Fragments
>      (must be >= previously seen Total Fragments for this message) in
>      Encrypted Fragment Payload are valid. If not - message MUST be
>      silently discarded.

OK.

> It should clearly say that if Total Fragments is less than previously
> seen then this fragment needs to be discarded.
>
> Now when I am reading th text several times through I see that all the
> required things are there, but when I was reading it first time
> through it was really hard to parse what is going on, and especially
> as I missed the dual role of the Total Fragments I got confused.
>
> It might be enough to just add more text in the Total Fragments
> description, clearly stating that it has dual role, i.e. telling what
> is the max fragment number of this message, and to separate the
> different fragmentation boundaries. My text above does not yet
> properly describe that either, so some better text is needed.

OK, I'll try to explain in more details.

Thanks,
Valery.