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.

E.g. if one wants to use it for reducing ESP overhead for small packets
(with no intention to hide traffic flow), and references your document,
then it'll look like he/she uses mechanism explicitly defined for IP-TFS
for some other purpose it wasn't designed for.

Note, that I don't suggest recast/redirect this work from IP-TFS,
I only want to reorganize the document so that the mechanism
itself is defined independently from its primary application (IP-TFS).
In other words:
- make name and abstract more neutral regarding IP-TFS
  (instead "we wanted IP-TFS and we defined a special mechanism for it"
  use "we defined a new mechanism and we described how it can be used for 
IP-TFS").
 - mention at least two possible applications in Introduction (with focus on 
IP-TFS)
- reorganize the document so that first the mechanism is defined with 
   as a generic one and then it's use for IP-TFS is described in details

Note, that all these are editorial changes, but they are essential,
so that if future documents defining non-IP-TFS use case reference this 
document,
it'll be clear that they reference the multi-purpose mechanism and not
the mechanism that was explicitly designed for IP-TFS.

> 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.

> > 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.

> > 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.

> > 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".

> > 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).

> > 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.

> > 21. IANA Considerations.
> >
> > I wonder why newly defined registry "AGGFRAG_PAYLOAD Sub-Type Registry "
> > has an allocation policy "Standards Action". From my understanding
> > this is too restrictive. Why "RFC Required" or "Expert Review" is 
> > insufficient?
> > Out of 256 possible values only 2 are defined and I don't expect
> > a lot of allocation requests for this registry, so why do you need such
> > a restrictive policy?
> 
> You're the second person to suggest this so I've changed it to "Expert 
> Review". :)

Thanks :-)

> > Another issue - I would have mark value 0 as "Reserved" (just in case
> > a special value is needed in future), but it's a matter of taste.
> 
> 0 is a very useful value b/c a typical non-congestion control header can 
> simply be zero'd efficiently. I've
> definitely used this fact in our implementation.

As I said, it's a matter of taste - I prefer to have at least one value
in registry that can never appear on the wire under current specification.
This can help protocol extensibility (and it happened in IPsec
a couple of times). But that's up to you.

> > 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.

Regards,
Valery.

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

Reply via email to