From a8e4ac6a2fe67c19672ecf0c6aeafa15801ce3a5 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Tue, 9 Jun 2015 14:17:50 +0200 Subject: [PATCH] Remove SSL_OP_TLS_BLOCK_PADDING_BUG This is a workaround so old that nobody remembers what buggy clients it was for. It's also been broken in stable branches for two years and nobody noticed (see https://boringssl-review.googlesource.com/#/c/1694/). Reviewed-by: Tim Hudson --- CHANGES | 5 +++++ apps/s_server.c | 3 --- doc/ssl/SSL_CTX_set_options.pod | 4 ---- include/openssl/ssl.h | 3 ++- include/openssl/ssl3.h | 3 ++- ssl/record/ssl3_record.c | 22 ---------------------- 6 files changed, 9 insertions(+), 31 deletions(-) diff --git a/CHANGES b/CHANGES index e1b33929d5..1bd9e1afe8 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,11 @@ _______________ Changes between 1.0.2 and 1.1.0 [xx XXX xxxx] + *) Remove SSL_OP_TLS_BLOCK_PADDING_BUG. This is SSLeay legacy, we're + not aware of clients that still exhibit this bug, and the workaround + hasn't been working properly for a while. + [Emilia Käsper] + *) The return type of BIO_number_read() and BIO_number_written() as well as the corresponding num_read and num_write members in the BIO structure has changed from unsigned long to uint64_t. On platforms where an unsigned diff --git a/apps/s_server.c b/apps/s_server.c index 8354386ba4..072d30d8e3 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -2462,9 +2462,6 @@ static int init_ssl_connection(SSL *con) #endif if (SSL_cache_hit(con)) BIO_printf(bio_s_out, "Reused session-id\n"); - if (SSL_ctrl(con, SSL_CTRL_GET_FLAGS, 0, NULL) & - TLS1_FLAGS_TLS_PADDING_BUG) - BIO_printf(bio_s_out, "Peer has incorrect TLSv1 block padding\n"); BIO_printf(bio_s_out, "Secure Renegotiation IS%s supported\n", SSL_get_secure_renegotiation_support(con) ? "" : " NOT"); if (keymatexportlabel != NULL) { diff --git a/doc/ssl/SSL_CTX_set_options.pod b/doc/ssl/SSL_CTX_set_options.pod index 1078f09837..84dde2805e 100644 --- a/doc/ssl/SSL_CTX_set_options.pod +++ b/doc/ssl/SSL_CTX_set_options.pod @@ -94,10 +94,6 @@ OS X 10.8..10.8.3 has broken support for ECDHE-ECDSA ciphers. ... -=item SSL_OP_TLS_BLOCK_PADDING_BUG - -... - =item SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS Disables a countermeasure against a SSL 3.0/TLS 1.0 protocol diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 4e18b65605..cd932e5edf 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -360,7 +360,8 @@ typedef int (*custom_ext_parse_cb) (SSL *s, unsigned int ext_type, # define SSL_OP_SAFARI_ECDHE_ECDSA_BUG 0x00000040L # define SSL_OP_SSLEAY_080_CLIENT_DH_BUG 0x00000080L # define SSL_OP_TLS_D5_BUG 0x00000100L -# define SSL_OP_TLS_BLOCK_PADDING_BUG 0x00000200L +/* Removed from OpenSSL 1.1.0 */ +# define SSL_OP_TLS_BLOCK_PADDING_BUG 0x0L /* Hasn't done anything since OpenSSL 0.9.7h, retained for compatibility */ # define SSL_OP_MSIE_SSLV2_RSA_PADDING 0x0 diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 66bc8c6e85..138b80c81a 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -362,7 +362,8 @@ extern "C" { # define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 # define SSL3_FLAGS_DELAY_CLIENT_FINISHED 0x0002 # define SSL3_FLAGS_POP_BUFFER 0x0004 -# define TLS1_FLAGS_TLS_PADDING_BUG 0x0008 +/* Removed from OpenSSL 1.1.0 */ +# define TLS1_FLAGS_TLS_PADDING_BUG 0x0 # define TLS1_FLAGS_SKIP_CERT_VERIFY 0x0010 # define TLS1_FLAGS_KEEP_HANDSHAKE 0x0020 /* diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index dbec5f1fc2..1865f24241 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -748,10 +748,6 @@ int tls1_enc(SSL *s, int send) /* we need to add 'i' padding bytes of value j */ 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++) rec->input[k] = j; l += i; @@ -1064,24 +1060,6 @@ int tls1_cbc_remove_padding(const SSL *s, padding_length = rec->data[rec->length - 1]; - /* - * NB: if compression is in operation the first packet may not be of even - * length so the padding bug check cannot be performed. This bug - * workaround has been around since SSLeay so hopefully it is either - * fixed now or no buggy implementation supports compression [steve] - */ - if ((s->options & SSL_OP_TLS_BLOCK_PADDING_BUG) && !s->expand) { - /* First packet is even in size, so check */ - if ((CRYPTO_memcmp(RECORD_LAYER_get_read_sequence(&s->rlayer), - "\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--; - } - } - if (EVP_CIPHER_flags(s->enc_read_ctx->cipher) & EVP_CIPH_FLAG_AEAD_CIPHER) { /* padding is already verified */ rec->length -= padding_length + 1; -- GitLab