Last Call Review of draft-ietf-tls-downgrade-scsv-03

Request Review of draft-ietf-tls-downgrade-scsv
Requested rev. no specific revision (document currently at 05)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2015-01-23
Requested 2015-01-15
Draft last updated 2015-01-22
Completed reviews Genart Last Call review of -03 by Russ Housley (diff)
Secdir Last Call review of -03 by Yoav Nir (diff)
Opsdir Last Call review of -03 by Al Morton (diff)
Assignment Reviewer Yoav Nir
State Completed
Review review-ietf-tls-downgrade-scsv-03-secdir-lc-nir-2015-01-22
Reviewed rev. 03 (document currently at 05)
Review result Has Issues
Review completed: 2015-01-22


Fear not, 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 for publication with a few minor issues.

The draft describes a mechanism for mitigating the security issue introduced by TLS version fallback mechanisms. TLS servers exist that do not conform to the TLS RFCs with regards to version negotiation. Such servers fail the negotiation even when there is a version of TLS that is supported by both them and the client. To interoperate with such servers, clients often retry the TLS handshake at a lower version. This is sometimes called the "downgrade dance". The downgrade dance allows a version downgrade to happen by either an active attacker or because of a network glitch.  This document introduces an indication in the TLS ClientHello that tells the server that a downgrade dance is in effect, allowing the server to terminate such a connection.

There has been much discussion on the TLS list about the downgrade dance being inherently insecure, and that publishing this document can be perceived as "blessing" this practice. The draft takes no position for or against, but rather assumes that the practice exists, and seeks to prevent such downgrades that are not necessary for connectivity. 

The Security Considerations section has some strange text about omitting the SCSV when "the protocol version given by ClientHello.client_version still provides an acceptable level of protection". This implies that it is appropriate to use a protocol version that does not provide and acceptable level of protection as long as the client sends the SCSV. This is not the case, and this is the reason that several browsers are disabling the downgrade to SSLv3. I realize it is difficult to write text that turns the secure/insecure dichotomy into a trichotomy (secure/so-so secure/insecure), but if we're going to make including the SCSV optional we need such text.

A scenario that bothers me is a server that returns an inappropriate_fallback whenever it receives a TLS_FALLBACK_SCSV with version TLS 1.1, but breaks at TLS 1.2. Such an (admittedly broken) server could potentially send the client into an endless loop of regular and fallback handshake attempts. An attacker could simulate this by RST-ing all TCP connections with TLS 1.2 in ClientHello. I think the draft should warn against doing the regular-fallback-regular cycle more than once. IOW: after receiving the alert, clients MUST NOT perform the downgrade dance on the following connection. Especially, don't implement the following downgrade dance:
 - regular handshake
 - fallback with SCSV
 - regular handshake
 - fallback without SCSV
If anyone ends up feeling the need to implement this, then we've failed miserably.

Nits and minor issues:

Since the client speaks first, it would make sense to have section 4 (Client Behavior) before section 3 (Server Behavior). Section 3 describes how the server responds to this SCSV when the document had only hinted about when the client should send it.

The IANA Considerations are very confusing, saying on the one hand that no values have been allocated, and on the other hand that IANA has assigned 0x56,0x00. Unless the authors are requesting that IANA assign the numbers that they have used in early implementations, it's better to just write "IANA is requested to assign..." and replace the numbers in the rest of the documents with TBDs (if you leave actual numbers and IANA assigns different identifiers, it's likely to be overlooked by the RFC editor).

A few missing articles:
"The fallback SCSV defined in this document is not *a* suitable substitute"
"if *the* TLS implementations also include support for (the) predecessor protocol SSL 3.0"