Last Call Review of draft-ietf-tsvwg-sctp-failover-14
review-ietf-tsvwg-sctp-failover-14-secdir-lc-sparks-2015-12-22-00

Request Review of draft-ietf-tsvwg-sctp-failover
Requested rev. no specific revision (document currently at 16)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2015-12-23
Requested 2015-12-10
Other Reviews Genart Last Call review of -14 by Alexey Melnikov (diff)
Opsdir Last Call review of -14 by Jürgen Schönwälder (diff)
Review State Completed
Reviewer Robert Sparks
Review review-ietf-tsvwg-sctp-failover-14-secdir-lc-sparks-2015-12-22
Posted at https://www.ietf.org/mail-archive/web/secdir/current/msg06297.html
Reviewed rev. 14 (document currently at 16)
Review result Has Nits
Draft last updated 2015-12-22
Review completed: 2015-12-22

Review
review-ietf-tsvwg-sctp-failover-14-secdir-lc-sparks-2015-12-22

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 primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.



Summary: Ready for publication as Proposed Standard but with nits that 


should be considered






This document defines a set of new SCTP sender behaviors that lead to 


quicker failover. The behaviors have an obvious security consideration 


(it is much easier for an attacker trigger failover, and potentially 


steer traffic to the most favorable of the available paths for the 


attacker's purposes). The document makes this very clear, and has a 


reasonable discussion in the Security Considerations section.






The shepherd writeup notes that what is here is widely (if partially in 


some cases) implemented and deployed.




There are a few places I suggest would benefit from more attention:



There are several places in the text that allow (MAY) the sender to 


choose not to behave in the manner this document is trying to 


standardize. See item B in section 3.2, and item C in section 4 for 


example. These seem out of place - if something is doing those things, 


you might as well say they're implementing some other unstandardized 


extension for failover. Why do you need to include these statements? 


They don't standardize whatever private "because I know better" behavior 


the endpoints they are discussing are engaging in. Consider deleting 


them, or restructuring the text to make this look less like protocol 


definition. (It's particularly odd that B in 3.2 uses MAY, but A does 


not use SHOULD).






The third paragraph of section 4 has a "does not compromise the fault 


tolerance" condition, leaving what would be considered a compromise very 


vague. Again, an endpoint choosing a different dormant state operation 


than the one you're standardizing here isn't behaving in a standard way. 


Why do you need this paragraph? If you're going to keep it, consider 


pointing to or providing more discussion on what you mean by the condition.






There are many unusual phrases used in the text. These are harder to 


read than they need to be, and will be even harder to translate. Please 


consider an editorial pass _before_ this is in the RFC Editor's hands. 


Search for "compromisation", "at exceed of which", "failover to deploy", 


and "unequivocally information" to get a feel for what I'm pointing to.