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. > But should we allow something as big as this just because its > theoretically possible? As a way of future-proofing OpenSSL (Hubert mentioned a few reasons) or just to be more standard-compliant I'd say yes, but it's obviously not an urgent problem. FWIW I checked a couple of TLS implementations I have around (GnuTLS and s2n), and AFAICT they don't check for a maximum size at all. Cheers _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev