Update SSL_OP_ALL to account for SSL_OP_CRYPTOPRO_TLSEXT_BUG being gone, and update ssl3_setup_write_buffer to account for SSL_MODE_CBC_RECORD_SPLITTING rather than the now defunct SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS. Also remove SSL_OP_TLS_BLOCK_PADDING_BUG. This is to allow for a buggy peer which pads CBC with N bytes of value N rather than N+1 bytes of value N. This quirk has been broken since CBC padding checks became constant-time, as demonstrated by this attempt at a test. (Instead of just decrementing padding_length, it needs to also keep track of a separate padding_value and not decrement that one.) https://boringssl-review.googlesource.com/#/c/1690/ (The quirk would also fall over anyway if the buggy client ever did a session resumption; then the server speaks first rather than the client, and the quirk triggered on reading the first encrypted record from the peer.) Change-Id: I19942dc629a47832aead77a46bb50e0b0a9780b3 Reviewed-on: https://boringssl-review.googlesource.com/1694 Reviewed-by: Adam Langley <agl@google.com>kris/onging/CECPQ3_patch15
@@ -471,33 +471,21 @@ struct ssl_session_st | |||||
#endif | #endif | ||||
#define SSL_OP_MICROSOFT_SESS_ID_BUG 0x00000001L | |||||
#define SSL_OP_NETSCAPE_CHALLENGE_BUG 0x00000002L | |||||
/* Allow initial connection to servers that don't support RI */ | |||||
/* SSL_OP_LEGACY_SERVER_CONNECT allows initial connection to servers | |||||
* that don't support RI */ | |||||
#define SSL_OP_LEGACY_SERVER_CONNECT 0x00000004L | #define SSL_OP_LEGACY_SERVER_CONNECT 0x00000004L | ||||
#define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0x00000010L | |||||
/* SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER allows for record sizes | |||||
* SSL3_RT_MAX_EXTRA bytes above the maximum record size. */ | |||||
#define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L | #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L | ||||
#define SSL_OP_SAFARI_ECDHE_ECDSA_BUG 0x00000040L | |||||
#define SSL_OP_TLS_D5_BUG 0x00000100L | |||||
#define SSL_OP_TLS_BLOCK_PADDING_BUG 0x00000200L | |||||
/* Hasn't done anything since OpenSSL 0.9.7h, retained for compatibility */ | |||||
#define SSL_OP_MSIE_SSLV2_RSA_PADDING 0x0 | |||||
/* SSL_OP_TLS_D5_BUG accepts an RSAClientKeyExchange in TLS encoded as | |||||
* SSL3, without a length prefix. */ | |||||
#define SSL_OP_TLS_D5_BUG 0x00000100L | |||||
/* 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. | |||||
* This used to be 0x000FFFFFL before 0.9.7. */ | |||||
#define SSL_OP_ALL 0x80000BFFL | |||||
/* SSL_OP_ALL enables the above bug workarounds that should be rather | |||||
* harmless. */ | |||||
#define SSL_OP_ALL 0x00000BFFL | |||||
/* DTLS options */ | /* DTLS options */ | ||||
#define SSL_OP_NO_QUERY_MTU 0x00001000L | #define SSL_OP_NO_QUERY_MTU 0x00001000L | ||||
@@ -340,7 +340,6 @@ typedef struct ssl3_buffer_st | |||||
#define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 | #define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 | ||||
#define SSL3_FLAGS_POP_BUFFER 0x0004 | #define SSL3_FLAGS_POP_BUFFER 0x0004 | ||||
#define TLS1_FLAGS_TLS_PADDING_BUG 0x0008 | |||||
/* TODO(davidben): This flag can probably be merged into s3->change_cipher_spec | /* TODO(davidben): This flag can probably be merged into s3->change_cipher_spec | ||||
* to something tri-state. (Normal / Expect CCS / Between CCS and Finished). */ | * to something tri-state. (Normal / Expect CCS / Between CCS and Finished). */ | ||||
#define SSL3_FLAGS_EXPECT_CCS 0x0080 | #define SSL3_FLAGS_EXPECT_CCS 0x0080 | ||||
@@ -202,11 +202,6 @@ int dtls1_enc(SSL *s, int send) | |||||
/* we need to add 'i' padding bytes of value j */ | /* we need to add 'i' padding bytes of value j */ | ||||
j=i-1; | j=i-1; | ||||
if (s->options & SSL_OP_TLS_BLOCK_PADDING_BUG) | |||||
{ | |||||
if (s->s3->flags & TLS1_FLAGS_TLS_PADDING_BUG) | |||||
j++; | |||||
} | |||||
for (k=(int)l; k<(int)(l+i); k++) | for (k=(int)l; k<(int)(l+i); k++) | ||||
rec->input[k]=j; | rec->input[k]=j; | ||||
l+=i; | l+=i; | ||||
@@ -696,8 +696,9 @@ int ssl3_setup_write_buffer(SSL *s) | |||||
len = s->max_send_fragment | len = s->max_send_fragment | ||||
+ SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD | + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD | ||||
+ headerlen + align; | + headerlen + align; | ||||
if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) | |||||
len += headerlen + align | |||||
/* Account for 1/n-1 record splitting. */ | |||||
if (s->mode & SSL_MODE_CBC_RECORD_SPLITTING) | |||||
len += headerlen + align + 1 | |||||
+ SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD; | + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD; | ||||
if ((p=OPENSSL_malloc(len)) == NULL) | if ((p=OPENSSL_malloc(len)) == NULL) | ||||
@@ -165,21 +165,6 @@ int tls1_cbc_remove_padding(const SSL* s, | |||||
padding_length = rec->data[rec->length-1]; | padding_length = rec->data[rec->length-1]; | ||||
if (s->options & SSL_OP_TLS_BLOCK_PADDING_BUG) | |||||
{ | |||||
/* First packet is even in size, so check */ | |||||
if ((memcmp(s->s3->read_sequence, "\0\0\0\0\0\0\0\0",8) == 0) && | |||||
!(padding_length & 1)) | |||||
{ | |||||
s->s3->flags|=TLS1_FLAGS_TLS_PADDING_BUG; | |||||
} | |||||
if ((s->s3->flags & TLS1_FLAGS_TLS_PADDING_BUG) && | |||||
padding_length > 0) | |||||
{ | |||||
padding_length--; | |||||
} | |||||
} | |||||
good = constant_time_ge(rec->length, overhead+padding_length); | good = constant_time_ge(rec->length, overhead+padding_length); | ||||
/* The padding consists of a length byte at the end of the record and | /* The padding consists of a length byte at the end of the record and | ||||
* then that many bytes of padding, all with the same value as the | * then that many bytes of padding, all with the same value as the | ||||
@@ -311,7 +311,7 @@ static int ssl3_get_record(SSL *s) | |||||
extra=0; | extra=0; | ||||
if (extra && !s->s3->init_extra) | if (extra && !s->s3->init_extra) | ||||
{ | { | ||||
/* An application error: SLS_OP_MICROSOFT_BIG_SSLV3_BUFFER | |||||
/* An application error: SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER | |||||
* set after ssl3_setup_buffers() was done */ | * set after ssl3_setup_buffers() was done */ | ||||
OPENSSL_PUT_ERROR(SSL, ssl3_get_record, ERR_R_INTERNAL_ERROR); | OPENSSL_PUT_ERROR(SSL, ssl3_get_record, ERR_R_INTERNAL_ERROR); | ||||
return -1; | return -1; | ||||
@@ -903,11 +903,6 @@ int tls1_enc(SSL *s, int send) | |||||
/* we need to add 'i' padding bytes of value j */ | /* we need to add 'i' padding bytes of value j */ | ||||
j=i-1; | j=i-1; | ||||
if (s->options & SSL_OP_TLS_BLOCK_PADDING_BUG) | |||||
{ | |||||
if (s->s3->flags & TLS1_FLAGS_TLS_PADDING_BUG) | |||||
j++; | |||||
} | |||||
for (k=(int)l; k<(int)(l+i); k++) | for (k=(int)l; k<(int)(l+i); k++) | ||||
rec->input[k]=j; | rec->input[k]=j; | ||||
l+=i; | l+=i; | ||||