> On Feb 10, 2021, at 3:36 AM, Valery Smyslov <smyslov.i...@gmail.com> wrote:
> 
> Hi Christian,
> 
>>> On Feb 8, 2021, at 6:44 AM, Valery Smyslov <smyslov.i...@gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> I think that in its current form the draft is too focused on a single 
>>> application for
>>> the Aggregation and Fragmentation mode - IP-TFS. From architectural
>>> point of view I'd like to see the draft first defining the mechanism itself
>>> and then describing possible applications for it, focusing on IP-TFS
>>> as the primary application.
>>> 
>>> We discussed this with Christian off the list in length and came to
>>> a good compromise regarding naming of new entities defined in the draft.
>>> After re-reading the draft I still think that its structure of the document 
>>> should be
>>> changed to better decouple mechanism from its applications.
>> 
>> Hi Valery,
>> 
>> Thanks for your continued reviews and suggestions.
> 
> No problem :-)
> 
>> I agree with what Lou with regards to it going too far to recast/redirect 
>> this work any further. I did do a round
>> of changes based on our agreement to help with future uses, and while it's 
>> nice that this work could lead to
>> these uses, those should be documented in another document at this point. 
>> The focus of this work has and
>> should continue to be traffic flow security.
> 
> Here we disagree. My point is that you define a very good mechanism
> that can be used not only for IP-TFS, but for other purposes too.
> I fully understand that IP-TFS was your primary focus and I agree
> the document to remain focused on it. However, if the document
> is kept in its current form any attempt to use it for other applications
> will look as an improper use of mechanism, because the way document
> is organized highly ties the mechanism and its application.

A compromise and to address your concern that other uses might look improper 
I've changed the abstract to include this final sentence:

"The mechanisms defined in this document are generic with the intent of 
allowing for non-TFS uses, but such uses are outside the scope of this 
document."

The introduction to include this final paragraph:

"The mechanisms defined in this document are generic with the intent of 
allowing for non-TFS uses, but such uses are outside the scope of this 
document."

And the "Packets and Data Formats" top level section to start:

"The packet and data formats defined below are generic with the intent of 
allowing for non-IP-TFS uses, but such uses are outside the scope of this 
document."

I believe this addresses your concern about other uses looking improper.

>> I've incorporated your other suggestions from below.
> 
> Please see my comments (I removed parts where we are in concert).
> 
>>> 9. Section 2.2.3:
>>> 
>>>  When using the AGGFRAG_PAYLOAD in conjunction with replay detection,
>>>  the window size for both MAY be reduced to share the smaller of the
>>>  two window sizes.  This is b/c packets outside of the smaller window
>>>  but inside the larger would still be dropped by the mechanism with
>>>  the smaller window size.
>>> 
>>> I wonder why MAY is used here. It should be MUST instead.
>>> As you explained there is no point for the sizes to be different.
>> 
>> They remain different mechanisms and the user may wish to have them treated 
>> differently (e.g., logging
>> replayed packets).
> 
> My question was regarding "MAY" vs "MUST". Even if these are different
> mechanisms, what is the reason for having different window sizes if both 
> mechanisms
> are employed? I may yet understand that reassembly window may be shorter
> then replay window (you will just have a penalty of dropping too old packets
> even when replay protection allows them in), but what may be the reason
> for having reassembly window longer than replay window? If you have some gaps
> at the far left end of reassembly window waiting for missing packets,
> you'll never receive them if replay window is shorter - they will fall
> outside it. So, it's just a waste of resources.

The implementation may wish to allow the user to have replayed packets logged 
(one can have a very large replay window w/o consuming many resources).

>>> 10. Section 2.2.3:
>>> 
>>>  Finally, as sequence numbers are reset when switching SAs (e.g., when
>>>  re-keying a child SA), an implementation SHOULD NOT send initial
>>>  fragments of an inner packet using one SA and subsequent fragments in
>>>  a different SA.
>>> 
>>> Two issues here - first why SHOULD NOT and not MUST NOT?
>>> In general you cannot reliably reassemble packet if it is fragmented over
>>> several SAs, so it will be dropped. Why do you allow this?
>> 
>> Changed to MUST NOT.
>> 
>>> Then, IPsec architecture allows several parallel ESP SAs
>>> to co-exist with the intention that kernel may use any of these SAs to send 
>>> packets
>>> (e.g. for improving performance, see draft-pwouters-multi-sa-performance).
>>> I think you should mention that in this case a care must be taken not to
>>> fragment outgoing packets over several parallel SAs. I.e. if a packet get 
>>> fragmented,
>>> all its fragments must be sent over single ESP SA.
>> 
>> Covered by the switch to MUST NOT.
> 
> Well, this text is explicitly about "sequence numbers are reset when 
> switching SAs".
> I think it's better to generalize it and say that "packet fragmentation MUST 
> not take
> place over different SAs". This will cover both cases - rekeying and parallel 
> SAs.

It's explaining the restriction. This adds information/justification w/o 
changing the actual requirement. I think that's a good thing not bad.

>>> 13. Section 2.3:
>>> 
>>>  While it's possible to
>>>  envision making the algorithm work in the presence of sequence number
>>>  skips in the AGGFRAG_PAYLOAD payload stream, the added complexity is
>>>  not deemed worthwhile.
>>> 
>>> I have trouble understanding applicability of this text to the section
>>> describing Exclusive SA Use. Shouldn't it be in 2..2.3?
>> 
>> It's just referring to what happens (sequence number skips) if one 
>> intermixes which we are disallowing.
> 
> Oh, I see. But I think that it's better to remove this sentence, as it only
> adds confusion and is not important for the mechanism itself.

This also represents removing the justification for the decision, but I agree 
the text is a little hard to comprehend, so I will remove the last 2 sentences.

> 
>>> 17. Section 5.1:
>>> 
>>>  The USE_AGGFRAG notification contains a 1 octet payload of flags that
>>>  specify any requirements from the sender of the message.  If any
>>>  requirement flags are not understood or cannot be supported by the
>>>  receiver then the receiver should not enable use of AGGFRAG_PAYLOAD
>>>  payload type (either by not responding with the USE_AGGFRAG
>>>  notification, or in the case of the initiator, by deleting the child
>>>  SA if the now established non-AGGFRAG_PAYLOAD using SA is
>>>  unacceptable).
>>> 
>>> It is not clear for me from this text whether these flags are negotiated
>>> or independently announced by peers. In other words - are they
>>> independent in both direction or they must be the same?
>> 
>> They are not negotiated as they are "requirements from the sender of the 
>> message" and the receiver either
>> understands and accepts them or doesn't use AGGFRAG_PAYLOADs.
>> 
>> I've changed
>>  "that specify any requirements from the sender"
>> 
>> to
>> 
>> "that specify requirements from the sender"
>> 
>> perhaps that makes it more clear?
> 
> I think even better would be "that specify requirements from the sender of 
> this notification".

OK, changed "of the message" to "of the notification".

> 
>>> 19. Section 6.1:
>>> 
>>>  An IP-TFS payload is identified by the ESP payload type
>>>  AGGFRAG_PAYLOAD which has the value 0x5.
>>> 
>>> Where this value has come from? Isn't it registered with IANA?
>>> We had a long discussion about this in the WG, current
>>> text looks like the value was squatted...
>> 
>> The discussion lead to the understanding that the ESP next-header field is 
>> not actually required to be IP
>> protocol number, and people don't generically use it that way so we are free 
>> to choose whatever non-used
>> value we would like. We chose 0x5 simply b/c that *was* an IP protocol 
>> number that is allocated but will not
>> used; however, this wasn't required -- it's just trying to be smart about 
>> picking a value. :)
>> 
>> I don't think we need to go into this in the document though -- it's enough 
>> that its a valid value to use.
> 
> I still think that some words about this must be added. Otherwise it looks 
> like
> the value 5 has come to you as a miracle and you cannot explain why it is 
> exactly 5
> (and not any other value).

Ok well then I've added "The value 5 was chosen to not conflict with other used 
values". What I don't want to do here, in the packet and data format definition 
section, is get into some distracting explanation about why this value isn't 
conflicting -- this does not help people trying to implement the specification.

>>> 20. Section 6.1.4:
>>> 
>>>  0:
>>>     6 bits - reserved, MUST be zero on send, unless defined by later
>>>     specifications.
>>> 
>>> Add a sentence that these bits MUST be ignored on receipt.
>> 
>> They must not be ignored. If they are set and they and not understood then 
>> AGGFRAG mode will not be
>> enabled as indicated in section 5.1
> 
> Section 5.1 is about USE_AGGFRAG notification, here we talk about
> Reserved bits AGGFRAG_PAYLOAD. How they are related?
> 
> As implementer I have a question - what should I do if these bits
> are non-zero on receipt? The draft is silent about this.

I think maybe you mixed something up in your reading? Section 6.1.4 is:

"6.1.4 <https://tools.ietf.org/html/draft-ietf-ipsecme-iptfs-06#section-6.1.4>. 
 IKEv2 USE_AGGFRAG Notification Message"

It is not about the AGGFRAG_PAYLOAD.

>>> 22. Security Considerations.
>>> 
>>> Add a text that correct functionality of the AGGFRAG mode
>>> requires restoring packets order on the receiver. Since this
>>> is done by utilizing ESP Sequence Number field, the ESP
>>> header must be authenticated, and thus the ESP SA MUST
>>> be created with authentication other than NONE
>>> or with AEAD cipher.
>> 
>> I think this falls under the non-IPTFS use case. I'd like to leave it out as 
>> it would be confusing.
> 
> No, it's about any use case of this mechanism.
> ESP can be used without authentication
> (although it's strictly discouraged) and in this
> if AGGFRAG is employed (regardless of IP-TFS),
> then an attacker can re-order packets on his/her will,
> by playing with SN field, so your reassembly mechanism won't work.

You think I need to state that things are not secure if the user turns off 
authentication?

I would think that's covered already by:

"
   Other than
   the additional security afforded by using this mechanism, IP-TFS
   utilizes the security protocols [RFC4303 
<https://tools.ietf.org/html/rfc4303>] and [RFC7296 
<https://tools.ietf.org/html/rfc7296>] and so their
   security considerations apply to IP-TFS as well.
"

Thanks,
Chris.

> 
> Regards,
> Valery.

Attachment: signature.asc
Description: Message signed with OpenPGP

_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec

Reply via email to