Fixes #346614 (HttpConnection.handle() spins in case of SSL truncation attacks).
SslSelectChannelEndPoint has been modified to not override shutdownInput() (so behavior is that of the base class, like it should), and when it detects a remote close, it calls SSLEngine.closeInbound(), which throws in case of a truncation attack. The exception is handled and the endpoint closed. git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@3224 7e9141cc-0065-0410-87d8-b60c137991c4
This commit is contained in:
parent
e438787325
commit
fc6ea06106
|
@ -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();
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue