Index: net/third_party/nss/ssl/ssl3con.c |
=================================================================== |
--- net/third_party/nss/ssl/ssl3con.c (revision 227672) |
+++ net/third_party/nss/ssl/ssl3con.c (working copy) |
@@ -2890,7 +2890,7 @@ |
SSL_TRC(3, ("%d: SSL3[%d] SendRecord type: %s nIn=%d", |
SSL_GETPID(), ss->fd, ssl3_DecodeContentType(type), |
nIn)); |
- PRINT_BUF(3, (ss, "Send record (plain text)", pIn, nIn)); |
+ PRINT_BUF(50, (ss, "Send record (plain text)", pIn, nIn)); |
PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) ); |
@@ -7344,36 +7344,47 @@ |
return rv; |
} |
-PRBool |
-ssl3_CanFalseStart(sslSocket *ss) { |
- PRBool rv; |
+static SECStatus |
+ssl3_CheckFalseStart(sslSocket *ss) |
+{ |
+ SECStatus rv; |
PORT_Assert( ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss) ); |
+ PORT_Assert( !ss->ssl3.hs.authCertificatePending ); |
- /* XXX: does not take into account whether we are waiting for |
- * SSL_AuthCertificateComplete or SSL_RestartHandshakeAfterCertReq. If/when |
- * that is done, this function could return different results each time it |
- * would be called. |
- */ |
+ if (ss->canFalseStartCallback) { |
+ PRBool maybeFalseStart; |
- ssl_GetSpecReadLock(ss); |
- rv = ss->opt.enableFalseStart && |
- !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 always require an 80-bit cipher. */ |
+ ssl_GetSpecReadLock(ss); |
+ maybeFalseStart = ss->ssl3.cwSpec->cipher_def->secret_key_size >= 10; |
+ ssl_ReleaseSpecReadLock(ss); |
- /* 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->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; |
+ if (maybeFalseStart) { |
+ rv = (ss->canFalseStartCallback)(ss->fd, |
+ ss->canFalseStartCallbackData, |
+ &ss->ssl3.hs.canFalseStart); |
+ if (rv == SECSuccess) { |
+ SSL_TRC(3, ("%d: SSL[%d]: false start callback returned %s", |
+ SSL_GETPID(), ss->fd, |
+ ss->ssl3.hs.canFalseStart ? "TRUE" : "FALSE")); |
+ } else { |
+ SSL_TRC(3, ("%d: SSL[%d]: false start callback failed (%s)", |
+ SSL_GETPID(), ss->fd, |
+ PR_ErrorToName(PR_GetError()))); |
+ } |
+ return rv; |
+ } |
+ } |
+ |
+ SSL_TRC(3, ("%d: SSL[%d]: no false start callback so no false start", |
+ SSL_GETPID(), ss->fd)); |
+ |
+ ss->ssl3.hs.canFalseStart = PR_FALSE; |
+ return SECSuccess; |
wtc
2013/10/17 00:10:53
Nit: it seems better to handle the "no false start
briansmith
2013/10/17 01:42:40
Done.
|
} |
static SECStatus ssl3_SendClientSecondRound(sslSocket *ss); |
@@ -7463,6 +7474,9 @@ |
} |
if (ss->ssl3.hs.authCertificatePending && |
(sendClientCert || ss->ssl3.sendEmptyCert || ss->firstHsDone)) { |
+ SSL_TRC(3, ("%d: SSL3[%p]: deferring ssl3_SendClientSecondRound because" |
+ " certificate authentication is still pending.", |
+ SSL_GETPID(), ss->fd)); |
ss->ssl3.hs.restartTarget = ssl3_SendClientSecondRound; |
return SECWouldBlock; |
} |
@@ -7500,20 +7514,59 @@ |
goto loser; /* err code was set. */ |
} |
- /* XXX: If the server's certificate hasn't been authenticated by this |
- * point, then we may be leaking this NPN message to an attacker. |
+ /* This must be done after we've set ss->ssl3.cwSpec in |
+ * ssl3_SendChangeCipherSpecs because SSL_GetChannelInfo uses information |
+ * from cwSpec. This must be done before we call ssl3_CheckFalseStart |
+ * because the false start callback (if any) may need the information from |
+ * the functions that depend on this being set. |
*/ |
+ ss->enoughFirstHsDone = PR_TRUE; |
+ |
if (!ss->firstHsDone) { |
+ /* XXX: If the server's certificate hasn't been authenticated by this |
+ * point, then we may be leaking this NPN message to an attacker. |
+ */ |
rv = ssl3_SendNextProto(ss); |
if (rv != SECSuccess) { |
goto loser; /* err code was set. */ |
} |
} |
+ |
rv = ssl3_SendEncryptedExtensions(ss); |
if (rv != SECSuccess) { |
goto loser; /* err code was set. */ |
} |
+ if (!ss->firstHsDone) { |
+ if (ss->opt.enableFalseStart) { |
+ if (!ss->ssl3.hs.authCertificatePending) { |
+ /* When we fix bug 589047, we will need to know whether we are |
+ * false starting before we try to flush the client second |
+ * round to the network. With that in mind, we purposefully |
+ * call ssl3_CheckFalseStart before calling ssl3_SendFinished, |
+ * which includes a call to ssl3_FlushHandshake, so that |
+ * no application develops a reliance on such flushing being |
+ * done before its false start callback is called. |
+ */ |
+ ssl_ReleaseXmitBufLock(ss); |
+ rv = ssl3_CheckFalseStart(ss); |
+ ssl_GetXmitBufLock(ss); |
+ if (rv != SECSuccess) { |
+ goto loser; |
+ } |
+ } else { |
+ /* The certificate authentication and the server's Finished |
+ * message are racing each other. If the certificate |
+ * authentication wins, then we will try to false start in |
+ * ssl3_AuthCertificateComplete. |
+ */ |
+ SSL_TRC(3, ("%d: SSL3[%p]: deferring false start check because" |
+ " certificate authentication is still pending.", |
+ SSL_GETPID(), ss->fd)); |
+ } |
+ } |
+ } |
+ |
rv = ssl3_SendFinished(ss, 0); |
if (rv != SECSuccess) { |
goto loser; /* err code was set. */ |
@@ -7526,11 +7579,6 @@ |
else |
ss->ssl3.hs.ws = wait_change_cipher; |
- /* Do the handshake callback for sslv3 here, if we can false start. */ |
- if (ss->handshakeCallback != NULL && ssl3_CanFalseStart(ss)) { |
- (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); |
- } |
- |
return SECSuccess; |
loser: |
@@ -10147,13 +10195,6 @@ |
ss->ssl3.hs.authCertificatePending = PR_TRUE; |
rv = SECSuccess; |
- |
- /* XXX: Async cert validation and False Start don't work together |
- * safely yet; if we leave False Start enabled, we may end up false |
- * starting (sending application data) before we |
- * SSL_AuthCertificateComplete has been called. |
- */ |
- ss->opt.enableFalseStart = PR_FALSE; |
} |
if (rv != SECSuccess) { |
@@ -10278,6 +10319,12 @@ |
} else if (ss->ssl3.hs.restartTarget != NULL) { |
sslRestartTarget target = ss->ssl3.hs.restartTarget; |
ss->ssl3.hs.restartTarget = NULL; |
+ |
+ if (target == ssl3_FinishHandshake) { |
+ SSL_TRC(3,("%d: SSL3[%p]: certificate authentication lost the race" |
+ " with peer's finished message", SSL_GETPID(), ss->fd)); |
+ } |
wtc
2013/10/17 00:10:53
If |target| is ssl3_SendClientSecondRound here, do
briansmith
2013/10/17 01:42:40
if target == ssl3_SendClientSecondRound then that
|
+ |
rv = target(ss); |
/* Even if we blocked here, we have accomplished enough to claim |
* success. Any remaining work will be taken care of by subsequent |
@@ -10287,7 +10334,30 @@ |
rv = SECSuccess; |
} |
} else { |
- rv = SECSuccess; |
+ SSL_TRC(3, ("%d: SSL3[%p]: certificate authentication won the race with" |
+ " peer's finished message", SSL_GETPID(), ss->fd)); |
+ |
+ PORT_Assert(!ss->firstHsDone); |
+ PORT_Assert(!ss->sec.isServer); |
+ PORT_Assert(!ss->ssl3.hs.isResuming); |
+ PORT_Assert(ss->ssl3.hs.ws == wait_change_cipher || |
+ ss->ssl3.hs.ws == wait_finished || |
+ ss->ssl3.hs.ws == wait_new_session_ticket); |
+ |
+ /* ssl3_SendClientSecondRound deferred the false start check because |
+ * certificate authentication was pending, so we have to do it now. |
+ */ |
+ if (ss->opt.enableFalseStart && |
+ !ss->firstHsDone && |
+ !ss->sec.isServer && |
+ !ss->ssl3.hs.isResuming && |
+ (ss->ssl3.hs.ws == wait_change_cipher || |
+ ss->ssl3.hs.ws == wait_finished || |
+ ss->ssl3.hs.ws == wait_new_session_ticket)) { |
+ rv = ssl3_CheckFalseStart(ss); |
+ } else { |
+ rv = SECSuccess; |
+ } |
} |
done: |
@@ -10913,9 +10983,6 @@ |
return rv; |
} |
- ss->gs.writeOffset = 0; |
- ss->gs.readOffset = 0; |
- |
if (ss->ssl3.hs.kea_def->kea == kea_ecdhe_rsa) { |
effectiveExchKeyType = kt_rsa; |
} else { |
@@ -10980,6 +11047,9 @@ |
return rv; |
} |
+/* The return type is SECStatus instead of void because this function needs |
+ * to have type sslRestartTarget. |
+ */ |
SECStatus |
ssl3_FinishHandshake(sslSocket * ss) |
{ |
@@ -10989,19 +11059,16 @@ |
/* The first handshake is now completed. */ |
ss->handshake = NULL; |
- ss->firstHsDone = PR_TRUE; |
if (ss->ssl3.hs.cacheSID) { |
(*ss->sec.cache)(ss->sec.ci.sid); |
ss->ssl3.hs.cacheSID = PR_FALSE; |
} |
+ ss->ssl3.hs.canFalseStart = PR_FALSE; /* False Start phase is complete */ |
ss->ssl3.hs.ws = idle_handshake; |
- /* Do the handshake callback for sslv3 here, if we cannot false start. */ |
- if (ss->handshakeCallback != NULL && !ssl3_CanFalseStart(ss)) { |
- (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData); |
- } |
+ ssl_FinishHandshake(ss); |
return SECSuccess; |
} |
@@ -11569,11 +11636,10 @@ |
unsigned int headerLen; |
PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) ); |
+ PORT_Assert( ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss) ); |
wtc
2013/10/17 00:10:53
Why do we need to call ssl3_HandleRecord while hol
briansmith
2013/10/17 01:42:40
ssl3_GatherCompleteHandshake is the function that
wtc
2013/10/17 15:28:14
I see. We should reconsider this change. This func
|
if (!ss->ssl3.initialized) { |
- ssl_GetSSL3HandshakeLock(ss); |
rv = ssl3_InitState(ss); |
- ssl_ReleaseSSL3HandshakeLock(ss); |
if (rv != SECSuccess) { |
return rv; /* ssl3_InitState has set the error code. */ |
} |
@@ -11933,7 +11999,6 @@ |
/* XXX Get the xmit lock here. Odds are very high that we'll be xmiting |
* data ang getting the xmit lock here prevents deadlocks. |
*/ |
- ssl_GetSSL3HandshakeLock(ss); |
/* All the functions called in this switch MUST set error code if |
** they return SECFailure or SECWouldBlock. |
@@ -11964,9 +12029,7 @@ |
break; |
} |
- ssl_ReleaseSSL3HandshakeLock(ss); |
return rv; |
- |
} |
/* |