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 d75718c4c1f6..4d8d6f685d9b 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 @@ -807,4 +807,120 @@ protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuff Throwable cause = failure.getCause(); assertThat(cause, Matchers.instanceOf(SSLHandshakeException.class)); } + + @Test + public void testTLSLargeFragments() throws Exception + { + CountDownLatch serverLatch = new CountDownLatch(1); + SslContextFactory serverTLSFactory = createServerSslContextFactory(); + 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 = 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) + { + 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)); + } } 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 9763ba7b98af..bb202a8f72b5 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 @@ -25,12 +25,14 @@ import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; +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; @@ -308,10 +310,37 @@ private boolean isHandshakeComplete() return state == HandshakeState.SUCCEEDED || state == HandshakeState.FAILED; } + private int getApplicationBufferSize() + { + return getBufferSize(SSLSession::getApplicationBufferSize); + } + + private int getPacketBufferSize() + { + return getBufferSize(SSLSession::getPacketBufferSize); + } + + private int getBufferSize(ToIntFunction 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(_sslEngine.getSession().getPacketBufferSize(), _encryptedDirectBuffers); + _encryptedInput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers); + } + + private void acquireEncryptedOutput() + { + if (_encryptedOutput == null) + _encryptedOutput = _bufferPool.acquire(getPacketBufferSize(), _encryptedDirectBuffers); } @Override @@ -409,6 +438,24 @@ public String toConnectionString() connection instanceof AbstractConnection ? ((AbstractConnection)connection).toConnectionString() : connection); } + 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; + } + } + private void releaseEncryptedOutputBuffer() { if (!Thread.holdsLock(_decryptedEndPoint)) @@ -544,9 +591,12 @@ public void setConnection(Connection connection) { 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); } @@ -613,12 +663,13 @@ public int fill(ByteBuffer buffer) throws IOException // can we use the passed buffer if it is big enough ByteBuffer appIn; + int appBufferSize = getApplicationBufferSize(); if (_decryptedInput == null) { - if (BufferUtil.space(buffer) > _sslEngine.getSession().getApplicationBufferSize()) + if (BufferUtil.space(buffer) > appBufferSize) appIn = buffer; else - appIn = _decryptedInput = _bufferPool.acquire(_sslEngine.getSession().getApplicationBufferSize(), _decryptedDirectBuffers); + appIn = _decryptedInput = _bufferPool.acquire(appBufferSize, _decryptedDirectBuffers); } else { @@ -698,8 +749,21 @@ public int fill(ByteBuffer buffer) throws IOException } return filled = netFilled; + 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(); + continue; + } + throw new IllegalStateException("Unexpected unwrap result " + unwrap); + case OK: - { if (unwrapResult.getHandshakeStatus() == HandshakeStatus.FINISHED) handshakeSucceeded(); @@ -717,7 +781,6 @@ public int fill(ByteBuffer buffer) throws IOException } break; - } default: throw new IllegalStateException("Unexpected unwrap result " + unwrap); @@ -737,17 +800,8 @@ public int fill(ByteBuffer buffer) throws IOException } finally { - if (_encryptedInput != null && !_encryptedInput.hasRemaining()) - { - _bufferPool.release(_encryptedInput); - _encryptedInput = null; - } - - if (_decryptedInput != null && !_decryptedInput.hasRemaining()) - { - _bufferPool.release(_decryptedInput); - _decryptedInput = null; - } + releaseEncryptedInputBuffer(); + releaseDecryptedInputBuffer(); if (_flushState == FlushState.WAIT_FOR_FILL) { @@ -974,8 +1028,8 @@ public boolean flush(ByteBuffer... appOuts) throws IOException throw new IllegalStateException("Unexpected HandshakeStatus " + status); } - if (_encryptedOutput == null) - _encryptedOutput = _bufferPool.acquire(_sslEngine.getSession().getPacketBufferSize(), _encryptedDirectBuffers); + int packetBufferSize = getPacketBufferSize(); + acquireEncryptedOutput(); if (_handshake.compareAndSet(HandshakeState.INITIAL, HandshakeState.HANDSHAKE)) { @@ -983,7 +1037,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; @@ -1032,7 +1087,18 @@ public boolean flush(ByteBuffer... appOuts) throws IOException case BUFFER_OVERFLOW: if (!flushed) return result = false; - continue; + // 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; + } + throw new IllegalStateException("Unexpected wrap result " + wrap); case OK: if (wrapResult.getHandshakeStatus() == HandshakeStatus.FINISHED)