[openssl-dev] [openssl.org #4063] Client Hello longer than 2^14 bytes are rejected

[Alessandro Ghedini via RT]

On Fri, Sep 25, 2015 at 05:11:39pm +0000, Hubert Kario via RT wrote:
> On Friday 25 September 2015 16:54:02 Alessandro Ghedini via RT wrote:
> > On Fri, Sep 25, 2015 at 04:17:33PM +0000, Matt Caswell via RT wrote:
> > > On 25/09/15 17:05, Alessandro Ghedini via RT wrote:
> > > > On Fri, Sep 25, 2015 at 03:02:27pm +0000, Hubert Kario via RT 
> wrote:
> > > >> On Friday 25 September 2015 14:51:17 Alessandro Ghedini via RT 
> wrote:
> > > >>> As a matter of test I changed the ssl_get_message() in
> > > >>> ssl3_get_client_hello() to use 0xFFFFFF (uint24 max) as maximum
> > > >>> size,
> > > >> 
> > > >> it doesn't have in theory, but it does in practice, as extensions
> > > >> can
> > > >> only be 2^16 long, same for cipher suites, and you can't have
> > > >> data
> > > >> trailing the messages, so the actual size is limited to something
> > > >> closer 2^18, so if the client hello parser is correct, it will
> > > >> be limited by it> > 
> > > > Yeah, but OpenSSL first tries to "get" the handshake body and its
> > > > length before parsing it (this is done by ssl3_get_message()). So
> > > > the "max" argument is intended to be used, I imagine, as a sanity
> > > > check: if the message exceeds that, then it's obviously broken
> > > > and an "illegal parameters" alert is sent. This is done
> > > > regardless of the message type, so the ClientHello parser has to
> > > > do this as well.
> > > > 
> > > > This max length check is not exactly smart (e.g. the max size of
> > > > the SSLv3 ClientHello is very different from that of TLS) and
> > > > could probably be removed completely, but I don't really know
> > > > what the consequences of this would be. So the best next fix
> > > > would simply be to provide an approximation of an absolute
> > > > maximum length for the ClientHello (or just use 0xFFFFFF). I
> > > > opened a pull request [0] with just this minimal fix. Anyone is
> > > > very welcome to propose a better fix for this though.
> > > 
> > > 0xffffff = 16777215 or 16Mb
> > > 
> > > Allowing a ClientHello as big as this could enable a DoS attack.
> > > 
> > > If I did my sums right I make the biggest possible valid ClientHello
> > > to be 131396.
> > 
> > I updated my patch to use this value now.
> 
> embedding magic values is rather bad form. 
> A define with extended comment describing how we arrived at this value 
> would be much better

Fair enough. Updated the patch again to do this.

Cheers