Cap the record layer version number of TLS ClientHello to TLS 1.0

net/third_party/nss/ssl/ssl3con.c

Index: net/third_party/nss/ssl/ssl3con.c
===================================================================
--- net/third_party/nss/ssl/ssl3con.c	(revision 146623)
+++ net/third_party/nss/ssl/ssl3con.c	(working copy)
@@ -2057,6 +2057,7 @@
 ssl3_CompressMACEncryptRecord(ssl3CipherSpec *   cwSpec,
 		              PRBool             isServer,
 			      PRBool             isDTLS,
+			      PRBool             capsRecordVersion,

[Ryan Sleevi, 2012/07/13 21:20:53]

should this be singular - capRecordVersion - to indicate it's a command?

with capsRecordVersion, "caps" seems like a verb, so it's not clear what/who the
subject is. If I substitute in "peer", as in "the peer caps the record version",
this doesn't seem accurate, since it's not the peer who caps, it's the client
who needs to cap to talk to the peer.

Same for other occurrences of the pluralized form

                               SSL3ContentType    type,
 		              const SSL3Opaque * pIn,
 		              PRUint32           contentLen,
@@ -2216,8 +2217,14 @@
 	wrBuf->buf[11] = MSB(cipherBytes);
 	wrBuf->buf[12] = LSB(cipherBytes);
     } else {
-	wrBuf->buf[1] = MSB(cwSpec->version);
-	wrBuf->buf[2] = LSB(cwSpec->version);
+	SSL3ProtocolVersion version;
+
+	version = cwSpec->version;

[Ryan Sleevi, 2012/07/13 21:20:53]

nit:

SSL3ProtocolVersion version = cwSpec->version?

+	if (capsRecordVersion) {
+	    version = PR_MIN(SSL_LIBRARY_VERSION_TLS_1_0, version);
+	}
+	wrBuf->buf[1] = MSB(version);
+	wrBuf->buf[2] = LSB(version);
 	wrBuf->buf[3] = MSB(cipherBytes);
 	wrBuf->buf[4] = LSB(cipherBytes);
     }
@@ -2247,7 +2254,14 @@
  *    all ciphertext into the pending ciphertext buffer.
  * ssl_SEND_FLAG_USE_EPOCH (for DTLS)
  *    Forces the use of the provided epoch
- *
+ * ssl_SEND_FLAG_CAP_RECORD_VERSION
+ *    Caps the record layer version number of TLS ClientHello to { 3, 1 }
+ *    (TLS 1.0). Some TLS 1.0 servers (which seem to use F5 BIG-IP) ignore 
+ *    ClientHello.client_version and use the record layer version number
+ *    (TLSPlaintext.version) instead when negotiating protocol versions. In
+ *    addition, if the record layer version number of ClientHello is { 3, 2 }
+ *    (TLS 1.1) or higher, these servers reset the TCP connections. Set this
+ *    flag to work around such servers.
  */
 PRInt32
 ssl3_SendRecord(   sslSocket *        ss,
@@ -2260,6 +2274,7 @@
     sslBuffer      *          wrBuf 	  = &ss->sec.writeBuf;
     SECStatus                 rv;
     PRInt32                   totalSent   = 0;
+    PRBool                    capsRecordVersion;
 
     SSL_TRC(3, ("%d: SSL3[%d] SendRecord type: %s nIn=%d",
 		SSL_GETPID(), ss->fd, ssl3_DecodeContentType(type),
@@ -2268,6 +2283,16 @@
 
     PORT_Assert( ss->opt.noLocks || ssl_HaveXmitBufLock(ss) );
 
+    capsRecordVersion = ((flags & ssl_SEND_FLAG_CAP_RECORD_VERSION) != 0);
+
+    if (capsRecordVersion) {
+	/* ssl_SEND_FLAG_CAP_RECORD_VERSION can only be used with
+	 * TLS ClientHello. */
+	PORT_Assert(!IS_DTLS(ss));
+	PORT_Assert(type == content_handshake);
+	PORT_Assert(ss->ssl3.hs.ws == wait_server_hello);

[Ryan Sleevi, 2012/07/13 21:20:53]

Is this true for client-initiated renegotiation? I would think the hs.ws may be
different then

[wtc, 2012/07/13 22:11:39]

Yes, ss->ssl3.hs.ws is also wait_server_hello for
client-initiated renegotiation.  ssl3_SendClientHello calls
ssl3_InitState at the beginning, which sets ss->ssl3.hs.ws
to wait_server_hello. Note that ssl3_SendClientHello also
sets ss->ssl3.hs.ws to wait_server_hello before returning,
but this assertion is checked before that.  This is a little
confusing because the side effect of ssl3_InitState is not
obvious.

Do you think I should remove this assertion to avoid that
confusion?  Alternatively I can change ssl3_SendClientHello
to explicitly set ss->ssl3.hs.ws to wait_server_hello earlier.

+    }
+
     if (ss->ssl3.initialized == PR_FALSE) {
 	/* This can happen on a server if the very first incoming record
 	** looks like a defective ssl3 record (e.g. too long), and we're
@@ -2324,7 +2349,8 @@
 
 	    rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
 					       ss->sec.isServer, IS_DTLS(ss),
-					       type, pIn, 1, wrBuf);
+					       capsRecordVersion, type, pIn,
+					       1, wrBuf);
 	    if (rv != SECSuccess)
 	        goto spec_locked_loser;
 
@@ -2337,7 +2363,8 @@
 
 	    rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
 	                                       ss->sec.isServer, IS_DTLS(ss),
-					       type, pIn + 1, contentLen - 1,
+					       capsRecordVersion, type,
+					       pIn + 1, contentLen - 1,
 	                                       &secondRecord);
 	    if (rv == SECSuccess) {
 	        PRINT_BUF(50, (ss, "send (encrypted) record data [2/2]:",
@@ -2349,6 +2376,7 @@
 		rv = ssl3_CompressMACEncryptRecord(ss->ssl3.cwSpec,
 						   ss->sec.isServer,
 						   IS_DTLS(ss),
+						   capsRecordVersion,
 						   type, pIn,
 						   contentLen, wrBuf);
 	    } else {
@@ -2560,6 +2588,8 @@
 static SECStatus
 ssl3_FlushHandshakeMessages(sslSocket *ss, PRInt32 flags)
 {
+    static const PRInt32 allowedFlags = ssl_SEND_FLAG_FORCE_INTO_BUFFER |
+                                        ssl_SEND_FLAG_CAP_RECORD_VERSION;
     PRInt32 rv = SECSuccess;
 
     PORT_Assert( ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
@@ -2568,9 +2598,9 @@
     if (!ss->sec.ci.sendBuf.buf || !ss->sec.ci.sendBuf.len)
 	return rv;
 
-    /* only this flag is allowed */
-    PORT_Assert(!(flags & ~ssl_SEND_FLAG_FORCE_INTO_BUFFER));
-    if ((flags & ~ssl_SEND_FLAG_FORCE_INTO_BUFFER) != 0) {
+    /* only these flags are allowed */
+    PORT_Assert(!(flags & ~allowedFlags));
+    if ((flags & ~allowedFlags) != 0) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	rv = SECFailure;
     } else {
@@ -3981,8 +4011,10 @@
     int              num_suites;
     int              actual_count = 0;
     PRBool           isTLS = PR_FALSE;
+    PRBool           serverVersionKnown = PR_FALSE;
     PRInt32          total_exten_len = 0;
     unsigned         numCompressionMethods;
+    PRInt32          flags;
 
     SSL_TRC(3, ("%d: SSL3[%d]: send client_hello handshake", SSL_GETPID(),
 		ss->fd));
@@ -4070,6 +4102,7 @@
     }
 
     if (sid) {
+	serverVersionKnown = PR_TRUE;
 	SSL_AtomicIncrementLong(& ssl3stats.sch_sid_cache_hits );
 
 	/* Are we attempting a stateless session resume? */
@@ -4305,7 +4338,11 @@
 	    ssl_renegotiation_info_xtn;
     }
 
-    rv = ssl3_FlushHandshake(ss, 0);
+    flags = 0;
+    if (!serverVersionKnown && !IS_DTLS(ss)) {
+	flags |= ssl_SEND_FLAG_CAP_RECORD_VERSION;
+    }
+    rv = ssl3_FlushHandshake(ss, flags);
     if (rv != SECSuccess) {
 	return rv;	/* error code set by ssl3_FlushHandshake */
     }