1From e6102d2ac84a55e4a50d9edfc36ec894c6174fb7 Mon Sep 17 00:00:00 2001 2From: Adam Langley <agl@chromium.org> 3Date: Thu, 31 Oct 2013 13:22:54 -0400 4 5This patch removes support for empty records (which is almost 6universally disabled via SSL_OP_ALL) and adds optional support for 1/n-1 7record splitting. 8 9The latter is not enabled by default, since it's not typically used on 10servers, but it should be enabled in web browsers since there are known 11attacks in that case (see BEAST). 12 13(Of course, this is a poor workaround for using TLS 1.2 and an AEAD 14cipher suite). 15--- 16 apps/s_client.c | 16 +++++--- 17 ssl/d1_pkt.c | 50 ++++--------------------- 18 ssl/s3_enc.c | 17 ++++----- 19 ssl/s3_pkt.c | 113 +++++++++++++++++++++++++++++++------------------------- 20 ssl/ssl.h | 20 +++++++--- 21 ssl/ssl3.h | 4 +- 22 ssl/ssl_locl.h | 2 - 23 ssl/t1_enc.c | 10 ++--- 24 8 files changed, 110 insertions(+), 122 deletions(-) 25 26diff --git a/apps/s_client.c b/apps/s_client.c 27index cb1efcd..0c70580 100644 28--- a/apps/s_client.c 29+++ b/apps/s_client.c 30@@ -363,6 +363,7 @@ static void sc_usage(void) 31 # endif 32 #endif 33 BIO_printf(bio_err," -cutthrough - enable 1-RTT full-handshake for strong ciphers\n"); 34+ BIO_printf(bio_err," -no_record_splitting - disable 1/n-1 record splitting in CBC mode\n"); 35 BIO_printf(bio_err," -legacy_renegotiation - enable use of legacy renegotiation (dangerous)\n"); 36 #ifndef OPENSSL_NO_SRTP 37 BIO_printf(bio_err," -use_srtp profiles - Offer SRTP key management with a colon-separated profile list\n"); 38@@ -579,7 +580,7 @@ int MAIN(int argc, char **argv) 39 EVP_PKEY *key = NULL; 40 char *CApath=NULL,*CAfile=NULL,*cipher=NULL; 41 int reconnect=0,badop=0,verify=SSL_VERIFY_NONE,bugs=0; 42- int cutthrough=0; 43+ int cutthrough=0, no_record_splitting=0; 44 int crlf=0; 45 int write_tty,read_tty,write_ssl,read_ssl,tty_on,ssl_pending; 46 SSL_CTX *ctx=NULL; 47@@ -594,6 +595,7 @@ int MAIN(int argc, char **argv) 48 char *inrand=NULL; 49 int mbuf_len=0; 50 struct timeval timeout, *timeoutp; 51+ int ssl_mode; 52 #ifndef OPENSSL_NO_ENGINE 53 char *engine_id=NULL; 54 char *ssl_client_engine_id=NULL; 55@@ -894,6 +896,8 @@ int MAIN(int argc, char **argv) 56 #endif 57 else if (strcmp(*argv,"-cutthrough") == 0) 58 cutthrough=1; 59+ else if (strcmp(*argv,"-no_record_splitting") == 0) 60+ no_record_splitting=1; 61 else if (strcmp(*argv,"-serverpref") == 0) 62 off|=SSL_OP_CIPHER_SERVER_PREFERENCE; 63 else if (strcmp(*argv,"-legacy_renegotiation") == 0) 64@@ -1183,14 +1187,16 @@ bad: 65 } 66 #endif 67 68- /* Enable handshake cutthrough for client connections using 69- * strong ciphers. */ 70+ ssl_mode = SSL_CTX_get_mode(ctx); 71+ if (!no_record_splitting) 72+ ssl_mode |= SSL_MODE_CBC_RECORD_SPLITTING; 73 if (cutthrough) 74 { 75- int ssl_mode = SSL_CTX_get_mode(ctx); 76+ /* Enable handshake cutthrough for client connections using 77+ * strong ciphers. */ 78 ssl_mode |= SSL_MODE_HANDSHAKE_CUTTHROUGH; 79- SSL_CTX_set_mode(ctx, ssl_mode); 80 } 81+ SSL_CTX_set_mode(ctx, ssl_mode); 82 83 if (state) SSL_CTX_set_info_callback(ctx,apps_ssl_info_callback); 84 if (cipher != NULL) 85diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c 86index 438c091..363fc8c 100644 87--- a/ssl/d1_pkt.c 88+++ b/ssl/d1_pkt.c 89@@ -179,6 +179,8 @@ static int dtls1_record_needs_buffering(SSL *s, SSL3_RECORD *rr, 90 static int dtls1_buffer_record(SSL *s, record_pqueue *q, 91 unsigned char *priority); 92 static int dtls1_process_record(SSL *s); 93+static int do_dtls1_write(SSL *s, int type, const unsigned char *buf, 94+ unsigned int len); 95 96 /* copy buffered record into SSL structure */ 97 static int 98@@ -1464,11 +1466,12 @@ int dtls1_write_bytes(SSL *s, int type, const void *buf, int len) 99 100 OPENSSL_assert(len <= SSL3_RT_MAX_PLAIN_LENGTH); 101 s->rwstate=SSL_NOTHING; 102- i=do_dtls1_write(s, type, buf, len, 0); 103+ i=do_dtls1_write(s, type, buf, len); 104 return i; 105 } 106 107-int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len, int create_empty_fragment) 108+static int do_dtls1_write(SSL *s, int type, const unsigned char *buf, 109+ unsigned int len) 110 { 111 unsigned char *p,*pseq; 112 int i,mac_size,clear=0; 113@@ -1495,7 +1498,7 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len, 114 /* if it went, fall through and send more stuff */ 115 } 116 117- if (len == 0 && !create_empty_fragment) 118+ if (len == 0) 119 return 0; 120 121 wr= &(s->s3->wrec); 122@@ -1516,37 +1519,6 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len, 123 goto err; 124 } 125 126- /* DTLS implements explicit IV, so no need for empty fragments */ 127-#if 0 128- /* 'create_empty_fragment' is true only when this function calls itself */ 129- if (!clear && !create_empty_fragment && !s->s3->empty_fragment_done 130- && SSL_version(s) != DTLS1_VERSION && SSL_version(s) != DTLS1_BAD_VER) 131- { 132- /* countermeasure against known-IV weakness in CBC ciphersuites 133- * (see http://www.openssl.org/~bodo/tls-cbc.txt) 134- */ 135- 136- if (s->s3->need_empty_fragments && type == SSL3_RT_APPLICATION_DATA) 137- { 138- /* recursive function call with 'create_empty_fragment' set; 139- * this prepares and buffers the data for an empty fragment 140- * (these 'prefix_len' bytes are sent out later 141- * together with the actual payload) */ 142- prefix_len = s->method->do_ssl_write(s, type, buf, 0, 1); 143- if (prefix_len <= 0) 144- goto err; 145- 146- if (s->s3->wbuf.len < (size_t)prefix_len + SSL3_RT_MAX_PACKET_SIZE) 147- { 148- /* insufficient space */ 149- SSLerr(SSL_F_DO_DTLS1_WRITE, ERR_R_INTERNAL_ERROR); 150- goto err; 151- } 152- } 153- 154- s->s3->empty_fragment_done = 1; 155- } 156-#endif 157 p = wb->buf + prefix_len; 158 159 /* write the header */ 160@@ -1652,14 +1624,6 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf, unsigned int len, 161 162 ssl3_record_sequence_update(&(s->s3->write_sequence[0])); 163 164- if (create_empty_fragment) 165- { 166- /* we are in a recursive call; 167- * just return the length, don't write out anything here 168- */ 169- return wr->length; 170- } 171- 172 /* now let's set up wb */ 173 wb->left = prefix_len + wr->length; 174 wb->offset = 0; 175@@ -1756,7 +1720,7 @@ int dtls1_dispatch_alert(SSL *s) 176 } 177 #endif 178 179- i = do_dtls1_write(s, SSL3_RT_ALERT, &buf[0], sizeof(buf), 0); 180+ i = do_dtls1_write(s, SSL3_RT_ALERT, &buf[0], sizeof(buf)); 181 if (i <= 0) 182 { 183 s->s3->alert_dispatch=1; 184diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c 185index 191b86b..6358e1b 100644 186--- a/ssl/s3_enc.c 187+++ b/ssl/s3_enc.c 188@@ -434,27 +434,26 @@ int ssl3_setup_key_block(SSL *s) 189 190 ret = ssl3_generate_key_block(s,p,num); 191 192- if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) 193+ /* enable vulnerability countermeasure for CBC ciphers with 194+ * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt) */ 195+ if ((s->mode & SSL_MODE_CBC_RECORD_SPLITTING) != 0) 196 { 197- /* enable vulnerability countermeasure for CBC ciphers with 198- * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt) 199- */ 200- s->s3->need_empty_fragments = 1; 201+ s->s3->need_record_splitting = 1; 202 203 if (s->session->cipher != NULL) 204 { 205 if (s->session->cipher->algorithm_enc == SSL_eNULL) 206- s->s3->need_empty_fragments = 0; 207- 208+ s->s3->need_record_splitting = 0; 209+ 210 #ifndef OPENSSL_NO_RC4 211 if (s->session->cipher->algorithm_enc == SSL_RC4) 212- s->s3->need_empty_fragments = 0; 213+ s->s3->need_record_splitting = 0; 214 #endif 215 } 216 } 217 218 return ret; 219- 220+ 221 err: 222 SSLerr(SSL_F_SSL3_SETUP_KEY_BLOCK,ERR_R_MALLOC_FAILURE); 223 return(0); 224diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c 225index 706ef1f..957d7c6 100644 226--- a/ssl/s3_pkt.c 227+++ b/ssl/s3_pkt.c 228@@ -119,7 +119,7 @@ 229 #include <openssl/rand.h> 230 231 static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 232- unsigned int len, int create_empty_fragment); 233+ unsigned int len, char fragment, char is_fragment); 234 static int ssl3_get_record(SSL *s); 235 236 int ssl3_read_n(SSL *s, int n, int max, int extend) 237@@ -636,15 +636,36 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len) 238 n=(len-tot); 239 for (;;) 240 { 241- if (n > s->max_send_fragment) 242- nw=s->max_send_fragment; 243+ /* max contains the maximum number of bytes that we can put 244+ * into a record. */ 245+ unsigned max = s->max_send_fragment; 246+ /* fragment is true if do_ssl3_write should send the first byte 247+ * in its own record in order to randomise a CBC IV. */ 248+ int fragment = 0; 249+ 250+ if (n > 1 && 251+ s->s3->need_record_splitting && 252+ type == SSL3_RT_APPLICATION_DATA && 253+ !s->s3->record_split_done) 254+ { 255+ fragment = 1; 256+ /* record_split_done records that the splitting has 257+ * been done in case we hit an SSL_WANT_WRITE condition. 258+ * In that case, we don't need to do the split again. */ 259+ s->s3->record_split_done = 1; 260+ } 261+ 262+ if (n > max) 263+ nw=max; 264 else 265 nw=n; 266 267- i=do_ssl3_write(s, type, &(buf[tot]), nw, 0); 268+ i=do_ssl3_write(s, type, &(buf[tot]), nw, fragment, 0); 269 if (i <= 0) 270 { 271 s->s3->wnum=tot; 272+ /* Try to write the fragment next time. */ 273+ s->s3->record_split_done = 0; 274 return i; 275 } 276 277@@ -652,10 +673,10 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len) 278 (type == SSL3_RT_APPLICATION_DATA && 279 (s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE))) 280 { 281- /* next chunk of data should get another prepended empty fragment 282- * in ciphersuites with known-IV weakness: */ 283- s->s3->empty_fragment_done = 0; 284- 285+ /* next chunk of data should get another prepended, 286+ * one-byte fragment in ciphersuites with known-IV 287+ * weakness. */ 288+ s->s3->record_split_done = 0; 289 return tot+i; 290 } 291 292@@ -664,11 +685,16 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len) 293 } 294 } 295 296+/* do_ssl3_write writes an SSL record of the given type. If |fragment| is 1 297+ * then it splits the record into a one byte record and a record with the rest 298+ * of the data in order to randomise a CBC IV. If |is_fragment| is true then 299+ * this call resulted from do_ssl3_write calling itself in order to create that 300+ * one byte fragment. */ 301 static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 302- unsigned int len, int create_empty_fragment) 303+ unsigned int len, char fragment, char is_fragment) 304 { 305 unsigned char *p,*plen; 306- int i,mac_size,clear=0; 307+ int i,mac_size; 308 int prefix_len=0; 309 int eivlen; 310 long align=0; 311@@ -691,11 +717,11 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 312 /* if it went, fall through and send more stuff */ 313 } 314 315- if (wb->buf == NULL) 316+ if (wb->buf == NULL) 317 if (!ssl3_setup_write_buffer(s)) 318 return -1; 319 320- if (len == 0 && !create_empty_fragment) 321+ if (len == 0) 322 return 0; 323 324 wr= &(s->s3->wrec); 325@@ -705,11 +731,6 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 326 (s->enc_write_ctx == NULL) || 327 (EVP_MD_CTX_md(s->write_hash) == NULL)) 328 { 329-#if 1 330- clear=s->enc_write_ctx?0:1; /* must be AEAD cipher */ 331-#else 332- clear=1; 333-#endif 334 mac_size=0; 335 } 336 else 337@@ -719,42 +740,33 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 338 goto err; 339 } 340 341- /* 'create_empty_fragment' is true only when this function calls itself */ 342- if (!clear && !create_empty_fragment && !s->s3->empty_fragment_done) 343+ if (fragment) 344 { 345 /* countermeasure against known-IV weakness in CBC ciphersuites 346 * (see http://www.openssl.org/~bodo/tls-cbc.txt) */ 347+ prefix_len = do_ssl3_write(s, type, buf, 1 /* length */, 348+ 0 /* fragment */, 349+ 1 /* is_fragment */); 350+ if (prefix_len <= 0) 351+ goto err; 352 353- if (s->s3->need_empty_fragments && type == SSL3_RT_APPLICATION_DATA) 354+ if (prefix_len > (SSL3_RT_HEADER_LENGTH + 355+ SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD)) 356 { 357- /* recursive function call with 'create_empty_fragment' set; 358- * this prepares and buffers the data for an empty fragment 359- * (these 'prefix_len' bytes are sent out later 360- * together with the actual payload) */ 361- prefix_len = do_ssl3_write(s, type, buf, 0, 1); 362- if (prefix_len <= 0) 363- goto err; 364- 365- if (prefix_len > 366- (SSL3_RT_HEADER_LENGTH + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD)) 367- { 368- /* insufficient space */ 369- SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); 370- goto err; 371- } 372+ /* insufficient space */ 373+ SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR); 374+ goto err; 375 } 376- 377- s->s3->empty_fragment_done = 1; 378 } 379 380- if (create_empty_fragment) 381+ if (is_fragment) 382 { 383 #if defined(SSL3_ALIGN_PAYLOAD) && SSL3_ALIGN_PAYLOAD!=0 384- /* extra fragment would be couple of cipher blocks, 385- * which would be multiple of SSL3_ALIGN_PAYLOAD, so 386- * if we want to align the real payload, then we can 387- * just pretent we simply have two headers. */ 388- align = (long)wb->buf + 2*SSL3_RT_HEADER_LENGTH; 389+ /* The extra fragment would be couple of cipher blocks, and 390+ * that will be a multiple of SSL3_ALIGN_PAYLOAD. So, if we 391+ * want to align the real payload, we can just pretend that we 392+ * have two headers and a byte. */ 393+ align = (long)wb->buf + 2*SSL3_RT_HEADER_LENGTH + 1; 394 align = (-align)&(SSL3_ALIGN_PAYLOAD-1); 395 #endif 396 p = wb->buf + align; 397@@ -791,7 +803,7 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 398 *(p++)=s->version&0xff; 399 400 /* field where we are to write out packet length */ 401- plen=p; 402+ plen=p; 403 p+=2; 404 /* Explicit IV length, block ciphers and TLS version 1.1 or later */ 405 if (s->enc_write_ctx && s->version >= TLS1_1_VERSION) 406@@ -819,8 +831,8 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 407 408 /* lets setup the record stuff. */ 409 wr->data=p + eivlen; 410- wr->length=(int)len; 411- wr->input=(unsigned char *)buf; 412+ wr->length=(int)(len - (fragment != 0)); 413+ wr->input=(unsigned char *)buf + (fragment != 0); 414 415 /* we now 'read' from wr->input, wr->length bytes into 416 * wr->data */ 417@@ -873,11 +885,10 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf, 418 wr->type=type; /* not needed but helps for debugging */ 419 wr->length+=SSL3_RT_HEADER_LENGTH; 420 421- if (create_empty_fragment) 422+ if (is_fragment) 423 { 424- /* we are in a recursive call; 425- * just return the length, don't write out anything here 426- */ 427+ /* we are in a recursive call; just return the length, don't 428+ * write out anything. */ 429 return wr->length; 430 } 431 432@@ -1548,7 +1559,7 @@ int ssl3_dispatch_alert(SSL *s) 433 void (*cb)(const SSL *ssl,int type,int val)=NULL; 434 435 s->s3->alert_dispatch=0; 436- i = do_ssl3_write(s, SSL3_RT_ALERT, &s->s3->send_alert[0], 2, 0); 437+ i = do_ssl3_write(s, SSL3_RT_ALERT, &s->s3->send_alert[0], 2, 0, 0); 438 if (i <= 0) 439 { 440 s->s3->alert_dispatch=1; 441diff --git a/ssl/ssl.h b/ssl/ssl.h 442index ef85428..ce65664 100644 443--- a/ssl/ssl.h 444+++ b/ssl/ssl.h 445@@ -578,11 +578,15 @@ struct ssl_session_st 446 /* Refers to ancient SSLREF and SSLv2, retained for compatibility */ 447 #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0x0 448 449-/* Disable SSL 3.0/TLS 1.0 CBC vulnerability workaround that was added 450- * in OpenSSL 0.9.6d. Usually (depending on the application protocol) 451- * the workaround is not needed. Unfortunately some broken SSL/TLS 452- * implementations cannot handle it at all, which is why we include 453- * it in SSL_OP_ALL. */ 454+/* SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS is vestigial. Previously it disabled the 455+ * insertion of empty records in CBC mode, but the empty records were commonly 456+ * misinterpreted as EOF by other TLS stacks and so this was disabled by 457+ * SSL_OP_ALL. 458+ * 459+ * This has been replaced by 1/n-1 record splitting, which is enabled by 460+ * SSL_MODE_CBC_RECORD_SPLITTING in SSL_set_mode. This involves sending a 461+ * one-byte record rather than an empty record and has much better 462+ * compatibility. */ 463 #define SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS 0x00000800L /* added in 0.9.6e */ 464 465 /* SSL_OP_ALL: various bug workarounds that should be rather harmless. 466@@ -675,6 +679,12 @@ struct ssl_session_st 467 * one RTT. */ 468 #define SSL_MODE_HANDSHAKE_CUTTHROUGH 0x00000080L 469 470+/* When set, TLS 1.0 and SSLv3, multi-byte, CBC records will be split in two: 471+ * the first record will contain a single byte and the second will contain the 472+ * rest of the bytes. This effectively randomises the IV and prevents BEAST 473+ * attacks. */ 474+#define SSL_MODE_CBC_RECORD_SPLITTING 0x00000100L 475+ 476 /* Note: SSL[_CTX]_set_{options,mode} use |= op on the previous value, 477 * they cannot be used to clear bits. */ 478 479diff --git a/ssl/ssl3.h b/ssl/ssl3.h 480index 16c389d..8e3e449 100644 481--- a/ssl/ssl3.h 482+++ b/ssl/ssl3.h 483@@ -419,8 +419,8 @@ typedef struct ssl3_state_st 484 unsigned char client_random[SSL3_RANDOM_SIZE]; 485 486 /* flags for countermeasure against known-IV weakness */ 487- int need_empty_fragments; 488- int empty_fragment_done; 489+ int need_record_splitting; 490+ int record_split_done; 491 492 /* The value of 'extra' when the buffers were initialized */ 493 int init_extra; 494diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h 495index 10baaee..6d4bc14 100644 496--- a/ssl/ssl_locl.h 497+++ b/ssl/ssl_locl.h 498@@ -1093,8 +1093,6 @@ int dtls1_shutdown(SSL *s); 499 500 long dtls1_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok); 501 int dtls1_get_record(SSL *s); 502-int do_dtls1_write(SSL *s, int type, const unsigned char *buf, 503- unsigned int len, int create_empty_fragement); 504 int dtls1_dispatch_alert(SSL *s); 505 int dtls1_enc(SSL *s, int snd); 506 507diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c 508index 9963a80..4ca1549 100644 509--- a/ssl/t1_enc.c 510+++ b/ssl/t1_enc.c 511@@ -774,22 +774,22 @@ printf("\nkey block\n"); 512 { int z; for (z=0; z<num; z++) printf("%02X%c",p1[z],((z+1)%16)?' ':'\n'); } 513 #endif 514 515- if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS) 516- && s->method->version <= TLS1_VERSION) 517+ if (s->method->version <= TLS1_VERSION && 518+ (s->mode & SSL_MODE_CBC_RECORD_SPLITTING) != 0) 519 { 520 /* enable vulnerability countermeasure for CBC ciphers with 521 * known-IV problem (http://www.openssl.org/~bodo/tls-cbc.txt) 522 */ 523- s->s3->need_empty_fragments = 1; 524+ s->s3->need_record_splitting = 1; 525 526 if (s->session->cipher != NULL) 527 { 528 if (s->session->cipher->algorithm_enc == SSL_eNULL) 529- s->s3->need_empty_fragments = 0; 530+ s->s3->need_record_splitting = 0; 531 532 #ifndef OPENSSL_NO_RC4 533 if (s->session->cipher->algorithm_enc == SSL_RC4) 534- s->s3->need_empty_fragments = 0; 535+ s->s3->need_record_splitting = 0; 536 #endif 537 } 538 } 539-- 5402.0.0.526.g5318336 541 542