Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(154)

Unified Diff: net/third_party/nss/ssl/ssl3gthr.c

Issue 27254004: Make SSL False Start work with asynchronous certificate validation (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: New patch from Brian Smith on 2013-10-29 11:03 PDT Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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,12 +275,18 @@
{
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 {
+
+ for (;;) {
PRBool handleRecordNow = PR_FALSE;
ssl_GetSSL3HandshakeLock(ss);
@@ -364,24 +370,56 @@
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;
+ }
}
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 SSL_ForceHandshake would become be a no-op after
+ * receiving the ServerHelloDone. By doing this check here, we
+ * ensure that SSL_ForceHandshake always tries to make a little
+ * progress.
+ */
+ 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);
+ if (!keepGoing) {
+ break;
+ }
+ }
ss->gs.readOffset = 0;
ss->gs.writeOffset = ss->gs.buf.len;
@@ -404,7 +442,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);

Powered by Google App Engine
This is Rietveld 408576698