> 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.
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec