From: Emilia Kasper <emilia@openssl.org>
Date: Tue, 9 Jun 2015 12:17:50 +0000 (+0200)
Subject: Remove SSL_OP_TLS_BLOCK_PADDING_BUG
X-Git-Tag: OpenSSL_1_1_0-pre1~1005
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a8e4ac6a2fe67c19672ecf0c6aeafa15801ce3a5;p=openssl

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 <tjh@openssl.org>
---

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;