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

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

Issue 23621040: Make SSL False Start work with asynchronous certificate validation (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Update the patch file 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
« no previous file with comments | « net/third_party/nss/ssl/ssl.h ('k') | net/third_party/nss/ssl/ssl3gthr.c » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/third_party/nss/ssl/ssl3con.c
===================================================================
--- net/third_party/nss/ssl/ssl3con.c (revision 227363)
+++ 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,35 +7344,42 @@
return rv;
}
-PRBool
-ssl3_CanFalseStart(sslSocket *ss) {
- PRBool rv;
+static SECStatus
+ssl3_CheckFalseStart(sslSocket *ss)
+{
+ SECStatus rv;
+ PRBool maybeFalseStart = PR_TRUE;
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.
- */
+ /* 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);
- rv = ss->opt.enableFalseStart &&
- !ss->sec.isServer &&
- !ss->ssl3.hs.isResuming &&
- ss->ssl3.cwSpec &&
+ if (ss->ssl3.cwSpec->cipher_def->secret_key_size < 10) {
+ ss->ssl3.hs.canFalseStart = PR_FALSE;
+ maybeFalseStart = PR_FALSE;
+ }
+ ssl_ReleaseSpecReadLock(ss);
+ if (!maybeFalseStart) {
+ return SECSuccess;
+ }
- /* 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);
+ if (!ss->canFalseStartCallback) {
+ rv = SSL_DefaultCanFalseStart(ss->fd, &ss->ssl3.hs.canFalseStart);
+ } else {
+ rv = (ss->canFalseStartCallback)(ss->fd,
+ ss->canFalseStartCallbackData,
+ &ss->ssl3.hs.canFalseStart);
+ }
+
+ if (rv != SECSuccess) {
+ ss->ssl3.hs.canFalseStart = PR_FALSE;
+ }
+
return rv;
}
@@ -7500,20 +7507,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,8 +7572,16 @@
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)) {
+ if (ss->handshakeCallback &&
+ (ss->ssl3.hs.canFalseStart && !ss->canFalseStartCallback)) {
+ /* Call the handshake callback here for backwards compatibility with
+ * applications that were using false start before
+ * canFalseStartCallback was added. Note that we do this after calling
+ * ssl3_SendFinished, which includes a call to ssl3_FlushHandshake,
+ * just in case the application is relying on having the handshake
+ * messages flushed to the network before its handshake callback is
+ * called.
+ */
(ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData);
}
@@ -10147,13 +10201,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 +10325,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));
+ }
+
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 +10340,39 @@
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);
+ if (rv == SECSuccess &&
+ ss->handshakeCallback &&
+ (ss->ssl3.hs.canFalseStart && !ss->canFalseStartCallback)) {
+ /* Call the handshake callback here for backwards compatibility
+ * with applications that were using false start before
+ * canFalseStartCallback was added.
+ */
+ (ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData);
+ }
+ } else {
+ rv = SECSuccess;
+ }
}
done:
@@ -10983,6 +11068,8 @@
SECStatus
ssl3_FinishHandshake(sslSocket * ss)
{
+ PRBool falseStarted;
+
PORT_Assert( ss->opt.noLocks || ssl_HaveRecvBufLock(ss) );
PORT_Assert( ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss) );
PORT_Assert( ss->ssl3.hs.restartTarget == NULL );
@@ -10990,6 +11077,7 @@
/* The first handshake is now completed. */
ss->handshake = NULL;
ss->firstHsDone = PR_TRUE;
+ ss->enoughFirstHsDone = PR_TRUE;
if (ss->ssl3.hs.cacheSID) {
(*ss->sec.cache)(ss->sec.ci.sid);
@@ -10997,9 +11085,14 @@
}
ss->ssl3.hs.ws = idle_handshake;
+ falseStarted = ss->ssl3.hs.canFalseStart;
+ ss->ssl3.hs.canFalseStart = PR_FALSE; /* False Start phase is complete */
- /* Do the handshake callback for sslv3 here, if we cannot false start. */
- if (ss->handshakeCallback != NULL && !ssl3_CanFalseStart(ss)) {
+ /* Call the handshake callback for sslv3 here, unless we called it already
+ * for the case where false start was done without a canFalseStartCallback.
+ */
+ if (ss->handshakeCallback &&
+ !(falseStarted && !ss->canFalseStartCallback)) {
(ss->handshakeCallback)(ss->fd, ss->handshakeCallbackData);
}
« no previous file with comments | « net/third_party/nss/ssl/ssl.h ('k') | net/third_party/nss/ssl/ssl3gthr.c » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698