Skip to content

Commit

Permalink
Fixes #4217 - SslConnection.DecryptedEnpoint.flush eternal busy loop.
Browse files Browse the repository at this point in the history
Releasing the encrypted output buffer so that it can be re-acquired
with an expanded capacity.

Signed-off-by: Simone Bordet <[email protected]>
  • Loading branch information
sbordet committed Oct 17, 2019
1 parent 894fc9b commit 2e633a4
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.SSLEngine;
Expand All @@ -48,6 +49,7 @@
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.ClientConnectionFactory;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.MappedByteBufferPool;
import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.io.ssl.SslHandshakeListener;
Expand All @@ -73,6 +75,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -807,4 +810,80 @@ protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuff
Throwable cause = failure.getCause();
assertThat(cause, Matchers.instanceOf(SSLHandshakeException.class));
}

@Test
public void testTLSLargeFragments() throws Exception
{
// The SSLEngine implementation may read a TLS packet that is larger than the default size,
// because the other peer uses large TLS fragments.
// When this happens, the SSLEngine implementation expands the SSLSession.packetBufferSize
// _during_ the TLS handshake, which causes a BUFFER_OVERFLOW result when trying to wrap().

SslContextFactory serverTLSFactory = createServerSslContextFactory();
startServer(serverTLSFactory, new EmptyServerHandler());

SslContextFactory clientTLSFactory = createClientSslContextFactory();
QueuedThreadPool clientThreads = new QueuedThreadPool();
clientThreads.setName("client");
client = new HttpClient(clientTLSFactory)
{
@Override
protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory)
{
if (sslContextFactory == null)
sslContextFactory = getSslContextFactory();
return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory)
{
@Override
protected SslConnection newSslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine engine)
{
AtomicInteger outputHash = new AtomicInteger();
byteBufferPool = new MappedByteBufferPool()
{
@Override
public void release(ByteBuffer buffer)
{
// Discard the TLS buffer so that it cannot be re-acquired.
if (outputHash.get() != System.identityHashCode(buffer))
super.release(buffer);
}
};
return new SslConnection(byteBufferPool, executor, endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption())
{
private int wrapCount;

@Override
protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuffer output) throws SSLException
{
if (++wrapCount == 1)
{
outputHash.set(System.identityHashCode(output));
// Replace the output buffer with one that will cause BUFFER_OVERFLOW.
output = ByteBuffer.allocate(1);
SSLEngineResult result = super.wrap(sslEngine, input, output);
assertEquals(SSLEngineResult.Status.BUFFER_OVERFLOW, result.getStatus());
return result;
}
else
{
// Make sure that if there was a BUFFER_OVERFLOW, we re-acquire
// the output buffer with potentially a different capacity due
// to the buffer expansion performed by the TLS implementation.
assertNotEquals(outputHash, System.identityHashCode(output));
return super.wrap(sslEngine, input, output);
}
}
};
}
};
}
};
client.setExecutor(clientThreads);
client.start();

ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(HttpScheme.HTTPS.asString())
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,8 @@ public boolean flush(ByteBuffer... appOuts) throws IOException
LOG.debug("flush starting handshake {}", SslConnection.this);
}

// We call sslEngine.wrap to try to take bytes from appOut buffers and encrypt them into the _netOut buffer
// We call sslEngine.wrap to try to take bytes from appOuts
// buffers and encrypt them into the _encryptedOutput buffer.
BufferUtil.compact(_encryptedOutput);
int pos = BufferUtil.flipToFill(_encryptedOutput);
SSLEngineResult wrapResult;
Expand Down Expand Up @@ -1032,6 +1033,13 @@ public boolean flush(ByteBuffer... appOuts) throws IOException
case BUFFER_OVERFLOW:
if (!flushed)
return result = false;
// It's possible that SSLSession.packetBufferSize has been expanded
// during the TLS handshake. Wrapping a large application buffer
// causes BUFFER_OVERFLOW because the (old) packetBufferSize is
// too small. Release the encrypted output buffer so that it will
// be re-acquired with the larger capacity.
// See also system property "jsse.SSLEngine.acceptLargeFragments".
releaseEncryptedOutputBuffer();
continue;

case OK:
Expand Down

0 comments on commit 2e633a4

Please sign in to comment.