Move the TLS padding extension under an SSL_OP_TLSEXT_PADDING option, which

is off by default (instead of being enabled unconditionally).

The TLS padding extension was added as a workaround for a bug in F5 SSL
terminators, however appears to trigger bugs in IronPort SMTP appliances.
Now the SSL client gets to choose which of these devices it wants to
trigger bugs in...

Ported from OpenSSL.

Discussed with many.

ok miod@

4 files changed, 36 insertions, 20 deletions

diff --git a/src/lib/libssl/src/ssl/ssl.h b/src/lib/libssl/src/ssl/ssl.h
index 857709f7c5..1851cd9525 100644
--- a/src/lib/libssl/src/ssl/ssl.h
+++ b/src/lib/libssl/src/ssl/ssl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl.h,v 1.63 2014/08/10 14:42:56 jsing Exp $ */
+/* $OpenBSD: ssl.h,v 1.64 2014/09/21 17:11:04 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -514,7 +514,7 @@ struct ssl_session_st {
 /* Allow initial connection to servers that don't support RI */
 #define SSL_OP_LEGACY_SERVER_CONNECT                    0x00000004L
 #define SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG         0x00000008L
-#define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG              0x00000010L
+#define SSL_OP_TLSEXT_PADDING                           0x00000010L
 #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER               0x00000020L
 #define SSL_OP_SAFARI_ECDHE_ECDSA_BUG                   0x00000040L
 #define SSL_OP_SSLEAY_080_CLIENT_DH_BUG                 0x00000080L
@@ -524,6 +524,9 @@ struct ssl_session_st {
 /* Hasn't done anything since OpenSSL 0.9.7h, retained for compatibility */
 #define SSL_OP_MSIE_SSLV2_RSA_PADDING                   0x0
 
+/* 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.
diff --git a/src/lib/libssl/src/ssl/t1_lib.c b/src/lib/libssl/src/ssl/t1_lib.c
index c25f10bfab..87a65e3db2 100644
--- a/src/lib/libssl/src/ssl/t1_lib.c
+++ b/src/lib/libssl/src/ssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.54 2014/08/07 22:27:28 guenther Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.55 2014/09/21 17:11:04 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -635,18 +635,24 @@ skip_ext:
         }
 #endif
 
-#ifdef TLSEXT_TYPE_padding
-        /* Add padding to workaround bugs in F5 terminators.
+        /*
+         * Add padding to workaround bugs in F5 terminators.
          * See https://tools.ietf.org/html/draft-agl-tls-padding-03
          *
+         * Note that this seems to trigger issues with IronPort SMTP
+         * appliances.
+         *
          * NB: because this code works out the length of all existing
          * extensions it MUST always appear last.
          */
-        {
+        if (s->options & SSL_OP_TLSEXT_PADDING) {
                 int hlen = ret - (unsigned char *)s->init_buf->data;
-        /* The code in s23_clnt.c to build ClientHello messages includes the
-         * 5-byte record header in the buffer, while the code in s3_clnt.c does
-         * not. */
+
+                /*
+                 * The code in s23_clnt.c to build ClientHello messages
+                 * includes the 5-byte record header in the buffer, while the
+                 * code in s3_clnt.c does not.
+                 */
                 if (s->state == SSL23_ST_CW_CLNT_HELLO_A)
                         hlen -= 5;
                 if (hlen > 0xff && hlen < 0x200) {
@@ -662,7 +668,6 @@ skip_ext:
                         ret += hlen;
                 }
         }
-#endif
 
         if ((extdatalen = ret - p - 2) == 0)
                 return p;
diff --git a/src/lib/libssl/ssl.h b/src/lib/libssl/ssl.h
index 857709f7c5..1851cd9525 100644
--- a/src/lib/libssl/ssl.h
+++ b/src/lib/libssl/ssl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl.h,v 1.63 2014/08/10 14:42:56 jsing Exp $ */
+/* $OpenBSD: ssl.h,v 1.64 2014/09/21 17:11:04 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -514,7 +514,7 @@ struct ssl_session_st {
 /* Allow initial connection to servers that don't support RI */
 #define SSL_OP_LEGACY_SERVER_CONNECT                    0x00000004L
 #define SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG         0x00000008L
-#define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG              0x00000010L
+#define SSL_OP_TLSEXT_PADDING                           0x00000010L
 #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER               0x00000020L
 #define SSL_OP_SAFARI_ECDHE_ECDSA_BUG                   0x00000040L
 #define SSL_OP_SSLEAY_080_CLIENT_DH_BUG                 0x00000080L
@@ -524,6 +524,9 @@ struct ssl_session_st {
 /* Hasn't done anything since OpenSSL 0.9.7h, retained for compatibility */
 #define SSL_OP_MSIE_SSLV2_RSA_PADDING                   0x0
 
+/* 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.
diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index c25f10bfab..87a65e3db2 100644
--- a/src/lib/libssl/t1_lib.c
+++ b/src/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.54 2014/08/07 22:27:28 guenther Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.55 2014/09/21 17:11:04 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -635,18 +635,24 @@ skip_ext:
         }
 #endif
 
-#ifdef TLSEXT_TYPE_padding
-        /* Add padding to workaround bugs in F5 terminators.
+        /*
+         * Add padding to workaround bugs in F5 terminators.
          * See https://tools.ietf.org/html/draft-agl-tls-padding-03
          *
+         * Note that this seems to trigger issues with IronPort SMTP
+         * appliances.
+         *
          * NB: because this code works out the length of all existing
          * extensions it MUST always appear last.
          */
-        {
+        if (s->options & SSL_OP_TLSEXT_PADDING) {
                 int hlen = ret - (unsigned char *)s->init_buf->data;
-        /* The code in s23_clnt.c to build ClientHello messages includes the
-         * 5-byte record header in the buffer, while the code in s3_clnt.c does
-         * not. */
+
+                /*
+                 * The code in s23_clnt.c to build ClientHello messages
+                 * includes the 5-byte record header in the buffer, while the
+                 * code in s3_clnt.c does not.
+                 */
                 if (s->state == SSL23_ST_CW_CLNT_HELLO_A)
                         hlen -= 5;
                 if (hlen > 0xff && hlen < 0x200) {
@@ -662,7 +668,6 @@ skip_ext:
                         ret += hlen;
                 }
         }
-#endif
 
         if ((extdatalen = ret - p - 2) == 0)
                 return p;