Bug 373276 ? Enhance SSL's Bypass feature to withstand failures. r=neil,alexei

4 files changed, 136 insertions, 88 deletions

diff --git a/security/nss/lib/ssl/config.mk b/security/nss/lib/ssl/config.mk
index 35e4147b5..27bee978e 100644
--- a/security/nss/lib/ssl/config.mk
+++ b/security/nss/lib/ssl/config.mk
@@ -39,6 +39,10 @@ ifdef NISCC_TEST
 DEFINES += -DNISCC_TEST
 endif
 
+ifdef NSS_SURVIVE_DOUBLE_BYPASS_FAILURE
+DEFINES += -DNSS_SURVIVE_DOUBLE_BYPASS_FAILURE
+endif
+
 ifeq (,$(filter-out WIN%,$(OS_TARGET)))
 
 # don't want the 32 in the shared library name
diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c
index bf37f19d3..68ddd5057 100644
--- a/security/nss/lib/ssl/ssl3con.c
+++ b/security/nss/lib/ssl/ssl3con.c
@@ -86,6 +86,9 @@ static SECStatus ssl3_SendFinished(          sslSocket *ss, PRInt32 flags);
 static SECStatus ssl3_SendServerHello(       sslSocket *ss);
 static SECStatus ssl3_SendServerHelloDone(   sslSocket *ss);
 static SECStatus ssl3_SendServerKeyExchange( sslSocket *ss);
+static SECStatus ssl3_NewHandshakeHashes(    sslSocket *ss);
+static SECStatus ssl3_UpdateHandshakeHashes( sslSocket *ss, unsigned char *b, 
+                                             unsigned int l);
 
 static SECStatus Null_Cipher(void *ctx, unsigned char *output, int *outputLen,
 			     int maxOutputLen, const unsigned char *input,
@@ -1523,8 +1526,8 @@ ssl3_InitPendingCipherSpec(sslSocket *ss, PK11SymKey *pms)
 	    goto done;  /* err code set by ssl3_DeriveMasterSecret */
 	}
     }
-    if (pwSpec->msItem.len && pwSpec->msItem.data) {
-	/* Double Bypass */
+    if (ss->opt.bypassPKCS11 && pwSpec->msItem.len && pwSpec->msItem.data) {
+	/* Double Bypass succeeded in extracting the master_secret */
 	const ssl3KEADef * kea_def = ss->ssl3.hs.kea_def;
 	PRBool             isTLS   = (PRBool)(kea_def->tls_keygen ||
                                 (pwSpec->version > SSL_LIBRARY_VERSION_3_0));
@@ -2652,6 +2655,18 @@ ssl3_DeriveMasterSecret(sslSocket *ss, PK11SymKey *pms)
 	 */
 	rv = PK11_ExtractKeyValue(pwSpec->master_secret);
 	if (rv != SECSuccess) {
+#if defined(NSS_SURVIVE_DOUBLE_BYPASS_FAILURE)
+	    /* The double bypass failed.  
+	     * Attempt to revert to an all PKCS#11, non-bypass method.
+	     * Do we need any unacquired locks here?
+	     */
+	    ss->opt.bypassPKCS11 = 0;
+	    rv = ssl3_NewHandshakeHashes(ss);
+	    if (rv == SECSuccess) {
+		rv = ssl3_UpdateHandshakeHashes(ss, ss->ssl3.hs.messages.buf, 
+		                                    ss->ssl3.hs.messages.len);
+	    }
+#endif
 	    return rv;
 	} 
 	/* This returns the address of the secItem inside the key struct,
@@ -2809,6 +2824,75 @@ loser:
     return SECFailure;
 }
 
+static SECStatus 
+ssl3_RestartHandshakeHashes(sslSocket *ss)
+{
+    SECStatus rv = SECSuccess;
+
+    if (ss->opt.bypassPKCS11) {
+	ss->ssl3.hs.messages.len = 0;
+	MD5_Begin((MD5Context *)ss->ssl3.hs.md5_cx);
+	SHA1_Begin((SHA1Context *)ss->ssl3.hs.sha_cx);
+    } else {
+	rv = PK11_DigestBegin(ss->ssl3.hs.md5);
+	if (rv != SECSuccess) {
+	    ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
+	    return rv;
+	}
+	rv = PK11_DigestBegin(ss->ssl3.hs.sha);
+	if (rv != SECSuccess) {
+	    ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
+	    return rv;
+	}
+    }
+    return rv;
+}
+
+static SECStatus
+ssl3_NewHandshakeHashes(sslSocket *ss)
+{
+    PK11Context *md5  = NULL;
+    PK11Context *sha  = NULL;
+
+    /*
+     * note: We should probably lookup an SSL3 slot for these
+     * handshake hashes in hopes that we wind up with the same slots
+     * that the master secret will wind up in ...
+     */
+    SSL_TRC(30,("%d: SSL3[%d]: start handshake hashes", SSL_GETPID(), ss->fd));
+    if (ss->opt.bypassPKCS11) {
+	PORT_Assert(!ss->ssl3.hs.messages.buf && !ss->ssl3.hs.messages.space);
+	ss->ssl3.hs.messages.buf = NULL;
+	ss->ssl3.hs.messages.space = 0;
+    } else {
+	ss->ssl3.hs.md5 = md5 = PK11_CreateDigestContext(SEC_OID_MD5);
+	ss->ssl3.hs.sha = sha = PK11_CreateDigestContext(SEC_OID_SHA1);
+	if (md5 == NULL) {
+	    ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
+	    goto loser;
+	}
+	if (sha == NULL) {
+	    ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
+	    goto loser;
+	}
+    }
+    if (SECSuccess == ssl3_RestartHandshakeHashes(ss)) {
+	return SECSuccess;
+    }
+
+loser:
+    if (md5 != NULL) {
+    	PK11_DestroyContext(md5, PR_TRUE);
+	ss->ssl3.hs.md5 = NULL;
+    }
+    if (sha != NULL) {
+    	PK11_DestroyContext(sha, PR_TRUE);
+	ss->ssl3.hs.sha = NULL;
+    }
+    return SECFailure;
+
+}
+
 /*
  * Handshake messages
  */
@@ -2830,6 +2914,9 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, unsigned char *b, unsigned int l)
     if (ss->opt.bypassPKCS11) {
 	MD5_Update((MD5Context *)ss->ssl3.hs.md5_cx, b, l);
 	SHA1_Update((SHA1Context *)ss->ssl3.hs.sha_cx, b, l);
+#if defined(NSS_SURVIVE_DOUBLE_BYPASS_FAILURE)
+	rv = sslBuffer_Append(&ss->ssl3.hs.messages, b, l);
+#endif
 	return rv;
     }
     rv = PK11_DigestOp(ss->ssl3.hs.md5, b, l);
@@ -3383,21 +3470,11 @@ ssl3_SendClientHello(sslSocket *ss)
 
     SSL_TRC(30,("%d: SSL3[%d]: reset handshake hashes",
 	    SSL_GETPID(), ss->fd ));
-    if (ss->opt.bypassPKCS11) {
-	MD5_Begin((MD5Context *)ss->ssl3.hs.md5_cx);
-	SHA1_Begin((SHA1Context *)ss->ssl3.hs.sha_cx);
-    } else {
-	rv = PK11_DigestBegin(ss->ssl3.hs.md5);
-	if (rv != SECSuccess) {
-	    ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
-	    return rv;
-	}
-	rv = PK11_DigestBegin(ss->ssl3.hs.sha);
-	if (rv != SECSuccess) {
-	    ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
-	    return rv;
-	}
+    rv = ssl3_RestartHandshakeHashes(ss);
+    if (rv != SECSuccess) {
+	return rv;
     }
+
     /* We ignore ss->sec.ci.sid here, and use ssl_Lookup because Lookup
      * handles expired entries and other details.
      * XXX If we've been called from ssl2_BeginClientHandshake, then
@@ -6477,9 +6554,8 @@ ssl3_HandleRSAClientKeyExchange(sslSocket *ss,
 	rv = PK11_PrivDecryptPKCS1(serverKey, rsaPmsBuf, &outLen, 
 				   sizeof rsaPmsBuf, enc_pms.data, enc_pms.len);
 	if (rv != SECSuccess) {
-	    /* avoid Bleichenbacker attack.  generate faux pms. */
-	    rv = PK11_GenerateRandom(rsaPmsBuf, sizeof rsaPmsBuf);
-	    /* ignore failure */
+	    /* triple bypass failed.  Let's try for a double bypass. */
+	    goto double_bypass;
 	} else if (ss->opt.detectRollBack) {
 	    SSL3ProtocolVersion client_version = 
 					 (rsaPmsBuf[0] << 8) | rsaPmsBuf[1];
@@ -6498,6 +6574,7 @@ ssl3_HandleRSAClientKeyExchange(sslSocket *ss,
 	}
 	rv = ssl3_InitPendingCipherSpec(ss,  NULL);
     } else {
+double_bypass:
 	/*
 	 * unwrap pms out of the incoming buffer
 	 * Note: CKM_SSL3_MASTER_KEY_DERIVE is NOT the mechanism used to do 
@@ -6526,6 +6603,7 @@ ssl3_HandleRSAClientKeyExchange(sslSocket *ss,
 	    return SECFailure;
 	}
 
+	/* This step will derive the MS from the PMS, among other things. */
 	rv = ssl3_InitPendingCipherSpec(ss,  pms);
 	PK11_FreeSymKey(pms);
     }
@@ -7620,20 +7698,9 @@ ssl3_HandleHandshakeMessage(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
     if (ss->ssl3.hs.msg_type == client_hello) {
 	SSL_TRC(30,("%d: SSL3[%d]: reset handshake hashes",
 		SSL_GETPID(), ss->fd ));
-	if (ss->opt.bypassPKCS11) {
-	    MD5_Begin((MD5Context *)ss->ssl3.hs.md5_cx);
-	    SHA1_Begin((SHA1Context *)ss->ssl3.hs.sha_cx);
-	} else {
-	    rv = PK11_DigestBegin(ss->ssl3.hs.md5);
-	    if (rv != SECSuccess) {
-		ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
-		return rv;
-	    }
-	    rv = PK11_DigestBegin(ss->ssl3.hs.sha);
-	    if (rv != SECSuccess) {
-		ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
-		return rv;
-	    }
+	rv = ssl3_RestartHandshakeHashes(ss);
+	if (rv != SECSuccess) {
+	    return rv;
 	}
     }
     /* We should not include hello_request messages in the handshake hashes */
@@ -8131,8 +8198,6 @@ ssl3_InitCipherSpec(sslSocket *ss, ssl3CipherSpec *spec)
 static SECStatus
 ssl3_InitState(sslSocket *ss)
 {
-    PK11Context *md5  = NULL;
-    PK11Context *sha  = NULL;
     SECStatus    rv;
     PORT_Assert( ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
 
@@ -8154,47 +8219,12 @@ ssl3_InitState(sslSocket *ss)
 #endif
     ssl_ReleaseSpecWriteLock(ss);
 
-    /*
-     * note: We should probably lookup an SSL3 slot for these
-     * handshake hashes in hopes that we wind up with the same slots
-     * that the master secret will wind up in ...
-     */
-    SSL_TRC(30,("%d: SSL3[%d]: start handshake hashes", SSL_GETPID(), ss->fd));
-    if (ss->opt.bypassPKCS11) {
-	MD5_Begin((MD5Context *)ss->ssl3.hs.md5_cx);
-	SHA1_Begin((SHA1Context *)ss->ssl3.hs.sha_cx);
-    } else {
-	ss->ssl3.hs.md5 = md5 = PK11_CreateDigestContext(SEC_OID_MD5);
-	if (md5 == NULL) {
-	    ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
-	    goto loser;
-	}
-	rv = PK11_DigestBegin(ss->ssl3.hs.md5);
-	if (rv != SECSuccess) {
-	    ssl_MapLowLevelError(SSL_ERROR_MD5_DIGEST_FAILURE);
-	    goto loser;
-	}
-
-	sha = ss->ssl3.hs.sha = PK11_CreateDigestContext(SEC_OID_SHA1);
-	if (sha == NULL) {
-	    ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
-	    goto loser;
-	}
-	rv = PK11_DigestBegin(ss->ssl3.hs.sha);
-	if (rv != SECSuccess) {
-	    ssl_MapLowLevelError(SSL_ERROR_SHA_DIGEST_FAILURE);
-	    goto loser;
-	}
+    rv = ssl3_NewHandshakeHashes(ss);
+    if (rv == SECSuccess) {
+	ss->ssl3.initialized = PR_TRUE;
     }
 
-    ss->ssl3.initialized = PR_TRUE;
-
-    return SECSuccess;
-
-loser:
-    if (md5 != NULL) PK11_DestroyContext(md5, PR_TRUE);
-    if (sha != NULL) PK11_DestroyContext(sha, PR_TRUE);
-    return SECFailure;
+    return rv;
 }
 
 /* Returns a reference counted object that contains a key pair.
@@ -8461,7 +8491,7 @@ ssl3_RedoHandshake(sslSocket *ss, PRBool flushCache)
     return rv;
 }
 
-/* Called from ssl_FreeSocket() in sslsock.c */
+/* Called from ssl_DestroySocketContents() in sslsock.c */
 void
 ssl3_DestroySSL3Info(sslSocket *ss)
 {
@@ -8491,6 +8521,12 @@ ssl3_DestroySSL3Info(sslSocket *ss)
     if (ss->ssl3.hs.sha) {
 	PK11_DestroyContext(ss->ssl3.hs.sha,PR_TRUE);
     }
+    if (ss->ssl3.hs.messages.buf) {
+    	PORT_Free(ss->ssl3.hs.messages.buf);
+	ss->ssl3.hs.messages.buf = NULL;
+	ss->ssl3.hs.messages.len = 0;
+	ss->ssl3.hs.messages.space = 0;
+    }
 
     /* free the SSL3Buffer (msg_body) */
     PORT_Free(ss->ssl3.hs.msg_body.buf);
diff --git a/security/nss/lib/ssl/sslimpl.h b/security/nss/lib/ssl/sslimpl.h
index 3540b2eee..c5c57a867 100644
--- a/security/nss/lib/ssl/sslimpl.h
+++ b/security/nss/lib/ssl/sslimpl.h
@@ -721,6 +721,7 @@ const ssl3CipherSuiteDef *suite_def;
     PRBool                usedStepDownKey;  /* we did a server key exchange. */
     sslBuffer             msgState;    /* current state for handshake messages*/
                                        /* protected by recvBufLock */
+    sslBuffer             messages;    /* Accumulated handshake messages */
 #ifdef NSS_ENABLE_ECC
     PRUint32              negotiatedECCurves; /* bit mask */
 #endif /* NSS_ENABLE_ECC */
@@ -1144,6 +1145,8 @@ extern SECStatus ssl2_BeginServerHandshake(sslSocket *ss);
 extern int       ssl_Do1stHandshake(sslSocket *ss);
 
 extern SECStatus sslBuffer_Grow(sslBuffer *b, unsigned int newLen);
+extern SECStatus sslBuffer_Append(sslBuffer *b, const void * data, 
+		                  unsigned int len);
 
 extern void      ssl2_UseClearSendFunc(sslSocket *ss);
 extern void      ssl_ChooseSessionIDProcs(sslSecurityInfo *sec);
diff --git a/security/nss/lib/ssl/sslsecur.c b/security/nss/lib/ssl/sslsecur.c
index b0fda391f..46aeffae3 100644
--- a/security/nss/lib/ssl/sslsecur.c
+++ b/security/nss/lib/ssl/sslsecur.c
@@ -433,6 +433,20 @@ sslBuffer_Grow(sslBuffer *b, unsigned int newLen)
     return SECSuccess;
 }
 
+SECStatus 
+sslBuffer_Append(sslBuffer *b, const void * data, unsigned int len)
+{
+    unsigned int newLen = b->len + len;
+    SECStatus rv;
+
+    rv = sslBuffer_Grow(b, newLen);
+    if (rv != SECSuccess)
+    	return rv;
+    PORT_Memcpy(b->buf + b->len, data, len);
+    b->len += len;
+    return SECSuccess;
+}
+
 /*
 ** Save away write data that is trying to be written before the security
 ** handshake has been completed. When the handshake is completed, we will
@@ -442,22 +456,13 @@ sslBuffer_Grow(sslBuffer *b, unsigned int newLen)
 SECStatus 
 ssl_SaveWriteData(sslSocket *ss, const void *data, unsigned int len)
 {
-    unsigned int newlen;
     SECStatus    rv;
 
     PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) );
-    newlen = ss->pendingBuf.len + len;
-    if (newlen > ss->pendingBuf.space) {
-	rv = sslBuffer_Grow(&ss->pendingBuf, newlen);
-	if (rv) {
-	    return rv;
-	}
-    }
-    SSL_TRC(5, ("%d: SSL[%d]: saving %d bytes of data (%d total saved so far)",
-		 SSL_GETPID(), ss->fd, len, newlen));
-    PORT_Memcpy(ss->pendingBuf.buf + ss->pendingBuf.len, data, len);
-    ss->pendingBuf.len = newlen;
-    return SECSuccess;
+    rv = sslBuffer_Append(&ss->pendingBuf, data, len);
+    SSL_TRC(5, ("%d: SSL[%d]: saving %u bytes of data (%u total saved so far)",
+		 SSL_GETPID(), ss->fd, len, ss->pendingBuf.len));
+    return rv;
 }
 
 /*