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

Issue #4209 - Unused TLS connection is not closed in Java 11. 

Code cleanup.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2019-10-16 13:02:24 +02:00
parent 20e7aa01f2
commit 4769de8a2b
3 changed files with 99 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java
View File

@ -67,8 +67,6 @@ public class HttpClientTLSTest
private ServerConnector connector;
private HttpClient client;
private SSLSocket sslSocket;
private void startServer(SslContextFactory sslContextFactory, Handler handler) throws Exception
{
ExecutorThreadPool serverThreads = new ExecutorThreadPool();
@ -420,16 +418,16 @@ public class HttpClientTLSTest
String host = "localhost";
int port = connector.getLocalPort();
Socket socket = new Socket(host, port);
sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true);
Socket socket1 = new Socket(host, port);
SSLSocket sslSocket1 = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket1, host, port, true);
CountDownLatch handshakeLatch1 = new CountDownLatch(1);
AtomicReference<byte[]> session1 = new AtomicReference<>();
sslSocket.addHandshakeCompletedListener(event ->
sslSocket1.addHandshakeCompletedListener(event ->
{
session1.set(event.getSession().getId());
handshakeLatch1.countDown();
});
sslSocket.startHandshake();
sslSocket1.startHandshake();
assertTrue(handshakeLatch1.await(5, TimeUnit.SECONDS));
// In TLS 1.3 the server sends a NewSessionTicket post-handshake message
@ -437,29 +435,29 @@ public class HttpClientTLSTest
assertThrows(SocketTimeoutException.class, () ->
{
sslSocket.setSoTimeout(1000);
sslSocket.getInputStream().read();
sslSocket1.setSoTimeout(1000);
sslSocket1.getInputStream().read();
});
// The client closes abruptly.
socket.close();
socket1.close();
// Try again and compare the session ids.
socket = new Socket(host, port);
sslSocket = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket, host, port, true);
Socket socket2 = new Socket(host, port);
SSLSocket sslSocket2 = (SSLSocket)clientTLSFactory.getSslContext().getSocketFactory().createSocket(socket2, host, port, true);
CountDownLatch handshakeLatch2 = new CountDownLatch(1);
AtomicReference<byte[]> session2 = new AtomicReference<>();
sslSocket.addHandshakeCompletedListener(event ->
sslSocket2.addHandshakeCompletedListener(event ->
{
session2.set(event.getSession().getId());
handshakeLatch2.countDown();
});
sslSocket.startHandshake();
sslSocket2.startHandshake();
assertTrue(handshakeLatch2.await(5, TimeUnit.SECONDS));
assertArrayEquals(session1.get(), session2.get());
sslSocket.close();
sslSocket2.close();
}
@Test
@ -477,7 +475,7 @@ public class HttpClientTLSTest
protected ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory sslContextFactory, ClientConnectionFactory connectionFactory)
{
SslClientConnectionFactory ssl = (SslClientConnectionFactory)super.newSslClientConnectionFactory(sslContextFactory, connectionFactory);
ssl.setAllowMissingCloseMessage(false);
ssl.setRequireCloseMessage(true);
return ssl;
}
};
@ -505,19 +503,19 @@ public class HttpClientTLSTest
break;
}
// If the response is Content-Length delimited, allowing the
// missing TLS Close Message is fine because the application
// will see a EOFException anyway.
// If the response is connection delimited, allowing the
// missing TLS Close Message is bad because the application
// will see a successful response with truncated content.
// If the response is Content-Length delimited, the lack of
// the TLS Close Message is fine because the application
// will see a EOFException anyway: the Content-Length and
// the actual content bytes count won't match.
// If the response is connection delimited, the lack of the
// TLS Close Message is bad because the application will
// see a successful response, but with truncated content.
// Verify that by not allowing the missing
// TLS Close Message we get a response failure.
// Verify that by requiring the TLS Close Message we get
// a response failure.
byte[] half = new byte[8];
String response = "HTTP/1.1 200 OK\r\n" +
// "Content-Length: " + (half.length * 2) + "\r\n" +
"Connection: close\r\n" +
"\r\n";
OutputStream output = sslSocket.getOutputStream();

36
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslClientConnectionFactory.java
View File

@ -47,7 +47,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
private final ClientConnectionFactory connectionFactory;
private boolean _directBuffersForEncryption = true;
private boolean _directBuffersForDecryption = true;
private boolean allowMissingCloseMessage = true;
private boolean _requireCloseMessage;
public SslClientConnectionFactory(SslContextFactory sslContextFactory, ByteBufferPool byteBufferPool, Executor executor, ClientConnectionFactory connectionFactory)
{
@ -77,14 +77,42 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
return _directBuffersForEncryption;
}
/**
* @return whether is not required that peers send the TLS {@code close_notify} message
* @deprecated use {@link #isRequireCloseMessage()} instead
*/
@Deprecated
public boolean isAllowMissingCloseMessage()
{
return allowMissingCloseMessage;
return !isRequireCloseMessage();
}
/**
* @param allowMissingCloseMessage whether is not required that peers send the TLS {@code close_notify} message
* @deprecated use {@link #setRequireCloseMessage(boolean)} instead
*/
@Deprecated
public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage)
{
this.allowMissingCloseMessage = allowMissingCloseMessage;
setRequireCloseMessage(!allowMissingCloseMessage);
}
/**
* @return whether peers must send the TLS {@code close_notify} message
* @see SslConnection#isRequireCloseMessage()
*/
public boolean isRequireCloseMessage()
{
return _requireCloseMessage;
}
/**
* @param requireCloseMessage whether peers must send the TLS {@code close_notify} message
* @see SslConnection#setRequireCloseMessage(boolean)
*/
public void setRequireCloseMessage(boolean requireCloseMessage)
{
_requireCloseMessage = requireCloseMessage;
}
@Override
@ -120,7 +148,7 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
SslConnection sslConnection = (SslConnection)connection;
sslConnection.setRenegotiationAllowed(sslContextFactory.isRenegotiationAllowed());
sslConnection.setRenegotiationLimit(sslContextFactory.getRenegotiationLimit());
sslConnection.setAllowMissingCloseMessage(isAllowMissingCloseMessage());
sslConnection.setRequireCloseMessage(isRequireCloseMessage());
ContainerLifeCycle connector = (ContainerLifeCycle)context.get(ClientConnectionFactory.CONNECTOR_CONTEXT_KEY);
connector.getBeans(SslHandshakeListener.class).forEach(sslConnection::addHandshakeListener);
}

57
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
View File

@ -113,7 +113,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
private boolean _renegotiationAllowed;
private int _renegotiationLimit = -1;
private boolean _closedOutbound;
private boolean _allowMissingCloseMessage = true;
private boolean _requireCloseMessage;
private FlushState _flushState = FlushState.IDLE;
private FillState _fillState = FillState.IDLE;
private AtomicReference<Handshake> _handshake = new AtomicReference<>(Handshake.INITIAL);
@ -231,7 +231,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
}
/**
* @return The number of renegotions allowed for this connection. When the limit
* @return The number of renegotiations allowed for this connection. When the limit
* is 0 renegotiation will be denied. If the limit is less than 0 then no limit is applied.
*/
public int getRenegotiationLimit()
@ -240,7 +240,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
}
/**
* @param renegotiationLimit The number of renegotions allowed for this connection.
* @param renegotiationLimit The number of renegotiations allowed for this connection.
* When the limit is 0 renegotiation will be denied. If the limit is less than 0 then no limit is applied.
* Default -1.
*/
@ -249,14 +249,46 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
_renegotiationLimit = renegotiationLimit;
}
/**
* @return whether is not required that peers send the TLS {@code close_notify} message
* @deprecated use inverted {@link #isRequireCloseMessage()} instead
*/
@Deprecated
public boolean isAllowMissingCloseMessage()
{
return _allowMissingCloseMessage;
return !isRequireCloseMessage();
}
/**
* @param allowMissingCloseMessage whether is not required that peers send the TLS {@code close_notify} message
* @deprecated use inverted {@link #setRequireCloseMessage(boolean)} instead
*/
@Deprecated
public void setAllowMissingCloseMessage(boolean allowMissingCloseMessage)
{
this._allowMissingCloseMessage = allowMissingCloseMessage;
setRequireCloseMessage(!allowMissingCloseMessage);
}
/**
* @return whether peers must send the TLS {@code close_notify} message
*/
public boolean isRequireCloseMessage()
{
return _requireCloseMessage;
}
/**
* <p>Sets whether it is required that a peer send the TLS {@code close_notify} message
* to indicate the will to close the connection, otherwise it may be interpreted as a
* truncation attack.</p>
* <p>This option is only useful on clients, since typically servers cannot accept
* connection-delimited content that may be truncated.</p>
*
* @param requireCloseMessage whether peers must send the TLS {@code close_notify} message
*/
public void setRequireCloseMessage(boolean requireCloseMessage)
{
_requireCloseMessage = requireCloseMessage;
}
private void acquireEncryptedInput()
@ -1096,15 +1128,15 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
@Override
public void doShutdownOutput()
{
final EndPoint endp = getEndPoint();
EndPoint endPoint = getEndPoint();
try
{
boolean close;
boolean flush = false;
synchronized (_decryptedEndPoint)
{
boolean ishut = endp.isInputShutdown();
boolean oshut = endp.isOutputShutdown();
boolean ishut = endPoint.isInputShutdown();
boolean oshut = endPoint.isOutputShutdown();
if (LOG.isDebugEnabled())
LOG.debug("shutdownOutput: {} oshut={}, ishut={}", SslConnection.this, oshut, ishut);
@ -1128,19 +1160,19 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
// let's just flush the encrypted output in the background.
ByteBuffer write = _encryptedOutput;
if (BufferUtil.hasContent(write))
endp.write(Callback.from(Callback.NOOP::succeeded, t -> endp.close()), write);
endPoint.write(Callback.from(Callback.NOOP::succeeded, t -> endPoint.close()), write);
}
}
if (close)
endp.close();
endPoint.close();
else
ensureFillInterested();
}
catch (Throwable x)
{
LOG.ignore(x);
endp.close();
endPoint.close();
}
}
@ -1152,7 +1184,8 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr
}
catch (Throwable x)
{
LOG.ignore(x);
if (LOG.isDebugEnabled())
LOG.debug(x);
}
}