On Thu, Oct 08, 2015 at 06:26:27pm +0000, Alessandro Ghedini via RT wrote: > On Thu, Oct 08, 2015 at 06:14:00pm +0000, Alessandro Ghedini via RT wrote: > > On Thu, Oct 08, 2015 at 05:19:06pm +0000, Alessandro Ghedini via RT wrote: > > > On Thu, Oct 08, 2015 at 04:12:50pm +0000, Hubert Kario via RT wrote: > > > > The server does not abort connection upon receiving a Client Hello > > > > message with malformed session_id field. > > > > > > > > Affects 1.0.1, 1.0.2 and master. > > > > > > > > In SSLv3 and all versions of TLS (e.g. RFC 5246), the SessionID is > > > > defined as > > > > > > > > opaque SessionID<0..32>; > > > > > > > > that means, that any SessionID longer than 32 bytes creates an > > > > incorrectly formatted Client Hello message, and as such, should be > > > > rejected. > > > > > > Looking at the code in master, for non-v2 ClientHello messages the code > > > uses > > > the PACKET_get_length_prefixed_1() function to extract the SessionID, > > > however I > > > see no way to pass a maximum allowed length to it. I think a new function > > > would > > > have to be added to the PACKET_* interface (I can look into this). Haven't > > > checked older branches yet. > > > > So, it turns out the check was already performed, but this failure didn't > > cause > > the session to be aborted (the original PACKET was advanced anyway though, > > even > > is the session_id isn't actually extracted), I don't know if this was on > > purpose though. > > Another thing to consider is that the client already aborts when an invalid > session_id is received in the ServerHello and sends the ILLEGAL_PARAMETER > alert. > > My patch doesn't actually send any alert so the check should be moved earlier > to allow an alert to be sent, if that is desired.
FYI, I just pushed another patch that does the above (moving the check and sending an alert) which I think is the best option (although, as Hubert pointed out, sending the decode_error alert is not a MUST). If that's ok with you, I can squash the two commits together and give them a better message, otherwise just ignore the second patch. Cheers _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
