Last Comment Bug 810582 - Only do SSL False Start with forward secret servers
: Only do SSL False Start with forward secret servers
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.12.8
: All All
: P2 normal with 1 vote (vote)
: 3.14.1
Assigned To: Adam Langley
:
:
Mentors:
Depends on: 525092
Blocks: 713933
  Show dependency treegraph
 
Reported: 2012-11-10 04:45 PST by Wan-Teh Chang
Modified: 2012-11-15 12:16 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch by Adam Langley (1.52 KB, patch)
2012-11-10 04:45 PST, Wan-Teh Chang
wtc: review+
brian: superreview+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2012-11-10 04:45:34 PST
Created attachment 680354 [details] [diff] [review]
Patch by Adam Langley

This patch was originally written by Adam Langley in
http://codereview.chromium.org/10136001. Here is his
description of the patch.

  [Bodo Moeller] made the point that we originally sacrificed
  an aspect of forward secrecy in order to use False Start
  widely. Specifically, an attacker can alter the handshake
  and cause a non-forward secure ciphersuite to be selected
  and the client's initial write will not be forward secret.

  Since we are no longer trying to use False Start everywhere,
  we can close that gap by only allowing it for forward secret
  connections.
Comment 1 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-11-10 08:22:19 PST
I sr+d the patch but I am not sure if this is the exactly the right policy for all applications. But, if we choose a different policy then we'll write a patch on top of this to enable a more flexible policy. 

Another potential problem with allowing False Start with RSA handshakes is that the peer hasn't demonstrated any proof of possession of the private key by the time the client has false started. I am not sure if this is actually an exploitable problem though.

When the server chooses a (EC)DHE cipher suite, we actually receive a signed proof that they chose that cipher suite because they signed their ServerKeyExchange with parameters of the given type; that means that we'd already have detected tampering with the cipher suite selection during the processing of ServerKeyExchange. So, AFAICT, this patch doesn't just prevent the attacker from choosing a non-(EC)DHE cipher suite; it actually completely prevents the attacker from altering the cipher suite at all, right?

BTW, I am not sure that servers can be confident that clients will never False Start with RSA cipher suites. I heard that IE10 is doing False Start aggressively now, and I am not sure they are limiting themselves to forward-secret cipher suites.
Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-11-10 09:07:58 PST
(In reply to Brian Smith (:bsmith) from comment #1)
> processing of ServerKeyExchange. So, AFAICT, this patch doesn't just prevent
> the attacker from choosing a non-(EC)DHE cipher suite; it actually
> completely prevents the attacker from altering the cipher suite at all,
> right?

It prevents an attacker from choosing the key exchange algorithm and keys, but it doesn't prevent them from choosing the symmetric algorithms.
Comment 3 Adam Langley 2012-11-12 08:35:20 PST
Unfortunately the ServerKeyExchange signature doesn't cover the previous handshake messages in the same way that client certs do. So, if we assume the existence of a server which does DHE on a 512-bit group and (in preference) ECDHE on P-256, it's still possible for the attacker to return a valid, signed DHE ServerKeyExchange and have the client use it.

However, this attack isn't limited to False Start because the same can be done simply by making the client downgrade to SSLv3.

So the attacker can still choose the weakest, forward-secret key exchange that the server supports.
Comment 4 Wan-Teh Chang 2012-11-12 17:30:33 PST
Patch checked in on the NSS trunk (NSS 3.14.1).

Checking in ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.194; previous revision: 1.193
done

Note You need to log in before you can comment on or make changes to this bug.