Only call SSL_OptionSet in an initial handshake.

This works around the locking problem with SSL_OptionSet in
a renegotiation.

The previous fix r134584 is reverted.

R=agl@chromium.org,rsleevi@chromium.org
BUG=125299
TEST=Run a Chrome debug build on Windows. Visit a site that
does SSL renegotiation. There should be no assertion failure.
For example, visit http://foaf.me, click the
"Login to your account" link at the upperright corner of the
page. Then press the "sign in" button.

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

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

5 files changed, 19 insertions, 25 deletions

diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 7c3440f..54152e9 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -2056,6 +2056,25 @@ SECStatus SSLClientSocketNSS::OwnAuthCertHandler(void* arg,
                                                  PRFileDesc* socket,
                                                  PRBool checksig,
                                                  PRBool is_server) {
+#ifdef SSL_ENABLE_FALSE_START
+  SSLClientSocketNSS* that = reinterpret_cast<SSLClientSocketNSS*>(arg);
+  if (!that->server_cert_nss_) {
+    // Only need to turn off False Start in the initial handshake. Also, it is
+    // unsafe to call SSL_OptionSet in a renegotiation because the "first
+    // handshake" lock isn't already held, which will result in an assertion
+    // failure in the ssl_Get1stHandshakeLock call in SSL_OptionSet.
+    PRBool npn;
+    SECStatus rv = SSL_HandshakeNegotiatedExtension(socket,
+                                                    ssl_next_proto_nego_xtn,
+                                                    &npn);
+    if (rv != SECSuccess || !npn) {
+      // If the server doesn't support NPN, then we don't do False Start with
+      // it.
+      SSL_OptionSet(socket, SSL_ENABLE_FALSE_START, PR_FALSE);
+    }
+  }
+#endif
+
   // Tell NSS to not verify the certificate.
   return SECSuccess;
 }
diff --git a/net/third_party/nss/README.chromium b/net/third_party/nss/README.chromium
index 22e7a15..8f968ba 100644
--- a/net/third_party/nss/README.chromium
+++ b/net/third_party/nss/README.chromium
@@ -68,10 +68,6 @@ Patches:
   * Enable False Start only when the server supports forward secrecy.
     patches/falsestartnpn.patch
 
-  * Enable False Start only when the server supports NPN. (Not for
-    upstreaming.)
-    patches/falsestartnpn2.patch
-
   * Reject DH generators and public values equal to zero or one.
     patches/dhvalues.patch
 
diff --git a/net/third_party/nss/patches/applypatches.sh b/net/third_party/nss/patches/applypatches.sh
index e3c00eb..55167bf 100755
--- a/net/third_party/nss/patches/applypatches.sh
+++ b/net/third_party/nss/patches/applypatches.sh
@@ -38,6 +38,4 @@ patch -p4 < $patches_dir/dtls.patch
 
 patch -p5 < $patches_dir/falsestartnpn.patch
 
-patch -p5 < $patches_dir/falsestartnpn2.patch
-
 patch -p5 < $patches_dir/dhvalues.patch
diff --git a/net/third_party/nss/patches/falsestartnpn2.patch b/net/third_party/nss/patches/falsestartnpn2.patch
deleted file mode 100644
index 00b2934..0000000
--- a/net/third_party/nss/patches/falsestartnpn2.patch
+++ /dev/null
@@ -1,15 +0,0 @@
-diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
-index 5476fa5..1465558 100644
---- a/net/third_party/nss/ssl/ssl3con.c
-+++ b/net/third_party/nss/ssl/ssl3con.c
-@@ -6108,6 +6108,10 @@ ssl3_CanFalseStart(sslSocket *ss) {
-     rv = ss->opt.enableFalseStart &&
- 	 !ss->sec.isServer &&
- 	 !ss->ssl3.hs.isResuming &&
-+	 /* This check for NPN is performed here because we can't call
-+	  * SSL_HandshakeNegotiatedExtension in the auth callback because of
-+	  * lock ordering issues. See crbug.com/125299 */
-+	 ssl3_ExtensionNegotiated(ss, ssl_next_proto_nego_xtn) &&
- 	 ss->ssl3.cwSpec &&
- 
- 	 /* An attacker can control the selected ciphersuite so we only wish to
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index 1465558..5476fa5 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -6108,10 +6108,6 @@ ssl3_CanFalseStart(sslSocket *ss) {
     rv = ss->opt.enableFalseStart &&
 	 !ss->sec.isServer &&
 	 !ss->ssl3.hs.isResuming &&
-	 /* This check for NPN is performed here because we can't call
-	  * SSL_HandshakeNegotiatedExtension in the auth callback because of
-	  * lock ordering issues. See crbug.com/125299 */
-	 ssl3_ExtensionNegotiated(ss, ssl_next_proto_nego_xtn) &&
 	 ss->ssl3.cwSpec &&
 
 	 /* An attacker can control the selected ciphersuite so we only wish to