From ecc01da9ddf18790ac96019710ccaed50133607c Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 20 Jul 2015 15:14:17 +0200 Subject: [PATCH] [#3968] Disallow pass-through of non ByteBufs in SslHandler Motivation: We pass-through non ByteBuf when SslHandler.write(...) is called which can lead to have unencrypted data to be send (like for example if a FileRegion is written). Modifications: - Fail ChannelPromise with UnsupportedMessageException if a non ByteBuf is written. Result: Only allow ByteBuf to be written when using SslHandler. --- .../java/io/netty/handler/ssl/SslHandler.java | 10 +++++----- .../java/io/netty/handler/ssl/SslHandlerTest.java | 15 ++++----------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 3b9b216543..4da91b9fca 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -33,6 +33,7 @@ import io.netty.channel.ChannelPromise; import io.netty.channel.ChannelPromiseNotifier; import io.netty.channel.PendingWriteQueue; import io.netty.handler.codec.ByteToMessageDecoder; +import io.netty.handler.codec.UnsupportedMessageTypeException; import io.netty.util.concurrent.DefaultPromise; import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.Future; @@ -466,6 +467,10 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH @Override public void write(final ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { + if (!(msg instanceof ByteBuf)) { + promise.setFailure(new UnsupportedMessageTypeException(msg, ByteBuf.class)); + return; + } pendingUnencryptedWrites.add(msg, promise); } @@ -504,11 +509,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH break; } - if (!(msg instanceof ByteBuf)) { - pendingUnencryptedWrites.removeAndWrite(); - continue; - } - ByteBuf buf = (ByteBuf) msg; if (out == null) { out = allocateOutNetBuf(ctx, buf.readableBytes()); diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java index dc381b3f6b..151f0a7233 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java @@ -19,6 +19,7 @@ package io.netty.handler.ssl; import io.netty.buffer.Unpooled; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.DecoderException; +import io.netty.handler.codec.UnsupportedMessageTypeException; import org.junit.Test; import javax.net.ssl.SSLContext; @@ -53,21 +54,13 @@ public class SslHandlerTest { } } - @Test - public void testNonByteBufPassthrough() throws Exception { + @Test(expected = UnsupportedMessageTypeException.class) + public void testNonByteBufNotPassThrough() throws Exception { SSLEngine engine = SSLContext.getDefault().createSSLEngine(); engine.setUseClientMode(false); EmbeddedChannel ch = new EmbeddedChannel(new SslHandler(engine)); - Object msg1 = new Object(); - ch.writeOutbound(msg1); - assertThat(ch.readOutbound(), is(sameInstance(msg1))); - - Object msg2 = new Object(); - ch.writeInbound(msg2); - assertThat(ch.readInbound(), is(sameInstance(msg2))); - - ch.finish(); + ch.writeOutbound(new Object()); } }