Hi Valery,

I think you make a number of good mechanism specific technical points that are worth addressing in the document, but I think that recasting/redirecting this work goes too far.  This work has always been focused on a specific application (TFS) and it's utility beyond that application is certainly a bonus -- the very recent name changes (made at your request) to accommodate such wider usage certainly provides a solid foundation for non-TFS usage.  This all said, I think the usage of the mechanisms defined in this document beyond TFS should be covered in its own document and not shoehorned in to this document.

I support publishing this document, largely as is and without a major focus change, once clarifications and nits are addressed.

Lou

On 2/8/2021 6:44 AM, Valery Smyslov 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.

Below are some comments and ad hoc text proposals.

0. General note:
The text constantly mixes up the mechanism (Aggregation and Fragmentation Mode)
with one of its application (IP-TFS). IP-TFS is not the only possible 
application for this mechanism and
when the mechanism itself is being described IP-TFS must not be mentioned 
unless the feature being described
is important for this application (like congestion control). E.g., the payload 
must be
named AGGFRAG (or something like that) payload and not IP-TFS payload. And so 
on.

1. I propose to rename the document:

"Aggregation and Fragmentation Mode for ESP and its Use for IP Traffic Flow Security 
(IP-TFS)"
(with short form "AGGFRAG ESP Mode and its use for IP-TFS").

2. Abstract:

I propose to change it as follows:

    This document describes a mechanism for aggregation and fragmentation of IP 
packets
    when they are being encapsulated in ESP payload. This mechanism can be used
    for various purposes, such as: improving performance when IP traffic flow 
confidentiality is in use,
    decreasing encapsulation overhead for small IP packets and so on. The 
document
    also describes the use of this mechanism for IP traffic flow confidentiality
    in detail.

3. Introduction:

I propose to leave the first two paras as is and to rewrite the rest of the 
section:
Modify text to clearly separate mechanism from its applications
(e.g. s/The IP-TFS solution provides/The Aggregation and Fragmentation mode 
provides).
Introduce IP-TFS acronym as one possible application for the mechanism
and the one that is the draft is focused on. Move last para of Section 2.1 to 
Introduction
to mention others possible applications.

4. Section 2:

I believe the structure of this section is incorrect. It should first describe 
the
Aggregation and Fragmentation mode (sections 2.2-2.4 and some text from 2.1)
and only then describe the use of AGGFRAG for IP-TFS (section 2 up to 2.1).
The name of the section should be "The Aggregation and Fragmentation Mode of 
Operation".

5. Section 2.1:

    This is accomplished using a new Encapsulating Security Payload (ESP,
    [RFC4303]) type which is identified by the number AGGFRAG_PAYLOAD
    (Section 6.1).

RFC 4303 doesn't define what is "Encapsulating Security Payload type ".
You should clarify this in more detail and also mention that
the AGGFRAG_PAYLOAD number must be in the ESP "Next Header" field.

6. Section 2.2:

s/Figure 1: Layout of an IP-TFS IPsec Packet/Figure 1: Layout of ESP Packet in 
Aggregation and Fragmentation Mode

7. Section 2.2.1:

s/ Figure 2: Layout of IP-TFS data block/ Figure 2: Layout of Data Block

8. Section 2.2.2:

    Only
    when there is no data to encapsulated is end padding required, and
    then an explicit "Pad Data Block" would be used to identify the
    padding.

Nit: isn't better:

s/no data to encapsulated/no data to encapsulate

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.

And please s/b\/c/because

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?

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.

11. Section 2.2.4:

IKEv2 always creates ESP SAs in pairs, so you don't need to send empty 
AGGFRAG_PAYLOAD payloads
over non-AGGFRAG enabled SAs. I understand your intention to use this mode in 
non-IKEv2
environments, but I think that in case of IPsec you don't need to send
AGGFRAG_PAYLOAD payload over non- AGGFRAG enabled SAs.
I think some text should be added that this section is only applicable
for non-IKEv2 use case.

12. Section 2.3:

    Empty AGGFRAG_PAYLOAD payloads (Section 2.2.4) are used to
    transmit congestion control information on non-IP-TFS enabled SAs, so
    intermixing is allowed in this specific case.

As I said before it is not needed in case of IKEv2, so I'd rather to
stress that this is only allowed if AGGFREAG SAs are not being created in pairs
(non-IKEv2 use case).

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?

14. Section 2.4.2:

This section is concerned with IP-TFS use of AGGFREG mode.
First, I think it must be mentioned in the text.
Then, the text lacks discussion on how to deal with congestion when ESP over 
TCP is used (RFC 8229).
>From my understanding it is TCP that will take care of congestion in this case.
If both mechanisms are in use then I suspect that it may affect performance
(like TCP in TCP), because in this case congestion information (RTT etc.)
will be highly inaccurate.

More general note - using ESP over TCP may also add some other implications
on using AGGFRAG (e.g. header values mapping, ECN, fixed packet size etc.),
I think it's worth to add some text. Is IP-TFS use of AGGFRAG ever make sense
when ESP is sent over TCP?

15. Section 3:

    In order to obtain these values the receiver sends
    congestion control information on it's SA back to the sender.

s/it's/its

16. Section 5.1:

    The USE_AGGFRAG notification MUST NOT be sent, and MUST be ignored,
    during a CREATE_CHILD_SA rekeying exchange as it is not allowed to
    change use of the AGGFRAG_PAYLOAD payload type during rekeying.  A
    new child SA due to re-keying inherits the use of AGGFRAG_PAYLOAD
    from the re-keyed child SA.

In IKEv2 re-keying of Child SA is semantically almost equal to creating a new 
Child SA
with the only exception that the SA which is being rekeyed is indicated.
However, all the properties of newly created substitute SA are negotiated as if 
it is not a rekey
(transforms, mode of operation). I don't see any reason why to break this with 
AGGFRAG.
I prefer to re-negotiate AGGFRAG in this case just to follow
current IKEv2 practice. This will simplify implementations.
And I see no problem if after AGGFRAG SA is replaced with normal one
and visa versa. Am I missing something?

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?

18. Section 5.1:

Add some text that USE_AGGFRAG cannot be combined with USE_TRANSPORT_MODE.

General note: I think some words should be added somewhere (Section 2?)
that AGGFRAG implies encapsulation whole IP packets, so it cannot be
used for Transport mode ESP SA.

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

Another issue: what is "ESP payload type"?
RFC 4303 doesn't have this term. You should first
introduce it.

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.

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?

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.

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.

Probably it's worth to mention this somewhere in Section 2.2.3...

  23. Appendix C.1.1:

    The overhead of IP-TFS is 40 bytes per outer packet.

I failed to understand where 40 bytes came from. From my understanding the 
overhead
of AGGFRAG is either 4 bytes or 24 bytes depending on mode used (comparing with 
usual ESP).
Am I missing something?

s/bytes/octets
24. Appendix C.1.2:

    The overhead per inner packet for constant-send-rate padded ESP
    (i.e., traditional IPsec TFC) is 36 octets plus any padding, unless
    fragmentation is required.

Again, I failed to understand where 36 octets came from.
In general ESP overhead depends on transforms in use (which
influence IV and ICV sizes), so you should mention under
which condition this overhead was computed.

Regards,
Valery.



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

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

Reply via email to