diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index fe4de6bfe34..2aa21e9fba6 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -18,19 +18,45 @@ package org.eclipse.jetty.client; +import java.io.OutputStream; +import java.net.Socket; +import java.nio.ByteBuffer; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLException; +import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.client.api.Result; +import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; +import org.eclipse.jetty.client.http.HttpDestinationOverHTTP; 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.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.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; @@ -180,4 +206,133 @@ public class HttpClientTLSTest // Expected. } } + + @Test + public void testTLSLargeFragments() throws Exception + { + final 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; + + final CountDownLatch clientLatch = new CountDownLatch(1); + final SslContextFactory clientTLSFactory = createSslContextFactory(); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client = new HttpClient( + new HttpClientTransportOverHTTP() + { + @Override + public HttpDestination newHttpDestination(Origin origin) + { + return new HttpDestinationOverHTTP(getHttpClient(), origin) + { + @Override + protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory) + { + SslContextFactory sslContextFactory = getHttpClient().getSslContextFactory(); + ByteBufferPool bufferPool = getHttpClient().getByteBufferPool(); + Executor executor = getHttpClient().getExecutor(); + return new SslClientConnectionFactory(sslContextFactory, bufferPool, executor, 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); + } + } + }; + } + }; + } + }; + } + }, clientTLSFactory); + client.setIdleTimeout(idleTimeout); + client.setExecutor(clientThreads); + client.start(); + + String host = "localhost"; + int port = connector.getLocalPort(); + + final CountDownLatch responseLatch = new CountDownLatch(1); + client.newRequest(host, port) + .scheme(HttpScheme.HTTPS.asString()) + .send(new Response.CompleteListener() + { + public void onComplete(Result 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)); + } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 9cb3cf94bda..905ab7b592b 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -23,12 +23,12 @@ import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; import java.util.Arrays; import java.util.concurrent.Executor; - 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.SSLSession; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.AbstractEndPoint; @@ -143,6 +143,57 @@ public class SslConnection extends AbstractConnection this._renegotiationAllowed = renegotiationAllowed; } + private int getApplicationBufferSize() + { + SSLSession hsSession = _sslEngine.getHandshakeSession(); + SSLSession session = _sslEngine.getSession(); + int size = session.getApplicationBufferSize(); + if (hsSession == null || hsSession == session) + return size; + int hsSize = hsSession.getApplicationBufferSize(); + return Math.max(hsSize, size); + } + + private int getPacketBufferSize() + { + SSLSession hsSession = _sslEngine.getHandshakeSession(); + SSLSession session = _sslEngine.getSession(); + int size = session.getPacketBufferSize(); + if (hsSession == null || hsSession == session) + return size; + int hsSize = hsSession.getPacketBufferSize(); + 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() { @@ -230,6 +281,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() { @@ -471,9 +532,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); } @@ -500,18 +564,22 @@ 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); if (DEBUG) @@ -525,14 +593,19 @@ 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 (DEBUG) - LOG.debug("{} unwrap {}", SslConnection.this, unwrapResult); + if (LOG.isDebugEnabled()) + 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(); @@ -585,6 +658,19 @@ public class SslConnection extends AbstractConnection } } } + 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: { @@ -684,16 +770,9 @@ public class SslConnection extends AbstractConnection getExecutor().execute(_runCompletWrite); } - if (_encryptedInput != null && !_encryptedInput.hasRemaining()) - { - _bufferPool.release(_encryptedInput); - _encryptedInput = null; - } - if (_decryptedInput != null && !_decryptedInput.hasRemaining()) - { - _bufferPool.release(_decryptedInput); - _decryptedInput = null; - } + releaseEncryptedInputBuffer(); + releaseDecryptedInputBuffer(); + if (DEBUG) LOG.debug("{} fill exit", SslConnection.this); } @@ -745,19 +824,20 @@ 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 { @@ -802,6 +882,26 @@ public class SslConnection extends AbstractConnection case BUFFER_UNDERFLOW: 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 (DEBUG) LOG.debug("{} {} {}", this, wrapResultStatus, BufferUtil.toDetailString(_encryptedOutput)); diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketProtocolTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketProtocolTest.java index 5500495dcb8..63648a5894a 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketProtocolTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketProtocolTest.java @@ -40,7 +40,7 @@ public class WebSocketProtocolTest public void startServer() throws Exception { server = new BrowserDebugTool(); - server.prepare(0); + server.prepare(0, 0); server.start(); } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java index b0ae23f126d..a0b46bd4fa7 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java @@ -21,14 +21,19 @@ package org.eclipse.jetty.websocket.server.browser; import java.util.ArrayList; import java.util.List; +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.server.handler.ResourceHandler; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.common.extensions.FrameDebugExtension; -import org.eclipse.jetty.websocket.common.extensions.compress.PerMessageDeflateExtension; import org.eclipse.jetty.websocket.server.WebSocketHandler; import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest; import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse; @@ -44,10 +49,12 @@ import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory; public class BrowserDebugTool implements WebSocketCreator { private static final Logger LOG = Log.getLogger(BrowserDebugTool.class); + private ServerConnector secureConnector; public static void main(String[] args) { int port = 8080; + int securePort = 8443; for (int i = 0; i < args.length; i++) { @@ -56,12 +63,17 @@ public class BrowserDebugTool implements WebSocketCreator { port = Integer.parseInt(args[++i]); } + + if ("-sP".equals(a) || "--securePort".equals(a)) + { + securePort = Integer.parseInt(args[++i]); + } } try { BrowserDebugTool tool = new BrowserDebugTool(); - tool.prepare(port); + tool.prepare(port, securePort); tool.start(); } catch (Throwable t) @@ -119,13 +131,39 @@ public class BrowserDebugTool implements WebSocketCreator return connector.getLocalPort(); } - public void prepare(int port) + public int getSecurePort() + { + return secureConnector.getLocalPort(); + } + + public void prepare(int port, int securePort) { server = new Server(); - connector = new ServerConnector(server); + + HttpConfiguration httpConfiguration = new HttpConfiguration(); + httpConfiguration.setSecureScheme("https"); + httpConfiguration.setSecurePort(securePort); + + connector = new ServerConnector(server, new HttpConnectionFactory(httpConfiguration)); connector.setPort(port); server.addConnector(connector); + SslContextFactory sslContextFactory = new SslContextFactory(); + sslContextFactory.setKeyStorePath(MavenTestingUtils.getTestResourceFile("keystore").getAbsolutePath()); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setKeyManagerPassword("keypwd"); + + // SSL HTTP Configuration + HttpConfiguration httpsConfiguration = new HttpConfiguration(httpConfiguration); + httpsConfiguration.addCustomizer(new SecureRequestCustomizer()); + + // SSL Connector + secureConnector = new ServerConnector(server, + new SslConnectionFactory(sslContextFactory,"http/1.1"), + new HttpConnectionFactory(httpsConfiguration)); + secureConnector.setPort(securePort); + server.addConnector(secureConnector); + WebSocketHandler wsHandler = new WebSocketHandler() { @Override @@ -162,7 +200,8 @@ public class BrowserDebugTool implements WebSocketCreator public void start() throws Exception { server.start(); - LOG.info("Server available on port {}",getPort()); + LOG.info("Server available on port {}", getPort()); + LOG.info("Server available on secure (TLS) port {}", getSecurePort()); } public void stop() throws Exception