Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

419344 - NPNServerConnection does not close the EndPoint if it reads 

-1.

Fixed by correctly handling the -1 read, closing the connection.
Also covered additional error cases, making sure the connection is
closed.
This commit is contained in:
Simone Bordet 2013-10-14 13:26:00 +02:00
parent b8c8abae2e
commit 8fec401b06
2 changed files with 180 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
jetty-spdy/spdy-server/src/main/java/org/eclipse/jetty/spdy/server/NextProtoNegoServerConnection.java
View File

@ -20,7 +20,6 @@ package org.eclipse.jetty.spdy.server;
import java.io.IOException;
import java.util.List;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
@ -63,33 +62,58 @@ public class NextProtoNegoServerConnection extends AbstractConnection implements
@Override
public void onFillable()
{
while (true)
{
int filled = fill();
if (filled == 0 && nextProtocol == null)
fillInterested();
if (filled <= 0 || nextProtocol != null)
break;
}
int filled = fill();
if (nextProtocol == null && engine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING)
if (filled == 0)
{
// The client sent the NPN extension, but did not send the NextProtocol
// message with the chosen protocol so we need to close
LOG.debug("{} missing next protocol. SSLEngine: {}", this, engine);
if (nextProtocol == null)
{
if (engine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING)
{
// Here the SSL handshake is finished, but while the client sent
// the NPN extension, the server application did not select the
// protocol; we need to close as the protocol cannot be negotiated.
LOG.debug("{} missing next protocol. SSLEngine: {}", this, engine);
close();
}
else
{
// Here the SSL handshake is not finished yet but we filled 0 bytes,
// so we need to read more.
fillInterested();
}
}
else
{
ConnectionFactory connectionFactory = connector.getConnectionFactory(nextProtocol);
if (connectionFactory == null)
{
LOG.debug("{} application selected protocol '{}', but no correspondent {} has been configured",
this, nextProtocol, ConnectionFactory.class.getName());
close();
}
else
{
EndPoint endPoint = getEndPoint();
Connection oldConnection = endPoint.getConnection();
Connection newConnection = connectionFactory.newConnection(connector, endPoint);
LOG.debug("{} switching from {} to {}", this, oldConnection, newConnection);
oldConnection.onClose();
endPoint.setConnection(newConnection);
getEndPoint().getConnection().onOpen();
}
}
}
else if (filled < 0)
{
// Something went bad, we need to close.
LOG.debug("{} closing on client close", this);
close();
}
if (nextProtocol != null)
else
{
ConnectionFactory connectionFactory = connector.getConnectionFactory(nextProtocol);
EndPoint endPoint = getEndPoint();
Connection oldConnection = endPoint.getConnection();
oldConnection.onClose();
Connection connection = connectionFactory.newConnection(connector, endPoint);
LOG.debug("{} switching from {} to {}", this, oldConnection, connection);
endPoint.setConnection(connection);
getEndPoint().getConnection().onOpen();
// Must never happen, since we fill using an empty buffer
throw new IllegalStateException();
}
}
@ -102,8 +126,7 @@ public class NextProtoNegoServerConnection extends AbstractConnection implements
catch (IOException x)
{
LOG.debug(x);
NextProtoNego.remove(engine);
getEndPoint().close();
close();
return -1;
}
}
@ -127,4 +150,13 @@ public class NextProtoNegoServerConnection extends AbstractConnection implements
nextProtocol = protocol != null ? protocol : defaultProtocol;
NextProtoNego.remove(engine);
}
@Override
public void close()
{
NextProtoNego.remove(engine);
EndPoint endPoint = getEndPoint();
endPoint.shutdownOutput();
endPoint.close();
}
}

123
jetty-spdy/spdy-server/src/test/java/org/eclipse/jetty/spdy/server/SSLSynReplyTest.java
View File

@ -19,14 +19,22 @@
package org.eclipse.jetty.spdy.server;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.SocketChannel;
import java.util.List;
import java.util.concurrent.Executor;
import javax.net.ssl.SSLEngine;
import org.eclipse.jetty.npn.NextProtoNego;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.spdy.api.server.ServerSessionFrameListener;
import org.eclipse.jetty.spdy.client.SPDYClient;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
public class SSLSynReplyTest extends SynReplyTest
{
@ -51,4 +59,119 @@ public class SSLSynReplyTest extends SynReplyTest
{
NextProtoNego.debug = true;
}
@Test
public void testGentleCloseDuringHandshake() throws Exception
{
InetSocketAddress address = startServer(version, null);
SslContextFactory sslContextFactory = newSslContextFactory();
sslContextFactory.start();
SSLEngine sslEngine = sslContextFactory.newSSLEngine(address);
sslEngine.setUseClientMode(true);
NextProtoNego.put(sslEngine, new NextProtoNego.ClientProvider()
{
@Override
public boolean supports()
{
return true;
}
@Override
public void unsupported()
{
}
@Override
public String selectProtocol(List<String> protocols)
{
return null;
}
});
sslEngine.beginHandshake();
ByteBuffer encrypted = ByteBuffer.allocate(sslEngine.getSession().getPacketBufferSize());
sslEngine.wrap(BufferUtil.EMPTY_BUFFER, encrypted);
encrypted.flip();
try (SocketChannel channel = SocketChannel.open(address))
{
// Send ClientHello, immediately followed by TLS Close Alert and then by FIN
channel.write(encrypted);
sslEngine.closeOutbound();
encrypted.clear();
sslEngine.wrap(BufferUtil.EMPTY_BUFFER, encrypted);
encrypted.flip();
channel.write(encrypted);
channel.shutdownOutput();
// Read ServerHello from server
encrypted.clear();
int read = channel.read(encrypted);
encrypted.flip();
Assert.assertTrue(read > 0);
// Cannot decrypt, as the SSLEngine has been already closed
// Now if we read more, we should either read the TLS Close Alert, or directly -1
encrypted.clear();
read = channel.read(encrypted);
// Sending a TLS Close Alert during handshake results in an exception when
// unwrapping that the server react to by closing the connection abruptly.
Assert.assertTrue(read < 0);
}
}
@Test
public void testAbruptCloseDuringHandshake() throws Exception
{
InetSocketAddress address = startServer(version, null);
SslContextFactory sslContextFactory = newSslContextFactory();
sslContextFactory.start();
SSLEngine sslEngine = sslContextFactory.newSSLEngine(address);
sslEngine.setUseClientMode(true);
NextProtoNego.put(sslEngine, new NextProtoNego.ClientProvider()
{
@Override
public boolean supports()
{
return true;
}
@Override
public void unsupported()
{
}
@Override
public String selectProtocol(List<String> protocols)
{
return null;
}
});
sslEngine.beginHandshake();
ByteBuffer encrypted = ByteBuffer.allocate(sslEngine.getSession().getPacketBufferSize());
sslEngine.wrap(BufferUtil.EMPTY_BUFFER, encrypted);
encrypted.flip();
try (SocketChannel channel = SocketChannel.open(address))
{
// Send ClientHello, immediately followed by FIN (no TLS Close Alert)
channel.write(encrypted);
channel.shutdownOutput();
// Read ServerHello from server
encrypted.clear();
int read = channel.read(encrypted);
encrypted.flip();
Assert.assertTrue(read > 0);
ByteBuffer decrypted = ByteBuffer.allocate(sslEngine.getSession().getApplicationBufferSize());
sslEngine.unwrap(encrypted, decrypted);
// Now if we read more, we should either read the TLS Close Alert, or directly -1
encrypted.clear();
read = channel.read(encrypted);
// Since we have close the connection abruptly, the server also does so
Assert.assertTrue(read < 0);
}
}
}