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