users@grizzly.java.net

Re: Grizzly 2.0 M3 infinite loop hang in SSL handshake

From: Bo Li <b.li_at_sun.com>
Date: Wed, 08 Jul 2009 17:58:15 -0500

Hi Alexey

Thank you for integrating the changes. However it seems like the changes
still suffers from issue 655. I have attached a patch that fixes it in
the latest revision. Please see the comments in the patch for an
explanation.

Thanks again
Bo

Oleksiy Stashok wrote:
> 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
>>>> <mailto:users-unsubscribe_at_grizzly.dev.java.net>
>>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>>> <mailto:users-help_at_grizzly.dev.java.net>
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: users-unsubscribe_at_grizzly.dev.java.net
>>> <mailto:users-unsubscribe_at_grizzly.dev.java.net>
>>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>>> <mailto: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
>> <mailto:users-unsubscribe_at_grizzly.dev.java.net>
>> For additional commands, e-mail: users-help_at_grizzly.dev.java.net
>> <mailto:users-help_at_grizzly.dev.java.net>
>