patches/0009-cbc_record_splitting.patch

From e6102d2ac84a55e4a50d9edfc36ec894c6174fb7 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@chromium.org>
Date: Thu, 31 Oct 2013 13:22:54 -0400

This patch removes support for empty records (which is almost
universally disabled via SSL_OP_ALL) and adds optional support for 1/n-1
record splitting.

The latter is not enabled by default, since it's not typically used on
servers, but it should be enabled in web browsers since there are known
attacks in that case (see BEAST).

(Of course, this is a poor workaround for using TLS 1.2 and an AEAD
cipher suite).
---
 apps/s_client.c |  16 +++++---
 ssl/d1_pkt.c    |  50 ++++---------------------
 ssl/s3_enc.c    |  17 ++++-----
 ssl/s3_pkt.c    | 113 +++++++++++++++++++++++++++++++-------------------------
 ssl/ssl.h       |  20 +++++++---
 ssl/ssl3.h      |   4 +-
 ssl/ssl_locl.h  |   2 -
 ssl/t1_enc.c    |  10 ++---
 8 files changed, 110 insertions(+), 122 deletions(-)

diff --git a/apps/s_client.c b/apps/s_client.c
index cb1efcd..0c70580 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -363,6 +363,7 @@ static void sc_usage(void)
 # endif
 #endif
 	BIO_printf(bio_err," -cutthrough       - enable 1-RTT full-handshake for strong ciphers\n");
+	BIO_printf(bio_err," -no_record_splitting  - disable 1/n-1 record splitting in CBC mode\n");
 	BIO_printf(bio_err," -legacy_renegotiation - enable use of legacy renegotiation (dangerous)\n");
 #ifndef OPENSSL_NO_SRTP
 	BIO_printf(bio_err," -use_srtp profiles - Offer SRTP key management with a colon-separated profile list\n");
@@ -579,7 +580,7 @@ int MAIN(int argc, char **argv)
 	EVP_PKEY *key = NULL;
 	char *CApath=NULL,*CAfile=NULL,*cipher=NULL;
 	int reconnect=0,badop=0,verify=SSL_VERIFY_NONE,bugs=0;
-	int cutthrough=0;
+	int cutthrough=0, no_record_splitting=0;
 	int crlf=0;
 	int write_tty,read_tty,write_ssl,read_ssl,tty_on,ssl_pending;
 	SSL_CTX *ctx=NULL;
@@ -594,6 +595,7 @@ int MAIN(int argc, char **argv)
 	char *inrand=NULL;
 	int mbuf_len=0;
 	struct timeval timeout, *timeoutp;
+	int ssl_mode;
 #ifndef OPENSSL_NO_ENGINE
 	char *engine_id=NULL;
 	char *ssl_client_engine_id=NULL;
@@ -894,6 +896,8 @@ int MAIN(int argc, char **argv)
 #endif
 		else if (strcmp(*argv,"-cutthrough") == 0)
 			cutthrough=1;
+		else if (strcmp(*argv,"-no_record_splitting") == 0)
+			no_record_splitting=1;
 		else if (strcmp(*argv,"-serverpref") == 0)
 			off|=SSL_OP_CIPHER_SERVER_PREFERENCE;
 		else if (strcmp(*argv,"-legacy_renegotiation") == 0)
@@ -1183,14 +1187,16 @@ bad:
 		}
 #endif
 
-	/* Enable handshake cutthrough for client connections using
-	 * strong ciphers. */
+	ssl_mode = SSL_CTX_get_mode(ctx);
+	if (!no_record_splitting)
+		ssl_mode |= SSL_MODE_CBC_RECORD_SPLITTING;
 	if (cutthrough)
 		{
-		int ssl_mode = SSL_CTX_get_mode(ctx);
+		/* Enable handshake cutthrough for client connections using
+		 * strong ciphers. */
 		ssl_mode |= SSL_MODE_HANDSHAKE_CUTTHROUGH;
-		SSL_CTX_set_mode(ctx, ssl_mode);
 		}
+	SSL_CTX_set_mode(ctx, ssl_mode);
 
 	if (state) SSL_CTX_set_info_callback(ctx,apps_ssl_info_callback);
 	if (cipher != NULL)
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 438c091..363fc8c 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -179,6 +179,8 @@ static int dtls1_record_needs_buffering(SSL *s, SSL3_RECORD *rr,
 static int dtls1_buffer_record(SSL *s, record_pqueue *q,
 	unsigned char *priority);
 static int dtls1_process_record(SSL *s);
+static int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
+			  unsigned int len);
 
 /* copy buffered record into SSL structure */
 static int
@@ -1464,11 +1466,12 @@ int dtls1_write_bytes(SSL *s, int type, const void *buf, int len)
 
 	OPENSSL_assert(len <= SSL3_RT_MAX_PLAIN_LENGTH);
 	s->rwstate=SSL_NOTHING;
-	i=do_dtls1_write(s, type, buf, len, 0);
+	i=do_dtls1_write(s, type, buf, len);
 	return i;
 	}
 
-int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len, int create_empty_fragment)
+static int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
+			  unsigned int len)
 	{
 	unsigned char *p,*pseq;
 	int i,mac_size,clear=0;
@@ -1495,7 +1498,7 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len,
 		/* if it went, fall through and send more stuff */
 		}
 
-	if (len == 0 && !create_empty_fragment)
+	if (len == 0)
 		return 0;
 
 	wr= &(s->s3->wrec);
@@ -1516,37 +1519,6 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len,
 			goto err;
 		}
 
-	/* DTLS implements explicit IV, so no need for empty fragments */
-#if 0
-	/* 'create_empty_fragment' is true only when this function calls itself */
-	if (!clear && !create_empty_fragment && !s->s3->empty_fragment_done
-	    && SSL_version(s) != DTLS1_VERSION && SSL_version(s) != DTLS1_BAD_VER)
-		{
-		/* countermeasure against known-IV weakness in CBC ciphersuites
-		 * (see http://www.openssl.org/~bodo/tls-cbc.txt) 
-		 */
-
-		if (s->s3->need_empty_fragments && type == SSL3_RT_APPLICATION_DATA)
-			{
-			/* recursive function call with 'create_empty_fragment' set;
-			 * this prepares and buffers the data for an empty fragment
-			 * (these 'prefix_len' bytes are sent out later
-			 * together with the actual payload) */
-			prefix_len = s->method->do_ssl_write(s, type, buf, 0, 1);
-			if (prefix_len <= 0)
-				goto err;
-
-			if (s->s3->wbuf.len < (size_t)prefix_len + SSL3_RT_MAX_PACKET_SIZE)
-				{
-				/* insufficient space */
-				SSLerr(SSL_F_DO_DTLS1_WRITE, ERR_R_INTERNAL_ERROR);
-				goto err;
-				}
-			}
-		
-		s->s3->empty_fragment_done = 1;
-		}
-#endif
 	p = wb->buf + prefix_len;
 
 	/* write the header */
@@ -1652,14 +1624,6 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len,
 
 	ssl3_record_sequence_update(&(s->s3->write_sequence[0]));
 
-	if (create_empty_fragment)
-		{
-		/* we are in a recursive call;
-		 * just return the length, don't write out anything here
-		 */
-		return wr->length;
-		}
-
 	/* now let's set up wb */
 	wb->left = prefix_len + wr->length;
 	wb->offset = 0;
@@ -1756,7 +1720,7 @@ int dtls1_dispatch_alert(SSL *s)
 		}
 #endif
 
-	i = do_dtls1_write(s, SSL3_RT_ALERT, &buf[0], sizeof(buf), 0);
+	i = do_dtls1_write(s, SSL3_RT_ALERT, &buf[0], sizeof(buf));
 	if (i <= 0)
 		{
 		s->s3->alert_dispatch=1;
diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index 191b86b..6358e1b 100644
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -434,27 +434,26 @@ int ssl3_setup_key_block(SSL *s)
 
 	ret = ssl3_generate_key_block(s,p,num);
 
-	if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS))
+	/* enable vulnerability countermeasure for CBC ciphers with
+	 * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt) */
+	if ((s->mode & SSL_MODE_CBC_RECORD_SPLITTING) != 0)
 		{
-		/* enable vulnerability countermeasure for CBC ciphers with
-		 * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt)
-		 */
-		s->s3->need_empty_fragments = 1;
+		s->s3->need_record_splitting = 1;
 
 		if (s->session->cipher != NULL)
 			{
 			if (s->session->cipher->algorithm_enc == SSL_eNULL)
-				s->s3->need_empty_fragments = 0;
-			
+				s->s3->need_record_splitting = 0;
+
 #ifndef OPENSSL_NO_RC4
 			if (s->session->cipher->algorithm_enc == SSL_RC4)
-				s->s3->need_empty_fragments = 0;
+				s->s3->need_record_splitting = 0;
 #endif
 			}
 		}
 
 	return ret;
-		
+
 err:
 	SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK,ERR_R_MALLOC_FAILURE);
 	return(0);
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 706ef1f..957d7c6 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -119,7 +119,7 @@
 #include <openssl/rand.h>
 
 static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
-			 unsigned int len, int create_empty_fragment);
+			 unsigned int len, char fragment, char is_fragment);
 static int ssl3_get_record(SSL *s);
 
 int ssl3_read_n(SSL *s, int n, int max, int extend)
@@ -636,15 +636,36 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
 	n=(len-tot);
 	for (;;)
 		{
-		if (n > s->max_send_fragment)
-			nw=s->max_send_fragment;
+		/* max contains the maximum number of bytes that we can put
+		 * into a record. */
+		unsigned max = s->max_send_fragment;
+		/* fragment is true if do_ssl3_write should send the first byte
+		 * in its own record in order to randomise a CBC IV. */
+		int fragment = 0;
+
+		if (n > 1 &&
+		    s->s3->need_record_splitting &&
+		    type == SSL3_RT_APPLICATION_DATA &&
+		    !s->s3->record_split_done)
+			{
+			fragment = 1;
+			/* record_split_done records that the splitting has
+			 * been done in case we hit an SSL_WANT_WRITE condition.
+			 * In that case, we don't need to do the split again. */
+			s->s3->record_split_done = 1;
+			}
+
+		if (n > max)
+			nw=max;
 		else
 			nw=n;
 
-		i=do_ssl3_write(s, type, &(buf[tot]), nw, 0);
+		i=do_ssl3_write(s, type, &(buf[tot]), nw, fragment, 0);
 		if (i <= 0)
 			{
 			s->s3->wnum=tot;
+			/* Try to write the fragment next time. */
+			s->s3->record_split_done = 0;
 			return i;
 			}
 
@@ -652,10 +673,10 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
 			(type == SSL3_RT_APPLICATION_DATA &&
 			 (s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE)))
 			{
-			/* next chunk of data should get another prepended empty fragment
-			 * in ciphersuites with known-IV weakness: */
-			s->s3->empty_fragment_done = 0;
-			
+			/* next chunk of data should get another prepended,
+			 * one-byte fragment in ciphersuites with known-IV
+			 * weakness. */
+			s->s3->record_split_done = 0;
 			return tot+i;
 			}
 
@@ -664,11 +685,16 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
 		}
 	}
 
+/* do_ssl3_write writes an SSL record of the given type. If |fragment| is 1
+ * then it splits the record into a one byte record and a record with the rest
+ * of the data in order to randomise a CBC IV. If |is_fragment| is true then
+ * this call resulted from do_ssl3_write calling itself in order to create that
+ * one byte fragment. */
 static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
-			 unsigned int len, int create_empty_fragment)
+			 unsigned int len, char fragment, char is_fragment)
 	{
 	unsigned char *p,*plen;
-	int i,mac_size,clear=0;
+	int i,mac_size;
 	int prefix_len=0;
 	int eivlen;
 	long align=0;
@@ -691,11 +717,11 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 		/* if it went, fall through and send more stuff */
 		}
 
- 	if (wb->buf == NULL)
+	if (wb->buf == NULL)
 		if (!ssl3_setup_write_buffer(s))
 			return -1;
 
-	if (len == 0 && !create_empty_fragment)
+	if (len == 0)
 		return 0;
 
 	wr= &(s->s3->wrec);
@@ -705,11 +731,6 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 		(s->enc_write_ctx == NULL) ||
 		(EVP_MD_CTX_md(s->write_hash) == NULL))
 		{
-#if 1
-		clear=s->enc_write_ctx?0:1;	/* must be AEAD cipher */
-#else
-		clear=1;
-#endif
 		mac_size=0;
 		}
 	else
@@ -719,42 +740,33 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 			goto err;
 		}
 
-	/* 'create_empty_fragment' is true only when this function calls itself */
-	if (!clear && !create_empty_fragment && !s->s3->empty_fragment_done)
+	if (fragment)
 		{
 		/* countermeasure against known-IV weakness in CBC ciphersuites
 		 * (see http://www.openssl.org/~bodo/tls-cbc.txt) */
+		prefix_len = do_ssl3_write(s, type, buf, 1 /* length */,
+					   0 /* fragment */,
+					   1 /* is_fragment */);
+		if (prefix_len <= 0)
+			goto err;
 
-		if (s->s3->need_empty_fragments && type == SSL3_RT_APPLICATION_DATA)
+		if (prefix_len > (SSL3_RT_HEADER_LENGTH +
+				  SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD))
 			{
-			/* recursive function call with 'create_empty_fragment' set;
-			 * this prepares and buffers the data for an empty fragment
-			 * (these 'prefix_len' bytes are sent out later
-			 * together with the actual payload) */
-			prefix_len = do_ssl3_write(s, type, buf, 0, 1);
-			if (prefix_len <= 0)
-				goto err;
-
-			if (prefix_len >
-		(SSL3_RT_HEADER_LENGTH + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD))
-				{
-				/* insufficient space */
-				SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR);
-				goto err;
-				}
+			/* insufficient space */
+			SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR);
+			goto err;
 			}
-		
-		s->s3->empty_fragment_done = 1;
 		}
 
-	if (create_empty_fragment)
+	if (is_fragment)
 		{
 #if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0
-		/* extra fragment would be couple of cipher blocks,
-		 * which would be multiple of SSL3_ALIGN_PAYLOAD, so
-		 * if we want to align the real payload, then we can
-		 * just pretent we simply have two headers. */
-		align = (long)wb->buf + 2*SSL3_RT_HEADER_LENGTH;
+		/* The extra fragment would be couple of cipher blocks, and
+		 * that will be a multiple of SSL3_ALIGN_PAYLOAD. So, if we
+		 * want to align the real payload, we can just pretend that we
+		 * have two headers and a byte. */
+		align = (long)wb->buf + 2*SSL3_RT_HEADER_LENGTH + 1;
 		align = (-align)&(SSL3_ALIGN_PAYLOAD-1);
 #endif
 		p = wb->buf + align;
@@ -791,7 +803,7 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 		*(p++)=s->version&0xff;
 
 	/* field where we are to write out packet length */
-	plen=p; 
+	plen=p;
 	p+=2;
 	/* Explicit IV length, block ciphers and TLS version 1.1 or later */
 	if (s->enc_write_ctx && s->version >= TLS1_1_VERSION)
@@ -819,8 +831,8 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 
 	/* lets setup the record stuff. */
 	wr->data=p + eivlen;
-	wr->length=(int)len;
-	wr->input=(unsigned char *)buf;
+	wr->length=(int)(len - (fragment != 0));
+	wr->input=(unsigned char *)buf + (fragment != 0);
 
 	/* we now 'read' from wr->input, wr->length bytes into
 	 * wr->data */
@@ -873,11 +885,10 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 	wr->type=type; /* not needed but helps for debugging */
 	wr->length+=SSL3_RT_HEADER_LENGTH;
 
-	if (create_empty_fragment)
+	if (is_fragment)
 		{
-		/* we are in a recursive call;
-		 * just return the length, don't write out anything here
-		 */
+		/* we are in a recursive call; just return the length, don't
+		 * write out anything. */
 		return wr->length;
 		}
 
@@ -1548,7 +1559,7 @@ int ssl3_dispatch_alert(SSL *s)
 	void (*cb)(const SSL *ssl,int type,int val)=NULL;
 
 	s->s3->alert_dispatch=0;
-	i = do_ssl3_write(s, SSL3_RT_ALERT, &s->s3->send_alert[0], 2, 0);
+	i = do_ssl3_write(s, SSL3_RT_ALERT, &s->s3->send_alert[0], 2, 0, 0);
 	if (i <= 0)
 		{
 		s->s3->alert_dispatch=1;
diff --git a/ssl/ssl.h b/ssl/ssl.h
index ef85428..ce65664 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -578,11 +578,15 @@ struct ssl_session_st
 /* Refers to ancient SSLREF and SSLv2, retained for compatibility */
 #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG		0x0
 
-/* Disable SSL 3.0/TLS 1.0 CBC vulnerability workaround that was added
- * in OpenSSL 0.9.6d.  Usually (depending on the application protocol)
- * the workaround is not needed.  Unfortunately some broken SSL/TLS
- * implementations cannot handle it at all, which is why we include
- * it in SSL_OP_ALL. */
+/* SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS is vestigial. Previously it disabled the
+ * insertion of empty records in CBC mode, but the empty records were commonly
+ * misinterpreted as EOF by other TLS stacks and so this was disabled by
+ * SSL_OP_ALL.
+ *
+ * This has been replaced by 1/n-1 record splitting, which is enabled by
+ * SSL_MODE_CBC_RECORD_SPLITTING in SSL_set_mode. This involves sending a
+ * one-byte record rather than an empty record and has much better
+ * compatibility. */
 #define SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS              0x00000800L /* added in 0.9.6e */
 
 /* SSL_OP_ALL: various bug workarounds that should be rather harmless.
@@ -675,6 +679,12 @@ struct ssl_session_st
  * one RTT. */
 #define SSL_MODE_HANDSHAKE_CUTTHROUGH 0x00000080L
 
+/* When set, TLS 1.0 and SSLv3, multi-byte, CBC records will be split in two:
+ * the first record will contain a single byte and the second will contain the
+ * rest of the bytes. This effectively randomises the IV and prevents BEAST
+ * attacks. */
+#define SSL_MODE_CBC_RECORD_SPLITTING 0x00000100L
+
 /* Note: SSL[_CTX]_set_{options,mode} use |= op on the previous value,
  * they cannot be used to clear bits. */
 
diff --git a/ssl/ssl3.h b/ssl/ssl3.h
index 16c389d..8e3e449 100644
--- a/ssl/ssl3.h
+++ b/ssl/ssl3.h
@@ -419,8 +419,8 @@ typedef struct ssl3_state_st
 	unsigned char client_random[SSL3_RANDOM_SIZE];
 
 	/* flags for countermeasure against known-IV weakness */
-	int need_empty_fragments;
-	int empty_fragment_done;
+	int need_record_splitting;
+	int record_split_done;
 
 	/* The value of 'extra' when the buffers were initialized */
 	int init_extra;
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 10baaee..6d4bc14 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1093,8 +1093,6 @@ int dtls1_shutdown(SSL *s);
 
 long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok);
 int dtls1_get_record(SSL *s);
-int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
-	unsigned int len, int create_empty_fragement);
 int dtls1_dispatch_alert(SSL *s);
 int dtls1_enc(SSL *s, int snd);
 
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 9963a80..4ca1549 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -774,22 +774,22 @@ printf("\nkey block\n");
 { int z; for (z=0; z<num; z++) printf("%02X%c",p1[z],((z+1)%16)?' ':'\n'); }
 #endif
 
-	if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)
-		&& s->method->version <= TLS1_VERSION)
+	if (s->method->version <= TLS1_VERSION &&
+	    (s->mode & SSL_MODE_CBC_RECORD_SPLITTING) != 0)
 		{
 		/* enable vulnerability countermeasure for CBC ciphers with
 		 * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt)
 		 */
-		s->s3->need_empty_fragments = 1;
+		s->s3->need_record_splitting = 1;
 
 		if (s->session->cipher != NULL)
 			{
 			if (s->session->cipher->algorithm_enc == SSL_eNULL)
-				s->s3->need_empty_fragments = 0;
+				s->s3->need_record_splitting = 0;
 			
 #ifndef OPENSSL_NO_RC4
 			if (s->session->cipher->algorithm_enc == SSL_RC4)
-				s->s3->need_empty_fragments = 0;
+				s->s3->need_record_splitting = 0;
 #endif
 			}
 		}
-- 
2.0.0.526.g5318336