Merged branch 'jetty-9.3.x' into 'jetty-9.4.x'.

jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java

@@ -59,6 +59,7 @@ import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.MappedByteBufferPool;
+import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 import org.eclipse.jetty.util.Fields;
 import org.eclipse.jetty.util.Jetty;
 import org.eclipse.jetty.util.Promise;
@@ -1074,6 +1075,11 @@ public class HttpClient extends ContainerLifeCycle
         return HttpScheme.HTTPS.is(scheme) || HttpScheme.WSS.is(scheme);
     }
 
+    protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
+    {
+        return new SslClientConnectionFactory(getSslContextFactory(), getByteBufferPool(), getExecutor(), connectionFactory);
+    }
+
     private class ContentDecoderFactorySet implements Set<ContentDecoder.Factory>
     {
         private final Set<ContentDecoder.Factory> set = new HashSet<>();

jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java

@@ -34,7 +34,6 @@ import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.io.ClientConnectionFactory;
-import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.HostPort;
@@ -124,7 +123,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
 
     protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
     {
-        return new SslClientConnectionFactory(client.getSslContextFactory(), client.getByteBufferPool(), client.getExecutor(), connectionFactory);
+        return client.newSslClientConnectionFactory(connectionFactory);
     }
 
     public boolean isSecure()

jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java

@@ -34,7 +34,6 @@ import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.EndPoint;
-import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 import org.eclipse.jetty.util.Promise;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
@@ -204,8 +203,7 @@ public class HttpProxy extends ProxyConfiguration.Proxy
                 context.put(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY, promise);
                 HttpDestination destination = (HttpDestination)context.get(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY);
                 HttpClient client = destination.getHttpClient();
-                ClientConnectionFactory sslConnectionFactory =
+                ClientConnectionFactory sslConnectionFactory = client.newSslClientConnectionFactory(connectionFactory);
-                        new SslClientConnectionFactory(client.getSslContextFactory(), client.getByteBufferPool(), client.getExecutor(), connectionFactory);
                 HttpConnectionOverHTTP oldConnection = (HttpConnectionOverHTTP)endPoint.getConnection();
                 org.eclipse.jetty.io.Connection newConnection = sslConnectionFactory.newConnection(endPoint, context);
                 // Creating the connection will link the new Connection the EndPoint,

jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java

@@ -30,7 +30,6 @@ import org.eclipse.jetty.client.api.Connection;
 import org.eclipse.jetty.io.AbstractConnection;
 import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.EndPoint;
-import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.Promise;
@@ -197,7 +196,7 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy
                 HttpClient client = destination.getHttpClient();
                 ClientConnectionFactory connectionFactory = this.connectionFactory;
                 if (destination.isSecure())
-                    connectionFactory = new SslClientConnectionFactory(client.getSslContextFactory(), client.getByteBufferPool(), client.getExecutor(), connectionFactory);
+                    connectionFactory = client.newSslClientConnectionFactory(connectionFactory);
                 org.eclipse.jetty.io.Connection newConnection = connectionFactory.newConnection(getEndPoint(), context);
                 getEndPoint().upgrade(newConnection);
                 if (LOG.isDebugEnabled())

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java

@@ -18,22 +18,34 @@
 
 package org.eclipse.jetty.client;
 
+import java.io.BufferedReader;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLSocket;
+
 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.ClientConnectionFactory;
+import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
 import org.eclipse.jetty.io.ssl.SslHandshakeListener;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
+import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
@@ -390,4 +402,116 @@ public class HttpClientTLSTest
         Assert.assertTrue(serverLatch.await(1, TimeUnit.SECONDS));
         Assert.assertTrue(clientLatch.await(1, TimeUnit.SECONDS));
     }
+
+    @Test
+    public void testClientRawCloseDoesNotInvalidateSession() throws Exception
+    {
+        SslContextFactory serverTLSFactory = createSslContextFactory();
+        startServer(serverTLSFactory, new EmptyServerHandler());
+
+        SslContextFactory clientTLSFactory = createSslContextFactory();
+        clientTLSFactory.start();
+
+        String host = "localhost";
+        int port = connector.getLocalPort();
+        Socket socket = new Socket(host, port);
+        SSLSocket sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true);
+        CountDownLatch handshakeLatch1 = new CountDownLatch(1);
+        AtomicReference<byte[]> session1 = new AtomicReference<>();
+        sslSocket.addHandshakeCompletedListener(event ->
+        {
+            session1.set(event.getSession().getId());
+            handshakeLatch1.countDown();
+        });
+        sslSocket.startHandshake();
+        Assert.assertTrue(handshakeLatch1.await(5, TimeUnit.SECONDS));
+
+        // The client closes abruptly.
+        socket.close();
+
+        // Try again and compare the session ids.
+        socket = new Socket(host, port);
+        sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true);
+        CountDownLatch handshakeLatch2 = new CountDownLatch(1);
+        AtomicReference<byte[]> session2 = new AtomicReference<>();
+        sslSocket.addHandshakeCompletedListener(event ->
+        {
+            session2.set(event.getSession().getId());
+            handshakeLatch2.countDown();
+        });
+        sslSocket.startHandshake();
+        Assert.assertTrue(handshakeLatch2.await(5, TimeUnit.SECONDS));
+
+        Assert.assertArrayEquals(session1.get(), session2.get());
+
+        sslSocket.close();
+    }
+
+    @Test
+    public void testServerRawCloseDetectedByClient() throws Exception
+    {
+        SslContextFactory serverTLSFactory = createSslContextFactory();
+        serverTLSFactory.start();
+        try (ServerSocket server = new ServerSocket(0))
+        {
+            QueuedThreadPool clientThreads = new QueuedThreadPool();
+            clientThreads.setName("client");
+            client = new HttpClient(createSslContextFactory())
+            {
+                @Override
+                protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
+                {
+                    SslClientConnectionFactory ssl = (SslClientConnectionFactory)super.newSslClientConnectionFactory(connectionFactory);
+                    ssl.setAllowMissingCloseMessage(false);
+                    return ssl;
+                }
+            };
+            client.setExecutor(clientThreads);
+            client.start();
+
+            CountDownLatch latch = new CountDownLatch(1);
+            client.newRequest("localhost", server.getLocalPort())
+                    .scheme(HttpScheme.HTTPS.asString())
+                    .send(result ->
+                    {
+                        Assert.assertThat(result.getResponseFailure(), Matchers.instanceOf(SSLException.class));
+                        latch.countDown();
+                    });
+
+            Socket socket = server.accept();
+            SSLSocket sslSocket = (SSLSocket)serverTLSFactory.getSslContext().getSocketFactory().createSocket(socket, null, socket.getPort(), true);
+            sslSocket.setUseClientMode(false);
+            BufferedReader reader = new BufferedReader(new InputStreamReader(sslSocket.getInputStream(), StandardCharsets.UTF_8));
+            while (true)
+            {
+                String line = reader.readLine();
+                if (line == null || line.isEmpty())
+                    break;
+            }
+
+            // If the response is Content-Length delimited, allowing the
+            // missing TLS Close Message is fine because the application
+            // will see a EOFException anyway.
+            // If the response is connection delimited, allowing the
+            // missing TLS Close Message is bad because the application
+            // will see a successful response with truncated content.
+
+            // Verify that by not allowing the missing
+            // TLS Close Message we get a response failure.
+
+            byte[] half = new byte[8];
+            String response = "HTTP/1.1 200 OK\r\n" +
+//                    "Content-Length: " + (half.length * 2) + "\r\n" +
+                    "Connection: close\r\n" +
+                    "\r\n";
+            OutputStream output = sslSocket.getOutputStream();
+            output.write(response.getBytes(StandardCharsets.UTF_8));
+            output.write(half);
+            output.flush();
+            // Simulate a truncation attack by raw closing.
+            socket.close();
+
+            Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
+        }
+    }
 }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java

@@ -40,8 +40,6 @@ import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.client.api.Request;
 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.client.util.BufferingResponseListener;
 import org.eclipse.jetty.client.util.InputStreamContentProvider;
 import org.eclipse.jetty.io.ByteBufferPool;
@@ -252,18 +250,12 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
         start(new TimeoutHandler(2 * timeout));
         client.stop();
         final AtomicBoolean sslIdle = new AtomicBoolean();
-        client = new HttpClient(new HttpClientTransportOverHTTP()
+        client = new HttpClient(sslContextFactory)
         {
             @Override
-            public HttpDestination newHttpDestination(Origin origin)
+            public ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
             {
-                return new HttpDestinationOverHTTP(getHttpClient(), origin)
+                return new SslClientConnectionFactory(getSslContextFactory(), getByteBufferPool(), getExecutor(), connectionFactory)
-                {
-                    @Override
-                    protected ClientConnectionFactory newSslClientConnectionFactory(ClientConnectionFactory connectionFactory)
-                    {
-                        HttpClient client = getHttpClient();
-                        return new SslClientConnectionFactory(client.getSslContextFactory(), client.getByteBufferPool(), client.getExecutor(), connectionFactory)
                 {
                     @Override
                     protected SslConnection newSslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine engine)
@@ -281,8 +273,6 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest
                 };
             }
         };
-            }
-        }, sslContextFactory);
         client.setIdleTimeout(timeout);
         client.start();
 

jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java

@@ -150,7 +150,7 @@ public class HTTP2Client extends ContainerLifeCycle
                 if (sslContextFactory != null)
                 {
                     ALPNClientConnectionFactory alpn = new ALPNClientConnectionFactory(getExecutor(), h2, getProtocols());
-                    factory = new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), alpn);
+                    factory = newSslClientConnectionFactory(sslContextFactory, alpn);
                 }
                 return factory.newConnection(endPoint, context);
             });
@@ -171,6 +171,11 @@ public class HTTP2Client extends ContainerLifeCycle
         return new ClientSelectorManager(getExecutor(), getScheduler(), getSelectors());
     }
 
+    protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory)
+    {
+        return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory);
+    }
+
     public Executor getExecutor()
     {
         return executor;

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java

@@ -42,6 +42,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
     private final ByteBufferPool byteBufferPool;
     private final Executor executor;
     private final ClientConnectionFactory connectionFactory;
+    private boolean allowMissingCloseMessage = true;
 
     public SslClientConnectionFactory(SslContextFactory sslContextFactory, ByteBufferPool byteBufferPool, Executor executor, ClientConnectionFactory connectionFactory)
     {
@@ -51,6 +52,16 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
         this.connectionFactory = connectionFactory;
     }
 
+    public boolean isAllowMissingCloseMessage()
+    {
+        return allowMissingCloseMessage;
+    }
+
+    public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage)
+    {
+        this.allowMissingCloseMessage = allowMissingCloseMessage;
+    }
+
     @Override
     public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context) throws IOException
     {
@@ -84,6 +95,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
             SslConnection sslConnection = (SslConnection)connection;
             sslConnection.setRenegotiationAllowed(sslContextFactory.isRenegotiationAllowed());
             sslConnection.setRenegotiationLimit(sslContextFactory.getRenegotiationLimit());
+            sslConnection.setAllowMissingCloseMessage(isAllowMissingCloseMessage());
             ContainerLifeCycle connector = (ContainerLifeCycle)context.get(ClientConnectionFactory.CONNECTOR_CONTEXT_KEY);
             connector.getBeans(SslHandshakeListener.class).forEach(sslConnection::addHandshakeListener);
         }

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

@@ -93,6 +93,7 @@ public class SslConnection extends AbstractConnection
     private boolean _renegotiationAllowed;
     private int _renegotiationLimit = -1;
     private boolean _closedOutbound;
+    private boolean _allowMissingCloseMessage = true;
 
     private abstract class RunnableTask  implements Runnable, Invocable
     {
@@ -231,6 +232,16 @@ public class SslConnection extends AbstractConnection
         _renegotiationLimit = renegotiationLimit;
     }
 
+    public boolean isAllowMissingCloseMessage()
+    {
+        return _allowMissingCloseMessage;
+    }
+
+    public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage)
+    {
+        this._allowMissingCloseMessage = allowMissingCloseMessage;
+    }
+
     @Override
     public void onOpen()
     {
@@ -662,7 +673,7 @@ public class SslConnection extends AbstractConnection
 
                                 if (_underFlown)
                                 {
-                                    if (net_filled < 0)
+                                    if (net_filled < 0 && _sslEngine.getUseClientMode())
                                         closeInbound();
                                     if (net_filled <= 0)
                                         return net_filled;
@@ -860,29 +871,45 @@ public class SslConnection extends AbstractConnection
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("Renegotiation denied {}", SslConnection.this);
-                closeInbound();
+                terminateInput();
                 return false;
             }
             
-            if (_renegotiationLimit==0)
+            if (getRenegotiationLimit()==0)
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("Renegotiation limit exceeded {}", SslConnection.this);
-                closeInbound();
+                terminateInput();
                 return false;
             }
             
             return true;
         }
 
-        private void closeInbound()
+        private void terminateInput()
         {
+            try
+            {
+                _sslEngine.closeInbound();
+            }
+            catch (Throwable x)
+            {
+                LOG.ignore(x);
+            }
+        }
+
+        private void closeInbound() throws SSLException
+        {
+            HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
             try
             {
                 _sslEngine.closeInbound();
             }
             catch (SSLException x)
             {
+                if (handshakeStatus == HandshakeStatus.NOT_HANDSHAKING && !isAllowMissingCloseMessage())
+                    throw x;
+                else
                     LOG.ignore(x);
             }
         }