Ensure calling ReferenceCountedOpenSslEngine.wrap(...) after closeOutbound() was called will not throw an SSLException

Motivation:

PR [#6238] added guards to be able to call wrap(...) / unwrap(...) after the engine was shutdown. Unfortunally one case was missed which is when closeOutbound() was called and produced some data while closeInbound() was not called yet.

Modifications:

Correctly guard against SSLException when closeOutbound() was called, produced data and someone calls wrap(...) after it.

Result:

No more SSLException. Fixes [#6260].
This commit is contained in:
Norman Maurer 2017-01-20 21:38:48 +01:00
parent 4e04b63746
commit e9fa40d770
2 changed files with 55 additions and 3 deletions

View File

@ -62,7 +62,6 @@ import static io.netty.handler.ssl.OpenSsl.memoryAddress;
import static io.netty.util.internal.EmptyArrays.EMPTY_CERTIFICATES;
import static io.netty.util.internal.EmptyArrays.EMPTY_JAVAX_X509_CERTIFICATES;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static java.lang.Math.max;
import static java.lang.Math.min;
import static javax.net.ssl.SSLEngineResult.HandshakeStatus.FINISHED;
import static javax.net.ssl.SSLEngineResult.HandshakeStatus.NEED_UNWRAP;
@ -570,6 +569,12 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return null;
}
private SSLEngineResult drainOutboundBuffer(ByteBuffer dst, SSLEngineResult.HandshakeStatus handshakeStatus)
throws SSLException {
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, 0, handshakeStatus);
return pendingNetResult != null ? pendingNetResult : NEED_UNWRAP_CLOSED;
}
@Override
public final SSLEngineResult wrap(
final ByteBuffer[] srcs, final int offset, final int length, final ByteBuffer dst) throws SSLException {
@ -593,10 +598,19 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
synchronized (this) {
if (isOutboundDone()) {
// All drained in the outbound buffer
return isInboundDone() || isDestroyed() ? CLOSED_NOT_HANDSHAKING : NEED_UNWRAP_CLOSED;
}
SSLEngineResult.HandshakeStatus status = NOT_HANDSHAKING;
// Explicit use outboundClosed as we want to drain any bytes that are still present.
if (outboundClosed) {
// There is something left to drain.
// See https://github.com/netty/netty/issues/6260
return drainOutboundBuffer(dst, status);
}
// Prepare OpenSSL to work in server mode and receive handshake
if (handshakeState != HandshakeState.FINISHED) {
if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
@ -613,8 +627,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// Explicit use outboundClosed and not outboundClosed() as we want to drain any bytes that are still
// present.
if (outboundClosed) {
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, 0, status);
return pendingNetResult != null ? pendingNetResult : NEED_UNWRAP_CLOSED;
return drainOutboundBuffer(dst, status);
}
}

View File

@ -1581,6 +1581,45 @@ public abstract class SSLEngineTest {
assertEquals(0, result.bytesProduced());
}
@Test
public void testWrapAfterCloseOutbound() throws Exception {
SelfSignedCertificate cert = new SelfSignedCertificate();
clientSslCtx = SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslProvider(sslClientProvider())
.build();
SSLEngine client = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
serverSslCtx = SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslProvider(sslServerProvider())
.build();
SSLEngine server = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
try {
ByteBuffer dst = allocateBuffer(server.getSession().getPacketBufferSize());
ByteBuffer src = allocateBuffer(1024);
handshake(client, server);
// This will produce a close_notify
client.closeOutbound();
SSLEngineResult result = client.wrap(src, dst);
assertEquals(SSLEngineResult.Status.CLOSED, result.getStatus());
assertEquals(0, result.bytesConsumed());
assertTrue(result.bytesProduced() > 0);
assertTrue(client.isOutboundDone());
assertFalse(client.isInboundDone());
} finally {
cert.delete();
cleanupClientSslEngine(client);
cleanupServerSslEngine(server);
}
}
@Test
public void testMultipleRecordsInOneBufferWithNonZeroPosition() throws Exception {
SelfSignedCertificate cert = new SelfSignedCertificate();