Split RC4-encrypted records.

net/third_party/nss/ssl/ssl3con.c

Index: net/third_party/nss/ssl/ssl3con.c
===================================================================
--- net/third_party/nss/ssl/ssl3con.c	(revision 185536)
+++ net/third_party/nss/ssl/ssl3con.c	(working copy)
@@ -1792,6 +1792,8 @@
 	goto done;
     }
 
+    pwSpec->rc4EncryptedBytes = 0;
+
     /* Generic behaviors -- common to all crypto methods */
     if (!IS_DTLS(ss)) {
 	pwSpec->read_seq_num.high = pwSpec->write_seq_num.high = 0;
@@ -2188,6 +2190,11 @@
 }
 
 /* Caller must hold the spec read lock. */
+/* The function uses the three members of wrBuf as follows:
+ * - Reads wrBuf->buf and wrBuf->space. Note that it does not read wrBuf->len.
+ * - Writes data into the buffer that wrBuf->buf points to.
+ * - Sets wrBuf->len to the number of bytes written on successful return.
+ */
 SECStatus
 ssl3_CompressMACEncryptRecord(ssl3CipherSpec *   cwSpec,
 		              PRBool             isServer,
@@ -2363,6 +2370,11 @@
 	wrBuf->buf[4] = LSB(cipherBytes);
     }
 
+    if (type == content_handshake &&
+	cwSpec->cipher_def->calg == ssl_calg_rc4) {
+	cwSpec->rc4EncryptedBytes += cipherBytes;
+    }
+
     ssl3_BumpSequenceNumber(&cwSpec->write_seq_num);
 
     return SECSuccess;
@@ -2416,6 +2428,7 @@
     PRINT_BUF(3, (ss, "Send record (plain text)", pIn, nIn));
 
     PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) );
+    PORT_Assert( wrBuf->len == 0 );
 
     capRecordVersion = ((flags & ssl_SEND_FLAG_CAP_RECORD_VERSION) != 0);
 
@@ -2449,7 +2462,7 @@
     while (nIn > 0) {
 	PRUint32  contentLen = PR_MIN(nIn, MAX_FRAGMENT_LENGTH);
 	unsigned int spaceNeeded;
-	unsigned int numRecords;
+	unsigned int numRecords = 1;
 
 	ssl_GetSpecReadLock(ss);    /********************************/
 
@@ -2461,8 +2474,32 @@
 	     * as explained in the documentation for SSL_CBC_RANDOM_IV in ssl.h
 	     */
 	    numRecords = 2;
-	} else {
-	    numRecords = 1;
+	} else if (nIn > 1 && ss->opt.cbcRandomIV &&
+	    type == content_application_data &&
+	    ss->ssl3.cwSpec->cipher_def->calg == ssl_calg_rc4 &&
+	    ss->ssl3.cwSpec->rc4EncryptedBytes < SSL3_BIASED_RC4_BYTES) {
+	    /* We will split the first few bytes of the record into their own
+	     * one-byte records so that the biased RC4 keystream bytes are
+	     * mostly used to encrypt record MACs.
+	     */
+	    PRInt32 inBytes = nIn;
+
+	    numRecords = 0;
+
+	    /* Count how many one-byte records are needed to exhaust the
+	     * biased RC4 keystream bytes. */
+	    while (inBytes > 0 &&
+		   ss->ssl3.cwSpec->rc4EncryptedBytes < SSL3_BIASED_RC4_BYTES) {
+		ss->ssl3.cwSpec->rc4EncryptedBytes +=

[agl, 2013/03/22 13:44:46]

I doesn't really matter, but these bytes will be accounted for twice in
rc4EncryptedBytes, no? Once here, and then again in
ssl3_CompressMACEncryptRecord.

[wtc, 2013/03/22 17:31:40]

No. In ssl3_CompressMACEncryptRecord, there is a
type == content_handshake check so it will only apply to
handshake records.

I am unhappy about this aspect of the CL because
application data records and handshake records are
counted in different places. I will try to fix this.

+		    1 + ss->ssl3.cwSpec->mac_size;
+		inBytes--;
+		numRecords++;
+	    }
+
+	    /* Send any remaining bytes in one record. */
+	    if (inBytes > 0) {
+		numRecords++;
+	    }
 	}
 
 	spaceNeeded = contentLen + (numRecords * SSL3_BUFFER_FUDGE);
@@ -2479,32 +2516,30 @@
 	    }
 	}
 
-	if (numRecords == 2) {
-	    sslBuffer secondRecord;
+	if (numRecords > 1) {
+	    sslBuffer recordBuf;
+	    unsigned int i;
 
-	    rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
-					       ss->sec.isServer, IS_DTLS(ss),
-					       capRecordVersion, type, pIn,
-					       1, wrBuf);
-	    if (rv != SECSuccess)
-	        goto spec_locked_loser;
+	    wrBuf->len = 0;
+	    for (i = 0; i < numRecords; i++) {
+		recordBuf.buf = wrBuf->buf + wrBuf->len;
+		recordBuf.len = 0;
+		recordBuf.space = wrBuf->space - wrBuf->len;
 
-	    PRINT_BUF(50, (ss, "send (encrypted) record data [1/2]:",
-	                   wrBuf->buf, wrBuf->len));
+		rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
+						   ss->sec.isServer,
+						   IS_DTLS(ss),
+						   capRecordVersion, type,
+						   pIn + i,
+						   (i != numRecords - 1) ?
+						   1 : contentLen - i,
+						   &recordBuf);
+		if (rv != SECSuccess)
+		    break;
 
-	    secondRecord.buf = wrBuf->buf + wrBuf->len;
-	    secondRecord.len = 0;
-	    secondRecord.space = wrBuf->space - wrBuf->len;
-
-	    rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
-	                                       ss->sec.isServer, IS_DTLS(ss),
-					       capRecordVersion, type,
-					       pIn + 1, contentLen - 1,
-	                                       &secondRecord);
-	    if (rv == SECSuccess) {
-	        PRINT_BUF(50, (ss, "send (encrypted) record data [2/2]:",
-	                       secondRecord.buf, secondRecord.len));
-	        wrBuf->len += secondRecord.len;
+		PRINT_BUF(50, (ss, "send (encrypted) record data:",
+			       recordBuf.buf, recordBuf.len));
+		wrBuf->len += recordBuf.len;
 	    }
 	} else {
 	    if (!IS_DTLS(ss)) {
@@ -2591,6 +2626,7 @@
 		    /* presumably a memory error, SEC_ERROR_NO_MEMORY */
 		    return SECFailure;
 		}
+		wrBuf->len = 0;	/* All cipher text is saved away. */
 	    }
 	}
 	totalSent += contentLen;
@@ -2621,7 +2657,6 @@
 
     if (ss->pendingBuf.len > SSL3_PENDING_HIGH_WATER &&
         !ssl_SocketIsBlocking(ss)) {
-	PORT_Assert(!ssl_SocketIsBlocking(ss));
 	PORT_SetError(PR_WOULD_BLOCK_ERROR);
 	return SECFailure;
     }
@@ -10798,6 +10833,8 @@
     spec->server.write_mac_key     = NULL;
     spec->server.write_mac_context = NULL;
 
+    spec->rc4EncryptedBytes        = 0;
+
     spec->write_seq_num.high       = 0;
     spec->write_seq_num.low        = 0;
 
@@ -10846,6 +10883,7 @@
 #endif
     ssl_ReleaseSpecWriteLock(ss);
 
+    /* XXX: move xtnData into ss->ssl3? */
     PORT_Memset(&ss->xtnData, 0, sizeof(TLSExtensionData));
 
     if (IS_DTLS(ss)) {