Make SSL False Start work with asynchronous certificate validation

net/third_party/nss/ssl/ssl3gthr.c

Index: net/third_party/nss/ssl/ssl3gthr.c
===================================================================
--- net/third_party/nss/ssl/ssl3gthr.c	(revision 227672)
+++ net/third_party/nss/ssl/ssl3gthr.c	(working copy)
@@ -275,11 +275,17 @@
 {
     SSL3Ciphertext cText;
     int            rv;
-    PRBool         canFalseStart = PR_FALSE;
+    PRBool         keepGoing = PR_TRUE;
 
     SSL_TRC(30, ("ssl3_GatherCompleteHandshake"));
 
+    /* ssl3_HandleRecord may end up eventually calling ssl_FinishHandshake,
+     * which requires the 1stHandshakeLock, which must be acquired before the
+     * RecvBufLock.
+     */
+    PORT_Assert( ss->opt.noLocks || ssl_Have1stHandshakeLock(ss) );
     PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) );
+
     do {
 	PRBool handleRecordNow = PR_FALSE;
 
@@ -364,24 +370,52 @@
 
 	    cText.buf     = &ss->gs.inbuf;
 	    rv = ssl3_HandleRecord(ss, &cText, &ss->gs.buf);
+
+	    if (rv == (int) SECSuccess && ss->gs.buf.len > 0) {
+		/* We have application data to return to the application. This
+		 * prioritizes returning application data to the application over
+		 * completing any renegotiation handshake we may be doing.
+		 */
+		PORT_Assert(ss->firstHsDone);
+		PORT_Assert(cText.type == content_application_data);
+		break;
+	    }

[wtc, 2013/11/26 02:45:17]

Brian: to match the original code better, it seems that this if statement (lines
374-382) should be moved after line 386, so that it also handles the result of
the ssl3_HandleRecord call on line 325.

 	}
 	if (rv < 0) {
 	    return ss->recvdCloseNotify ? 0 : rv;
 	}
 
-	/* If we kicked off a false start in ssl3_HandleServerHelloDone, break
-	 * out of this loop early without finishing the handshake.
-	 */
-	if (ss->opt.enableFalseStart) {
-	    ssl_GetSSL3HandshakeLock(ss);
-	    canFalseStart = (ss->ssl3.hs.ws == wait_change_cipher ||
-			     ss->ssl3.hs.ws == wait_new_session_ticket) &&
-		            ssl3_CanFalseStart(ss);
-	    ssl_ReleaseSSL3HandshakeLock(ss);
+	PORT_Assert(keepGoing);
+	ssl_GetSSL3HandshakeLock(ss);
+	if (ss->ssl3.hs.ws == idle_handshake) {
+	    /* We are done with the current handshake so stop trying to
+	     * handshake. Note that it would be safe to test ss->firstHsDone
+	     * instead of ss->ssl3.hs.ws. By testing ss->ssl3.hs.ws instead,
+	     * we prioritize completing a renegotiation handshake over sending
+	     * application data.
+	     */
+	    PORT_Assert(ss->firstHsDone);
+	    PORT_Assert(!ss->ssl3.hs.canFalseStart);
+	    keepGoing = PR_FALSE;
+	} else if (ss->ssl3.hs.canFalseStart) {
+	    /* Prioritize sending application data over trying to complete
+	     * the handshake if we're false starting.
+	     *
+	     * If we were to do this check at the beginning of the loop instead
+	     * of here, then this function would become be a no-op after
+	     * receiving the ServerHelloDone in the false start case, and we
+	     * would never complete the handshake.
+	     */
+	    PORT_Assert(!ss->firstHsDone);
+
+	    if (ssl3_WaitingForStartOfServerSecondRound(ss)) {
+		keepGoing = PR_FALSE;
+	    } else {
+		ss->ssl3.hs.canFalseStart = PR_FALSE;
+	    }
 	}
-    } while (ss->ssl3.hs.ws != idle_handshake &&
-             !canFalseStart &&
-             ss->gs.buf.len == 0);
+	ssl_ReleaseSSL3HandshakeLock(ss);
+    } while (keepGoing);
 
     ss->gs.readOffset = 0;
     ss->gs.writeOffset = ss->gs.buf.len;
@@ -404,7 +438,10 @@
 {
     int            rv;
 
+    /* ssl3_GatherCompleteHandshake requires both of these locks. */
+    PORT_Assert( ss->opt.noLocks || ssl_Have1stHandshakeLock(ss) );
     PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) );
+
     do {
 	rv = ssl3_GatherCompleteHandshake(ss, flags);
     } while (rv > 0 && ss->gs.buf.len == 0);