diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslSelectChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslSelectChannelEndPoint.java index 858711e6f90..e04dc4af45d 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslSelectChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslSelectChannelEndPoint.java @@ -187,13 +187,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint return _engine!=null && _engine.isInboundDone(); } - /* ------------------------------------------------------------ */ - @Override - public void shutdownInput() throws IOException - { - // Nothing to do here. If the socket is open, let the SSL close handshake work, else the socket is closed anyway. - } - /* ------------------------------------------------------------ */ @Override public void shutdownOutput() throws IOException @@ -687,7 +680,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _inNIOBuffer.clear(); int total_filled=0; - + boolean remoteClosed = false; // loop filling as much encrypted data as we can into the buffer while (_inNIOBuffer.space()>0 && super.isOpen()) { @@ -695,8 +688,9 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint { int filled=super.fill(_inNIOBuffer); if (_debug) __log.debug(_session+" unwrap filled "+filled); - // break the loop if no progress is made (we have read everything - // there is to read). + if (filled < 0) + remoteClosed = true; + // break the loop if no progress is made (we have read everything there is to read) if (filled<=0) break; total_filled+=filled; @@ -719,7 +713,24 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint // If we have no progress and no data if (total_filled==0 && _inNIOBuffer.length()==0) { - if(!isOpen()) + if (isOpen()) + { + if (remoteClosed) + { + try + { + _engine.closeInbound(); + } + catch (SSLException x) + { + // It may happen, for example, in case of truncation + // attacks, we close so that we do not spin forever. + freeOutBuffer(); + super.close(); + } + } + } + else { freeOutBuffer(); throw new EofException(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslTruncationAttackTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslTruncationAttackTest.java new file mode 100644 index 00000000000..d97be2f00b5 --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslTruncationAttackTest.java @@ -0,0 +1,164 @@ +package org.eclipse.jetty.server.ssl; + +import java.io.IOException; +import java.net.Socket; +import java.nio.channels.SelectionKey; +import java.nio.channels.SocketChannel; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import javax.net.ssl.HandshakeCompletedEvent; +import javax.net.ssl.HandshakeCompletedListener; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.http.ssl.SslContextFactory; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.nio.SelectChannelEndPoint; +import org.eclipse.jetty.io.nio.SelectorManager; +import org.eclipse.jetty.io.nio.SslSelectChannelEndPoint; +import org.eclipse.jetty.server.HttpConnection; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class SslTruncationAttackTest +{ + private Server server; + private SslSelectChannelConnector connector; + private SSLContext sslContext; + private AtomicBoolean endPointClosed; + private AtomicLong handleCount; + + @Before + public void initServer() throws Exception + { + handleCount = new AtomicLong(); + endPointClosed = new AtomicBoolean(); + + server = new Server(); + connector = new SslSelectChannelConnector() + { + @Override + protected SelectChannelEndPoint newEndPoint(SocketChannel channel, SelectorManager.SelectSet selectSet, SelectionKey key) throws IOException + { + return new SslSelectChannelEndPoint(getSslBuffers(), channel, selectSet, key, createSSLEngine(channel)) + { + @Override + public void close() throws IOException + { + endPointClosed.compareAndSet(false, true); + super.close(); + } + }; + } + + @Override + protected Connection newConnection(SocketChannel channel, SelectChannelEndPoint endpoint) + { + return new HttpConnection(this, endpoint, server) + { + @Override + public Connection handle() throws IOException + { + handleCount.incrementAndGet(); + return super.handle(); + } + }; + } + }; + server.addConnector(connector); + + String keystorePath = System.getProperty("basedir", ".") + "/src/test/resources/keystore"; + SslContextFactory sslContextFactory = connector.getSslContextFactory(); + sslContextFactory.setKeyStore(keystorePath); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setKeyManagerPassword("keypwd"); + sslContextFactory.setTrustStore(keystorePath); + sslContextFactory.setTrustStorePassword("storepwd"); + + server.setHandler(new EmptyHandler()); + + server.start(); + + sslContext = sslContextFactory.getSslContext(); + } + + @After + public void destroyServer() throws Exception + { + server.stop(); + server.join(); + } + + /** + * A SSL truncation attack is when the remote peer sends a TCP FIN + * without having sent the SSL close alert. + * We need to handle this condition carefully to avoid spinning + * on the selector and by closing the connection altogether. + * + * @throws Exception if the test fails + */ + @Test + public void testTruncationAttack() throws Exception + { + Socket socket = new Socket("localhost", connector.getLocalPort()); + SSLSocket sslSocket = (SSLSocket)sslContext.getSocketFactory().createSocket(socket, socket.getInetAddress().getHostName(), socket.getPort(), true); + sslSocket.setUseClientMode(true); + final CountDownLatch handshakeLatch = new CountDownLatch(1); + sslSocket.addHandshakeCompletedListener(new HandshakeCompletedListener() + { + public void handshakeCompleted(HandshakeCompletedEvent handshakeCompletedEvent) + { + handshakeLatch.countDown(); + } + }); + sslSocket.startHandshake(); + + Assert.assertTrue(handshakeLatch.await(1, TimeUnit.SECONDS)); + + String request = "" + + "GET / HTTP/1.1\r\n" + + "Host: localhost:" + connector.getLocalPort() + "\r\n" + + "\r\n"; + sslSocket.getOutputStream().write(request.getBytes("UTF-8")); + + byte[] buffer = new byte[1024]; + StringBuilder response = new StringBuilder(); + sslSocket.setSoTimeout(1000); + while (true) + { + int read = sslSocket.getInputStream().read(buffer); + response.append(new String(buffer, 0, read, "UTF-8")); + if (response.indexOf("\r\n\r\n") >= 0) + break; + } + + handleCount.set(0); + + // Send TCP FIN without SSL close alert + socket.close(); + + // Sleep for a while to detect eventual spin looping + TimeUnit.SECONDS.sleep(1); + + Assert.assertEquals("handle() invocations", 1, handleCount.get()); + Assert.assertTrue("endpoint closed", endPointClosed.get()); + } + + private class EmptyHandler extends AbstractHandler + { + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + } + } +}