Merge remote-tracking branch 'origin/jetty-9.3.x' into jetty-9.4.x

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

@@ -83,6 +83,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
         {
             SslConnection sslConnection = (SslConnection)connection;
             sslConnection.setRenegotiationAllowed(sslContextFactory.isRenegotiationAllowed());
+            sslConnection.setRenegotiationLimit(sslContextFactory.getRenegotiationLimit());
             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

@@ -91,6 +91,7 @@ public class SslConnection extends AbstractConnection
     private final boolean _encryptedDirectBuffers = true;
     private final boolean _decryptedDirectBuffers = false;
     private boolean _renegotiationAllowed;
+    private int _renegotiationLimit = -1;
     private boolean _closedOutbound;
 
     private abstract class RunnableTask  implements Runnable, Invocable
@@ -208,7 +209,26 @@ public class SslConnection extends AbstractConnection
 
     public void setRenegotiationAllowed(boolean renegotiationAllowed)
     {
-        this._renegotiationAllowed = renegotiationAllowed;
+        _renegotiationAllowed = renegotiationAllowed;
+    }
+
+    /**
+     * @return The number of renegotions allowed for this connection.  When the limit
+     * is 0 renegotiation will be denied. If the limit is <0 then no limit is applied. 
+     */
+    public int getRenegotiationLimit()
+    {
+        return _renegotiationLimit;
+    }
+
+    /**
+     * @param renegotiationLimit The number of renegotions allowed for this connection.  
+     * When the limit is 0 renegotiation will be denied. If the limit is <0 then no limit is applied.
+     * Default -1.
+     */
+    public void setRenegotiationLimit(int renegotiationLimit)
+    {
+        _renegotiationLimit = renegotiationLimit;
     }
 
     @Override
@@ -357,7 +377,7 @@ public class SslConnection extends AbstractConnection
                 synchronized (DecryptedEndPoint.this)
                 {
                     if (LOG.isDebugEnabled())
-                        LOG.debug("{} write failed", SslConnection.this, x);
+                        LOG.debug("write failed {}", SslConnection.this, x);
 
                     BufferUtil.clear(_encryptedOutput);
                     releaseEncryptedOutputBuffer();
@@ -627,8 +647,8 @@ public class SslConnection extends AbstractConnection
                                 }
                                 if (LOG.isDebugEnabled())
                                 {
-                                    LOG.debug("{} net={} unwrap {}", SslConnection.this, net_filled, unwrapResult.toString().replace('\n',' '));
+                                    LOG.debug("net={} unwrap {} {}", net_filled, unwrapResult.toString().replace('\n',' '), SslConnection.this);
-                                    LOG.debug("{} filled {}",SslConnection.this,BufferUtil.toHexSummary(buffer));
+                                    LOG.debug("filled {} {}",BufferUtil.toHexSummary(buffer), SslConnection.this);
                                 }
 
                                 HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
@@ -686,25 +706,13 @@ public class SslConnection extends AbstractConnection
                                     case BUFFER_UNDERFLOW:
                                     case OK:
                                     {
-                                        if (unwrapHandshakeStatus == HandshakeStatus.FINISHED && !_handshaken)
+                                        if (unwrapHandshakeStatus == HandshakeStatus.FINISHED)
-                                        {
+                                            handshakeFinished();
-                                            _handshaken = true;
-                                            if (LOG.isDebugEnabled())
-                                                LOG.debug("{} {} handshake succeeded {}/{}", SslConnection.this,
-                                                        _sslEngine.getUseClientMode() ? "client" : "resumed server",
-                                                        _sslEngine.getSession().getProtocol(),_sslEngine.getSession().getCipherSuite());
-                                            notifyHandshakeSucceeded(_sslEngine);
-                                        }
 
-                                        // Check whether renegotiation is allowed
+                                        // Check whether re-negotiation is allowed
-                                        if (_handshaken && handshakeStatus != HandshakeStatus.NOT_HANDSHAKING && !isRenegotiationAllowed())
+                                        if (!allowRenegotiate(handshakeStatus))
-                                        {
-                                            if (LOG.isDebugEnabled())
-                                                LOG.debug("{} renegotiation denied", SslConnection.this);
-                                            closeInbound();
                                             return -1;
-                                        }
+                                        
-
                                         // 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().
@@ -822,6 +830,51 @@ public class SslConnection extends AbstractConnection
             }
         }
 
+        private void handshakeFinished()
+        {
+            if (_handshaken)
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Renegotiated {}", SslConnection.this);
+                if (_renegotiationLimit>0)
+                    _renegotiationLimit--;
+            }
+            else
+            {
+                _handshaken = true;
+                if (LOG.isDebugEnabled())
+                    LOG.debug("{} handshake succeeded {}/{} {}",
+                        _sslEngine.getUseClientMode() ? "client" : "resumed server",
+                            _sslEngine.getSession().getProtocol(),_sslEngine.getSession().getCipherSuite(),
+                            SslConnection.this);
+                notifyHandshakeSucceeded(_sslEngine);
+            }
+        }
+
+        private boolean allowRenegotiate(HandshakeStatus handshakeStatus)
+        {   
+            if (!_handshaken || handshakeStatus == HandshakeStatus.NOT_HANDSHAKING)
+                return true;
+
+            if (!isRenegotiationAllowed())
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Renegotiation denied {}", SslConnection.this);
+                closeInbound();
+                return false;
+            }
+            
+            if (_renegotiationLimit==0)
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Renegotiation limit exceeded {}", SslConnection.this);
+                closeInbound();
+                return false;
+            }
+            
+            return true;
+        }
+
         private void closeInbound()
         {
             try
@@ -847,7 +900,7 @@ public class SslConnection extends AbstractConnection
             if (LOG.isDebugEnabled())
             {
                 for (ByteBuffer b : appOuts)
-                    LOG.debug("{} flush {}", SslConnection.this, BufferUtil.toHexSummary(b));
+                    LOG.debug("flush {} {}", BufferUtil.toHexSummary(b), SslConnection.this);
             }
 
             try
@@ -882,7 +935,7 @@ public class SslConnection extends AbstractConnection
                                 BufferUtil.flipToFlush(_encryptedOutput, pos);
                             }
                             if (LOG.isDebugEnabled())
-                                LOG.debug("{} wrap {}", SslConnection.this, wrapResult.toString().replace('\n',' '));
+                                LOG.debug("wrap {} {}", wrapResult.toString().replace('\n',' '), SslConnection.this);
 
                             Status wrapResultStatus = wrapResult.getStatus();
 
@@ -923,29 +976,20 @@ public class SslConnection extends AbstractConnection
                                 default:
                                 {
                                     if (LOG.isDebugEnabled())
-                                        LOG.debug("{} wrap {} {}", SslConnection.this, wrapResultStatus, BufferUtil.toHexSummary(_encryptedOutput));
+                                        LOG.debug("wrap {} {} {}", wrapResultStatus, BufferUtil.toHexSummary(_encryptedOutput), SslConnection.this);
 
-                                    if (wrapResult.getHandshakeStatus() == HandshakeStatus.FINISHED && !_handshaken)
+                                    if (wrapResult.getHandshakeStatus() == HandshakeStatus.FINISHED)
-                                    {
+                                        handshakeFinished();
-                                        _handshaken = true;
-                                        if (LOG.isDebugEnabled())
-                                            LOG.debug("{} {} handshake succeeded {}/{}", SslConnection.this,
-                                                    _sslEngine.getUseClientMode() ? "resumed client" : "server",
-                                                    _sslEngine.getSession().getProtocol(),_sslEngine.getSession().getCipherSuite());
-                                        notifyHandshakeSucceeded(_sslEngine);
-                                    }
 
                                     HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus();
 
-                                    // Check whether renegotiation is allowed
+                                    // Check whether re-negotiation is allowed
-                                    if (_handshaken && handshakeStatus != HandshakeStatus.NOT_HANDSHAKING && !isRenegotiationAllowed())
+                                    if (!allowRenegotiate(handshakeStatus))
                                     {
-                                        if (LOG.isDebugEnabled())
-                                            LOG.debug("{} renegotiation denied", SslConnection.this);
                                         getEndPoint().shutdownOutput();
                                         return allConsumed;
                                     }
-
+                                    
                                     // if we have net bytes, let's try to flush them
                                     if (BufferUtil.hasContent(_encryptedOutput))
                                         if (!getEndPoint().flush(_encryptedOutput))
@@ -1035,7 +1079,7 @@ public class SslConnection extends AbstractConnection
                     boolean ishut = isInputShutdown();
                     boolean oshut = isOutputShutdown();
                     if (LOG.isDebugEnabled())
-                        LOG.debug("{} shutdownOutput: oshut={}, ishut={}", SslConnection.this, oshut, ishut);
+                        LOG.debug("shutdownOutput: oshut={}, ishut={} {}", oshut, ishut, SslConnection.this);
 
                     if (oshut)
                         return;

jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java

@@ -78,6 +78,7 @@ public class SelectChannelEndPointSslTest extends SelectChannelEndPointTest
         engine.setUseClientMode(false);
         SslConnection sslConnection = new SslConnection(__byteBufferPool, _threadPool, endpoint, engine);
         sslConnection.setRenegotiationAllowed(__sslCtxFactory.isRenegotiationAllowed());
+        sslConnection.setRenegotiationLimit(__sslCtxFactory.getRenegotiationLimit());
         Connection appConnection = super.newConnection(channel,sslConnection.getDecryptedEndPoint());
         sslConnection.getDecryptedEndPoint().setConnection(appConnection);
         return sslConnection;

jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java

@@ -34,6 +34,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSocket;
 
 import org.eclipse.jetty.io.ssl.SslConnection;
@@ -82,6 +83,7 @@ public class SslConnectionTest
             engine.setUseClientMode(false);
             SslConnection sslConnection = new SslConnection(__byteBufferPool, getExecutor(), endpoint, engine);
             sslConnection.setRenegotiationAllowed(__sslCtxFactory.isRenegotiationAllowed());
+            sslConnection.setRenegotiationLimit(__sslCtxFactory.getRenegotiationLimit());
             Connection appConnection = new TestConnection(sslConnection.getDecryptedEndPoint());
             sslConnection.getDecryptedEndPoint().setConnection(appConnection);
             return sslConnection;
@@ -151,6 +153,8 @@ public class SslConnectionTest
         _threadPool.start();
         _scheduler.start();
         _manager.start();
+        __sslCtxFactory.setRenegotiationAllowed(true);
+        __sslCtxFactory.setRenegotiationLimit(-1);
 
     }
 
@@ -250,7 +254,7 @@ public class SslConnectionTest
             }
         }
     }
-    protected Socket newClient() throws IOException
+    protected SSLSocket newClient() throws IOException
     {
         SSLSocket socket = __sslCtxFactory.newSslSocket();
         socket.connect(_connector.socket().getLocalSocketAddress());
@@ -282,6 +286,112 @@ public class SslConnectionTest
         client.close();
     }
 
+    @Test
+    public void testRenegotiate() throws Exception
+    {
+        SSLSocket client = newClient();
+        client.setSoTimeout(60000);
+
+        SocketChannel server = _connector.accept();
+        server.configureBlocking(false);
+        _manager.accept(server);
+
+        client.getOutputStream().write("Hello".getBytes(StandardCharsets.UTF_8));
+        byte[] buffer = new byte[1024];
+        int len=client.getInputStream().read(buffer);
+        Assert.assertEquals(5, len);
+        Assert.assertEquals("Hello",new String(buffer,0,len,StandardCharsets.UTF_8));
+
+        client.startHandshake();
+        
+        client.getOutputStream().write("World".getBytes(StandardCharsets.UTF_8));
+        len=client.getInputStream().read(buffer);
+        Assert.assertEquals(5, len);
+        Assert.assertEquals("World",new String(buffer,0,len,StandardCharsets.UTF_8));
+
+        client.close();
+    }
+
+    @Test
+    public void testRenegotiateNotAllowed() throws Exception
+    {
+        __sslCtxFactory.setRenegotiationAllowed(false);
+        
+        SSLSocket client = newClient();
+        client.setSoTimeout(60000);
+
+        SocketChannel server = _connector.accept();
+        server.configureBlocking(false);
+        _manager.accept(server);
+
+        client.getOutputStream().write("Hello".getBytes(StandardCharsets.UTF_8));
+        byte[] buffer = new byte[1024];
+        int len=client.getInputStream().read(buffer);
+        Assert.assertEquals(5, len);
+        Assert.assertEquals("Hello",new String(buffer,0,len,StandardCharsets.UTF_8));
+
+        client.startHandshake();
+        
+        client.getOutputStream().write("World".getBytes(StandardCharsets.UTF_8));
+        try
+        {
+            client.getInputStream().read(buffer);
+            Assert.fail();
+        }
+        catch(SSLException e)
+        {
+            // expected
+        }
+    }
+
+    @Test
+    public void testRenegotiateLimit() throws Exception
+    {
+        __sslCtxFactory.setRenegotiationAllowed(true);
+        __sslCtxFactory.setRenegotiationLimit(2);
+        
+        SSLSocket client = newClient();
+        client.setSoTimeout(60000);
+
+        SocketChannel server = _connector.accept();
+        server.configureBlocking(false);
+        _manager.accept(server);
+
+        client.getOutputStream().write("Good".getBytes(StandardCharsets.UTF_8));
+        byte[] buffer = new byte[1024];
+        int len=client.getInputStream().read(buffer);
+        Assert.assertEquals(4, len);
+        Assert.assertEquals("Good",new String(buffer,0,len,StandardCharsets.UTF_8));
+        
+        client.startHandshake();
+
+        client.getOutputStream().write("Bye".getBytes(StandardCharsets.UTF_8));
+        len=client.getInputStream().read(buffer);
+        Assert.assertEquals(3, len);
+        Assert.assertEquals("Bye",new String(buffer,0,len,StandardCharsets.UTF_8));
+        
+        client.startHandshake();
+
+        client.getOutputStream().write("Cruel".getBytes(StandardCharsets.UTF_8));
+        len=client.getInputStream().read(buffer);
+        Assert.assertEquals(5, len);
+        Assert.assertEquals("Cruel",new String(buffer,0,len,StandardCharsets.UTF_8));
+
+        client.startHandshake();
+        
+        client.getOutputStream().write("World".getBytes(StandardCharsets.UTF_8));
+        try
+        {
+            client.getInputStream().read(buffer);
+            Assert.fail();
+        }
+        catch(SSLException e)
+        {
+            // expected
+        }
+    }
+    
+    
 
     @Test
     public void testWriteOnConnect() throws Exception

jetty-server/src/main/config/etc/jetty-ssl-context.xml

@@ -26,4 +26,6 @@
   <Set name="useCipherSuitesOrder"><Property name="jetty.sslContext.useCipherSuitesOrder" default="true"/></Set>
   <Set name="sslSessionCacheSize"><Property name="jetty.sslContext.sslSessionCacheSize" default="-1"/></Set>
   <Set name="sslSessionTimeout"><Property name="jetty.sslContext.sslSessionTimeout" default="-1"/></Set>
+  <Set name="RenegotiationAllowed"><Property name="jetty.sslContext.renegotiationAllowed" default="true"/></Set>
+  <Set name="RenegotiationLimit"><Property name="jetty.sslContext.renegotiationLimit" default="5"/></Set>
 </Configure>

jetty-server/src/main/config/modules/ssl.mod

@@ -97,3 +97,7 @@ basehome:modules/ssl/keystore|etc/keystore
 
 ## Set the timeout (in seconds) of the SslSession cache timeout
 # jetty.sslContext.sslSessionTimeout=-1
+
+## Allow SSL renegotiation
+# jetty.sslContext.renegotiationAllowed=true
+# jetty.sslContext.renegotiationLimit=5

jetty-server/src/main/java/org/eclipse/jetty/server/SslConnectionFactory.java

@@ -87,6 +87,7 @@ public class SslConnectionFactory extends AbstractConnectionFactory
 
         SslConnection sslConnection = newSslConnection(connector, endPoint, engine);
         sslConnection.setRenegotiationAllowed(_sslContextFactory.isRenegotiationAllowed());
+        sslConnection.setRenegotiationLimit(_sslContextFactory.getRenegotiationLimit());
         configure(sslConnection, connector, endPoint);
 
         ConnectionFactory next = connector.getConnectionFactory(_nextProtocol);

jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java

@@ -165,6 +165,7 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable
     private String _endpointIdentificationAlgorithm = null;
     private boolean _trustAll;
     private boolean _renegotiationAllowed = true;
+    private int _renegotiationLimit = 5;
     private Factory _factory;
 
     /**
@@ -911,6 +912,25 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable
         _renegotiationAllowed = renegotiationAllowed;
     }
 
+    /**
+     * @return The number of renegotions allowed for this connection.  When the limit
+     * is 0 renegotiation will be denied. If the limit is <0 then no limit is applied. 
+     */
+    public int getRenegotiationLimit()
+    {
+        return _renegotiationLimit;
+    }
+
+    /**
+     * @param renegotiationLimit The number of renegotions allowed for this connection.  
+     * When the limit is 0 renegotiation will be denied. If the limit is <0 then no limit is applied.
+     * Default 5.
+     */
+    public void setRenegotiationLimit(int renegotiationLimit)
+    {
+        _renegotiationLimit = renegotiationLimit;
+    }
+    
     /**
      * @return Path to file that contains Certificate Revocation List
      */