Improved SslSelectChannelEndPoint to free buffers when not needed, and added more truncation attack tests.

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@3253 7e9141cc-0065-0410-87d8-b60c137991c4
This commit is contained in:
Simone Bordet 2011-05-25 09:56:04 +00:00
parent 3ddabaf64f
commit 1bb56442a7
2 changed files with 138 additions and 27 deletions

View File

@ -183,10 +183,16 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
/* ------------------------------------------------------------ */
@Override
public void shutdownOutput() throws IOException
{
try
{
sslClose();
}
finally
{
super.shutdownOutput();
}
}
/* ------------------------------------------------------------ */
protected void sslClose() throws IOException
@ -291,9 +297,9 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
}
}
}
catch (Exception e)
catch (Exception x)
{
Log.debug(e);
Log.debug(x);
super.close();
}
}
@ -301,10 +307,16 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
/* ------------------------------------------------------------ */
@Override
public void close() throws IOException
{
try
{
sslClose();
}
finally
{
super.close();
}
}
/* ------------------------------------------------------------ */
/** Fill the buffer with unencrypted bytes.
@ -693,6 +705,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
{
if (_inNIOBuffer.length()==0)
{
freeInBuffer();
if (_outNIOBuffer!=null)
{
_outNIOBuffer.clear();
@ -707,9 +720,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
// If we have no progress and no data
if (total_filled==0 && _inNIOBuffer.length()==0)
{
if (isOpen())
{
if (remoteClosed)
if (isOpen() && remoteClosed)
{
try
{
@ -718,17 +729,17 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
catch (SSLException x)
{
// It may happen, for example, in case of truncation
// attacks, we close so that we do not spin forever.
freeOutBuffer();
// attacks, we close so that we do not spin forever
super.close();
}
}
}
else
{
freeInBuffer();
freeOutBuffer();
if (!isOpen())
throw new EofException();
}
return false;
}
@ -739,7 +750,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
// so update its position and limit from the inNIOBuffer.
in_buffer.position(_inNIOBuffer.getIndex());
in_buffer.limit(_inNIOBuffer.putIndex());
_result=null;
// Do the unwrap
_result=_engine.unwrap(in_buffer,buffer);
@ -751,6 +761,7 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint
catch(SSLException e)
{
Log.warn(getRemoteAddr() + ":" + getRemotePort() + " " + e);
freeOutBuffer();
super.close();
throw e;
}

View File

@ -28,6 +28,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
public class SslTruncationAttackTest
@ -108,7 +109,7 @@ public class SslTruncationAttackTest
* @throws Exception if the test fails
*/
@Test
public void testTruncationAttack() throws Exception
public void testTruncationAttackAfterReading() throws Exception
{
Socket socket = new Socket("localhost", connector.getLocalPort());
SSLSocket sslSocket = (SSLSocket)sslContext.getSocketFactory().createSocket(socket, socket.getInetAddress().getHostName(), socket.getPort(), true);
@ -151,7 +152,106 @@ public class SslTruncationAttackTest
TimeUnit.SECONDS.sleep(1);
Assert.assertEquals("handle() invocations", 1, handleCount.get());
Assert.assertTrue("endpoint closed", endPointClosed.get());
Assert.assertTrue("endpoint not closed", endPointClosed.get());
}
/**
* This test is currently failing because we are looping on SslSCEP.unwrap()
* to fill the buffer, so there is a case where we loop once, read some data
* loop again and read -1, but we can't close the connection yet as we have
* to notify the application (not sure that this is necessary... must assume
* the data is truncated, so it's not that safe to pass it to the application).
* This case needs to be revisited, and it also requires a review of the
* Connection:close case, especially on the client side.
* @throws Exception if the test fails
*/
@Ignore
@Test
public void testTruncationAttackBeforeReading() 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"));
// Do not read the response, just close the underlying socket
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 not closed", endPointClosed.get());
}
@Test
public void testTruncationAttackAfterHandshake() 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));
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 not closed", endPointClosed.get());
}
@Test
public void testTruncationAttackBeforeHandshake() 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);
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 not closed", endPointClosed.get());
}
private class EmptyHandler extends AbstractHandler