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 _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
