Last Call Review of draft-ietf-tcpm-newcwv-10
review-ietf-tcpm-newcwv-10-secdir-lc-kaduk-2015-05-21-00

Request Review of draft-ietf-tcpm-newcwv
Requested rev. no specific revision (document currently at 13)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2015-05-22
Requested 2015-05-15
Other Reviews Genart Last Call review of -10 by Roni Even (diff)
Review State Completed
Reviewer Benjamin Kaduk
Review review-ietf-tcpm-newcwv-10-secdir-lc-kaduk-2015-05-21
Posted at https://www.ietf.org/mail-archive/web/secdir/current/msg05697.html
Reviewed rev. 10 (document currently at 13)
Review result Has Nits
Draft last updated 2015-05-21
Review completed: 2015-05-21

Review
review-ietf-tcpm-newcwv-10-secdir-lc-kaduk-2015-05-21

[resending with the correct spelling of the draft alias.  Sorry for the
extra copy, everyone else.]

Hi all,

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written with the intent of improving
security requirements and considerations in IETF drafts.  Comments
not addressed in last call may be included in AD reviews during the
IESG review.  Document editors and WG chairs should treat these
comments just like any other last call comments.

I believe this document is ready with nits.

The security considerations defer to those of RFC 5681 with the claim that
this document just describes an algorithm for one aspect of the congestion
control procedures and thus that the generic security considerations
apply; this claim seems true.  The security considerations section of RFC
5681 feels a little bit short, but it does seem to cover the relevant
risks, namely that an attacker could make the sender send more slowly or
more quickly, with the latter possibly driving the network into congestion
collapse.  These are issues that protocol designers and implementors must
be aware of (and avoiding congestion collapse is more important).

The authors seem to have taken sufficient care to avoid this algorithm
contributing to congestion collapse, and the concern about an attacker
being able to slow down a sender is not really feasible to mitigate at
this level, so the security considerations of RFC 5681, short as they are,
seem adequate here.  One would hope that implementors know that failing to
follow the specification could lead to congestion collapse, so an explicit
warning is not needed.

The nits are basically all grammatical; I'll include them below and expect
people other than the document authors/shepherd to stop reading here.

-Ben


My apologies in advance if I am applying too much of an American English
perspective to this document written in British English.


In the abstract, "TCP is used to support traffic that [...]", is "support"
the best verb to use (as opposed to "transmit", "carry", "transport",
etc.)?

The first paragraph of the Introduction might have a little more text to
clarify how the cwnd differs from the FlightSize, for non-experts such as
myself.  (The cwnd is how much the sender is permitted to have
outstanding/unack'd and the FlightSize is how much the sender actually has
outstanding/unack'd?)  Just another sentence after the second one, "The
FlightSize is how many unacknowledged packets/bytes that are currently
outstanding, and the cwnd is the maximum value the sender will allow
FlightSize to take on" would be plenty.

Introduction, third paragraph, """[CWV] introduced the terminology of
"application limited periods".  This document describes [...]"""  Usually,
in an RFC, "this document" refers to the RFC itself, but in this case,
"this document" seems to be being used to refer to the other document, RFC
2861.  So, the references and text could be made more clear.

A couple sentences later, "or by changing the rate the application sends",
maybe put an "at which" in there somewhere?  Absent context, I would
expect "the rate the application sends" to mean that an application
protocol was conveying a number which is interpreted as a rate in some
fashion.

Still in the Introduction, in the summary of the document, paragraph for
section 4, maybe s/does this in a way/does so in a way/ ?  Also, in the
parenthetical at the end of that sentence, I think it reads better as
"including both application-limited and idle senders".

Still in the introduction, in the "goals of this update" list, first item,
do bulk transfers consume the cwnd or fill it?

I'm not sure that the fifth item is fully grammatical.  Maybe add in an
"to send extra traffic" somewhere?

In the sixth item, there's a singular/plural mismatch between flows and
network (unless it's supposed to be "the network").

Section 2, first paragraph, "the behaviour where a sender transmitted at a
rate less than allowed by cwnd", is "where" or "when" better?

Section 2, second paragraph, third sentence: I think it should start
"While this" instead of just "This".  The first comma in this sentence
could also be removed, if desired.

In section 3 (Terminology), it might be nice to clarify that the listed
terms are new to this document, or in addition to the RFC 5681 terms, or
something like that.  (I.e., "Additionally, the following terminology
...".)

Section 4.2, last paragraph, a space is missing after the [RFC6675]
citation.

In section 4.4: if I understand correctly, the sliding window for pipeAck
calculation is smaller than the NVP, so it is not the absolute amount of
data which the sender transmits which can push pipeAck over (1/2)*cwnd,
but rather the rate at which the data is transmitted.

Also in section 4.4, there is some superficial inconsistency between "a
sender that enters the non-validated phase SHOULD preserve the cwnd" and
the text telling the sender how to reduce cwnd if it encounters congestion
and the (outer) bullet point wherein "a sender determines whether to
increase the cwnd based on ...".  The last paragraph of the section helps
clarify the situation, but it might help readability to clarify the
exceptions to the SHOULD (and the rationale for them) right after the
claim is made.

In section 4.4.1, there is a missing space after "recovery" and before the
internal reference to Section 4.2.

Section 4.5.1, comma after "e.g.".