Make SSL False Start work with asynchronous certificate validation

net/third_party/nss/ssl/ssl3gthr.c

 /*
  * Gather (Read) entire SSL3 records from socket into buffer.  
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "cert.h"
 #include "ssl.h"
 #include "sslimpl.h"
@@ skipping 257 matching lines @@
  *    and from SSL_ForceHandshake in sslsecur.c
  *    and from ssl3_GatherAppDataRecord below (<- DoRecv in sslsecur.c).
  *
  * Caller must hold the recv buf lock.
  */
 int
 ssl3_GatherCompleteHandshake(sslSocket *ss, int flags)
 {
     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;
 
         ssl_GetSSL3HandshakeLock(ss);
 
         /* Without this, we may end up wrongly reporting
          * SSL_ERROR_RX_UNEXPECTED_* errors if we receive any records from the
          * peer while we are waiting to be restarted.
          */
         if (ss->ssl3.hs.restartTarget) {
@@ skipping 64 matching lines @@
                 cText.seq_num.high = 0; cText.seq_num.low = 0;
                 for (i = 0; i < 4; i++) {
                     cText.seq_num.high <<= 8; cText.seq_num.low <<= 8;
                     cText.seq_num.high |= ss->gs.hdr[3 + i];
                     cText.seq_num.low |= ss->gs.hdr[7 + i];
                 }
             }
 
             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;
     return 1;
 }
 
 /* Repeatedly gather in a record and when complete, Handle that record.
  * Repeat this until some application data is received.
  *
  * Returns  1 when application data is available.
  * Returns  0 if ssl3_GatherData hits EOF.
  * Returns -1 on read error, or PR_WOULD_BLOCK_ERROR, or handleRecord error.
  * Returns -2 on SECWouldBlock return from ssl3_HandleRecord.
  *
  * Called from DoRecv in sslsecur.c
  * Caller must hold the recv buf lock.
  */
 int
 ssl3_GatherAppDataRecord(sslSocket *ss, int flags)
 {
     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);
 
     return rv;
 }