Merge branch 'jetty-9.2.x' of github.com:eclipse/jetty.project into jetty-9.2.x

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));
+    }
 }

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;
+                // This is an optimization to avoid that upper layer connections use small
-                if (a.getInputBufferSize()<_sslEngine.getSession().getApplicationBufferSize())
+                // buffers and we need to copy decrypted data rather than decrypting in place.
-                    a.setInputBufferSize(_sslEngine.getSession().getApplicationBufferSize());
+                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)
+                        if (LOG.isDebugEnabled())
-                            LOG.debug("{} unwrap {}", SslConnection.this, unwrapResult);
+                            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())
+                releaseEncryptedInputBuffer();
-                {
+                releaseDecryptedInputBuffer();
-                    _bufferPool.release(_encryptedInput);
+
-                    _encryptedInput = null;
-                }
-                if (_decryptedInput != null && !_decryptedInput.hasRemaining())
-                {
-                    _bufferPool.release(_decryptedInput);
-                    _decryptedInput = null;
-                }
                 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));

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();
     }