net: Precede each CBC encrypted application data record with an empty one.

net/third_party/nss/ssl/ssl3con.c

Index: net/third_party/nss/ssl/ssl3con.c
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index 2cc1e05bd8a5bfe7ac5f0bfc92192494d205e88c..74d9e564dada0ca547c7628a471938b2fcc68957 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -2229,7 +2229,7 @@ ssl3_SendRecord(   sslSocket *        ss,
 	return SECFailure;
     }
 
-    while (nIn > 0) {
+    do {
 	PRUint32  contentLen = PR_MIN(nIn, MAX_FRAGMENT_LENGTH);
 
 	if (wrBuf->space < contentLen + SSL3_BUFFER_FUDGE) {
@@ -2306,7 +2306,7 @@ ssl3_SendRecord(   sslSocket *        ss,
 	    }
 	}
 	totalSent += contentLen;
-    }
+    } while (nIn > 0);
     return totalSent;
 }
 
@@ -2321,6 +2321,7 @@ ssl3_SendApplicationData(sslSocket *ss, const unsigned char *in,
 {
     PRInt32   totalSent	= 0;
     PRInt32   discarded = 0;
+    PRBool    isBlockCipher;
 
     PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) );
     if (len < 0 || !in) {
@@ -2345,6 +2346,28 @@ ssl3_SendApplicationData(sslSocket *ss, const unsigned char *in,
 	len--;
 	discarded = 1;
     }
+
+    ssl_GetSpecReadLock(ss);
+    isBlockCipher = ss->ssl3.cwSpec->cipher_def->type == type_block;
+    ssl_ReleaseSpecReadLock(ss);
+
+    if (isBlockCipher && len > 0) {
+	// We assume that block ciphers are used in CBC mode and prepend an
+	// empty record. This effectively randomizes the IV in a backwards
+	// compatible way.
+	PRInt32 sent = ssl3_SendRecord(ss, content_application_data,
+				       in, 0 /* no payload */, flags);
+	if (sent < 0) {
+	    return SECFailure; /* error code set by ssl3_SendRecord */
+	}
+	if (ss->pendingBuf.len) {
+	    /* must be a non-blocking socket */
+	    PORT_Assert(!ssl_SocketIsBlocking(ss));
+	    PORT_Assert(ss->lastWriteBlocked);
+	    goto writeBlocked;
+	}

[wtc, 2011/06/27 18:11:55]

I suspect what you described will indeed happen if we
simply fall into the while loop below.  But I think it's
better to deal with the if (ss->pendingBuf.len) condition
explicitly; at least it would help me convince myself the
new code is correct.

Because of your comment, I found that given totalSent = 0,
we can simplify it to avoid the goto statement:

        if (ss->pendingBuf.len) {
            /* must be a non-blocking socket */
            PORT_Assert(!ssl_SocketIsBlocking(ss));
            PORT_Assert(ss->lastWriteBlocked);
            PORT_SetError(PR_WOULD_BLOCK_ERROR);
            return SECFailure;
        }

[agl, 2011/06/27 19:05:30]

Done.

+    }
+
     while (len > totalSent) {
 	PRInt32   sent, toSend;
 
@@ -2377,6 +2400,8 @@ ssl3_SendApplicationData(sslSocket *ss, const unsigned char *in,
 	    break;	
 	}
     }
+
+  writeBlocked:

[wtc, 2011/06/27 18:11:55]

Nit: labels are not indented in the NSS source code.

[agl, 2011/06/27 19:05:30]

(mooted: line removed.)

     if (ss->pendingBuf.len) {
 	/* Must be non-blocking. */
 	PORT_Assert(!ssl_SocketIsBlocking(ss));