[secdir] Secdir review of draft-ietf-tls-session-hash-04

[Radia Perlman <[e-mail]>]

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,