Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4245 from eclipse/jetty-9.3.x-4217-sslconnection-flush-loop 

Issue #4217 - (9.3.x) SslConnection DecryptedEndpoint flush eternal busy loop
This commit is contained in:
Joakim Erdfelt 2019-10-31 11:42:08 -05:00 committed by GitHub
parent abc92e5c5d a3f3612fb4
commit 6c69c39ffa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 271 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java
View File

@ -23,13 +23,16 @@ import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.ServerSocket;
import java.net.Socket;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
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.AtomicReference;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSocket;
@ -37,12 +40,20 @@ import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpScheme;
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.ssl.SslClientConnectionFactory;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.io.ssl.SslHandshakeListener;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
@ -50,6 +61,9 @@ import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class HttpClientTLSTest
{
private Server server;
@ -516,4 +530,119 @@ public class HttpClientTLSTest
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
}
}
@Test
public void testTLSLargeFragments() throws Exception
{
CountDownLatch serverLatch = new CountDownLatch(1);
SslContextFactory serverTLSFactory = createSslContextFactory();
QueuedThreadPool serverThreads = new QueuedThreadPool();
serverThreads.setName("server");
server = new Server(serverThreads);
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.addCustomizer(new SecureRequestCustomizer());
HttpConnectionFactory http = new HttpConnectionFactory(httpConfig);
SslConnectionFactory ssl = new SslConnectionFactory(serverTLSFactory, http.getProtocol())
{
@Override
protected SslConnection newSslConnection(Connector connector, EndPoint endPoint, SSLEngine engine)
{
return new SslConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption())
{
@Override
protected SSLEngineResult unwrap(SSLEngine sslEngine, ByteBuffer input, ByteBuffer output) throws SSLException
{
int inputBytes = input.remaining();
SSLEngineResult result = super.unwrap(sslEngine, input, output);
if (inputBytes == 5)
serverLatch.countDown();
return result;
}
};
}
};
connector = new ServerConnector(server, 1, 1, ssl, http);
server.addConnector(connector);
server.setHandler(new EmptyServerHandler());
server.start();
long idleTimeout = 2000;
CountDownLatch clientLatch = new CountDownLatch(1);
SslContextFactory clientTLSFactory = createSslContextFactory();
QueuedThreadPool clientThreads = new QueuedThreadPool();
clientThreads.setName("client");
client = new HttpClient(clientTLSFactory)
{
@Override
protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
{
SslContextFactory sslContextFactory = getSslContextFactory();
return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory)
{
@Override
protected SslConnection newSslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine engine)
{
return new SslConnection(byteBufferPool, executor, endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption())
{
@Override
protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuffer output) throws SSLException
{
try
{
clientLatch.countDown();
assertTrue(serverLatch.await(5, TimeUnit.SECONDS));
return super.wrap(sslEngine, input, output);
}
catch (InterruptedException x)
{
throw new SSLException(x);
}
}
};
}
};
}
};
client.setIdleTimeout(idleTimeout);
client.setExecutor(clientThreads);
client.start();
String host = "localhost";
int port = connector.getLocalPort();
CountDownLatch responseLatch = new CountDownLatch(1);
client.newRequest(host, port)
.scheme(HttpScheme.HTTPS.asString())
.send(result ->
{
assertTrue(result.isSucceeded());
assertEquals(HttpStatus.OK_200, result.getResponse().getStatus());
responseLatch.countDown();
});
// Wait for the TLS buffers to be acquired by the client, then the
// HTTP request will be paused waiting for the TLS buffer to be expanded.
assertTrue(clientLatch.await(5, TimeUnit.SECONDS));
// Send the large frame bytes that will enlarge the TLS buffers.
try (Socket socket = new Socket(host, port))
{
OutputStream output = socket.getOutputStream();
byte[] largeFrameBytes = new byte[5];
largeFrameBytes[0] = 22; // Type = handshake
largeFrameBytes[1] = 3; // Major TLS version
largeFrameBytes[2] = 3; // Minor TLS version
// Frame length is 0x7FFF == 32767, i.e. a "large fragment".
// Maximum allowed by RFC 8446 is 16384, but SSLEngine supports up to 33093.
largeFrameBytes[3] = 0x7F; // Length hi byte
largeFrameBytes[4] = (byte)0xFF; // Length lo byte
output.write(largeFrameBytes);
output.flush();
// Just close the connection now, the large frame
// length was enough to trigger the buffer expansion.
}
// The HTTP request will resume and be forced to handle the TLS buffer expansion.
assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
}
}

5
jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesClientTest.java
View File

@ -30,7 +30,6 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLSocket;
@ -41,10 +40,12 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.util.FutureResponseListener;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.toolchain.test.JDK;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.After;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
@ -59,6 +60,8 @@ public class SslBytesClientTest extends SslBytesTest
@Before
public void init() throws Exception
{
// Disable for JDK 9 and above.
Assume.assumeFalse(JDK.IS_9);
threadPool = Executors.newCachedThreadPool();
client = new HttpClient(new SslContextFactory(true));

1
jetty-client/src/test/resources/jetty-logging.properties
View File

@ -1,3 +1,4 @@
org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
#org.eclipse.jetty.LEVEL=DEBUG
#org.eclipse.jetty.client.LEVEL=DEBUG
#org.eclipse.jetty.io.ssl.LEVEL=DEBUG

181
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
View File

@ -24,13 +24,14 @@ import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executor;
import java.util.function.ToIntFunction;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
import javax.net.ssl.SSLEngineResult.Status;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLSession;
import org.eclipse.jetty.io.AbstractConnection;
import org.eclipse.jetty.io.AbstractEndPoint;
@ -38,6 +39,7 @@ import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.EofException;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.io.SelectChannelEndPoint;
import org.eclipse.jetty.io.WriteFlusher;
import org.eclipse.jetty.util.BufferUtil;
@ -212,6 +214,57 @@ public class SslConnection extends AbstractConnection
this._allowMissingCloseMessage = allowMissingCloseMessage;
}
private int getApplicationBufferSize()
{
return getBufferSize(SSLSession::getApplicationBufferSize);
}
private int getPacketBufferSize()
{
return getBufferSize(SSLSession::getPacketBufferSize);
}
private int getBufferSize(ToIntFunction<SSLSession> bufferSizeFn)
{
SSLSession hsSession = _sslEngine.getHandshakeSession();
SSLSession session = _sslEngine.getSession();
int size = bufferSizeFn.applyAsInt(session);
if (hsSession == null || hsSession == session)
return size;
int hsSize = bufferSizeFn.applyAsInt(hsSession);
return Math.max(hsSize, size);
}
private void acquireEncryptedInput()
{
if (_encryptedInput == null)
_encryptedInput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers);
}
private void acquireEncryptedOutput()
{
if (_encryptedOutput == null)
_encryptedOutput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers);
}
private void releaseEncryptedInputBuffer()
{
if (_encryptedInput != null && !_encryptedInput.hasRemaining())
{
_bufferPool.release(_encryptedInput);
_encryptedInput = null;
}
}
protected void releaseDecryptedInputBuffer()
{
if (_decryptedInput != null && !_decryptedInput.hasRemaining())
{
_bufferPool.release(_decryptedInput);
_decryptedInput = null;
}
}
@Override
public void onOpen()
{
@ -298,6 +351,16 @@ public class SslConnection extends AbstractConnection
_decryptedEndPoint.getWriteFlusher().onFail(cause);
}
protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuffer output) throws SSLException
{
return sslEngine.wrap(input, output);
}
protected SSLEngineResult unwrap(SSLEngine sslEngine, ByteBuffer input, ByteBuffer output) throws SSLException
{
return sslEngine.unwrap(input, output);
}
@Override
public String toString()
{
@ -540,9 +603,12 @@ public class SslConnection extends AbstractConnection
{
if (connection instanceof AbstractConnection)
{
AbstractConnection a = (AbstractConnection)connection;
if (a.getInputBufferSize()<_sslEngine.getSession().getApplicationBufferSize())
a.setInputBufferSize(_sslEngine.getSession().getApplicationBufferSize());
// This is an optimization to avoid that upper layer connections use small
// buffers and we need to copy decrypted data rather than decrypting in place.
AbstractConnection c = (AbstractConnection)connection;
int appBufferSize = getApplicationBufferSize();
if (c.getInputBufferSize() < appBufferSize)
c.setInputBufferSize(appBufferSize);
}
super.setConnection(connection);
}
@ -572,18 +638,23 @@ public class SslConnection extends AbstractConnection
else
BufferUtil.compact(_encryptedInput);
// We also need an app buffer, but can use the passed buffer if it is big enough
ByteBuffer app_in;
if (BufferUtil.space(buffer) > _sslEngine.getSession().getApplicationBufferSize())
app_in = buffer;
else if (_decryptedInput == null)
app_in = _decryptedInput = _bufferPool.acquire(_sslEngine.getSession().getApplicationBufferSize(), _decryptedDirectBuffers);
else
app_in = _decryptedInput;
// loop filling and unwrapping until we have something
while (true)
{
// We also need an app buffer, but can use the passed buffer if it is big enough
ByteBuffer app_in;
int appBufferSize = getApplicationBufferSize();
if (BufferUtil.space(buffer) > appBufferSize)
app_in = buffer;
else if (_decryptedInput == null)
app_in = _decryptedInput = _bufferPool.acquire(appBufferSize, _decryptedDirectBuffers);
else
app_in = _decryptedInput;
acquireEncryptedInput();
// Let's try reading some encrypted data... even if we have some already.
int net_filled = getEndPoint().fill(_encryptedInput);
@ -598,17 +669,20 @@ public class SslConnection extends AbstractConnection
SSLEngineResult unwrapResult;
try
{
unwrapResult = _sslEngine.unwrap(_encryptedInput, app_in);
unwrapResult = unwrap(_sslEngine, _encryptedInput, app_in);
}
finally
{
BufferUtil.flipToFlush(app_in, pos);
}
if (LOG.isDebugEnabled())
{
LOG.debug("net={} unwrap {} {}", net_filled, unwrapResult.toString().replace('\n',' '), SslConnection.this);
LOG.debug("filled {} {}",BufferUtil.toHexSummary(buffer), SslConnection.this);
}
LOG.debug("unwrap net_filled={} {} encryptedBuffer={} unwrapBuffer={} appBuffer={}",
net_filled,
unwrapResult.toString().replace('\n',' '),
BufferUtil.toSummaryString(_encryptedInput),
BufferUtil.toDetailString(app_in),
BufferUtil.toDetailString(buffer));
HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
HandshakeStatus unwrapHandshakeStatus = unwrapResult.getHandshakeStatus();
@ -634,34 +708,38 @@ public class SslConnection extends AbstractConnection
switch (handshakeStatus)
{
case NOT_HANDSHAKING:
{
// We were not handshaking, so just tell the app we are closed
return -1;
}
case NEED_TASK:
{
_sslEngine.getDelegatedTask().run();
continue;
}
case NEED_WRAP:
{
// We need to send some handshake data (probably the close handshake).
// We return -1 so that the application can drive the close by flushing
// or shutting down the output.
return -1;
}
case NEED_UNWRAP:
{
// We expected to read more, but we got closed.
// Return -1 to indicate to the application to drive the close.
return -1;
}
default:
{
throw new IllegalStateException();
}
}
}
case BUFFER_OVERFLOW:
// It's possible that SSLSession.applicationBufferSize has been expanded
// by the SSLEngine implementation. Unwrapping a large encrypted buffer
// causes BUFFER_OVERFLOW because the (old) applicationBufferSize is
// too small. Release the decrypted input buffer so it will be re-acquired
// with the larger capacity.
// See also system property "jsse.SSLEngine.acceptLargeFragments".
if (BufferUtil.isEmpty(_decryptedInput) && appBufferSize < getApplicationBufferSize())
{
releaseDecryptedInputBuffer();
break decryption;
}
throw new IllegalStateException("Unexpected unwrap result " + unwrapResultStatus);
case BUFFER_UNDERFLOW:
case OK:
{
@ -671,7 +749,7 @@ public class SslConnection extends AbstractConnection
// Check whether re-negotiation is allowed
if (!allowRenegotiate(handshakeStatus))
return -1;
// If bytes were produced, don't bother with the handshake status;
// pass the decrypted data to the application, which will perform
// another call to fill() or flush().
@ -769,23 +847,17 @@ public class SslConnection extends AbstractConnection
getExecutor().execute(failure == null ? _runCompletWrite : new FailWrite(failure));
}
if (_encryptedInput != null && !_encryptedInput.hasRemaining())
{
_bufferPool.release(_encryptedInput);
_encryptedInput = null;
}
if (_decryptedInput != null && !_decryptedInput.hasRemaining())
{
_bufferPool.release(_decryptedInput);
_decryptedInput = null;
}
releaseEncryptedInputBuffer();
releaseDecryptedInputBuffer();
}
}
}
catch (Throwable x)
{
close(x);
throw x;
if(x instanceof IOException)
throw (IOException) x;
throw new RuntimeIOException(x);
}
}
@ -895,19 +967,19 @@ public class SslConnection extends AbstractConnection
return false;
}
// We will need a network buffer
if (_encryptedOutput == null)
_encryptedOutput = _bufferPool.acquire(_sslEngine.getSession().getPacketBufferSize(), _encryptedDirectBuffers);
while (true)
{
// We call sslEngine.wrap to try to take bytes from appOut buffers and encrypt them into the _netOut buffer
int packetBufferSize = getPacketBufferSize();
acquireEncryptedOutput();
// 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;
try
{
wrapResult = _sslEngine.wrap(appOuts, _encryptedOutput);
wrapResult = wrap(_sslEngine, appOuts,_encryptedOutput);
}
finally
{
@ -952,6 +1024,25 @@ public class SslConnection extends AbstractConnection
{
throw new IllegalStateException();
}
case BUFFER_OVERFLOW:
{
// It's possible that SSLSession.packetBufferSize has been expanded
// by the SSLEngine implementation. 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".
if (packetBufferSize < getPacketBufferSize())
{
releaseEncryptedOutputBuffer();
continue;
}
if (BufferUtil.isEmpty(_encryptedOutput))
{
throw new IllegalStateException("Unexpected wrap result " + wrapResultStatus);
}
// fall-through default case to flush()
}
default:
{
if (LOG.isDebugEnabled())