Subject: Re: [IPsec] WGLC for draft-ietf-ipsecme-iptfs

From: Sean Turner <[email-masked]>
Date: Wed, 3 Feb 2021 22:38:06 -0500

Hi! I mostly just have nits, but there are a couple of questions interspersed below.

s1, para 1: s/While one may directly obscure the data through the use of/While directly obscuring the data with

s1, para 1: s/it’s/its

s1, para 3: s/IP-TFS/IP-TFS (IP Traffic Flow Security)

s1, last para: s/IP-TFS provides for dealing with network congestion/IP-TFS addresses network congestion

s1: You mention full TFC. Is it partial TFC if you use a non-constant send-rate? if so that might be good to qualify in the 3rd paragraph with something like:

OLD:

 A non-
 constant send-rate is allowed, but the confidentiality properties of
 its use are outside the scope of this document.

NEW:

 A non-
 constant send-rate is allowed to support partial TFC, but the
 confidentiality properties of its use are outside the scope of
 this document.

s1.1, para 1: Use updated terminology para from 8174 ("BCP 14” is missing):

 The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
 NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
 "MAY", and "OPTIONAL" in this document are to be interpreted as
 described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
 appear in all capitals, as shown here.

s2, para 1: 

 is the "(SA)" needed?  ... tunnel (SA) ...
 s/it’s/its

s2, 2nd para: I tripped over this para a couple of times. Does this get the same point across?

OLD:

 The primary input to the tunnel algorithm is the requested bandwidth
 used by the tunnel.  Two values are then required to provide for this
 bandwidth, the fixed size of the encapsulating packets, and rate at
 which to send them.

NEW:

 The primary input to the tunnel algorithm is the requested
 bandwidth for the tunnel.  Two values needed to determine
 the bandwidth are the fixed size of the encapsulating
 packets and the rate at which to send them.

s2, 3rd para: s/or could be/or be

s2, 4th para: s/requested tunnel used bandwidth/
requested tunnel bandwidth

s1, last para to make it match the rest of the sentence: s/The egress of the IP-TFS/The egress (receiving) side of the IP-TFS 

s2.1, 1st para: s/In order to maximize bandwidth IP-TFS/
In order to maximize bandwidth, IP-TFS

s2.1, 3rd para: Does this say the same thing:

OLD:

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

NEW:

 IP-TFS uses a new Encapsulating Security Payload (ESP, [RFC4303])
 type identified by the number assigned to AGGFRAG_PAYLOAD
 (Section 6.1).

s2.2: I had a really hard time parsing this sentence:

 The AGGFRAG_PAYLOAD payload content defined in this document is
 comprised of a 4 or 24 octet header followed by either a partial, a
 full or multiple partial or full data blocks.

There are three options: partial, full or multiple partial, or full data? Maybe it’s just missing a comma: s/multiple partial or/multiple partial or,

s2.2.1: Should we be specific about the IPv4 and IPv6 length’s field name:

OLD:

 Likewise, the length of the data block is extracted from the
 encapsulated IPv4 or IPv6 packet's length field.

NEW:

 Likewise, the length of the data block is extracted from the
 encapsulated IPv4’s Total Length or IPv6’s Payload Length fields.

s2.2: s/It’s/It is
s2.2.3: s/to be able to reassemble/to reassemble 
s2.2.3: s/This possible interleaving/This interleaving
s2.2.3: s/sender to always be able to send a/sender to always send a 
s2.2.3, 4th para: s/Finally, we note/Finally, note
s2.2.3, 5th para: s/As the amount of reordering that may be present is hard to predict the window/As the amount of reordering that may be present is hard to predict, the window

2.2.3, 5th para: I am all about not littering I-Ds with 2119-language, but here it looks like you are burying the MUST in the parenthetical. BTW - I think the sentence stands on its own without the "i.e.” and it could safely be deleted. Would this rewording work:

 Gaps in the sequence numbers will not work for this document,
 therefore ESP sequence number stream MUST increase monotonically
 by 1 for each subsequent packet.

s2.2.3, penultimate para: s/b/c/because

s2.2.3, last para: Should the I-D state what happens if the implementation does send initial fragments of an inner packet using one SA and subsequent fragments in a different SA. I.e., motivate the SHOULD NOT.

s2.2.3, last para: implementation here refers to sender right so maybe: senders SHOULD NOT

s2.2.3.1: Implementations here refers to senders so maybe:
 s/An implementation/Senders
 s/Implementation implementing/Senders implementing ;)

s2.2.4: s/In order to support/To support

s2.2.5, 1st para:
 s/(by design!)/(by design)    ;)
 s/although an implementation/although a sender
 s/An implementation SHOULD/A sender SHOULD

s2.2.5, 2nd para:
 s/an implementation/a sender
 s/([RFC3168])/[RFC3168]

s2.2.6, 1st para: s/([RFC0791])/[RFC0791]

s2.2.6, 2nd para: 2119 the should?: s/should be/SHOULD be.  Is there a reason you would want to handle the errors differently? If not then would the following also be true (i.e., replace "should be" with "are": 

 Any errors (e.g., ICMP errors arriving back at the tunnel ingress due
 to tunnel traffic) are handled the same as with non IP-TFS
 IPsec tunnels.

s2.2.7, 1st para: s/am implementation/a sender

s2.2.7, 2nd para: s/([RFC4301])/[RFC4301]

s2.3: Does this work?

OLD:

 It is not the intention of this specification to allow
 for mixed use of an AGGFRAG_PAYLOAD enabled SA.

NEW:

 This document does not specify mixed use of an
 AGGFRAG_PAYLOAD enabled SA.

s2.4.1, 1st para: s/In the non-congestion controlled mode IP_TFS /In the non-congestion controlled mode, IP-TFS

s2.4.1, 2nd para:
 Should the should be SHOULD?
 s/In this case packet/In this case, packet
 There is a reference to a user. Is it really a user?

s2.4.2, 2nd para:
 s/the ingress sends/the ingress side sends
 Maybe just swap this around?

OLD:

 An example of an implementation of the [RFC5348] algorithm which
 matches the requirements of IP-TFS (i.e., designed for fixed-size
 packet and send rate varied based on congestion) is documented in
 [RFC4342].

NEW:

 [RFC4342] provides an example of the [RFC5348] algorithm which
 matches the requirements of IP-TFS (i.e., designed for fixed-size
 packet and send rate varied based on congestion.

s2.4.2, 3rd para: s/In particular these/In particular, these

s2.4.2, 4th para:
 Should the must be MUST?
 s/The lack of receiving this information/Not receiving this information

s2.4.2, 4th and 5th paras: s/it’s/its

s2.4.2, 6th para: Does this work?

OLD:

 When an implementation is choosing a congestion control algorithm (or
 a selection of algorithms) one should remember that IP-TFS is not
 providing for reliable delivery of IP traffic, and so per packet ACKs
 are not required and are not provided.

NEW:

 When choosing a congestion control algorithm (or
 a selection of algorithms) note that IP-TFS is not
 providing for reliable delivery of IP traffic, and so per packet ACKs
 are not required and are not provided.

s2.4.2, last para: s/It’s/It is

s3, 1st para:
 s/and also be able to approximate/and also to approximate 
 s/([RFC5348])/[RFC5348]
 s/In order to obtain these values the/
   In order to obtain these values, the 
 s/Thus in order, to support/Thus, to
 s/is used to convey/conveys
s3, 1st and 3rd para: s/it’s/its

s3, 3rd para: Nits complained about this so I am assuming it’s MUST NOT here - s/MUST not/MUST NOT

s3.1, 1st para: s/egress endpoint/egress (receiving) side

s3.1, last para: s/For this reason ECN/For this reason, ECN/

s4: s/should be able to be specified/should be specified

s4.1: s/For non-congestion controlled mode the/For non-congestion controlled mode, the

s4.1: Does this work:

OLD:

 For congestion controlled mode one can configure the
 bandwidth or have no configuration and let congestion
 control discover the maximum bandwidth available.

NEW:

 For congestion controlled mode, the bandwidth can be
 configured or the congestion controller discovers
 the maximum bandwidth available.

s5.1, 2nd para: s/is used to enable use of/enables the

s5.1, 3rd para:
 s/To request using the/To request use of the
 s/then response/then the response

s5.1, penultimate para: Should the should not be SHOULD NOT?

s6.1: s/8 bit/8-bit
s6.1: s/specification/document
s6.1.1: s/"DataBlocks” data/“DataBlocks" ?
s6.1.1: s/16 bit/16-bit
s6.1.2: s/7 bit/7-bit
s6.1.2: s/1 bit/1-bit
s6.1.2 x3: s/32 bit/32-bit
s6.1.2: s/22 bit/22-bit
s6.1.2 x2: s/21 bit/21-bit
s6.1.3: s/4 bit/4-bit
s6.1.3.1/s6.1.3.2/s6.1.3.3: s/4 bit/4-bit
s6.1.3.1/s6.1.3.2: s/16 bit/16-bit
s6.1.3.3: s/extends/Extends

s6.1.4: s/As discussed in Section 5.1 a/As discussed in Section 5.1, a

s6.1.4, D bullet (make it match C): s/Don't Fragment bit, if/Don't Fragment bit.  If

s7: I may have missed this in earlier discussions but why is the registration policy Standards Action? I thought that most of the registries were trending more towards Expert Review.

s8, 1st para: s/Traffic Flow Confidentiality/TFC

s8:

You warned in s1 about using a non-constant send-rate, but shouldn’t that be echoed here?

Likewise maybe repeat the ECN covert channel statement.

s9.2: r/[draft-iab-wire-image]/[RFC8546]

Appendix B: YMMV here but since this is an Appendix and you’re repeating the current practice s/SHOULD/should

Cheers,
spt

> On Jan 24, 2021, at 20:55, Tero Kivinen <[email-masked]> wrote:
> 
> This is the start of 3 week WGLC on the document, ending 2021-02-15.
> Please submit your comments to the list, also send a note if you have
> reviewed the document, so we can see how many people are interested in
> getting this out.
> -- 
> [email-masked]
> 
> _______________________________________________
> IPsec mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ipsec

---

From: Paul Wouters <[email-masked]>
Date: Thu, 4 Feb 2021 10:14:23 -0500 (EST)

> s1, last para: s/IP-TFS provides for dealing with network congestion/IP-TFS addresses network congestion

I dont think it "addresses" network congestion. It maybe improves
dealing with network congestion ?

> AGGFRAG_PAYLOAD enabled SA.

Maybe add "purposefully" ?

Otherwise agree with Sean's comments.

Paul

---

From: "Valery Smyslov" <[email-masked]>
Date: Mon, 8 Feb 2021 14:44:12 +0300

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:

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.

---

From: Lou Berger <[email-masked]>
Date: Mon, 8 Feb 2021 09:52:18 -0500

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

---

From: Christian Hopps <[email-masked]>
Date: Mon, 8 Feb 2021 18:19:32 -0500

> On Feb 3, 2021, at 10:38 PM, Sean Turner <[email-masked]> wrote:
> 

Hi Sean,

Thanks so much for this very thorough review!

I have made the vast majority of the suggested changes with only a few style exceptions (and incorporated Paul's input in his follow-on email).

I will post a new version soon with these changes (noted below) as well as addressing some from Valery as well.

> s1, para 1: s/While one may directly obscure the data through the use of/While directly obscuring the data with

Fixed

> s1, para 1: s/it’s/its

Fixed

> s1, para 3: s/IP-TFS/IP-TFS (IP Traffic Flow Security)

Fixed

> s1, last para: s/IP-TFS provides for dealing with network congestion/IP-TFS addresses network congestion

"Additionally, IP-TFS provides for operating fairly within congested networks"

> confidentiality properties of its use are outside the scope of

There are other ideas being studied to achieve full TFC w/o use of constant send-rate (e.g., using some sort of randomizing), I'm fairly sure that this just reduces to a lower logical fixed send rate that utilizes burstiness. In any case since I am aware of there being research being done in this area I think leaving the text as is makes sense (i.e., I don't want to claim anything not fixed-rate must be partial TFC).

> appear in all capitals, as shown here.

Fixed.

> is the "(SA)" needed?  ... tunnel (SA) ...

Fixed.

> packets and the rate at which to send them.

Changed to:

The primary input to the tunnel algorithm is the requested bandwidth
to be used by the tunnel. Two values are then required to provide for
this bandwidth use, the fixed size of the encapsulating packets, and
rate at which to send them.

> s2, 3rd para: s/or could be/or be

Fixed

> requested tunnel bandwidth

Changed to:

"requested bandwidth to be used"

and "The packet send rate is the requested bandwidth to be used divided by the size of the encapsulating packet."

> s1, last para to make it match the rest of the sentence: s/The egress of the IP-TFS/The egress (receiving) side of the IP-TFS

Fixed.

> In order to maximize bandwidth, IP-TFS

Fixed

> type identified by the number assigned to AGGFRAG_PAYLOAD

I think I prefer the original text, if that's OK, as it is saying how it does the paragraph above vs. just making another statement (i.e., I think it ties the text together better).

> There are three options: partial, full or multiple partial, or full data? Maybe it’s just missing a comma: s/multiple partial or/multiple partial or,

Changed to:

The AGGFRAG_PAYLOAD payload content defined in this document is
comprised of a 4 or 24 octet header followed by either a partial
datablock, a full datablock, or multiple partial or full datablocks.

> encapsulated IPv4’s Total Length or IPv6’s Payload Length fields.

Fixed

> s2.2.3, 5th para: s/As the amount of reordering that may be present is hard to predict the window/As the amount of reordering that may be present is hard to predict, the window

Fixed.

> by 1 for each subsequent packet.

Fixed

> s2.2.3, penultimate para: s/b/c/because

Fixed

> s2.2.3, last para: implementation here refers to sender right so maybe: senders SHOULD NOT

Simplified to "senders MUST NOT"... :)

> s/Implementation implementing/Senders implementing ;)

Fixed

> s2.2.4: s/In order to support/To support

Fixed

> s/An implementation SHOULD/A sender SHOULD

Fixed

> s/an implementation/a sender

Fixed

> s2.2.6, 1st para: s/([RFC0791])/[RFC0791]

Fixed, and update other single refs to not use parens.

> to tunnel traffic) are handled the same as with non IP-TFS

Fixed

> s2.2.7, 1st para: s/am implementation/a sender

Fixed

> s2.2.7, 2nd para: s/([RFC4301])/[RFC4301]

Fixed

> AGGFRAG_PAYLOAD enabled SA.

Fixed

> s2.4.1, 1st para: s/In the non-congestion controlled mode IP_TFS /In the non-congestion controlled mode, IP-TFS

Fixed

> There is a reference to a user. Is it really a user?

I think it's a should (non-normative), as this is guidance on the right way to deploy IPTFS, but not how to implement it.

> packet and send rate varied based on congestion.

Fixed.

> s2.4.2, 3rd para: s/In particular these/In particular, these

Fixed

> s/The lack of receiving this information/Not receiving this information

Yes, and fixed.

> s2.4.2, 4th and 5th paras: s/it’s/its

Fixed

> are not required and are not provided.

Fixed

> s2.4.2, last para: s/It’s/It is

Fixed

> s/Thus in order, to support/Thus, to

Fixed

> s/is used to convey/conveys

This is a style one I think that I'd like to leave be, I think it better highlights that one needs to send these empty payloads in this case.

> s3, 1st and 3rd para: s/it’s/its

Fixed.

> s3, 3rd para: Nits complained about this so I am assuming it’s MUST NOT here - s/MUST not/MUST NOT

Fixed

> s3.1, 1st para: s/egress endpoint/egress (receiving) side

Fixed

> s3.1, last para: s/For this reason ECN/For this reason, ECN/
Fixed

> s4: s/should be able to be specified/should be specified

Fixed

> s4.1: s/For non-congestion controlled mode the/For non-congestion controlled mode, the

Fixed

> the maximum bandwidth available.

Changed to:

"For congestion controlled mode, the bandwidth can be configured or the congestion control algorithm discovers and uses the maximum bandwidth available."

> s5.1, 2nd para: s/is used to enable use of/enables the

Fixed

> s/then response/then the response

Fixed

> s5.1, penultimate para: Should the should not be SHOULD NOT?

Fixed

> s6.1: s/specification/document

Fixed

> s6.1.1: s/"DataBlocks” data/“DataBlocks" ?

I'm going to leave this as the ``"DataBlocks" data'' as it is referring to that area of the packet data. DataBlocks may be spread across multiple packets or there may be multiple DataBlocks in a single packet.

> s6.1.3.3: s/extends/Extends

Fixed

> s6.1.4: s/As discussed in Section 5.1 a/As discussed in Section 5.1, a

Fixed

> s6.1.4, D bullet (make it match C): s/Don't Fragment bit, if/Don't Fragment bit.  If

Fixed

> s7: I may have missed this in earlier discussions but why is the registration policy Standards Action? I thought that most of the registries were trending more towards Expert Review.

No particular reason other than it was the conservative choice. :)

> s8, 1st para: s/Traffic Flow Confidentiality/TFC

Fixed

> You warned in s1 about using a non-constant send-rate, but shouldn’t that be echoed here?

As mentioned previously it's not necessarily less secure to use a non-constant send rate (research being done by others here), we are only claiming use of constant send rate is more secure.

> Likewise maybe repeat the ECN covert channel statement.

Repeated.

> s9.2: r/[draft-iab-wire-image]/[RFC8546]

Fixed.

> Appendix B: YMMV here but since this is an Appendix and you’re repeating the current practice s/SHOULD/should

Fixed.

Thanks again for the very thorough review!

Thanks,
Chris.

---

From: Christian Hopps <[email-masked]>
Date: Tue, 9 Feb 2021 02:57:09 -0500

> On Feb 8, 2021, at 6:44 AM, Valery Smyslov <[email-masked]> wrote:
> 
> Hi,
> 

Hi Valery,

Thanks for your continued reviews and suggestions.

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.

I've incorporated your other suggestions from below.

> the AGGFRAG_PAYLOAD number must be in the ESP "Next Header" field.

s/type which is identified by the number/Next Header field value/

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

Updated.

Fixed

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

Fixed.

> several SAs, so it will be dropped. Why do you allow this?

Changed to MUST NOT.

> all its fragments must be sent over single ESP SA.

Covered by the switch to MUST NOT.

> AGGFRAG_PAYLOAD payload over non- AGGFRAG enabled SAs.

SAs are a concept that extend beyond IKEv2 so I think that the text is OK here.

> I think some text should be added that this section is only applicable

Add: "Currently this situation is only applicable in non-IKEv2 use cases."

> stress that this is only allowed if AGGFREAG SAs are not being created in pairs

Covered with above change.

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

> when ESP is sent over TCP?

I'll add this to the previous (non-congestion control mode) section.

"Non-congestion control mode is also appropriate if ESP over TCP is in
use [RFC8229]."

Fixed.

> and visa versa. Am I missing something?

I think you have a good point on simplicity. I've removed that paragraph and removed "non-rekeying" in the text in the previous paragraph.

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

> used for Transport mode ESP SA.

I'll add this to 5.1:

"As the use of AGGFRAG_PAYLOAD payloads is currently only defined for
non-transport mode tunnels, the USE_AGGFRAG notification MUST NOT be
combined with USE_TRANSPORT notification."

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

> RFC 4303 doesn't have this term. You should first

I've changed payload type to "ESP Next Header value" in the document.

> 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

> a lot of allocation requests for this registry, so why do you need such

You're the second person to suggest this so I've changed it to "Expert Review". :)

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

> be created with authentication other than NONE

I think this falls under the non-IPTFS use case. I'd like to leave it out as it would be confusing.

> of AGGFRAG is either 4 bytes or 24 bytes depending on mode used (comparing with usual ESP).

See final comment below..

Fixed.

> which condition this overhead was computed.

These were done a couple years ago when we first published this, and for some reason I had the ESP overhead (shared by both IPTFS and regular IPsec) plugged into the table formulas as 16.

I'll rerun the tables using AES-GCM-256 values. New value IPsec/ESP overhead would be 54 (20 (IP) + 8+2 (ESPH/F) + 8 (IV) + (16 ICV))

The results will still highlight the same things, but thanks for catching this! :)

Thanks,
Chris.

---

From: "Valery Smyslov" <[email-masked]>
Date: Wed, 10 Feb 2021 11:36:40 +0300

Hi Christian,

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

> They remain different mechanisms and the user may wish to have them treated differently (e.g., logging

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.

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

> 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

I think even better would be "that specify requirements from the sender of this notification".

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

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

> You're the second person to suggest this so I've changed it to "Expert Review". :)

Thanks :-)

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

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

---

From: Christian Hopps <[email-masked]>
Date: Wed, 10 Feb 2021 10:02:33 -0500

> On Feb 10, 2021, at 3:36 AM, Valery Smyslov <[email-masked]> wrote:
> 
> Hi Christian,
> 

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.

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

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

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

> I think even better would be "that specify requirements from the sender of this notification".

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

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

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

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

---

From: Christian Hopps <[email-masked]>
Date: Wed, 10 Feb 2021 10:15:11 -0500

As there have been a number of editorial changes (nits etc), to help any remaining reviewers here's a pointer to the current text which includes the changes made so far during the WGLC:

https://app.circleci.com/pipelines/github/LabNConsulting/iptfs/6/workflows/6e885d99-8577-4899-82b7-6b4bd938d18e/jobs/6/artifacts <https://app.circleci.com/pipelines/github/LabNConsulting/iptfs/6/workflows/6e885d99-8577-4899-82b7-6b4bd938d18e/jobs/6/artifacts>

Thanks,
Chris.

> On Jan 24, 2021, at 8:55 PM, Tero Kivinen <[email-masked]> wrote:
>

---

From: "Valery Smyslov" <[email-masked]>
Date: Thu, 11 Feb 2021 11:23:34 +0300

Hi Christian,

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

          That changes are the very minimal ones and in my opinion they're not enough.

          The document still looks like a mess, because it constantly used term "IP-TFS"

          when describing a generic behavior of Aggregation and Fragmentation mode.

          Few examples:

2.  The IP-TFS Tunnel

   As mentioned in Section 1 IP-TFS utilizes an IPsec [RFC4303] tunnel

   (SA) as it's transport.  To provide for full TFC, fixed-sized

   encapsulating packets are sent at a constant rate on the tunnel.

   The egress of the IP-TFS tunnel MUST allow for and expect the ingress

   (sending) side of the IP-TFS tunnel to vary the size and rate of sent

   encapsulating packets, unless constrained by other policy.

   IP-TFS aggregates as well as fragments the inner IP traffic flow into

   fixed-sized encapsulating IPsec tunnel packets.  

   In particular IP-TFS never maps the inner DF bit as it is

   unrelated to the IP-TFS tunnel functionality; IP-TFS never IP

   fragments the inner packets and the inner packets will not affect the

   fragmentation of the outer encapsulation packets.  Likewise, the ECN

   value need not be mapped as any congestion related to the constant-

   send-rate IP-TFS tunnel is unrelated (by design!) to the inner

   traffic flow

   An example IP-TFS packet flow can be found in Appendix A.

          And so on. I think the text must be cleaned up to be more accurate

          with this regard.

9. Section 2.2.3:

They remain different mechanisms and the user may wish to have them treated differently (e.g., logging
replayed packets).

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

          Well, I probably was unclear, but you missed my point. I'll try again.

          The current text says that if both replay protection and AGGFRAG 

          are employed, then replay protection window and reassembling window MAY 

          be of the same size, which is the smaller of both.

          Since MAY means that you think it's OK for these windows to be of different

          sizes in this case, I wondered in which situations you think it's OK.

          You gave me an example, for situation when replay window is longer than 

          reassembly window, which I can reluctantly buy (I still think that it's a waste

          of resources: some delayed packets will pass the replay protection, 

          decrypted and then dropped because they would fall outside reassembly window).

          But can you please give me an example when it's useful to have reassembly

          window longer than replay protection window? 

          Consider an example when reassembly window size is 100 and replay protection window size is 10.

          For simplicity let's assume that reassembly window at the moment is filled with packets with odd SNs 

          starting from 1 to 99. So, you are waiting for the rest packets with even SNs from 0 to 98 to be able

          to complete reassembly process. But since replay protection window size is only 10 and the 

          most recently received ESP packet has SN 99, you never receive packets with SNs from 0 to 89, 

          (because the will be dropped by replay protection logic) and the first 90 packets will never be reassembled.

          It's just a waste of resources.

          So, I think that MAY above should be at least SHOULD. Or leave it as MAY and say that 

          reassembly window SHOULD NOT (MUST NOT?) be longer that replay protection (for the reason

          outlined above).

10. Section 2.2.3:

Changed to MUST NOT.

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

          If you have several parallel SAs their SNs are not reset, they are just different...

          OK, I can live with this, although I still think the text is a bit confusing.

20. Section 6.1.4:

 0:

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

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

          You are right, I somehow mixed up these sections. That's probably because

          the description of USE_AGGFRAG is split over 5.1 and 6.1.4, that 

          was a cause of my confusion. I'd rather merge them not to confuse 

          other readers :-)

          I think yes. You may have ESP SA  w/o authentication, it's not prohibited by RFC 4303.

          In this case replay protection won't work (it may be OK for some application).

          What I want is to stress that AGGFRAG won't work either in this situation.

          Regards,

          Valery.

"

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

"

Thanks,

Chris.

Regards,
Valery.

---

From: Christian Hopps <[email-masked]>
Date: Thu, 11 Feb 2021 09:01:56 -0500

Hi Valery,

> changed to better decouple mechanism from its applications.

The discussion in the WG over the last 2 years has focused on providing a better TFS solution, indeed this is what the work is chartered to do.

Your excellent observation that this work could be used in other ways was spot on, and is reflected in the current text.

Restructuring the draft to focus on a generic encapsulation solution goes beyond the WG discussion and our chartered task. If the working group consensus is to shift gears then that is what we can do; however, the document is in WGLC and it's rather late in the process. I think it's best to move forward with the current document as discussed in the WG, and let the WG discuss other use cases in future documents.

Thanks,
Chris.

> On Feb 11, 2021, at 3:23 AM, Valery Smyslov <[email-masked]> wrote:
> 
> Hi Christian,
> 
>           That changes are the very minimal ones and in my opinion they’re not enough.
>           The document still looks like a mess, because it constantly used term “IP-TFS”
>           Well, I probably was unclear, but you missed my point. I’ll try again.
>           Since MAY means that you think it’s OK for these windows to be of different
>           sizes in this case, I wondered in which situations you think it’s OK.
>           reassembly window, which I can reluctantly buy (I still think that it’s a waste
>           But can you please give me an example when it’s useful to have reassembly
>           For simplicity let’s assume that reassembly window at the moment is filled with packets with odd SNs
>           It’s just a waste of resources.
> 
> " <>6.1.4 <https://tools.ietf.org/html/draft-ietf-ipsecme-iptfs-06#section-6.1.4>.  IKEv2 USE_AGGFRAG Notification Message"
> 
>           You are right, I somehow mixed up these sections. That’s probably because
>           was a cause of my confusion. I’d rather merge them not to confuse
>           other readers :-)
> 
> 
>           I think yes. You may have ESP SA  w/o authentication, it’s not prohibited by RFC 4303.
>           In this case replay protection won’t work (it may be OK for some application).
>           What I want is to stress that AGGFRAG won’t work either in this situation.
> 
>           Regards,
>           Valery.
> 
>

---

From: Tero Kivinen <[email-masked]>
Date: Thu, 11 Feb 2021 21:16:53 +0200

Christian Hopps writes:
<something about WGLC beeing to late, but I  can't really quote it
properly as it was in HTML format and my client could not parse that
html out for quoting>

WGLC is not too late to do changes, it is quite early in the process,
we still need to go to the IETF LC, and then through IESG etc. If
making these changes now will make document easier to read, that will
most likely make IETF LC and further steps easier, so it might save
some time and effort from us later.

And about the charter, as long as we do get the TFS solution out of
that is fine for the charter. We are trying to provide solution to the
problem described in the our charter, the charter usually does not try
to dictate what the final solution will be, just describe the problem
that we need to solve.

Our charter says:

	The demand for Traffic Flow Confidentiality has been
	increasing in the user community, but the current method
	defined in RFC4303 (adding null padding to each ESP payload)
	is very inefficient in its use of network resources. The
	working group will develop an alternative TFC solution that
	uses network resources more efficiently.

An example of this is that when we started working on the post quantum
cryptography, we realized we need this auxiliary exchange to solve the
issue, but then we realized that this could also be used for something
else, and it was renamed to intermediate exchange etc. So while
working for one solution we can also generate more generic protocol
that can be used to perhaps solve other problems in the future, even
better. Of course making the protocol more complicated by making it
more general is not necessarely good idea, thus sometimes it is better
to make more restricted protocol just for simplicitys sake.

But my understanding is that here we do not need any protocol changes,
just reordering of the document to make it more clear that we have two
layers where the first one can be used for other things too, and that
the second part uses that first layer as it building block.
-- 
[email-masked]

---

From: Christian Hopps <[email-masked]>
Date: Thu, 11 Feb 2021 17:52:36 -0500

Hi Tero,

I'm sorry some of the previous messages HTML at some point the message got converted to that format. I actually prefer text as well.

It was not my intention to say anything was too late to change, only that the WG has been discussing an IP-TFS solution for 2 years. This reorganization of the document obfuscates that and turns the document into a generic encapsulation document, with IP-TFS as a after-though. If that's the consensus of the WG then that is what we will do of course.

I personally feel this is totally unneeded, it makes things harder to understand for someone actually trying to implement a TFS solution which is the chartered goal.

Is this the WG consensus, that we should have a generic new ESP encapsulation document? I have only seen Valery ask for these changes so far.

I would like to ask for WG consensus before we switch directions like this.

Thanks,
Chris.

> On Feb 11, 2021, at 2:16 PM, Tero Kivinen <[email-masked]> wrote:
> 
> Christian Hopps writes:

---

From: Christian Hopps <[email-masked]>
Date: Thu, 11 Feb 2021 23:36:07 -0500

Hi Tero,

Just to clarify, are you indicating Valery's changes should be made? If so then I will update the document so we can continue to make progress.

Thanks,
Chris.

> On Feb 11, 2021, at 5:52 PM, Christian Hopps <[email-masked]> wrote:
> 
> Signed PGP part
> Hi Tero,
>

---

From: Sean Turner <[email-masked]>
Date: Fri, 12 Feb 2021 09:30:47 -0500

Chris,

Thanks for wading through these. Incorporating these will certainly remove any issues I had. As far as the style suggestions goe, that’s fine the GENART, IESG, and RFC editor will also ask for their pound of flesh ;)

Spt

> On Feb 8, 2021, at 18:19, Christian Hopps <[email-masked]> wrote:
> 
> 
>

---

From: Tero Kivinen <[email-masked]>
Date: Sat, 13 Feb 2021 00:59:15 +0200

Christian Hopps writes:
> Just to clarify, are you indicating Valery's changes should be made?
> If so then I will update the document so we can continue to make
> progress. 

I have not checked out the draft yet, so I do not have opinion about
this. I was just pointing out that this is good time to do the change,
if those are needed, this is not too late...

And, yes I would think it would be worth of getting other people to
say their opinions too before doing too much work.

Sometimes having clear layers makes things easier, I think for example
having separate intermediate exchange instead of having that same
exchange buried inside the post quantum draft makes it easier to
understand how it works. Not sure if that is true in this case.
-- 
[email-masked]

---

From: Michael Richardson <[email-masked]>
Date: Sun, 14 Feb 2021 23:45:59 -0500

I have read draft-ietf-ipsecme-iptfs before it was adopted, and during the
adoption call, but have been busy.  So I have read it again today from
beginning to end before tackling the long thread that has developed.

EXEC SUM: I think that the document is not ready.
     There are a lot of MAYs and future work thoughts on the sender.
     That's fine.  But, in order for future senders to know what's legal and
     what's not, what we really need is a clearly articulated Receiver State Machine.
     I suggest that this is pretty important.

===

The first sentence of the Abstract needs rework.
The words "security" and "confidentiality" are used, but really it's about
traffic analysis.  So, if there is anything with increased confidentiality,
it's the pattern, right?  Abstracts are really hard to write.
Ah, the problem is that "traffic flow security" is not the term,
it's traffic flow confidentiality, and it is not capitalized!
So, that would help, but... it's not defined yet.

I suggest the following terms be added to section 1.1.
 - TFC

* I'm glad you recommend PLMTUD, I suggest PMTUD is dead.
  How do use PLMTUD?  Will you tell us later in the document, or is that new work?
      (does not look like you tell us)

* You are using "CamelCase" for variables, I found that jarring.
  I wondered what rfc4303 used, but found nothing.
  RFC7296 uses UPPER_CASE.
  RFC7296 does not use _ for field names.
  I might prefer snake_case, but whatever...

* I would prefer "If BlockOffset != 0" rather than
    Conversely, if the "BlockOffset" value is non-zero it points to the

>   The "BlockOffset" can point past the end of the "DataBlocks" data
>   which indicates that the next data block occurs in a subsequent
>   encapsulating packet.

I guess, it the actual value does not matter: it's not remembered.
As long it is bigger than the packet, it's good.  So 0xffff probably works?

> 2.2.2.  No Implicit End Padding Required

-- I think you are telling me that the IPv4/IPv6 length field is good enough
   to find the end of the packet.  If not, I didn't quite understand.
   Later in 6.1, I learn that AGGFRAG_PAYLOADS are squatting on 'IPv5'
   I hope this will be acceptable :-)
   I think it's a reasonable hack, and I don't see us wrapping around
   IP versions within a hundred years, soooo...
   And pad blocks are "IPv0"

> This possible interleaving of all-pad
>    payloads allows the sender to always be able to send a tunnel packet,
>   regardless of the encapsulation computational requirements.

I think that you are telling me that a sender can have some pad packets being
created regularly (perhaps on a CPU dedicated to this) and almost ready to
send, and the pad packet is just replaced by real data if it happens to be
ready.

Please split 2.2.5 up by flag type into subsubsubsection.
2.4.21: Maybe need to briefly explain circuit breakers to us non-transport-types.

> If the SA back to the sender is a non-
>    AGGFRAG_PAYLOAD enabled SA then an AGGFRAG_PAYLOAD empty payload
>    (i.e., header only) is used to convey the information.

is this really worth supporting?
Please break up third paragraph.

=====

--
Michael Richardson <[email-masked]>   . o O ( IPv6 IøT consulting )
           Sandelman Software Works Inc, Ottawa and Worldwide

---

From: Michael Richardson <[email-masked]>
Date: Sun, 14 Feb 2021 23:49:36 -0500

Lou Berger <[email-masked]> wrote:
    > 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

I agree.

---

From: Christian Hopps <[email-masked]>
Date: Tue, 16 Feb 2021 06:11:18 -0500

Hi Michael,

Thanks for the review, q's, comments, and changes inline..

> On Feb 14, 2021, at 11:45 PM, Michael Richardson <[email-masked]> wrote:
> 
> Signed PGP part
> 

What did you have in mind? There are purposefully no restrictions for the receiver to enforce. From section 2:

  The egress (receiving) side of the IP-TFS tunnel MUST allow for and
  expect the ingress (sending) side of the IP-TFS tunnel to vary the
  size and rate of sent encapsulating packets, unless constrained by
  other policy.

> I suggest the following terms be added to section 1.1.

Capitalized and "acronymized" TFC and called it out specifically now:

   This document assumes familiarity with IP security concepts including Traffic
   Flow Confidentiality as described in [RFC4301].

>      (does not look like you tell us)

I believed it was enough to just reference the mechanism (as we do for PMTUD as well). This was added during the transport area review.

>  I might prefer snake_case, but whatever...

You must not like Python very much. :) I used UPPER_CASE for constants as that's what seemed to be the prior art in ipsec. For field names camel case seemed to work pretty good differentiates them from constants and non-actual-field references. I could add spaces instead of using CamelCase if you think this is important, though.

>    Conversely, if the "BlockOffset" value is non-zero it points to the

Usually I don't use "=" or "!=" in text. I'd prefer to leave this as is, otherwise when reconciling that change with the rest of the text e.g.:

   The "BlockOffset" value is either zero or some offset into or past
   the end of the "DataBlocks" data.

Becomes

   The "BlockOffset" == 0 or some offset into or past the end of the "DataBlocks" data.

Doesn't seem correct to mix and match like that.

> As long it is bigger than the packet, it's good.  So 0xffff probably works?

Your right it just has to point past; however, it helps to have it point to the exact ending when implementing (the code is easy to implmeent and it caught multiple bugs for me as well).

>   to find the end of the packet.  If not, I didn't quite understand.

You understood.

> send, and the pad packet is just replaced by real data if it happens to be

You understood perfectly.

> Please split 2.2.5 up by flag type into subsubsubsection.

Done.

> 2.4.21: Maybe need to briefly explain circuit breakers to us non-transport-types.

I think the reference should serve here. I try to avoid paraphrasing other documents as one often just gets it wrong (i.e., prefer a single source of truth). :)

> is this really worth supporting?

It doesn't take much to continue to allow for unidirectional use at the lowest layer (ESP/SA). It isn't relevant once you get to IKE where bidirectionality is required.

> Please break up third paragraph.

Done.

Thanks again for the review!

Chris.

---

From: Michael Richardson <[email-masked]>
Date: Tue, 16 Feb 2021 12:47:44 -0500

Christian Hopps <[email-masked]> wrote:
    >> * I'm glad you recommend PLMTUD, I suggest PMTUD is dead.  How do use
    >> PLMTUD?  Will you tell us later in the document, or is that new work?

    > I believed it was enough to just reference the mechanism (as we do for
    > PMTUD as well). This was added during the transport area review.

PMTUD relies on ICMP to say "too big"

PLMTUD relies on TCP to send a repeated ack (packet loss) to tell us that we
guessed wrong.  We now have RFC8899 too, but I don't know how to apply it,
and I think that some advice is in order.
I think that we need a PL defined.

    >> I guess, it the actual value does not matter: it's not remembered.  As
    >> long it is bigger than the packet, it's good.  So 0xffff probably
    >> works?

    > Your right it just has to point past; however, it helps to have it
    > point to the exact ending when implementing (the code is easy to
    > implmeent and it caught multiple bugs for me as well).

So, then please tell us exactly what to do, and what the receiver is supposed
to pay attention to.  I didn't like "can" here, for instance.

    >> -- I think you are telling me that the IPv4/IPv6 length field is good
    >> enough to find the end of the packet.  If not, I didn't quite
    >> understand.

    > You understood.

okay, please hit me over the head harder here.

    >> Later in 6.1, I learn that AGGFRAG_PAYLOADS are squatting on 'IPv5' I
    >> hope this will be acceptable :-) I think it's a reasonable hack, and I
    >> don't see us wrapping around IP versions within a hundred years,
    >> soooo...  And pad blocks are "IPv0"
    >>
    >>> This possible interleaving of all-pad payloads allows the sender to
    >>> always be able to send a tunnel packet, regardless of the
    >>> encapsulation computational requirements.
    >>
    >> I think that you are telling me that a sender can have some pad
    >> packets being created regularly (perhaps on a CPU dedicated to this)
    >> and almost ready to send, and the pad packet is just replaced by real
    >> data if it happens to be ready.

    > You understood perfectly.

I think that motivating this design choice with this implementation advice is worthwhile.

    >>> If the SA back to the sender is a non- AGGFRAG_PAYLOAD enabled SA
    >>> then an AGGFRAG_PAYLOAD empty payload (i.e., header only) is used to
    >>> convey the information.
    >>

    > It doesn't take much to continue to allow for unidirectional use at the
    > lowest layer (ESP/SA). It isn't relevant once you get to IKE where
    > bidirectionality is required.

I think that maybe this is in the MAY.
It's isn't clear to me if I need to support that or not.

---

From: Christian Hopps <[email-masked]>
Date: Fri, 26 Feb 2021 00:50:00 -0500

> On Feb 16, 2021, at 12:47 PM, Michael Richardson <[email-masked]> wrote:
> 
> Signed PGP part
> 

+considered the more robust option. For PLMTUD, congestion control
+payloads can be used as in-band probes (see [[Congestion Control
+AGGFRAG_PAYLOAD Payload Format]] and [[RFC8899]]).

Additionally a "P-bit" was added to the CC header so that loss of the in-band probes can be disregarded as loss-events by the CC algorithm.

> to pay attention to.  I didn't like "can" here, for instance.

...
-                 new data block. ~BlockOffset~ can count past the end
-                 of the ~DataBlocks~ data in which case all the
...
+                 new data block. If the start of a new data block
+                 occurs in a subsequent payload the ~BlockOffset~
+                 will point past the end of the ~DataBlocks~ data.
+                 In this case all the ~DataBlocks~ data belongs to
+                 the current data block being assembled. When the
+                 ~BlockOffset~ extends into subsequent payloads it
+                 continues to only count ~DataBlocks~ data (i.e.,
+                 it does not count subsequent packets
+                 non-~DataBlocks~ data such as header octets).

> okay, please hit me over the head harder here.

Replaced this entire section (others also found it confusing) with something much simpler.

+*** End Padding
+
+Since a data block's type is identified in its first 4-bits, the only
+time padding is required is when there is no data to encapsulate. For
+this end padding a ~Pad Data Block~ is used.

> It's isn't clear to me if I need to support that or not.

Greatly simplified this as well:

 ** Exclusive SA Use

 This document does not specify mixed use of an AGGFRAG_PAYLOAD
+enabled SA. A sender MUST only send AGGFRAG_PAYLOAD payloads over an
+SA configured for AGGFRAG_PAYLOAD use.

Thanks,
Chris.

---

From: Christian Hopps <[email-masked]>
Date: Fri, 26 Feb 2021 00:55:19 -0500

> On Feb 16, 2021, at 6:11 AM, Christian Hopps <[email-masked]> wrote:
> 
> Hi Michael,
> 
>> On Feb 14, 2021, at 11:45 PM, Michael Richardson <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> Signed PGP part
>> 

I've added a summary of "Summary of Receiver Processing" to the latest revision. This gathers what needs to be paid attention to into a single place.

Thanks,
Chris.

---

From: Michael Richardson <[email-masked]>
Date: Fri, 26 Feb 2021 12:21:23 -0500

Christian Hopps <[email-masked]> wrote:
    >>>> * I'm glad you recommend PLMTUD, I suggest PMTUD is dead.  How do
    >>>> use PLMTUD?  Will you tell us later in the document, or is that new
    >>>> work?  (does not look like you tell us)
    >>
    >>> I believed it was enough to just reference the mechanism (as we do
    >>> for PMTUD as well). This was added during the transport area review.
    >>
    >> PLMTUD relies on TCP to send a repeated ack (packet loss) to tell us
    >> that we guessed wrong.  We now have RFC8899 too, but I don't know how
    >> to apply it, and I think that some advice is in order.

    > Additionally a "P-bit" was added to the CC header so that loss of the
    > in-band probes can be disregarded as loss-events by the CC algorithm.

I still don't really see enough explanation of:

  1) what do my probe packets look like?
     Can I, for instance, send regular traffic, padded to the extra size?
     That's an optimistic view of things, but maybe appropriate.
     How do I get positive response that it was accepted?

  2) How do I learn about traffic that was dropped?

I'm on about this, because I think that PMTU issues are among the biggest
problem for IPsec.
If we can auto-tune the tunnel size, that's really a killer use.

---

From: Christian Hopps <[email-masked]>
Date: Sat, 27 Feb 2021 14:57:15 -0500

> On Feb 26, 2021, at 12:21 PM, Michael Richardson <[email-masked]> wrote:
> 
> 

This is what https://tools.ietf.org/html/rfc8899 <https://tools.ietf.org/html/rfc8899> documents. All that this document should do is provide the on the wire mechanism to send a probe packet and get a reply that it was received, as well as provide for not considering probe loss as loss events for CC (the p-bit). The CC header does this (the sender timestamp is echo'd back for RTT estimates). The implementation of RFC8899 can choose to send user data or not (RFC8899 recommends that one should avoid doing this if possible).

Thanks,
Chris.

---

From: Michael Richardson <[email-masked]>
Date: Sat, 27 Feb 2021 15:14:46 -0500

Christian Hopps <[email-masked]> wrote:
    >> 1) what do my probe packets look like?  Can I, for instance, send
    >> regular traffic, padded to the extra size?  That's an optimistic view
    >> of things, but maybe appropriate.  How do I get positive response that
    >> it was accepted?
    >>

    > This is what https://tools.ietf.org/html/rfc8899
    > <https://tools.ietf.org/html/rfc8899> documents. All that this document
    > should do is provide the on the wire mechanism to send a probe packet
    > and get a reply that it was received, as well as provide for not
    > considering probe loss as loss events for CC (the p-bit). The CC header
    > does this (the sender timestamp is echo'd back for RTT estimates). The
    > implementation of RFC8899 can choose to send user data or not (RFC8899
    > recommends that one should avoid doing this if possible).

I'll read again, but I am still perceiving a gap between RFC8899 and TFS.
Perhaps it is obvious to you, having designed and implemented it all over
three or four years.  In reading it, I didn't understand.

As I understand it then, I have to send a AGGGRAG_PAYLOAD packet of the new
size I want to test, I include a sender timestamp in TVal, and I look for
that echoed back in the TEcho to confirm receipt.  That's my PL?

I would know that it failed if I get a sender timestamp X (getting X+1),
that the oversize packet was lost.