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

[Sean Turner <[e-mail]>]

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 <[e-mail]> wrote:
> 
> 
> 
>> On Feb 3, 2021, at 10:38 PM, Sean Turner <[e-mail]> wrote:
>> 
>> Hi! I mostly just have nits, but there are a couple of questions interspersed below.
> 
> 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"
> 
>> 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.
> 
> 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).
> 
>> 
>> 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.
> 
> Fixed.
> 
>> 
>> s2, para 1:
>> 
>> is the "(SA)" needed?  ... tunnel (SA) ...
>> s/it’s/its
> 
> Fixed.
> 
>> 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.
> 
> 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
> 
>> s2, 4th para: s/requested tunnel used bandwidth/
>> 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.
> 
>> 
>> s2.1, 1st para: s/In order to maximize bandwidth IP-TFS/
>> In order to maximize bandwidth, IP-TFS
> 
> Fixed
> 
>> 
>> 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).
> 
> 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).
> 
>> 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,
> 
> 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.
> 
>> 
>> 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.
> 
> Fixed
> 
>> 
>> 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
> 
> Fixed.
> 
>> 
>> 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.
> 
> Fixed
> 
>> 
>> s2.2.3, penultimate para: s/b/c/because
> 
> Fixed
> 
>> 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
> 
> Simplified to "senders MUST NOT"... :)
> 
>> s2.2.3.1: Implementations here refers to senders so maybe:
>> s/An implementation/Senders
>> s/Implementation implementing/Senders implementing ;)
> 
> Fixed
> 
>> s2.2.4: s/In order to support/To support
> 
> Fixed
> 
>> s2.2.5, 1st para:
>> s/(by design!)/(by design)    ;)
>> s/although an implementation/although a sender
>> s/An implementation SHOULD/A sender SHOULD
> 
> Fixed
> 
>> s2.2.5, 2nd para:
>> s/an implementation/a sender
>> s/([RFC3168])/[RFC3168]
> 
> Fixed
> 
>> s2.2.6, 1st para: s/([RFC0791])/[RFC0791]
> 
> Fixed, and update other single refs to not use parens.
> 
>> 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.
> 
> Fixed
> 
>> s2.2.7, 1st para: s/am implementation/a sender
> 
> Fixed
> 
>> 
>> s2.2.7, 2nd para: s/([RFC4301])/[RFC4301]
> 
> Fixed
> 
>> 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.
> 
> Fixed
> 
>> s2.4.1, 1st para: s/In the non-congestion controlled mode IP_TFS /In the non-congestion controlled mode, IP-TFS
> 
> Fixed
> 
>> 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?
> 
> 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.
> 
>> 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.
> 
> Fixed.
> 
>> s2.4.2, 3rd para: s/In particular these/In particular, these
> 
> Fixed
> 
>> s2.4.2, 4th para:
>> Should the must be MUST?
>> 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
> 
>> 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.
> 
> Fixed
> 
>> s2.4.2, last para: s/It’s/It is
> 
> Fixed
> 
>> 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
> 
> 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
> 
>> 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.
> 
> 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
> 
>> s5.1, 3rd para:
>> s/To request using the/To request use of the
>> s/then response/then the response
> 
> Fixed
> 
>> s5.1, penultimate para: Should the should not be SHOULD NOT?
> 
> Fixed
> 
>> s6.1: s/8 bit/8-bit
>> 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.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
> 
> 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
> 
>> s8:
>> 
>> 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.
> 
>> 
>> Cheers,
>> spt
>> 
>>> On Jan 24, 2021, at 20:55, Tero Kivinen <[e-mail]> 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.
>>> --
>>> [e-mail]
>>> 
>>> _______________________________________________
>>> IPsec mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/ipsec
>> 
>> _______________________________________________
>> IPsec mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/ipsec
>