On Wed, Jul 23, 2014 at 10:20:23AM +0200, Hanno B?ck wrote: > Hi, > > Quick background: Some router firmwares from F5 have a bug that they > fail if the SSL handshake is between 256 and 511 bytes.
F5 should issue fixes for their firmware. > > Following up that openssl and other major ssl implementations > introduced a TLS padding extension that does nothing else than padding > the handshake if it is between these sizes. > > IMHO this is the wrong way of doing things, because it hides bugs > instead of fixing them. And a bunch of alike workarounds in the past > are one of the reasons TLS is such a mess these days. > Also, there have already been devices found that break with this > workaround because they fail if the ssl handshake is above 512 bytes > [3]. > > openssl unconditionally enables the padding extension. I propose a > simple step: Just remove it from libressl. > > > > [1] https://www.imperialviolet.org/2014/02/27/tlssymmetriccrypto.html > [2] http://www.ietf.org/mail-archive/web/tls/current/msg10423.html > [3] http://www.ietf.org/mail-archive/web/tls/current/msg12143.html > > cu, > -- > Hanno B?ck > http://hboeck.de/ > > mail/jabber: ha...@hboeck.de > GPG: BBB51E42 > diff -Naur libressl-2.0.3/include/openssl/tls1.h > libressl-2.0.3-1/include/openssl/tls1.h > --- libressl-2.0.3/include/openssl/tls1.h 2014-07-11 18:50:56.000000000 > +0200 > +++ libressl-2.0.3-1/include/openssl/tls1.h 2014-07-23 10:12:34.081669530 > +0200 > @@ -230,12 +230,6 @@ > /* ExtensionType value from RFC5620 */ > #define TLSEXT_TYPE_heartbeat 15 > > -/* ExtensionType value for TLS padding extension. > - * > http://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml > - * http://tools.ietf.org/html/draft-agl-tls-padding-03 > - */ > -#define TLSEXT_TYPE_padding 21 > - > /* ExtensionType value from RFC4507 */ > #define TLSEXT_TYPE_session_ticket 35 > > diff -Naur libressl-2.0.3/ssl/t1_lib.c libressl-2.0.3-1/ssl/t1_lib.c > --- libressl-2.0.3/ssl/t1_lib.c 2014-07-14 01:04:03.000000000 +0200 > +++ libressl-2.0.3-1/ssl/t1_lib.c 2014-07-23 10:12:13.006933000 +0200 > @@ -635,35 +635,6 @@ > } > #endif > > -#ifdef TLSEXT_TYPE_padding > - /* Add padding to workaround bugs in F5 terminators. > - * See https://tools.ietf.org/html/draft-agl-tls-padding-03 > - * > - * NB: because this code works out the length of all existing > - * extensions it MUST always appear last. > - */ > - { > - 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. */ > - if (s->state == SSL23_ST_CW_CLNT_HELLO_A) > - hlen -= 5; > - if (hlen > 0xff && hlen < 0x200) { > - hlen = 0x200 - hlen; > - if (hlen >= 4) > - hlen -= 4; > - else > - hlen = 0; > - > - s2n(TLSEXT_TYPE_padding, ret); > - s2n(hlen, ret); > - memset(ret, 0, hlen); > - ret += hlen; > - } > - } > -#endif > - > if ((extdatalen = ret - p - 2) == 0) > return p; >
