[IPsec] AD Review of draft-ietf-ipsecme-iptfs-12

[Roman Danyliw <[e-mail]>]

Hi!

I performed an AD review of draft-ietf-ipsecme-iptfs-12.  Thank you for this work and the patience of the WG in getting it processed.

I have a number of comments below, but the document is in good shape so please process them concurrently with the IETF LC feedback.  

** Thank you for getting an early TSVART review and addressing that feedback given congestion control issues this work raises

** Section 1.  Editorial.

OLD
While directly
   obscuring the data with encryption [RFC4303], the traffic pattern
   itself exposes information due to variations in its shape and timing

NEW
While directly obscuring the data with encryption [RFC4303], the patterns in the message traffic may 
exposes information due to variations in its shape and timing.

** Section 2.1.  Typo. s/the the/the/

** Section 2.4.2.1.

Enabling circuit breakers is also a reason a user
   may wish to enable congestion information reports ...

Consider if s/a user may wish to/local policy may/ to generalize who is doing the tuning.  There are a few other places were "user" is the noun tuning the stack.

** Section 3.

Thus, to support
   congestion control the receiver must have a paired SA back to the
   sender

Should the be a "MUST" (i.e., s/receiver must have/received MUST have)?

** Section 3.

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.

I'm missing something -- if the SA is not AGGFRAG_PAYLOAD-enabled, what how can it send anything AGGFRAG related?

** Section 4.

All IP-TFS specific configuration should be specified
   at the unidirectional tunnel ingress (sending) side

Should is used here.  Not in the normative sense.  What would be the alternative to specifying the unidirectional tunnels on the sending side?

** Section 4.1.

For non-congestion
   controlled mode, the bandwidth SHOULD be configured.  

What would it mean for this not be configured?  That the end-point set a packet size and rate?

** Section 4.3.  Is this a generic statement that how congestion control is done is a local configuration option, or is this a Boolean configuration of use congestion control or not (aka, Section 6.1.1 vs. 6.1.2.?)

** Section 5.1.  

If any
   requirement flags are not understood or cannot be supported by the
   receiver then the receiver SHOULD NOT enable use of AGGFRAG_PAYLOAD

Is the WG sure that this shouldn't be MUST NOT?  What's the user case where you want to continue?

** Section 6.1.4.  Typo. s/it's USE_AGGFRAG/its USE_AGGFRAG/

** Section 7.1.  Why the reference to RFC7120?  Since this registry is going to be "Expert Review", this document doesn't apply.

** Section 7.1.  To double check, there is no more guidance to provide to the expert reviewer?

** Section 8.  Editorial.

This document describes an aggregation and fragmentation mechanism
   and it use to add TFC to IP traffic.  The use described is expected
   to increase the security of the traffic being transported.  

The first sentence doesn't parse and I recommend more precision on the second. Perhaps:

NEW

This document describes an aggregation and fragmentation mechanism to efficiently implement TFC for IP traffic. This approach is expected to reduce the efficacy of traffic analysis on IPSec communication.

** Section 8. Editorial. s/As noted in (Section 3.1)/As noted in Section 3.1,/

** Section 8.

As noted previously in Section 2.4.2, for TFC to be fully maintained ...

What does it mean to for TFC to be "fully maintained"?

Regards,
Roman