users@grizzly.java.net

Re: Grizzly 2.0 M3 infinite loop hang in SSL handshake

From: Oleksiy Stashok <Oleksiy.Stashok_at_Sun.COM>
Date: Wed, 08 Jul 2009 12:30:43 +0200

Hi Bo,

>
> I have looked into this a little more and found another issue (697).
> After tracking down the cause of these problems, I went ahead and
> made a proposed fix for both issues (655 and 697). Its not the most
> elegant fix but at least it gets the job done. I ran a brief test
> using my code and it seems to work perfectly.
Thank you very much for patches.
I've reworked them just a little bit and integrated.

Thank you again.

WBR,
Alexey.

>
> Let me know what you think
> Bo
>
> Oleksiy Stashok wrote:
>> Hi Bo,
>>
>> sure, file the issue, I'll fix it once will come from vacation (in
>> a week).
>>
>> Thank you!
>>
>> WBR,
>> Alexey.
>>
>> On Jun 4, 2009, at 6:37 , Bo Li wrote:
>>
>>> While implementing StartTLS with Grizzly 2.0 M3, I seem to have
>>> uncovered a
>>> potential bug with the handshaking code:
>>>
>>> I started a TCPNIOTransport using the DefaultFilterChain with the
>>> TransportFilter and our own LDAPFilter. After successful
>>> negotiation of the
>>> StartTLS operation, I insert the SSLFilter between the
>>> TransportFilter and
>>> the LDAPFilter and initiated the handshake (this is the client
>>> side).
>>> However, the handshake gets stuck at the NEED_WRAP state because the
>>> underlying TCPNIOStreamWriter's buffer is too small for the
>>> SSLEngine.
>>>
>>> The default buffer size for the TCPNIOStreamWriter is 4k but the
>>> SSLEngine
>>> requires a 16k destination buffer when calling SSLEngine.wrap. The
>>> call to
>>> SSLStreamWriter.checkBuffers sets the new buffer size and then
>>> flushes the
>>> smaller buffer. However, since the buffer was empty, the flush
>>> didn't
>>> actually do anything. When SSLEngine.wrap() is called, its given a
>>> 4k
>>> destination buffer instead of the 16k and the wrap produces no
>>> network data
>>> and stays in the NEED_WRAP state. This whole process repeats in a
>>> loop.
>>>
>>> Stack trace of the failed buffer resize attempt:
>>> AbstractStreamWriter.overflow(..) : 124
>>> AbstractStreamWriter.flush(..) : 153
>>> AbstractStreamWriter.flush() : 145
>>> SSLStreamWriter.checkBuffers() : 106
>>> SSLStreamWriter.flush0(..) : 124
>>> SSLStreamWriter.handshakeWrap(..) : 88
>>> BlockingSSLHandshaker.handshake(..) : 128
>>>
>>> Should I open an issue?
>>>
>>> Thanks
>>> Bo
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>
> Index: modules/grizzly/src/main/java/com/sun/grizzly/streams/
> AbstractStreamWriter.java
> ===================================================================
> --- modules/grizzly/src/main/java/com/sun/grizzly/streams/
> AbstractStreamWriter.java (revision 3411)
> +++ modules/grizzly/src/main/java/com/sun/grizzly/streams/
> AbstractStreamWriter.java Mon Jul 06 17:17:13 CDT 2009
> @@ -119,7 +119,6 @@
> if (!future.isDone()) {
> buffer = newBuffer(bufferSize);
> }
> - initBuffer();
> } else {
> future = ZERO_READY_FUTURE;
> if (completionHandler != null) {
> @@ -128,15 +127,24 @@
> }
> } else {
> buffer = newBuffer(bufferSize);
> - initBuffer();
> }
>
> + initBuffer();
> return future;
> }
>
> private void initBuffer() {
> + if(buffer.capacity() < bufferSize)
> + {
> + // Buffer size increased.
> + buffer.dispose();
> + buffer = newBuffer(bufferSize);
> + }
> + else
> + {
> - buffer.clear();
> - }
> + buffer.clear();
> + }
> + }
>
> /**
> * Cause the overflow handler to be called even if buffer is not
> full.
> Index: modules/grizzly/src/main/java/com/sun/grizzly/ssl/
> SSLStreamWriter.java
> ===================================================================
> --- modules/grizzly/src/main/java/com/sun/grizzly/ssl/
> SSLStreamWriter.java (revision 3411)
> +++ modules/grizzly/src/main/java/com/sun/grizzly/ssl/
> SSLStreamWriter.java Mon Jul 06 17:16:12 CDT 2009
> @@ -67,14 +67,17 @@
>
> @Override
> public void setUnderlyingWriter(StreamWriter underlyingWriter) {
> - super.setUnderlyingWriter(underlyingWriter);
> -
> + super.setUnderlyingWriter(underlyingWriter);
> +
> - try {
> - checkBuffers();
> - } catch (IOException e) {
> - throw new IllegalStateException(e);
> + SSLEngine sslEngine = getSSLEngine();
> +
> + if (sslEngine != null) {
> + int appBufferSize =
> sslEngine.getSession().getApplicationBufferSize();
> + if (bufferSize < appBufferSize) {
> + bufferSize = appBufferSize;
> }
> - }
> + }
> + }
>
> public SSLEngine getSSLEngine() {
> SSLResourcesAccessor resourceAccessor =
> @@ -90,39 +93,25 @@
> completionHandler);
> }
>
> - private void checkBuffers() throws IOException {
> + @Override
> + protected Future<Integer> flush0(Buffer buffer,
> + CompletionHandler<Integer> completionHandler) throws
> IOException {
> +
> + Future lastWriterFuture = null;
> SSLEngine sslEngine = getSSLEngine();
> + int underlyingBufferSize =
> sslEngine.getSession().getPacketBufferSize();
> + int appBufferSize =
> sslEngine.getSession().getApplicationBufferSize();
>
> - if (sslEngine != null) {
> - if (underlyingWriter != null) {
> + if (underlyingWriter != null) {
> - int underlyingBufferSize =
> sslEngine.getSession().getPacketBufferSize();
> - if (underlyingWriter.getBufferSize() <
> underlyingBufferSize) {
> -
> underlyingWriter.setBufferSize(underlyingBufferSize);
> - }
> + if (underlyingWriter.getBufferSize() <
> underlyingBufferSize) {
> + underlyingWriter.setBufferSize(underlyingBufferSize);
> + }
> -
> - Buffer underlyingBuffer =
> underlyingWriter.getBuffer();
> - if (underlyingBuffer == null ||
> - (underlyingBuffer.remaining() <
> underlyingBufferSize)) {
> - underlyingWriter.flush();
> - }
> + }
> - }
> -
> +
> - int appBufferSize =
> sslEngine.getSession().getApplicationBufferSize();
> - if (bufferSize < appBufferSize) {
> - bufferSize = appBufferSize;
> - }
> + if (bufferSize < appBufferSize) {
> + bufferSize = appBufferSize;
> + }
> - }
> - }
>
> - @Override
> - protected Future<Integer> flush0(Buffer buffer,
> - CompletionHandler<Integer> completionHandler) throws
> IOException {
> -
> - Future lastWriterFuture = null;
> - SSLEngine sslEngine = getSSLEngine();
> -
> - checkBuffers();
> -
> if (buffer != null) {
> buffer.flip();
> if (buffer.remaining() > 0 &&
> SSLUtils.isHandshaking(sslEngine)) {
> @@ -132,6 +121,10 @@
> ByteBuffer byteBuffer = (ByteBuffer) buffer.underlying();
> do {
> Buffer underlyingBuffer =
> underlyingWriter.getBuffer();
> + if (underlyingBuffer.remaining() <
> underlyingBufferSize) {
> + underlyingWriter.flush();
> + underlyingBuffer = underlyingWriter.getBuffer();
> + }
> ByteBuffer underlyingByteBuffer = (ByteBuffer)
> underlyingBuffer.underlying();
> sslEngine.wrap(byteBuffer, underlyingByteBuffer);
> lastWriterFuture = underlyingWriter.flush();
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: users-help_at_grizzly.dev.java.net