EnsureBufferLength infinite wait

[mappendino]

I found that when peeking a client hello (using PeekClientHello) there are some cases when EnsureBufferLenght keeps waiting for more data when there isn't any more available.
I have a client that was sending a TLS 1.0 client hello without extensions and when PeekClientHello called ReadExtensions it would wait until the client connection timed out.

With a client that sends the extensions PeekClientHello is expecting this issue can be replicated by adding an extra EnsureBufferLenght check right after the call to ReadExtensions. The expected behavior is for the check to fail because there's no more data available but instead it will keep waiting until the connection times out.

[justcoding121]

Do we have a public site where we can reproduce this issue, TLS 1.0 with no extensions.

[justcoding121]

I see the issue here. But I don't think we can do anything about it.

Currently we expect the client to have TLS extensions if its protocol version is 3.1 or above (TLS 1.1 or above).
https://github.com/justcoding121/Stream-Extended/blob/master/src/StreamExtended/SslTools.cs#L199

When no extension is present the read call will wait for the timeout. Are you sure client is using TLS 1.0. It could be TLS 1.1.

[jgilbert2017]

wouldn't the proper thing to do be to check the ssl record length to make sure there are extension records before trying to read them (ie bytes 3-4 of the ssl record https://www.ibm.com/support/knowledgecenter/SSB23S_1.1.0.12/gtps7/s5rcd.html )?

[justcoding121]

I did'nt write this piece here and @honfika is not active nowadays. Would be glad if someone can create a PR fixing this.

[justcoding121]

I think one way would be to use the record length of whole client hello record and then keep subtracting the number of read bytes until its about to read the extension values. At that point if the remaining bytes to read is positive then there is extension data.

[jgilbert2017]

should be an easy fix. the stream already keeps track of it's position and the recordLength + 5 tells you how far you can go where recordLength is the first length field read.

int extenstionsStartPosition = peekStream.Position;

[justcoding121]

Thanks.

[jgilbert2017]

just checked the merge... i thought the if statement should be

if (extenstionsStartPosition < recordLength + 5) {

because the recordLength does not include the first 5 bytes that are read.

[justcoding121]

Thanks for the catch. I messed it up.