DefaultHttp2FrameWriter ping payload size check

Motivation:
The HTTP/2 spec states that the ping frame length must be 8 and is otherwise an error https://tools.ietf.org/html/rfc7540#section-6.7. The DefaultHttp2FrameReader enforces this, but the DefaultHttp2FrameWriter allows invalid frames to be written. We should not allow invalid ping frames to be written to the network.

Modifications:
- DefaultHttp2FrameWriter checks the frame size to be 8, or throws an exception

Result:
Fixes https://github.com/netty/netty/issues/3721
This commit is contained in:
Scott Mitchell 2015-09-09 13:40:07 -07:00
parent 59600f1812
commit 15450af2e7
2 changed files with 22 additions and 12 deletions

View File

@ -14,9 +14,15 @@
*/ */
package io.netty.handler.codec.http2; package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http2.Http2FrameReader.Configuration;
import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_FRAME_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_MAX_FRAME_SIZE;
import static io.netty.handler.codec.http2.Http2CodecUtil.FRAME_HEADER_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.FRAME_HEADER_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.INT_FIELD_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.INT_FIELD_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.PING_FRAME_PAYLOAD_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_ENTRY_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_ENTRY_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_INITIAL_WINDOW_SIZE;
import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_MAX_FRAME_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_MAX_FRAME_SIZE;
@ -39,10 +45,6 @@ import static io.netty.handler.codec.http2.Http2FrameTypes.PUSH_PROMISE;
import static io.netty.handler.codec.http2.Http2FrameTypes.RST_STREAM; import static io.netty.handler.codec.http2.Http2FrameTypes.RST_STREAM;
import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS; import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS;
import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE; import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http2.Http2FrameReader.Configuration;
/** /**
* A {@link Http2FrameReader} that supports all frame types defined by the HTTP/2 specification. * A {@link Http2FrameReader} that supports all frame types defined by the HTTP/2 specification.
@ -344,7 +346,7 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize
if (streamId != 0) { if (streamId != 0) {
throw connectionError(PROTOCOL_ERROR, "A stream ID must be zero."); throw connectionError(PROTOCOL_ERROR, "A stream ID must be zero.");
} }
if (payloadLength != 8) { if (payloadLength != PING_FRAME_PAYLOAD_LENGTH) {
throw connectionError(FRAME_SIZE_ERROR, throw connectionError(FRAME_SIZE_ERROR,
"Frame length %d incorrect size for ping.", payloadLength); "Frame length %d incorrect size for ping.", payloadLength);
} }

View File

@ -15,6 +15,13 @@
package io.netty.handler.codec.http2; package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.handler.codec.http2.Http2CodecUtil.SimpleChannelPromiseAggregator;
import io.netty.handler.codec.http2.Http2FrameWriter.Configuration;
import static io.netty.buffer.Unpooled.directBuffer; import static io.netty.buffer.Unpooled.directBuffer;
import static io.netty.buffer.Unpooled.unmodifiableBuffer; import static io.netty.buffer.Unpooled.unmodifiableBuffer;
import static io.netty.buffer.Unpooled.unreleasableBuffer; import static io.netty.buffer.Unpooled.unreleasableBuffer;
@ -29,6 +36,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_UNSIGNED_BYTE;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_UNSIGNED_INT; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_UNSIGNED_INT;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_WEIGHT; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_WEIGHT;
import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_WEIGHT; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_WEIGHT;
import static io.netty.handler.codec.http2.Http2CodecUtil.PING_FRAME_PAYLOAD_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_ENTRY_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_ENTRY_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_FRAME_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.PRIORITY_FRAME_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.PUSH_PROMISE_FRAME_HEADER_LENGTH; import static io.netty.handler.codec.http2.Http2CodecUtil.PUSH_PROMISE_FRAME_HEADER_LENGTH;
@ -53,13 +61,6 @@ import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS;
import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE; import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE;
import static io.netty.util.internal.ObjectUtil.checkNotNull; import static io.netty.util.internal.ObjectUtil.checkNotNull;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.handler.codec.http2.Http2CodecUtil.SimpleChannelPromiseAggregator;
import io.netty.handler.codec.http2.Http2FrameWriter.Configuration;
/** /**
* A {@link Http2FrameWriter} that supports all frame types defined by the HTTP/2 specification. * A {@link Http2FrameWriter} that supports all frame types defined by the HTTP/2 specification.
*/ */
@ -240,6 +241,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
SimpleChannelPromiseAggregator promiseAggregator = SimpleChannelPromiseAggregator promiseAggregator =
new SimpleChannelPromiseAggregator(promise, ctx.channel(), ctx.executor()); new SimpleChannelPromiseAggregator(promise, ctx.channel(), ctx.executor());
try { try {
verifyPingPayload(data);
Http2Flags flags = ack ? new Http2Flags().ack(true) : new Http2Flags(); Http2Flags flags = ack ? new Http2Flags().ack(true) : new Http2Flags();
ByteBuf buf = ctx.alloc().buffer(FRAME_HEADER_LENGTH); ByteBuf buf = ctx.alloc().buffer(FRAME_HEADER_LENGTH);
writeFrameHeaderInternal(buf, data.readableBytes(), PING, flags, 0); writeFrameHeaderInternal(buf, data.readableBytes(), PING, flags, 0);
@ -545,4 +547,10 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize
throw new IllegalArgumentException("WindowSizeIncrement must be >= 0"); throw new IllegalArgumentException("WindowSizeIncrement must be >= 0");
} }
} }
private static void verifyPingPayload(ByteBuf data) {
if (data == null || data.readableBytes() != PING_FRAME_PAYLOAD_LENGTH) {
throw new IllegalArgumentException("Opaque data must be " + PING_FRAME_PAYLOAD_LENGTH + " bytes");
}
}
} }