Merged branch 'master' into 'jetty-9.1'.

jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java

@@ -88,6 +88,7 @@ public class SslBytesServerTest extends SslBytesTest
     private ExecutorService threadPool;
     private Server server;
     private SslContextFactory sslContextFactory;
+    private int serverPort;
     private SSLContext sslContext;
     private SimpleProxy proxy;
     private Runnable idleHook;
@@ -215,7 +216,7 @@ public class SslBytesServerTest extends SslBytesTest
             }
         });
         server.start();
-        int serverPort = connector.getLocalPort();
+        serverPort = connector.getLocalPort();
 
         sslContext = sslContextFactory.getSslContext();
 
@@ -296,6 +297,88 @@ public class SslBytesServerTest extends SslBytesTest
         closeClient(client);
     }
 
+    @Test
+    public void testHandshakeWithResumedSessionThenClose() throws Exception
+    {
+        // First socket will establish the SSL session
+        SSLSocket client1 = newClient();
+        SimpleProxy.AutomaticFlow automaticProxyFlow = proxy.startAutomaticFlow();
+        client1.startHandshake();
+        client1.close();
+        Assert.assertTrue(automaticProxyFlow.stop(5, TimeUnit.SECONDS));
+        int proxyPort = proxy.getPort();
+        proxy.stop();
+
+        proxy = new SimpleProxy(threadPool, proxyPort, "localhost", serverPort);
+        proxy.start();
+        logger.info("proxy:{} <==> server:{}", proxy.getPort(), serverPort);
+
+        final SSLSocket client2 = newClient(proxy);
+
+        Future<Object> handshake = threadPool.submit(new Callable<Object>()
+        {
+            @Override
+            public Object call() throws Exception
+            {
+                client2.startHandshake();
+                return null;
+            }
+        });
+
+        // Client Hello with SessionID
+        TLSRecord record = proxy.readFromClient();
+        Assert.assertNotNull(record);
+        proxy.flushToServer(record);
+
+        // Server Hello
+        record = proxy.readFromServer();
+        Assert.assertNotNull(record);
+        proxy.flushToClient(record);
+
+        // Change Cipher Spec
+        record = proxy.readFromServer();
+        Assert.assertNotNull(record);
+        proxy.flushToClient(record);
+
+        // Server Done
+        record = proxy.readFromServer();
+        Assert.assertNotNull(record);
+        proxy.flushToClient(record);
+
+        // Client Key Exchange
+        record = proxy.readFromClient();
+        Assert.assertNotNull(record);
+        // Client Done
+        TLSRecord doneRecord = proxy.readFromClient();
+        Assert.assertNotNull(doneRecord);
+        // Close
+        client2.close();
+        TLSRecord closeRecord = proxy.readFromClient();
+        Assert.assertNotNull(closeRecord);
+        Assert.assertEquals(TLSRecord.Type.ALERT, closeRecord.getType());
+        // Flush to server Client Key Exchange + Client Done + Close in one chunk
+        byte[] recordBytes = record.getBytes();
+        byte[] doneBytes = doneRecord.getBytes();
+        byte[] closeRecordBytes = closeRecord.getBytes();
+        byte[] chunk = new byte[recordBytes.length + doneBytes.length + closeRecordBytes.length];
+        System.arraycopy(recordBytes, 0, chunk, 0, recordBytes.length);
+        System.arraycopy(doneBytes, 0, chunk, recordBytes.length, doneBytes.length);
+        System.arraycopy(closeRecordBytes, 0, chunk, recordBytes.length + doneBytes.length, closeRecordBytes.length);
+        proxy.flushToServer(0, chunk);
+        // Close the raw socket
+        proxy.flushToServer(null);
+
+        // Expect the server to send a FIN as well
+        record = proxy.readFromServer();
+        Assert.assertNull(record);
+
+        // Check that we did not spin
+        TimeUnit.MILLISECONDS.sleep(500);
+        Assert.assertThat(sslFills.get(), Matchers.lessThan(20));
+        Assert.assertThat(sslFlushes.get(), Matchers.lessThan(20));
+        Assert.assertThat(httpParses.get(), Matchers.lessThan(20));
+    }
+
     @Test
     public void testHandshakeWithSplitBoundary() throws Exception
     {
@@ -1849,6 +1932,11 @@ public class SslBytesServerTest extends SslBytesTest
     }
 
     private SSLSocket newClient() throws IOException, InterruptedException
+    {
+        return newClient(proxy);
+    }
+
+    private SSLSocket newClient(SimpleProxy proxy) throws IOException, InterruptedException
     {
         SSLSocket client = (SSLSocket)sslContext.getSocketFactory().createSocket("localhost", proxy.getPort());
         client.setUseClientMode(true);

jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesTest.java

@@ -105,6 +105,7 @@ public abstract class SslBytesTest
     {
         private final CountDownLatch latch = new CountDownLatch(1);
         private final ExecutorService threadPool;
+        private final int proxyPort;
         private final String serverHost;
         private final int serverPort;
         private volatile ServerSocket serverSocket;
@@ -112,15 +113,21 @@ public abstract class SslBytesTest
         private volatile Socket client;
 
         public SimpleProxy(ExecutorService threadPool, String serverHost, int serverPort)
+        {
+            this(threadPool, 0, serverHost, serverPort);
+        }
+
+        public SimpleProxy(ExecutorService threadPool, int proxyPort, String serverHost, int serverPort)
         {
             this.threadPool = threadPool;
+            this.proxyPort = proxyPort;
             this.serverHost = serverHost;
             this.serverPort = serverPort;
         }
 
         public void start() throws Exception
         {
-            serverSocket = new ServerSocket(0);
+            serverSocket = new ServerSocket(proxyPort);
             Thread acceptor = new Thread(this);
             acceptor.start();
             server = new Socket(serverHost, serverPort);

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

@@ -193,7 +193,7 @@ public class SslConnection extends AbstractConnection
 
         // We have received a close handshake, close the end point to send FIN.
         if (_decryptedEndPoint.isInputShutdown())
-            getEndPoint().close();
+            _decryptedEndPoint.close();
 
         // wake up whoever is doing the fill or the flush so they can
         // do all the filling, unwrapping, wrapping and flushing
@@ -508,56 +508,69 @@ public class SslConnection extends AbstractConnection
                     int net_filled = getEndPoint().fill(_encryptedInput);
                     if (DEBUG)
                         LOG.debug("{} filled {} encrypted bytes", SslConnection.this, net_filled);
-                    if (net_filled > 0)
-                        _underFlown = false;
 
-                    // Let's try the SSL thang even if we have no net data because in that
+                    decryption: while (true)
+                    {
+                        // Let's unwrap even if we have no net data because in that
                         // case we want to fall through to the handshake handling
                         int pos = BufferUtil.flipToFill(app_in);
-
                         SSLEngineResult unwrapResult = _sslEngine.unwrap(_encryptedInput, app_in);
-
                         BufferUtil.flipToFlush(app_in, pos);
                         if (DEBUG)
                             LOG.debug("{} unwrap {}", SslConnection.this, unwrapResult);
 
-                    Status unwrapResultStatus = unwrapResult.getStatus();
-                    HandshakeStatus unwrapHandshakeStatus = unwrapResult.getHandshakeStatus();
                         HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
+                        HandshakeStatus unwrapHandshakeStatus = unwrapResult.getHandshakeStatus();
+                        Status unwrapResultStatus = unwrapResult.getStatus();
+
+                        _underFlown = unwrapResultStatus == Status.BUFFER_UNDERFLOW;
+
+                        if (_underFlown)
+                        {
+                            if (net_filled < 0)
+                                closeInbound();
+                            if (net_filled <= 0)
+                                return net_filled;
+                        }
 
-                    // and deal with the results
                         switch (unwrapResultStatus)
                         {
-                        case BUFFER_OVERFLOW:
-                            throw new IllegalStateException();
-
                             case CLOSED:
-                            // Dang! we have to care about the handshake state specially for close
+                            {
                                 switch (handshakeStatus)
                                 {
                                     case NOT_HANDSHAKING:
+                                    {
                                         // We were not handshaking, so just tell the app we are closed
                                         return -1;
-
+                                    }
                                     case NEED_TASK:
-                                    // run the task
+                                    {
                                         _sslEngine.getDelegatedTask().run();
                                         continue;
-
+                                    }
                                     case NEED_WRAP:
-                                    // we need to send some handshake data (probably to send a close handshake).
+                                    {
-                                    // but that will not enable any extra data to fill, so we just return -1
+                                        // We need to send some handshake data (probably the close handshake).
-                                    // The wrapping can be done by any output drivers doing flushing or shutdown output.
+                                        // We return -1 so that the application can drive the close by flushing
+                                        // or shutting down the output.
                                         return -1;
                                     }
-                            throw new IllegalStateException();
-
                                     default:
+                                    {
+                                        throw new IllegalStateException();
+                                    }
+                                }
+                            }
+                            case BUFFER_UNDERFLOW:
+                            case OK:
+                            {
                                 if (unwrapHandshakeStatus == HandshakeStatus.FINISHED && !_handshaken)
                                 {
                                     _handshaken = true;
                                     if (DEBUG)
-                                    LOG.debug("{} handshake completed client-side", SslConnection.this);
+                                        LOG.debug("{} {} handshake completed", SslConnection.this,
+                                                _sslEngine.getUseClientMode() ? "client-side" : "resumed session server-side");
                                 }
 
                                 // Check whether renegotiation is allowed
@@ -569,9 +582,6 @@ public class SslConnection extends AbstractConnection
                                     return -1;
                                 }
 
-                            if (unwrapResultStatus == Status.BUFFER_UNDERFLOW)
-                                _underFlown = true;
-
                                 // 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().
@@ -582,70 +592,60 @@ public class SslConnection extends AbstractConnection
                                     return BufferUtil.flipPutFlip(_decryptedInput, buffer);
                                 }
 
-                            // Dang! we have to care about the handshake state
                                 switch (handshakeStatus)
                                 {
                                     case NOT_HANDSHAKING:
-                                    // we just didn't read anything.
-                                    if (net_filled < 0)
                                     {
-                                        closeInbound();
+                                        if (_underFlown)
-                                        return -1;
+                                            break decryption;
+                                        continue;
                                     }
-                                    return 0;
-
                                     case NEED_TASK:
-                                    // run the task
+                                    {
                                         _sslEngine.getDelegatedTask().run();
                                         continue;
-
+                                    }
                                     case NEED_WRAP:
-                                    // we need to send some handshake data
+                                    {
-
+                                        // If we are called from flush()
-                                    // if we are called from flush
+                                        // return to let it do the wrapping.
                                         if (buffer == __FLUSH_CALLED_FILL)
-                                        return 0; // let it do the wrapping
+                                            return 0;
 
                                         _fillRequiresFlushToProgress = true;
                                         flush(__FILL_CALLED_FLUSH);
                                         if (BufferUtil.isEmpty(_encryptedOutput))
                                         {
-                                        // the flush completed so continue
+                                            // The flush wrote all the encrypted bytes so continue to fill
                                             _fillRequiresFlushToProgress = false;
                                             continue;
                                         }
-                                    return 0;
-
-                                case NEED_UNWRAP:
-                                    // if we just filled some net data
-                                    if (net_filled < 0)
-                                    {
-                                        closeInbound();
-                                        return -1;
-                                    }
-                                    else if (net_filled > 0)
-                                    {
-                                        // maybe we will fill some more on a retry
-                                        continue;
-                                    }
                                         else
                                         {
-                                        if (_encryptedInput.hasRemaining())
+                                            // The flush did not complete, return from fill()
-                                        {
+                                            // and let the write completion mechanism to kick in.
-                                            // if there are more encrypted bytes,
-                                            // then we need to unwrap more, we don't
-                                            // care if net_filled is zero
-                                            continue;
-                                        }
-                                        // we need to wait for more net data
                                             return 0;
                                         }
-
+                                    }
-                                case FINISHED:
+                                    case NEED_UNWRAP:
+                                    {
+                                        if (_underFlown)
+                                            break decryption;
+                                        continue;
+                                    }
+                                    default:
+                                    {
                                         throw new IllegalStateException();
                                     }
                                 }
                             }
+                            default:
+                            {
+                                throw new IllegalStateException();
+                            }
+                        }
+                    }
+                }
             }
             catch (SSLException e)
             {
@@ -778,7 +778,7 @@ public class SslConnection extends AbstractConnection
                             {
                                 _handshaken = true;
                                 if (DEBUG)
-                                    LOG.debug("{} handshake completed server-side", SslConnection.this);
+                                    LOG.debug("{} {} handshake completed", SslConnection.this, "server-side");
                             }
 
                             HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
@@ -824,7 +824,7 @@ public class SslConnection extends AbstractConnection
                                         if (handshakeStatus == HandshakeStatus.NEED_WRAP)
                                             continue;
                                     }
-                                    return allConsumed&&BufferUtil.isEmpty(_encryptedOutput);
+                                    return allConsumed && BufferUtil.isEmpty(_encryptedOutput);
 
                                 case FINISHED:
                                     throw new IllegalStateException();
@@ -860,17 +860,18 @@ public class SslConnection extends AbstractConnection
         public void shutdownOutput()
         {
             boolean ishut = isInputShutdown();
+            boolean oshut = isOutputShutdown();
             if (DEBUG)
-                LOG.debug("{} shutdownOutput: oshut={}, ishut={}", SslConnection.this, isOutputShutdown(), ishut);
+                LOG.debug("{} shutdownOutput: oshut={}, ishut={}", SslConnection.this, oshut, ishut);
             if (ishut)
             {
                 // Aggressively close, since inbound close alert has already been processed
                 // and the TLS specification allows to close the connection directly, which
                 // is what most other implementations expect: a FIN rather than a TLS close
-                // reply. If a TLS close reply is sent, most implementation send a RST.
+                // reply. If a TLS close reply is sent, most implementations send a RST.
                 getEndPoint().close();
             }
-            else
+            else if (!oshut)
             {
                 try
                 {
@@ -895,6 +896,8 @@ public class SslConnection extends AbstractConnection
         @Override
         public void close()
         {
+            // First send the TLS Close Alert, then the FIN
+            shutdownOutput();
             getEndPoint().close();
         }