diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java index 648ec687a26..60662741ad8 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java +++ b/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); } 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 7c004311387..8d234ce64f4 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 @@ -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("{} filled {}",SslConnection.this,BufferUtil.toHexSummary(buffer)); + LOG.debug("net={} unwrap {} {}", net_filled, unwrapResult.toString().replace('\n',' '), SslConnection.this); + 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) - { - _handshaken = true; - if (LOG.isDebugEnabled()) - LOG.debug("{} {} handshake succeeded {}/{}", SslConnection.this, - _sslEngine.getUseClientMode() ? "client" : "resumed server", - _sslEngine.getSession().getProtocol(),_sslEngine.getSession().getCipherSuite()); - notifyHandshakeSucceeded(_sslEngine); - } + if (unwrapHandshakeStatus == HandshakeStatus.FINISHED) + handshakeFinished(); - // Check whether renegotiation is allowed - if (_handshaken && handshakeStatus != HandshakeStatus.NOT_HANDSHAKING && !isRenegotiationAllowed()) - { - if (LOG.isDebugEnabled()) - LOG.debug("{} renegotiation denied", SslConnection.this); - closeInbound(); + // Check whether re-negotiation is allowed + if (!allowRenegotiate(handshakeStatus)) 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) - { - _handshaken = true; - if (LOG.isDebugEnabled()) - LOG.debug("{} {} handshake succeeded {}/{}", SslConnection.this, - _sslEngine.getUseClientMode() ? "resumed client" : "server", - _sslEngine.getSession().getProtocol(),_sslEngine.getSession().getCipherSuite()); - notifyHandshakeSucceeded(_sslEngine); - } + if (wrapResult.getHandshakeStatus() == HandshakeStatus.FINISHED) + handshakeFinished(); HandshakeStatus handshakeStatus = _sslEngine.getHandshakeStatus(); - // Check whether renegotiation is allowed - if (_handshaken && handshakeStatus != HandshakeStatus.NOT_HANDSHAKING && !isRenegotiationAllowed()) + // Check whether re-negotiation is allowed + 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; diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java index 9d9627f5ea6..a4da692b838 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java +++ b/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; diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java index 9b45451e0ca..3c322fabc6a 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java +++ b/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 diff --git a/jetty-server/src/main/config/etc/jetty-ssl-context.xml b/jetty-server/src/main/config/etc/jetty-ssl-context.xml index 1b266d1c744..3e84a9e0e0c 100644 --- a/jetty-server/src/main/config/etc/jetty-ssl-context.xml +++ b/jetty-server/src/main/config/etc/jetty-ssl-context.xml @@ -26,4 +26,6 @@ + + diff --git a/jetty-server/src/main/config/modules/ssl.mod b/jetty-server/src/main/config/modules/ssl.mod index e8edec6d7d1..3efb90e3b93 100644 --- a/jetty-server/src/main/config/modules/ssl.mod +++ b/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 diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SslConnectionFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SslConnectionFactory.java index 7a8f0c66a9e..cdb63609022 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/SslConnectionFactory.java +++ b/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); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index cf008b12338..979cff5c3da 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/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 */