Last Call Review of draft-ietf-tls-session-hash-04
review-ietf-tls-session-hash-04-secdir-lc-perlman-2015-04-16-00

Request Review of draft-ietf-tls-session-hash
Requested rev. no specific revision (document currently at 06)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2015-04-13
Requested 2015-04-02
Authors Karthikeyan Bhargavan, Antoine Delignat-Lavaud, Alfredo Pironti, Adam Langley, Marsh Ray
Draft last updated 2015-04-16
Completed reviews Genart Last Call review of -04 by Russ Housley (diff)
Genart Telechat review of -05 by Russ Housley (diff)
Secdir Last Call review of -04 by Radia Perlman (diff)
Assignment Reviewer Radia Perlman 
State Completed
Review review-ietf-tls-session-hash-04-secdir-lc-perlman-2015-04-16
Reviewed rev. 04 (document currently at 06)
Review result Has Issues
Review completed: 2015-04-16

Review
review-ietf-tls-session-hash-04-secdir-lc-perlman-2015-04-16

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.

This document proposes an extension and a small computation change to TLS to

mitigate the "triple handshake" (and perhaps other related

yet-to-be-discovered) vulnerabilities in TLS. For details of the attack,

see: Bhargavan, K., Delignat-Lavaud, A., Fournet, C., Pironti, A., and P.

Strub, "Triple Handshakes and Cookie Cutters: Breaking and Fixing

Authentication over TLS", IEEE Symposium on Security and Privacy

(Oakland'14) , 2014.

In my judgement, the proposed change does a fine job of addressing the

vulnerability. I assume this has gotten lots of other eyes as well.

I hope that someone has tested the major server implementations of TLS to

assure that they follow the spec and will ignore the new unrecognized

extension if a client presents it. The TLS spec clearly says that they MUST

do so, but there has been a history of hacks done by implementers to get

around bugs in legacy implementations. If there are a significant number of

implementations that get this wrong, it would be worth considering

implementing the extension in some different way (e.g. as an additional

proposed cipher-suite).

This vulnerability is subtle and would not affect the security of most uses

of TLS, though evaluating whether it does or does not in any particular case

is difficult so it makes sense to implement this change universally over

time. This spec, however, takes a hard line in the trade-off between

security and interoperability that I believe needs to be carefully

considered. In particular, I believe lots of implementers will *not* follow

this spec in implementing the feature in order to get greater

interoperability with deployed systems.

In particular, section 5.2 paragraph 4 & 5 say:

"If the server receives a ClientHello without the extension, it SHOULD abort

the handshake. If it chooses to continue, then it MUST NOT include the

extension in the ServerHello."

"If a client receives a ServerHello without the extension, it SHOULD abort

the handshake."

Implementations that follow these SHOULDs will not interoperate with any

existing implementation of TLS, making a phased deployment of the new

software impossible. I believe it would be preferable to say that

implementations of TLS SHOULD be configurable to reject peers that do not

implement the extension so that improved security at the expense of

interoperability can be enabled after a critical mass of deployments exist.

In section 5.3, there is similar language to disable session resumption when

the extension is not present. While this may be more acceptable because it

will not break interoperability, it *will* cause a significant performance

degradation in some important scenarios. I would again recommend that it be

a configuration parameter so that no one will refuse to deploy this change

because of performance concerns.

Section 5.4 also mandates breaking behavior (and here it is a MUST) in

scenarios where there is most likely to be a triple handshake vulnerability.

Given that there are cases with no such vulnerability and given that people

will be reluctant to deploy software that turns a subtle security

vulnerability into non-interoperability, I believe this should be a matter

of configuration.

Section 5.4 paragraph 4 seems to be undermining earlier requirements, saying

that it MAY be safe (to break rules stated earlier). Taking this to heart, I

would propose that the "MUST abort" clauses in sections 5.2 and 5.3 be

softened to at least allow - and perhaps recommend - implementations to

support this scenario.

-------

Nits:

In section 6.2, it is claimed that the security of TLS depends on the hash

function used being collision resistant, use of MD5 and SHA1 is NOT

RECOMMENDED. That would rule out use of TLS 1.0 and TLS 1.1. While a worthy

goal, I don't believe this enhancement makes migration away from TLS 1.0 and

TLS 1.1 any more urgent. I also don't believe that TLS depends on the hash

function being collision resistant (but only that it is second preimage

resistant).

P9: "each other identity" -> "each other's identities"

P10: "that where hence" -> "that were hence"

P11: "server/ Hence," -> "server. Hence,