Fixes #4217 - SslConnection.DecryptedEnpoint.flush eternal busy loop.
Releasing the encrypted output buffer so that it can be re-acquired with an expanded capacity. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
parent
894fc9b115
commit
2e633a4e86
|
@ -32,6 +32,7 @@ import java.util.concurrent.CountDownLatch;
|
|||
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;
|
||||
|
@ -48,6 +49,7 @@ import org.eclipse.jetty.http.HttpStatus;
|
|||
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;
|
||||
|
@ -73,6 +75,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
|
|||
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;
|
||||
|
@ -807,4 +810,80 @@ public class HttpClientTLSTest
|
|||
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());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -983,7 +983,8 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
|
|||
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;
|
||||
|
@ -1032,6 +1033,13 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
|
|||
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:
|
||||
|
|
Loading…
Reference in New Issue