net: only False Start with forward secret servers.

Bodo 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.

This change also addresses follow up comments on
https://chromiumcodereview.appspot.com/10014010/ and adds the patch file that
was missing in that change.

BUG=none
TEST=net_unittests


Review URL: http://codereview.chromium.org/10136001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133255 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 66 insertions, 5 deletions

diff --git a/net/third_party/nss/patches/applypatches.sh b/net/third_party/nss/patches/applypatches.sh
index 766a0ca..9f099a2 100755
--- a/net/third_party/nss/patches/applypatches.sh
+++ b/net/third_party/nss/patches/applypatches.sh
@@ -35,4 +35,4 @@ patch -p6 < $patches_dir/encryptedclientcerts.patch
 
 patch -p4 < $patches_dir/dtls.patch
 
-patch -p6 < $patches_dir/falsestartnpn.patch
+patch -p5 < $patches_dir/falsestartnpn.patch
diff --git a/net/third_party/nss/patches/falsestartnpn.patch b/net/third_party/nss/patches/falsestartnpn.patch
new file mode 100644
index 0000000..5516fb7
--- /dev/null
+++ b/net/third_party/nss/patches/falsestartnpn.patch
@@ -0,0 +1,51 @@
+diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
+index e8a7f01..b6f4313 100644
+--- a/net/third_party/nss/ssl/ssl3con.c
++++ b/net/third_party/nss/ssl/ssl3con.c
+@@ -6087,10 +6087,17 @@ ssl3_CanFalseStart(sslSocket *ss) {
+ 	 !ss->sec.isServer &&
+ 	 !ss->ssl3.hs.isResuming &&
+ 	 ss->ssl3.cwSpec &&
++
++	 /* An attacker can control the selected ciphersuite so we only wish to
++	  * do False Start in the case that the selected ciphersuite is
++	  * sufficiently strong that the attack can gain no advantage.
++	  * Therefore we require an 80-bit cipher and a forward-secret key
++	  * exchange. */
+ 	 ss->ssl3.cwSpec->cipher_def->secret_key_size >= 10 &&
+-	(ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_rsa ||
+-	 ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_dh  ||
+-	 ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_ecdh);
++	(ss->ssl3.hs.kea_def->kea == kea_dhe_dss ||
++	 ss->ssl3.hs.kea_def->kea == kea_dhe_rsa ||
++	 ss->ssl3.hs.kea_def->kea == kea_ecdhe_ecdsa ||
++	 ss->ssl3.hs.kea_def->kea == kea_ecdhe_rsa);
+     ssl_ReleaseSpecReadLock(ss);
+     return rv;
+ }
+diff --git a/net/third_party/nss/ssl/ssl3ext.c b/net/third_party/nss/ssl/ssl3ext.c
+index 80c1f7f..6d5866b 100644
+--- a/net/third_party/nss/ssl/ssl3ext.c
++++ b/net/third_party/nss/ssl/ssl3ext.c
+@@ -567,6 +567,12 @@ ssl3_ServerHandleNextProtoNegoXtn(sslSocket * ss, PRUint16 ex_type, SECItem *dat
+ 	return SECFailure;
+     }
+ 
++    ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
++
++    /* TODO: server side NPN support would require calling
++     * ssl3_RegisterServerHelloExtensionSender here in order to echo the
++     * extension back to the client. */
++
+     return SECSuccess;
+ }
+ 
+@@ -635,6 +641,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type,
+ 	return SECFailure;
+     }
+ 
++    ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
++
+     SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE);
+     return SECITEM_CopyItem(NULL, &ss->ssl3.nextProto, &result);
+ }
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index d0eb042..b6f4313 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -6086,12 +6086,18 @@ ssl3_CanFalseStart(sslSocket *ss) {
     rv = ss->opt.enableFalseStart &&
 	 !ss->sec.isServer &&
 	 !ss->ssl3.hs.isResuming &&
-	 ssl3_ExtensionNegotiated(ss, ssl_next_proto_nego_xtn) &&
 	 ss->ssl3.cwSpec &&
+
+	 /* An attacker can control the selected ciphersuite so we only wish to
+	  * do False Start in the case that the selected ciphersuite is
+	  * sufficiently strong that the attack can gain no advantage.
+	  * Therefore we require an 80-bit cipher and a forward-secret key
+	  * exchange. */
 	 ss->ssl3.cwSpec->cipher_def->secret_key_size >= 10 &&
-	(ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_rsa ||
-	 ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_dh  ||
-	 ss->ssl3.hs.kea_def->exchKeyType == ssl_kea_ecdh);
+	(ss->ssl3.hs.kea_def->kea == kea_dhe_dss ||
+	 ss->ssl3.hs.kea_def->kea == kea_dhe_rsa ||
+	 ss->ssl3.hs.kea_def->kea == kea_ecdhe_ecdsa ||
+	 ss->ssl3.hs.kea_def->kea == kea_ecdhe_rsa);
     ssl_ReleaseSpecReadLock(ss);
     return rv;
 }
diff --git a/net/third_party/nss/ssl/ssl3ext.c b/net/third_party/nss/ssl/ssl3ext.c
index 4e043b8..6d5866b 100644
--- a/net/third_party/nss/ssl/ssl3ext.c
+++ b/net/third_party/nss/ssl/ssl3ext.c
@@ -569,6 +569,10 @@ ssl3_ServerHandleNextProtoNegoXtn(sslSocket * ss, PRUint16 ex_type, SECItem *dat
 
     ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
 
+    /* TODO: server side NPN support would require calling
+     * ssl3_RegisterServerHelloExtensionSender here in order to echo the
+     * extension back to the client. */
+
     return SECSuccess;
 }