From 42a4421f03dfe989a721ff6aa13f772495f3914c Mon Sep 17 00:00:00 2001 From: wangxiao65 <287608437@qq.com> Date: Tue, 6 Apr 2021 15:28:11 +0800 Subject: [PATCH] fix CVE-2021-21295 CVE-2021-21409 (cherry picked from commit df460aec6ed66a0b89f1f48f39144fd0b1932e27) --- CVE-2021-21295-pre1.patch | 240 ++++++++++++ CVE-2021-21295-pre2.patch | 676 +++++++++++++++++++++++++++++++++ CVE-2021-21295-pre3.patch | 566 ++++++++++++++++++++++++++++ CVE-2021-21295-pre4.patch | 771 ++++++++++++++++++++++++++++++++++++++ CVE-2021-21295.patch | 547 +++++++++++++++++++++++++++ CVE-2021-21409.patch | 51 +++ netty.spec | 13 +- 7 files changed, 2862 insertions(+), 2 deletions(-) create mode 100644 CVE-2021-21295-pre1.patch create mode 100644 CVE-2021-21295-pre2.patch create mode 100644 CVE-2021-21295-pre3.patch create mode 100644 CVE-2021-21295-pre4.patch create mode 100644 CVE-2021-21295.patch create mode 100644 CVE-2021-21409.patch diff --git a/CVE-2021-21295-pre1.patch b/CVE-2021-21295-pre1.patch new file mode 100644 index 0000000..6f5a17b --- /dev/null +++ b/CVE-2021-21295-pre1.patch @@ -0,0 +1,240 @@ +From bcb62be62bd989c0292e0f8e22a51127907cefdc Mon Sep 17 00:00:00 2001 +From: Bennett Lynch +Date: Thu, 11 Jun 2020 22:39:10 -0700 +Subject: [PATCH] Consolidate HttpObjectDecoder default values into + constants (#10344) + +Motivation + +HttpObjectDecoder and its associated classes make frequent use of +default values for maxInitialLineLength, maxHeaderSize, maxChunkSize, +etc. Today, these defaults are defined in-line in constructors and +duplicated across many classes. This repetition is more prone to error +and inconsistencies. + +Furthermore, due to the current lack of builder support, if a user wants +to change just one of these values (e.g., maxHeaderSize), they are also +required to know and repeat the other default values (e.g., +maxInitialLineLength and maxChunkSize). + +The primary motivation for this change is as we are considering adding +another constructor parameter (for multiple content length behavior), +appending this parameter may require some users to have prior knowledge +of the default initialBufferSize, and it would be cleaner to allow them +to reference the default constant. + +Modifications + +* Consolidate the HttpObjectDecoder default values into public constants +* Reference these constants where possible + +Result + +No functional change. Additional telescoping constructors will be easier +and safer to write. Users may have an easier experience changing single +parameters. +--- + .../netty/handler/codec/http/HttpClientCodec.java | 6 +++++- + .../handler/codec/http/HttpObjectDecoder.java | 15 ++++++++++++--- + .../handler/codec/http/HttpRequestDecoder.java | 7 ++++--- + .../handler/codec/http/HttpResponseDecoder.java | 7 ++++--- + .../netty/handler/codec/http/HttpServerCodec.java | 6 +++++- + .../io/netty/handler/codec/rtsp/RtspDecoder.java | 10 ---------- + .../handler/codec/rtsp/RtspObjectDecoder.java | 4 +++- + 7 files changed, 33 insertions(+), 22 deletions(-) + +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +index da4c440466..a832bfdff3 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +@@ -28,6 +28,10 @@ import java.util.List; + import java.util.Queue; + import java.util.concurrent.atomic.AtomicLong; + ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH; ++ + /** + * A combination of {@link HttpRequestEncoder} and {@link HttpResponseDecoder} + * which enables easier client side HTTP implementation. {@link HttpClientCodec} +@@ -61,7 +65,7 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler +Date: Mon, 4 Feb 2019 22:55:07 +0800 +Subject: [PATCH] use checkPositive/checkPositiveOrZero (#8835) + +Motivation: + +We can replace some "hand-rolled" integer checks with our own static utility method to simplify the code. + +Modifications: + +Use methods provided by `ObjectUtil`. + +Result: + +Cleaner code and less duplication +--- + .../handler/codec/dns/AbstractDnsRecord.java | 5 ++--- + .../codec/http/DefaultHttpHeaders.java | 3 +-- + .../handler/codec/http/HttpObjectDecoder.java | 21 ++++++------------- + .../codec/http/HttpResponseStatus.java | 7 +++---- + .../netty/handler/codec/http/HttpVersion.java | 10 ++++----- + .../multipart/AbstractMemoryHttpData.java | 3 +-- + .../codec/spdy/DefaultSpdyGoAwayFrame.java | 7 +++---- + .../codec/spdy/DefaultSpdyStreamFrame.java | 7 +++---- + .../codec/spdy/DefaultSpdySynReplyFrame.java | 3 +-- + .../codec/spdy/DefaultSpdySynStreamFrame.java | 8 +++---- + .../spdy/DefaultSpdyWindowUpdateFrame.java | 14 +++++-------- + .../handler/codec/spdy/SpdyFrameDecoder.java | 7 +++---- + .../handler/codec/spdy/SpdyHttpDecoder.java | 6 ++---- + .../codec/spdy/SpdySessionHandler.java | 19 ++++++++--------- + .../http2/DefaultHttp2ConnectionEncoder.java | 5 ++--- + .../codec/http2/DefaultHttp2FrameWriter.java | 14 +++++-------- + .../codec/http2/DefaultHttp2GoAwayFrame.java | 6 +++--- + .../DefaultHttp2LocalFlowController.java | 5 ++--- + .../DefaultHttp2RemoteFlowController.java | 5 ++--- + .../DelegatingDecompressorFrameListener.java | 5 ++--- + .../http2/UniformStreamByteDistributor.java | 5 ++--- + .../WeightedFairQueueByteDistributor.java | 11 +++++----- + .../binary/AbstractBinaryMemcacheDecoder.java | 6 +++--- + .../codec/stomp/StompSubframeDecoder.java | 13 +++--------- + 24 files changed, 75 insertions(+), 120 deletions(-) + +diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java +index 28b92c27f9..2ba6e573a7 100644 +--- a/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java ++++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/AbstractDnsRecord.java +@@ -21,6 +21,7 @@ import io.netty.util.internal.UnstableApi; + import java.net.IDN; + + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + + /** + * A skeletal implementation of {@link DnsRecord}. +@@ -62,9 +63,7 @@ public abstract class AbstractDnsRecord implements DnsRecord { + * @param timeToLive the TTL value of the record + */ + protected AbstractDnsRecord(String name, DnsRecordType type, int dnsClass, long timeToLive) { +- if (timeToLive < 0) { +- throw new IllegalArgumentException("timeToLive: " + timeToLive + " (expected: >= 0)"); +- } ++ checkPositiveOrZero(timeToLive, "timeToLive"); + // Convert to ASCII which will also check that the length is not too big. + // See: + // - https://github.com/netty/netty/issues/4937 +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java +index 6204f3ea7f..d18f196e8f 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java +@@ -341,8 +341,7 @@ public class DefaultHttpHeaders extends HttpHeaders { + default: + // Check to see if the character is not an ASCII character, or invalid + if (value < 0) { +- throw new IllegalArgumentException("a header name cannot contain non-ASCII character: " + +- value); ++ throw new IllegalArgumentException("a header name cannot contain non-ASCII character: " + value); + } + } + } +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java +index d4caf29c6d..ed7caa7801 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.http; + ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++ + import io.netty.buffer.ByteBuf; + import io.netty.buffer.Unpooled; + import io.netty.channel.ChannelHandlerContext; +@@ -177,21 +179,10 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + protected HttpObjectDecoder( + int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, + boolean chunkedSupported, boolean validateHeaders, int initialBufferSize) { +- if (maxInitialLineLength <= 0) { +- throw new IllegalArgumentException( +- "maxInitialLineLength must be a positive integer: " + +- maxInitialLineLength); +- } +- if (maxHeaderSize <= 0) { +- throw new IllegalArgumentException( +- "maxHeaderSize must be a positive integer: " + +- maxHeaderSize); +- } +- if (maxChunkSize <= 0) { +- throw new IllegalArgumentException( +- "maxChunkSize must be a positive integer: " + +- maxChunkSize); +- } ++ checkPositive(maxInitialLineLength, "maxInitialLineLength"); ++ checkPositive(maxHeaderSize, "maxHeaderSize"); ++ checkPositive(maxChunkSize, "maxChunkSize"); ++ + AppendableCharSequence seq = new AppendableCharSequence(initialBufferSize); + lineParser = new LineParser(seq, maxInitialLineLength); + headerParser = new HeaderParser(seq, maxHeaderSize); +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java +index 026866ebcc..9f24e0d3cc 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java +@@ -22,6 +22,8 @@ import io.netty.util.AsciiString; + import io.netty.util.ByteProcessor; + import io.netty.util.CharsetUtil; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + /** + * The response code and its description of HTTP or its derived protocols, such as + * RTSP and +@@ -577,10 +579,7 @@ public class HttpResponseStatus implements Comparable { + } + + private HttpResponseStatus(int code, String reasonPhrase, boolean bytes) { +- if (code < 0) { +- throw new IllegalArgumentException( +- "code: " + code + " (expected: 0+)"); +- } ++ checkPositiveOrZero(code, "code"); + + if (reasonPhrase == null) { + throw new NullPointerException("reasonPhrase"); +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java +index a643f42458..7ba40eed90 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.http; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.buffer.ByteBuf; + import io.netty.util.CharsetUtil; + +@@ -165,12 +167,8 @@ public class HttpVersion implements Comparable { + } + } + +- if (majorVersion < 0) { +- throw new IllegalArgumentException("negative majorVersion"); +- } +- if (minorVersion < 0) { +- throw new IllegalArgumentException("negative minorVersion"); +- } ++ checkPositiveOrZero(majorVersion, "majorVersion"); ++ checkPositiveOrZero(minorVersion, "minorVersion"); + + this.protocolName = protocolName; + this.majorVersion = majorVersion; +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java +index 31aa9ce64b..4cb7e567b2 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java +@@ -128,8 +128,7 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData { + } + long newsize = file.length(); + if (newsize > Integer.MAX_VALUE) { +- throw new IllegalArgumentException( +- "File too big to be loaded in memory"); ++ throw new IllegalArgumentException("File too big to be loaded in memory"); + } + checkSize(newsize); + FileInputStream inputStream = new FileInputStream(file); +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java +index 4d88875a6e..79c21f2404 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyGoAwayFrame.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.spdy; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.util.internal.StringUtil; + + /** +@@ -62,10 +64,7 @@ public class DefaultSpdyGoAwayFrame implements SpdyGoAwayFrame { + + @Override + public SpdyGoAwayFrame setLastGoodStreamId(int lastGoodStreamId) { +- if (lastGoodStreamId < 0) { +- throw new IllegalArgumentException("Last-good-stream-ID" +- + " cannot be negative: " + lastGoodStreamId); +- } ++ checkPositiveOrZero(lastGoodStreamId, "lastGoodStreamId"); + this.lastGoodStreamId = lastGoodStreamId; + return this; + } +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java +index 4618d4d4a9..487844ecd9 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyStreamFrame.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.spdy; + ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++ + /** + * The default {@link SpdyStreamFrame} implementation. + */ +@@ -39,10 +41,7 @@ public abstract class DefaultSpdyStreamFrame implements SpdyStreamFrame { + + @Override + public SpdyStreamFrame setStreamId(int streamId) { +- if (streamId <= 0) { +- throw new IllegalArgumentException( +- "Stream-ID must be positive: " + streamId); +- } ++ checkPositive(streamId, "streamId"); + this.streamId = streamId; + return this; + } +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java +index 7efc905641..f757d1dbd6 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynReplyFrame.java +@@ -20,8 +20,7 @@ import io.netty.util.internal.StringUtil; + /** + * The default {@link SpdySynReplyFrame} implementation. + */ +-public class DefaultSpdySynReplyFrame extends DefaultSpdyHeadersFrame +- implements SpdySynReplyFrame { ++public class DefaultSpdySynReplyFrame extends DefaultSpdyHeadersFrame implements SpdySynReplyFrame { + + /** + * Creates a new instance. +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java +index f8adc1c5f1..46fe301636 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdySynStreamFrame.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.spdy; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.util.internal.StringUtil; + + /** +@@ -77,11 +79,7 @@ public class DefaultSpdySynStreamFrame extends DefaultSpdyHeadersFrame + + @Override + public SpdySynStreamFrame setAssociatedStreamId(int associatedStreamId) { +- if (associatedStreamId < 0) { +- throw new IllegalArgumentException( +- "Associated-To-Stream-ID cannot be negative: " + +- associatedStreamId); +- } ++ checkPositiveOrZero(associatedStreamId, "associatedStreamId"); + this.associatedStreamId = associatedStreamId; + return this; + } +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java +index f14611bac6..22b0406c80 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/DefaultSpdyWindowUpdateFrame.java +@@ -15,6 +15,9 @@ + */ + package io.netty.handler.codec.spdy; + ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.util.internal.StringUtil; + + /** +@@ -43,10 +46,7 @@ public class DefaultSpdyWindowUpdateFrame implements SpdyWindowUpdateFrame { + + @Override + public SpdyWindowUpdateFrame setStreamId(int streamId) { +- if (streamId < 0) { +- throw new IllegalArgumentException( +- "Stream-ID cannot be negative: " + streamId); +- } ++ checkPositiveOrZero(streamId, "streamId"); + this.streamId = streamId; + return this; + } +@@ -58,11 +58,7 @@ public class DefaultSpdyWindowUpdateFrame implements SpdyWindowUpdateFrame { + + @Override + public SpdyWindowUpdateFrame setDeltaWindowSize(int deltaWindowSize) { +- if (deltaWindowSize <= 0) { +- throw new IllegalArgumentException( +- "Delta-Window-Size must be positive: " + +- deltaWindowSize); +- } ++ checkPositive(deltaWindowSize, "deltaWindowSize"); + this.deltaWindowSize = deltaWindowSize; + return this; + } +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java +index e0d1112813..fc432b6830 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyFrameDecoder.java +@@ -38,6 +38,8 @@ import static io.netty.handler.codec.spdy.SpdyCodecUtil.getSignedInt; + import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedInt; + import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedMedium; + import static io.netty.handler.codec.spdy.SpdyCodecUtil.getUnsignedShort; ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++ + import io.netty.buffer.ByteBuf; + import io.netty.buffer.Unpooled; + +@@ -95,10 +97,7 @@ public class SpdyFrameDecoder { + if (delegate == null) { + throw new NullPointerException("delegate"); + } +- if (maxChunkSize <= 0) { +- throw new IllegalArgumentException( +- "maxChunkSize must be a positive integer: " + maxChunkSize); +- } ++ checkPositive(maxChunkSize, "maxChunkSize"); + this.spdyVersion = spdyVersion.getVersion(); + this.delegate = delegate; + this.maxChunkSize = maxChunkSize; +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java +index 366ad15b66..5e16a6f4f2 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyHttpDecoder.java +@@ -38,6 +38,7 @@ import java.util.List; + import java.util.Map; + + import static io.netty.handler.codec.spdy.SpdyHeaders.HttpNames.*; ++import static io.netty.util.internal.ObjectUtil.checkPositive; + + /** + * Decodes {@link SpdySynStreamFrame}s, {@link SpdySynReplyFrame}s, +@@ -103,10 +104,7 @@ public class SpdyHttpDecoder extends MessageToMessageDecoder { + if (version == null) { + throw new NullPointerException("version"); + } +- if (maxContentLength <= 0) { +- throw new IllegalArgumentException( +- "maxContentLength must be a positive integer: " + maxContentLength); +- } ++ checkPositive(maxContentLength, "maxContentLength"); + spdyVersion = version.getVersion(); + this.maxContentLength = maxContentLength; + this.messageMap = messageMap; +diff --git a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java +index 394f6c2e9a..8f90864151 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/spdy/SpdySessionHandler.java +@@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicInteger; + + import static io.netty.handler.codec.spdy.SpdyCodecUtil.SPDY_SESSION_STREAM_ID; + import static io.netty.handler.codec.spdy.SpdyCodecUtil.isServerId; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + + /** + * Manages streams within a SPDY session. +@@ -77,16 +78,14 @@ public class SpdySessionHandler extends ChannelDuplexHandler { + } + + public void setSessionReceiveWindowSize(int sessionReceiveWindowSize) { +- if (sessionReceiveWindowSize < 0) { +- throw new IllegalArgumentException("sessionReceiveWindowSize"); +- } +- // This will not send a window update frame immediately. +- // If this value increases the allowed receive window size, +- // a WINDOW_UPDATE frame will be sent when only half of the +- // session window size remains during data frame processing. +- // If this value decreases the allowed receive window size, +- // the window will be reduced as data frames are processed. +- initialSessionReceiveWindowSize = sessionReceiveWindowSize; ++ checkPositiveOrZero(sessionReceiveWindowSize, "sessionReceiveWindowSize"); ++ // This will not send a window update frame immediately. ++ // If this value increases the allowed receive window size, ++ // a WINDOW_UPDATE frame will be sent when only half of the ++ // session window size remains during data frame processing. ++ // If this value decreases the allowed receive window size, ++ // the window will be reduced as data frames are processed. ++ initialSessionReceiveWindowSize = sessionReceiveWindowSize; + } + + @Override +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +index f0af13b394..18375db76a 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +@@ -29,6 +29,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGH + import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.connectionError; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import static java.lang.Integer.MAX_VALUE; + import static java.lang.Math.min; + +@@ -485,9 +486,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + + FlowControlledBase(final Http2Stream stream, int padding, boolean endOfStream, + final ChannelPromise promise) { +- if (padding < 0) { +- throw new IllegalArgumentException("padding must be >= 0"); +- } ++ checkPositiveOrZero(padding, "padding"); + this.padding = padding; + this.endOfStream = endOfStream; + this.stream = stream; +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java +index c7277561d6..77270f8343 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java +@@ -61,6 +61,8 @@ 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.WINDOW_UPDATE; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import static java.lang.Math.max; + import static java.lang.Math.min; + +@@ -547,15 +549,11 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize + } + + private static void verifyStreamId(int streamId, String argumentName) { +- if (streamId <= 0) { +- throw new IllegalArgumentException(argumentName + " must be > 0"); +- } ++ checkPositive(streamId, "streamId"); + } + + private static void verifyStreamOrConnectionId(int streamId, String argumentName) { +- if (streamId < 0) { +- throw new IllegalArgumentException(argumentName + " must be >= 0"); +- } ++ checkPositiveOrZero(streamId, "streamId"); + } + + private static void verifyWeight(short weight) { +@@ -571,9 +569,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize + } + + private static void verifyWindowSizeIncrement(int windowSizeIncrement) { +- if (windowSizeIncrement < 0) { +- throw new IllegalArgumentException("WindowSizeIncrement must be >= 0"); +- } ++ checkPositiveOrZero(windowSizeIncrement, "windowSizeIncrement"); + } + + private static void verifyPingPayload(ByteBuf data) { +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java +index 8f54b8e329..dc01f37482 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2GoAwayFrame.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.http2; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.buffer.ByteBuf; + import io.netty.buffer.DefaultByteBufHolder; + import io.netty.buffer.Unpooled; +@@ -97,9 +99,7 @@ public final class DefaultHttp2GoAwayFrame extends DefaultByteBufHolder implemen + + @Override + public Http2GoAwayFrame setExtraStreamIds(int extraStreamIds) { +- if (extraStreamIds < 0) { +- throw new IllegalArgumentException("extraStreamIds must be non-negative"); +- } ++ checkPositiveOrZero(extraStreamIds, "extraStreamIds"); + this.extraStreamIds = extraStreamIds; + return this; + } +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java +index 74dc3ae31c..cac715614d 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2LocalFlowController.java +@@ -24,6 +24,7 @@ import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.connectionError; + import static io.netty.handler.codec.http2.Http2Exception.streamError; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import static java.lang.Math.max; + import static java.lang.Math.min; + import io.netty.buffer.ByteBuf; +@@ -173,9 +174,7 @@ public class DefaultHttp2LocalFlowController implements Http2LocalFlowController + @Override + public boolean consumeBytes(Http2Stream stream, int numBytes) throws Http2Exception { + assert ctx != null && ctx.executor().inEventLoop(); +- if (numBytes < 0) { +- throw new IllegalArgumentException("numBytes must not be negative"); +- } ++ checkPositiveOrZero(numBytes, "numBytes"); + if (numBytes == 0) { + return false; + } +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java +index 034140c81f..125a394cae 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java +@@ -32,6 +32,7 @@ import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.streamError; + import static io.netty.handler.codec.http2.Http2Stream.State.HALF_CLOSED_LOCAL; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import static java.lang.Math.max; + import static java.lang.Math.min; + +@@ -652,9 +653,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll + } + + void initialWindowSize(int newWindowSize) throws Http2Exception { +- if (newWindowSize < 0) { +- throw new IllegalArgumentException("Invalid initial window size: " + newWindowSize); +- } ++ checkPositiveOrZero(newWindowSize, "newWindowSize"); + + final int delta = newWindowSize - initialWindowSize; + initialWindowSize = newWindowSize; +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java +index 78ef230c62..3e73bd68dd 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DelegatingDecompressorFrameListener.java +@@ -33,6 +33,7 @@ import static io.netty.handler.codec.http.HttpHeaderValues.X_GZIP; + import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.streamError; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + + /** + * A HTTP2 frame listener that will decompress data frames according to the {@code content-encoding} header for each +@@ -398,9 +399,7 @@ public class DelegatingDecompressorFrameListener extends Http2FrameListenerDecor + * @return The number of pre-decompressed bytes that have been consumed. + */ + int consumeBytes(int streamId, int decompressedBytes) throws Http2Exception { +- if (decompressedBytes < 0) { +- throw new IllegalArgumentException("decompressedBytes must not be negative: " + decompressedBytes); +- } ++ checkPositiveOrZero(decompressedBytes, "decompressedBytes"); + if (decompressed - decompressedBytes < 0) { + throw streamError(streamId, INTERNAL_ERROR, + "Attempting to return too many bytes for stream %d. decompressed: %d " + +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java +index c3e5e2faaa..6204c7bb9c 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/UniformStreamByteDistributor.java +@@ -24,6 +24,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.streamableBytes; + import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.connectionError; + import static io.netty.util.internal.ObjectUtil.checkNotNull; ++import static io.netty.util.internal.ObjectUtil.checkPositive; + import static java.lang.Math.max; + import static java.lang.Math.min; + +@@ -72,9 +73,7 @@ public final class UniformStreamByteDistributor implements StreamByteDistributor + * Must be > 0. + */ + public void minAllocationChunk(int minAllocationChunk) { +- if (minAllocationChunk <= 0) { +- throw new IllegalArgumentException("minAllocationChunk must be > 0"); +- } ++ checkPositive(minAllocationChunk, "minAllocationChunk"); + this.minAllocationChunk = minAllocationChunk; + } + +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java +index c215376c72..d26c088c62 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/WeightedFairQueueByteDistributor.java +@@ -36,6 +36,8 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGH + import static io.netty.handler.codec.http2.Http2CodecUtil.streamableBytes; + import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.connectionError; ++import static io.netty.util.internal.ObjectUtil.checkPositive; ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; + import static java.lang.Integer.MAX_VALUE; + import static java.lang.Math.max; + import static java.lang.Math.min; +@@ -95,9 +97,8 @@ public final class WeightedFairQueueByteDistributor implements StreamByteDistrib + } + + public WeightedFairQueueByteDistributor(Http2Connection connection, int maxStateOnlySize) { +- if (maxStateOnlySize < 0) { +- throw new IllegalArgumentException("maxStateOnlySize: " + maxStateOnlySize + " (expected: >0)"); +- } else if (maxStateOnlySize == 0) { ++ checkPositiveOrZero(maxStateOnlySize, "maxStateOnlySize"); ++ if (maxStateOnlySize == 0) { + stateOnlyMap = IntCollections.emptyMap(); + stateOnlyRemovalQueue = EmptyPriorityQueue.instance(); + } else { +@@ -280,9 +281,7 @@ public final class WeightedFairQueueByteDistributor implements StreamByteDistrib + * @param allocationQuantum the amount of bytes that will be allocated to each stream. Must be > 0. + */ + public void allocationQuantum(int allocationQuantum) { +- if (allocationQuantum <= 0) { +- throw new IllegalArgumentException("allocationQuantum must be > 0"); +- } ++ checkPositive(allocationQuantum, "allocationQuantum"); + this.allocationQuantum = allocationQuantum; + } + +diff --git a/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java b/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java +index 2c90382829..bec754afbd 100644 +--- a/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java ++++ b/codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheDecoder.java +@@ -15,6 +15,8 @@ + */ + package io.netty.handler.codec.memcache.binary; + ++import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; ++ + import io.netty.buffer.ByteBuf; + import io.netty.buffer.Unpooled; + import io.netty.channel.ChannelHandlerContext; +@@ -59,9 +61,7 @@ public abstract class AbstractBinaryMemcacheDecoder { + + public StompSubframeDecoder(int maxLineLength, int maxChunkSize) { + super(State.SKIP_CONTROL_CHARACTERS); +- if (maxLineLength <= 0) { +- throw new IllegalArgumentException( +- "maxLineLength must be a positive integer: " + +- maxLineLength); +- } +- if (maxChunkSize <= 0) { +- throw new IllegalArgumentException( +- "maxChunkSize must be a positive integer: " + +- maxChunkSize); +- } ++ checkPositive(maxLineLength, "maxLineLength"); ++ checkPositive(maxChunkSize, "maxChunkSize"); + this.maxChunkSize = maxChunkSize; + this.maxLineLength = maxLineLength; + } +-- +2.23.0 + diff --git a/CVE-2021-21295-pre3.patch b/CVE-2021-21295-pre3.patch new file mode 100644 index 0000000..2c002e8 --- /dev/null +++ b/CVE-2021-21295-pre3.patch @@ -0,0 +1,566 @@ +From 9557c88da2aaa49b3c3fd7525462dc0694681c19 Mon Sep 17 00:00:00 2001 +From: Bennett Lynch +Date: Mon, 6 Jul 2020 01:25:13 -0700 +Subject: [PATCH] Add option to HttpObjectDecoder to allow duplicate + Content-Lengths (#10349) + +Motivation: + +Since https://github.com/netty/netty/pull/9865 (Netty 4.1.44) the +default behavior of the HttpObjectDecoder has been to reject any HTTP +message that is found to have multiple Content-Length headers when +decoding. This behavior is well-justified as per the risks outlined in +https://github.com/netty/netty/issues/9861, however, we can see from the +cited RFC section that there are multiple possible options offered for +responding to this scenario: + +> If a message is received that has multiple Content-Length header +> fields with field-values consisting of the same decimal value, or a +> single Content-Length header field with a field value containing a +> list of identical decimal values (e.g., "Content-Length: 42, 42"), +> indicating that duplicate Content-Length header fields have been +> generated or combined by an upstream message processor, then the +> recipient MUST either reject the message as invalid or replace the +> duplicated field-values with a single valid Content-Length field +> containing that decimal value prior to determining the message body +> length or forwarding the message. + +https://tools.ietf.org/html/rfc7230#section-3.3.2 + +Netty opted for the first option (rejecting as invalid), which seems +like the safest, but the second option (replacing duplicate values with +a single value) is also valid behavior. + +Modifications: + +* Introduce "allowDuplicateContentLengths" parameter to +HttpObjectDecoder (defaulting to false). +* When set to true, will allow multiple Content-Length headers only if +they are all the same value. The duplicated field-values will be +replaced with a single valid Content-Length field. +* Add new parameterized test class for testing different variations of +multiple Content-Length headers. + +Result: + +This is a backwards-compatible change with no functional change to the +existing behavior. + +Note that the existing logic would result in NumberFormatExceptions +for header values like "Content-Length: 42, 42". The new logic correctly +reports these as IllegalArgumentException with the proper error message. + +Additionally note that this behavior is only applied to HTTP/1.1, but I +suspect that we may want to expand that to include HTTP/1.0 as well... +That behavior is not modified here to minimize the scope of this change. +--- + .../handler/codec/http/HttpClientCodec.java | 35 ++++- + .../handler/codec/http/HttpObjectDecoder.java | 75 ++++++++- + .../codec/http/HttpRequestDecoder.java | 7 + + .../codec/http/HttpResponseDecoder.java | 7 + + .../handler/codec/http/HttpServerCodec.java | 16 ++ + .../MultipleContentLengthHeadersTest.java | 144 ++++++++++++++++++ + 6 files changed, 268 insertions(+), 16 deletions(-) + create mode 100644 codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java + +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +index a832bfdff3..9a99fff97f 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +@@ -28,9 +28,11 @@ import java.util.List; + import java.util.Queue; + import java.util.concurrent.atomic.AtomicLong; + ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS; + import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE; + import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE; + import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_VALIDATE_HEADERS; + + /** + * A combination of {@link HttpRequestEncoder} and {@link HttpResponseDecoder} +@@ -48,6 +50,8 @@ import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_ + */ + public final class HttpClientCodec extends CombinedChannelDuplexHandler + implements HttpClientUpgradeHandler.SourceCodec { ++ public static final boolean DEFAULT_FAIL_ON_MISSING_RESPONSE = false; ++ public static final boolean DEFAULT_PARSE_HTTP_AFTER_CONNECT_REQUEST = false; + + /** A queue that is used for correlating a request and a response. */ + private final Queue queue = new ArrayDeque(); +@@ -65,14 +69,15 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandlerParameters that prevents excessive memory consumption + * + * +- * ++ * + * + * + * ++ * + * + * + * ++ * + * + * + * + * ++ * + * + *
NameMeaningNameDefault valueMeaning
{@code maxInitialLineLength}{@value #DEFAULT_MAX_INITIAL_LINE_LENGTH}The maximum length of the initial line + * (e.g. {@code "GET / HTTP/1.0"} or {@code "HTTP/1.0 200 OK"}) + * If the length of the initial line exceeds this value, a +@@ -48,11 +51,13 @@ import java.util.List; + *
{@code maxHeaderSize}{@value #DEFAULT_MAX_HEADER_SIZE}The maximum length of all headers. If the sum of the length of each + * header exceeds this value, a {@link TooLongFrameException} will be raised.
{@code maxChunkSize}{@value #DEFAULT_MAX_CHUNK_SIZE}The maximum length of the content or each chunk. If the content length + * (or the length of each chunk) exceeds this value, the content or chunk + * will be split into multiple {@link HttpContent}s whose length is +@@ -60,6 +65,21 @@ import java.util.List; + *
+ * ++ *

Parameters that control parsing behavior

++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ * ++ *
NameDefault valueMeaning
{@code allowDuplicateContentLengths}{@value #DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS}When set to {@code false}, will reject any messages that contain multiple Content-Length header fields. ++ * When set to {@code true}, will allow multiple Content-Length headers only if they are all the same decimal value. ++ * The duplicated field-values will be replaced with a single valid Content-Length field. ++ * See RFC 7230, Section 3.3.2.
++ * + *

Chunked Content

+ * + * If the content of an HTTP message is greater than {@code maxChunkSize} or +@@ -108,12 +128,15 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + public static final int DEFAULT_MAX_CHUNK_SIZE = 8192; + public static final boolean DEFAULT_VALIDATE_HEADERS = true; + public static final int DEFAULT_INITIAL_BUFFER_SIZE = 128; ++ public static final boolean DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS = false; + + private static final String EMPTY_VALUE = ""; ++ private static final Pattern COMMA_PATTERN = Pattern.compile(","); + + private final int maxChunkSize; + private final boolean chunkedSupported; + protected final boolean validateHeaders; ++ private final boolean allowDuplicateContentLengths; + private final HeaderParser headerParser; + private final LineParser lineParser; + +@@ -176,9 +199,20 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + DEFAULT_INITIAL_BUFFER_SIZE); + } + ++ /** ++ * Creates a new instance with the specified parameters. ++ */ + protected HttpObjectDecoder( + int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, + boolean chunkedSupported, boolean validateHeaders, int initialBufferSize) { ++ this(maxInitialLineLength, maxHeaderSize, maxChunkSize, chunkedSupported, validateHeaders, initialBufferSize, ++ DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS); ++ } ++ ++ protected HttpObjectDecoder( ++ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, ++ boolean chunkedSupported, boolean validateHeaders, int initialBufferSize, ++ boolean allowDuplicateContentLengths) { + checkPositive(maxInitialLineLength, "maxInitialLineLength"); + checkPositive(maxHeaderSize, "maxHeaderSize"); + checkPositive(maxChunkSize, "maxChunkSize"); +@@ -189,6 +223,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + this.maxChunkSize = maxChunkSize; + this.chunkedSupported = chunkedSupported; + this.validateHeaders = validateHeaders; ++ this.allowDuplicateContentLengths = allowDuplicateContentLengths; + } + + @Override +@@ -602,10 +637,9 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + name = null; + value = null; + +- List values = headers.getAll(HttpHeaderNames.CONTENT_LENGTH); +- int contentLengthValuesCount = values.size(); ++ List contentLengthFields = headers.getAll(HttpHeaderNames.CONTENT_LENGTH); + +- if (contentLengthValuesCount > 0) { ++ if (!contentLengthFields.isEmpty()) { + // Guard against multiple Content-Length headers as stated in + // https://tools.ietf.org/html/rfc7230#section-3.3.2: + // +@@ -619,17 +653,42 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + // duplicated field-values with a single valid Content-Length field + // containing that decimal value prior to determining the message body + // length or forwarding the message. +- if (contentLengthValuesCount > 1 && message.protocolVersion() == HttpVersion.HTTP_1_1) { +- throw new IllegalArgumentException("Multiple Content-Length headers found"); ++ boolean multipleContentLengths = ++ contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0; ++ if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) { ++ if (allowDuplicateContentLengths) { ++ // Find and enforce that all Content-Length values are the same ++ String firstValue = null; ++ for (String field : contentLengthFields) { ++ String[] tokens = COMMA_PATTERN.split(field, -1); ++ for (String token : tokens) { ++ String trimmed = token.trim(); ++ if (firstValue == null) { ++ firstValue = trimmed; ++ } else if (!trimmed.equals(firstValue)) { ++ throw new IllegalArgumentException( ++ "Multiple Content-Length values found: " + contentLengthFields); ++ } ++ } ++ } ++ // Replace the duplicated field-values with a single valid Content-Length field ++ headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue); ++ contentLength = Long.parseLong(firstValue); ++ } else { ++ // Reject the message as invalid ++ throw new IllegalArgumentException( ++ "Multiple Content-Length values found: " + contentLengthFields); ++ } ++ } else { ++ contentLength = Long.parseLong(contentLengthFields.get(0)); + } +- contentLength = Long.parseLong(values.get(0)); + } + + if (isContentAlwaysEmpty(message)) { + HttpUtil.setTransferEncodingChunked(message, false); + return State.SKIP_CONTROL_CHARS; + } else if (HttpUtil.isTransferEncodingChunked(message)) { +- if (contentLengthValuesCount > 0 && message.protocolVersion() == HttpVersion.HTTP_1_1) { ++ if (!contentLengthFields.isEmpty() && message.protocolVersion() == HttpVersion.HTTP_1_1) { + handleTransferEncodingChunkedWithContentLength(message); + } + return State.READ_CHUNK_SIZE; +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java +index 70c1db5540..ba2d79ecb4 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpRequestDecoder.java +@@ -82,6 +82,13 @@ public class HttpRequestDecoder extends HttpObjectDecoder { + initialBufferSize); + } + ++ public HttpRequestDecoder( ++ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders, ++ int initialBufferSize, boolean allowDuplicateContentLengths) { ++ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders, ++ initialBufferSize, allowDuplicateContentLengths); ++ } ++ + @Override + protected HttpMessage createMessage(String[] initialLine) throws Exception { + return new DefaultHttpRequest( +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java +index 39d4d6a5ad..62f6dd3554 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java +@@ -113,6 +113,13 @@ public class HttpResponseDecoder extends HttpObjectDecoder { + initialBufferSize); + } + ++ public HttpResponseDecoder( ++ int maxInitialLineLength, int maxHeaderSize, int maxChunkSize, boolean validateHeaders, ++ int initialBufferSize, boolean allowDuplicateContentLengths) { ++ super(maxInitialLineLength, maxHeaderSize, maxChunkSize, DEFAULT_CHUNKED_SUPPORTED, validateHeaders, ++ initialBufferSize, allowDuplicateContentLengths); ++ } ++ + @Override + protected HttpMessage createMessage(String[] initialLine) { + return new DefaultHttpResponse( +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java +index 8ae6295cf7..b2b905e083 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerCodec.java +@@ -75,6 +75,16 @@ public final class HttpServerCodec extends CombinedChannelDuplexHandler out) throws Exception { + int oldSize = out.size(); +diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java +new file mode 100644 +index 0000000000..29c7d84b71 +--- /dev/null ++++ b/codec-http/src/test/java/io/netty/handler/codec/http/MultipleContentLengthHeadersTest.java +@@ -0,0 +1,144 @@ ++/* ++ * Copyright 2020 The Netty Project ++ * ++ * The Netty Project licenses this file to you under the Apache License, ++ * version 2.0 (the "License"); you may not use this file except in compliance ++ * with the License. You may obtain a copy of the License at: ++ * ++ * http://www.apache.org/licenses/LICENSE-2.0 ++ * ++ * Unless required by applicable law or agreed to in writing, software ++ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT ++ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the ++ * License for the specific language governing permissions and limitations ++ * under the License. ++ */ ++package io.netty.handler.codec.http; ++ ++import io.netty.buffer.Unpooled; ++import io.netty.channel.embedded.EmbeddedChannel; ++import io.netty.util.CharsetUtil; ++import org.junit.Before; ++import org.junit.Test; ++import org.junit.runner.RunWith; ++import org.junit.runners.Parameterized; ++import org.junit.runners.Parameterized.Parameters; ++ ++import java.util.Arrays; ++import java.util.Collection; ++import java.util.List; ++ ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_INITIAL_BUFFER_SIZE; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_CHUNK_SIZE; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH; ++import static io.netty.handler.codec.http.HttpObjectDecoder.DEFAULT_VALIDATE_HEADERS; ++import static org.hamcrest.MatcherAssert.assertThat; ++import static org.hamcrest.Matchers.contains; ++import static org.hamcrest.Matchers.containsString; ++import static org.hamcrest.Matchers.is; ++import static org.hamcrest.core.IsInstanceOf.instanceOf; ++ ++@RunWith(Parameterized.class) ++public class MultipleContentLengthHeadersTest { ++ ++ private final boolean allowDuplicateContentLengths; ++ private final boolean sameValue; ++ private final boolean singleField; ++ ++ private EmbeddedChannel channel; ++ ++ @Parameters ++ public static Collection parameters() { ++ return Arrays.asList(new Object[][] { ++ { false, false, false }, ++ { false, false, true }, ++ { false, true, false }, ++ { false, true, true }, ++ { true, false, false }, ++ { true, false, true }, ++ { true, true, false }, ++ { true, true, true } ++ }); ++ } ++ ++ public MultipleContentLengthHeadersTest( ++ boolean allowDuplicateContentLengths, boolean sameValue, boolean singleField) { ++ this.allowDuplicateContentLengths = allowDuplicateContentLengths; ++ this.sameValue = sameValue; ++ this.singleField = singleField; ++ } ++ ++ @Before ++ public void setUp() { ++ HttpRequestDecoder decoder = new HttpRequestDecoder( ++ DEFAULT_MAX_INITIAL_LINE_LENGTH, ++ DEFAULT_MAX_HEADER_SIZE, ++ DEFAULT_MAX_CHUNK_SIZE, ++ DEFAULT_VALIDATE_HEADERS, ++ DEFAULT_INITIAL_BUFFER_SIZE, ++ allowDuplicateContentLengths); ++ channel = new EmbeddedChannel(decoder); ++ } ++ ++ @Test ++ public void testMultipleContentLengthHeadersBehavior() { ++ String requestStr = setupRequestString(); ++ assertThat(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)), is(true)); ++ HttpRequest request = channel.readInbound(); ++ ++ if (allowDuplicateContentLengths) { ++ if (sameValue) { ++ assertValid(request); ++ List contentLengths = request.headers().getAll(HttpHeaderNames.CONTENT_LENGTH); ++ assertThat(contentLengths, contains("1")); ++ LastHttpContent body = channel.readInbound(); ++ assertThat(body.content().readableBytes(), is(1)); ++ assertThat(body.content().readCharSequence(1, CharsetUtil.US_ASCII).toString(), is("a")); ++ } else { ++ assertInvalid(request); ++ } ++ } else { ++ assertInvalid(request); ++ } ++ assertThat(channel.finish(), is(false)); ++ } ++ ++ private String setupRequestString() { ++ String firstValue = "1"; ++ String secondValue = sameValue ? firstValue : "2"; ++ String contentLength; ++ if (singleField) { ++ contentLength = "Content-Length: " + firstValue + ", " + secondValue + "\r\n\r\n"; ++ } else { ++ contentLength = "Content-Length: " + firstValue + "\r\n" + ++ "Content-Length: " + secondValue + "\r\n\r\n"; ++ } ++ return "PUT /some/path HTTP/1.1\r\n" + ++ contentLength + ++ "ab"; ++ } ++ ++ @Test ++ public void testDanglingComma() { ++ String requestStr = "GET /some/path HTTP/1.1\r\n" + ++ "Content-Length: 1,\r\n" + ++ "Connection: close\n\n" + ++ "ab"; ++ assertThat(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)), is(true)); ++ HttpRequest request = channel.readInbound(); ++ assertInvalid(request); ++ assertThat(channel.finish(), is(false)); ++ } ++ ++ private static void assertValid(HttpRequest request) { ++ assertThat(request.decoderResult().isFailure(), is(false)); ++ } ++ ++ private static void assertInvalid(HttpRequest request) { ++ assertThat(request.decoderResult().isFailure(), is(true)); ++ assertThat(request.decoderResult().cause(), instanceOf(IllegalArgumentException.class)); ++ assertThat(request.decoderResult().cause().getMessage(), ++ containsString("Multiple Content-Length values found")); ++ } ++} +-- +2.23.0 + diff --git a/CVE-2021-21295-pre4.patch b/CVE-2021-21295-pre4.patch new file mode 100644 index 0000000..fb65aea --- /dev/null +++ b/CVE-2021-21295-pre4.patch @@ -0,0 +1,771 @@ +From a91df58ca17d5b30c57c46dde5b1d60bb659b029 Mon Sep 17 00:00:00 2001 +From: Scott Mitchell +Date: Tue, 11 Jul 2017 14:53:49 -0700 +Subject: [PATCH] HTTP/2 enforce HTTP message flow + +Motivation: +codec-http2 currently does not strictly enforce the HTTP/1.x semantics with respect to the number of headers defined in RFC 7540 Section 8.1 [1]. We currently don't validate the number of headers nor do we validate that the trailing headers should indicate EOS. + +[1] https://tools.ietf.org/html/rfc7540#section-8.1 + +Modifications: +- DefaultHttp2ConnectionDecoder should only allow decoding of a single headers and a single trailers +- DefaultHttp2ConnectionEncoder should only allow encoding of a single headers and optionally a single trailers + +Result: +Constraints of RFC 7540 restricting the number of headers/trailers is enforced. +--- + .../handler/codec/http/HttpStatusClass.java | 21 ++ + .../codec/http2/DefaultHttp2Connection.java | 52 +++-- + .../http2/DefaultHttp2ConnectionDecoder.java | 11 + + .../http2/DefaultHttp2ConnectionEncoder.java | 18 +- + .../handler/codec/http2/Http2Stream.java | 32 ++- + .../DefaultHttp2ConnectionDecoderTest.java | 103 ++++++++- + .../DefaultHttp2ConnectionEncoderTest.java | 203 +++++++++++++++++- + .../http2/InboundHttp2ToHttpAdapterTest.java | 43 ---- + 8 files changed, 419 insertions(+), 64 deletions(-) + +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java +index 9f57e18984..0a4f4c11ab 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpStatusClass.java +@@ -74,6 +74,27 @@ public enum HttpStatusClass { + return UNKNOWN; + } + ++ /** ++ * Returns the class of the specified HTTP status code. ++ * @param code Just the numeric portion of the http status code. ++ */ ++ public static HttpStatusClass valueOf(CharSequence code) { ++ if (code != null && code.length() == 3) { ++ char c0 = code.charAt(0); ++ return isDigit(c0) && isDigit(code.charAt(1)) && isDigit(code.charAt(2)) ? valueOf(digit(c0) * 100) ++ : UNKNOWN; ++ } ++ return UNKNOWN; ++ } ++ ++ private static int digit(char c) { ++ return c - '0'; ++ } ++ ++ private static boolean isDigit(char c) { ++ return c >= '0' && c <= '9'; ++ } ++ + private final int min; + private final int max; + private final AsciiString defaultReasonPhrase; +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java +index 2789423bc7..12815c225c 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java +@@ -373,13 +373,16 @@ public class DefaultHttp2Connection implements Http2Connection { + * Simple stream implementation. Streams can be compared to each other by priority. + */ + private class DefaultStream implements Http2Stream { +- private static final byte SENT_STATE_RST = 0x1; +- private static final byte SENT_STATE_HEADERS = 0x2; +- private static final byte SENT_STATE_PUSHPROMISE = 0x4; ++ private static final byte META_STATE_SENT_RST = 1; ++ private static final byte META_STATE_SENT_HEADERS = 1 << 1; ++ private static final byte META_STATE_SENT_TRAILERS = 1 << 2; ++ private static final byte META_STATE_SENT_PUSHPROMISE = 1 << 3; ++ private static final byte META_STATE_RECV_HEADERS = 1 << 4; ++ private static final byte META_STATE_RECV_TRAILERS = 1 << 5; + private final int id; + private final PropertyMap properties = new PropertyMap(); + private State state; +- private byte sentState; ++ private byte metaState; + + DefaultStream(int id, State state) { + this.id = id; +@@ -398,35 +401,60 @@ public class DefaultHttp2Connection implements Http2Connection { + + @Override + public boolean isResetSent() { +- return (sentState & SENT_STATE_RST) != 0; ++ return (metaState & META_STATE_SENT_RST) != 0; + } + + @Override + public Http2Stream resetSent() { +- sentState |= SENT_STATE_RST; ++ metaState |= META_STATE_SENT_RST; + return this; + } + + @Override +- public Http2Stream headersSent() { +- sentState |= SENT_STATE_HEADERS; ++ public Http2Stream headersSent(boolean isInformational) { ++ if (!isInformational) { ++ metaState |= isHeadersSent() ? META_STATE_SENT_TRAILERS : META_STATE_SENT_HEADERS; ++ } + return this; + } + + @Override + public boolean isHeadersSent() { +- return (sentState & SENT_STATE_HEADERS) != 0; ++ return (metaState & META_STATE_SENT_HEADERS) != 0; ++ } ++ ++ @Override ++ public boolean isTrailersSent() { ++ return (metaState & META_STATE_SENT_TRAILERS) != 0; ++ } ++ ++ @Override ++ public Http2Stream headersReceived(boolean isInformational) { ++ if (!isInformational) { ++ metaState |= isHeadersReceived() ? META_STATE_RECV_TRAILERS : META_STATE_RECV_HEADERS; ++ } ++ return this; ++ } ++ ++ @Override ++ public boolean isHeadersReceived() { ++ return (metaState & META_STATE_RECV_HEADERS) != 0; ++ } ++ ++ @Override ++ public boolean isTrailersReceived() { ++ return (metaState & META_STATE_RECV_TRAILERS) != 0; + } + + @Override + public Http2Stream pushPromiseSent() { +- sentState |= SENT_STATE_PUSHPROMISE; ++ metaState |= META_STATE_SENT_PUSHPROMISE; + return this; + } + + @Override + public boolean isPushPromiseSent() { +- return (sentState & SENT_STATE_PUSHPROMISE) != 0; ++ return (metaState & META_STATE_SENT_PUSHPROMISE) != 0; + } + + @Override +@@ -599,7 +627,7 @@ public class DefaultHttp2Connection implements Http2Connection { + } + + @Override +- public Http2Stream headersSent() { ++ public Http2Stream headersSent(boolean isInformational) { + throw new UnsupportedOperationException(); + } + +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +index ef643fafad..4027978651 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +@@ -16,6 +16,7 @@ package io.netty.handler.codec.http2; + + import io.netty.buffer.ByteBuf; + import io.netty.channel.ChannelHandlerContext; ++import io.netty.handler.codec.http.HttpStatusClass; + import io.netty.handler.codec.http2.Http2Connection.Endpoint; + import io.netty.util.internal.UnstableApi; + import io.netty.util.internal.logging.InternalLogger; +@@ -23,6 +24,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; + + import java.util.List; + ++import static io.netty.handler.codec.http.HttpStatusClass.INFORMATIONAL; + import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT; + import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR; + import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; +@@ -282,6 +284,14 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + return; + } + ++ boolean isInformational = !connection.isServer() && ++ HttpStatusClass.valueOf(headers.status()) == INFORMATIONAL; ++ if ((isInformational || !endOfStream) && stream.isHeadersReceived() || stream.isTrailersReceived()) { ++ throw streamError(streamId, PROTOCOL_ERROR, ++ "Stream %d received too many headers EOS: %s state: %s", ++ streamId, endOfStream, stream.state()); ++ } ++ + switch (stream.state()) { + case RESERVED_REMOTE: + stream.open(endOfStream); +@@ -305,6 +315,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + stream.state()); + } + ++ stream.headersReceived(isInformational); + encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive); + + listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream); +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +index 18375db76a..f129f63fd8 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java +@@ -21,10 +21,12 @@ import io.netty.channel.ChannelFutureListener; + import io.netty.channel.ChannelHandlerContext; + import io.netty.channel.ChannelPromise; + import io.netty.channel.CoalescingBufferQueue; ++import io.netty.handler.codec.http.HttpStatusClass; + import io.netty.util.internal.UnstableApi; + + import java.util.ArrayDeque; + ++import static io.netty.handler.codec.http.HttpStatusClass.INFORMATIONAL; + import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT; + import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR; + import static io.netty.handler.codec.http2.Http2Exception.connectionError; +@@ -146,6 +148,15 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + return writeHeaders(ctx, streamId, headers, 0, DEFAULT_PRIORITY_WEIGHT, false, padding, endStream, promise); + } + ++ private static boolean validateHeadersSentState(Http2Stream stream, Http2Headers headers, boolean isServer, ++ boolean endOfStream) { ++ boolean isInformational = isServer && HttpStatusClass.valueOf(headers.status()) == INFORMATIONAL; ++ if ((isInformational || !endOfStream) && stream.isHeadersSent() || stream.isTrailersSent()) { ++ throw new IllegalStateException("Stream " + stream.id() + " sent too many headers EOS: " + endOfStream); ++ } ++ return isInformational; ++ } ++ + @Override + public ChannelFuture writeHeaders(final ChannelHandlerContext ctx, final int streamId, + final Http2Headers headers, final int streamDependency, final short weight, +@@ -181,6 +192,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + // for this stream. + Http2RemoteFlowController flowController = flowController(); + if (!endOfStream || !flowController.hasFlowControlled(stream)) { ++ boolean isInformational = validateHeadersSentState(stream, headers, connection.isServer(), endOfStream); + if (endOfStream) { + final Http2Stream finalStream = stream; + final ChannelFutureListener closeStreamLocalListener = new ChannelFutureListener() { +@@ -191,6 +203,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + }; + promise = promise.unvoid().addListener(closeStreamLocalListener); + } ++ + ChannelFuture future = frameWriter.writeHeaders(ctx, streamId, headers, streamDependency, + weight, exclusive, padding, endOfStream, promise); + // Writing headers may fail during the encode state if they violate HPACK limits. +@@ -198,7 +211,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + if (failureCause == null) { + // Synchronously set the headersSent flag to ensure that we do not subsequently write + // other headers containing pseudo-header fields. +- stream.headersSent(); ++ stream.headersSent(isInformational); + } else { + lifecycleManager.onError(ctx, failureCause); + } +@@ -452,6 +465,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + + @Override + public void write(ChannelHandlerContext ctx, int allowedBytes) { ++ boolean isInformational = validateHeadersSentState(stream, headers, connection.isServer(), endOfStream); + if (promise.isVoid()) { + promise = ctx.newPromise(); + } +@@ -462,7 +476,7 @@ public class DefaultHttp2ConnectionEncoder implements Http2ConnectionEncoder { + // Writing headers may fail during the encode state if they violate HPACK limits. + Throwable failureCause = f.cause(); + if (failureCause == null) { +- stream.headersSent(); ++ stream.headersSent(isInformational); + } else { + lifecycleManager.onError(ctx, failureCause); + } +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java +index 167087551b..3b654425cf 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java +@@ -130,15 +130,41 @@ public interface Http2Stream { + V removeProperty(Http2Connection.PropertyKey key); + + /** +- * Indicates that headers has been sent to the remote on this stream. ++ * Indicates that headers have been sent to the remote endpoint on this stream. The first call to this method would ++ * be for the initial headers (see {@link #isHeadersSent()}} and the second call would indicate the trailers ++ * (see {@link #isTrailersReceived()}). ++ * @param isInformational {@code true} if the headers contain an informational status code (for responses only). + */ +- Http2Stream headersSent(); ++ Http2Stream headersSent(boolean isInformational); + + /** +- * Indicates whether or not headers was sent to the remote endpoint. ++ * Indicates whether or not headers were sent to the remote endpoint. + */ + boolean isHeadersSent(); + ++ /** ++ * Indicates whether or not trailers were sent to the remote endpoint. ++ */ ++ boolean isTrailersSent(); ++ ++ /** ++ * Indicates that headers have been received. The first call to this method would be for the initial headers ++ * (see {@link #isHeadersReceived()}} and the second call would indicate the trailers ++ * (see {@link #isTrailersReceived()}). ++ * @param isInformational {@code true} if the headers contain an informational status code (for responses only). ++ */ ++ Http2Stream headersReceived(boolean isInformational); ++ ++ /** ++ * Indicates whether or not the initial headers have been received. ++ */ ++ boolean isHeadersReceived(); ++ ++ /** ++ * Indicates whether or not the trailers have been received. ++ */ ++ boolean isTrailersReceived(); ++ + /** + * Indicates that a push promise was sent to the remote endpoint. + */ +diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +index 6dc4266799..3fcf560eff 100644 +--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java ++++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +@@ -21,6 +21,7 @@ import io.netty.channel.ChannelFuture; + import io.netty.channel.ChannelHandlerContext; + import io.netty.channel.ChannelPromise; + import io.netty.channel.DefaultChannelPromise; ++import io.netty.handler.codec.http.HttpResponseStatus; + import junit.framework.AssertionFailedError; + import org.junit.Before; + import org.junit.Test; +@@ -50,10 +51,10 @@ import static org.mockito.Mockito.anyBoolean; + import static org.mockito.Mockito.anyInt; + import static org.mockito.Mockito.anyLong; + import static org.mockito.Mockito.anyShort; +-import static org.mockito.Mockito.eq; +-import static org.mockito.Mockito.isNull; + import static org.mockito.Mockito.doAnswer; + import static org.mockito.Mockito.doNothing; ++import static org.mockito.Mockito.eq; ++import static org.mockito.Mockito.isNull; + import static org.mockito.Mockito.never; + import static org.mockito.Mockito.times; + import static org.mockito.Mockito.verify; +@@ -67,6 +68,8 @@ public class DefaultHttp2ConnectionDecoderTest { + private static final int STREAM_ID = 3; + private static final int PUSH_STREAM_ID = 2; + private static final int STREAM_DEPENDENCY_ID = 5; ++ private static final int STATE_RECV_HEADERS = 1; ++ private static final int STATE_RECV_TRAILERS = 1 << 1; + + private Http2ConnectionDecoder decoder; + private ChannelPromise promise; +@@ -122,11 +125,49 @@ public class DefaultHttp2ConnectionDecoderTest { + + promise = new DefaultChannelPromise(channel); + ++ final AtomicInteger headersReceivedState = new AtomicInteger(); + when(channel.isActive()).thenReturn(true); + when(stream.id()).thenReturn(STREAM_ID); + when(stream.state()).thenReturn(OPEN); + when(stream.open(anyBoolean())).thenReturn(stream); + when(pushStream.id()).thenReturn(PUSH_STREAM_ID); ++ doAnswer(new Answer() { ++ @Override ++ public Boolean answer(InvocationOnMock in) throws Throwable { ++ return (headersReceivedState.get() & STATE_RECV_HEADERS) != 0; ++ } ++ }).when(stream).isHeadersReceived(); ++ doAnswer(new Answer() { ++ @Override ++ public Boolean answer(InvocationOnMock in) throws Throwable { ++ return (headersReceivedState.get() & STATE_RECV_TRAILERS) != 0; ++ } ++ }).when(stream).isTrailersReceived(); ++ doAnswer(new Answer() { ++ @Override ++ public Http2Stream answer(InvocationOnMock in) throws Throwable { ++ boolean isInformational = in.getArgument(0); ++ if (isInformational) { ++ return stream; ++ } ++ for (;;) { ++ int current = headersReceivedState.get(); ++ int next = current; ++ if ((current & STATE_RECV_HEADERS) != 0) { ++ if ((current & STATE_RECV_TRAILERS) != 0) { ++ throw new IllegalStateException("already sent headers!"); ++ } ++ next |= STATE_RECV_TRAILERS; ++ } else { ++ next |= STATE_RECV_HEADERS; ++ } ++ if (headersReceivedState.compareAndSet(current, next)) { ++ break; ++ } ++ } ++ return stream; ++ } ++ }).when(stream).headersReceived(anyBoolean()); + doAnswer(new Answer() { + @Override + public Http2Stream answer(InvocationOnMock in) throws Throwable { +@@ -452,7 +493,65 @@ public class DefaultHttp2ConnectionDecoderTest { + eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false)); + } + ++ @Test(expected = Http2Exception.class) ++ public void trailersDoNotEndStreamThrows() throws Exception { ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); ++ // Trailers must end the stream! ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); ++ } ++ ++ @Test(expected = Http2Exception.class) ++ public void tooManyHeadersEOSThrows() throws Exception { ++ tooManyHeaderThrows(true); ++ } ++ ++ @Test(expected = Http2Exception.class) ++ public void tooManyHeadersNoEOSThrows() throws Exception { ++ tooManyHeaderThrows(false); ++ } ++ ++ private void tooManyHeaderThrows(boolean eos) throws Exception { ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, true); ++ // We already received the trailers! ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, eos); ++ } ++ ++ private static Http2Headers informationalHeaders() { ++ Http2Headers headers = new DefaultHttp2Headers(); ++ headers.status(HttpResponseStatus.CONTINUE.codeAsText()); ++ return headers; ++ } ++ ++ @Test ++ public void infoHeadersAndTrailersAllowed() throws Exception { ++ infoHeadersAndTrailersAllowed(true, 1); ++ } ++ + @Test ++ public void multipleInfoHeadersAndTrailersAllowed() throws Exception { ++ infoHeadersAndTrailersAllowed(true, 10); ++ } ++ ++ @Test(expected = Http2Exception.class) ++ public void infoHeadersAndTrailersNoEOSThrows() throws Exception { ++ infoHeadersAndTrailersAllowed(false, 1); ++ } ++ ++ @Test(expected = Http2Exception.class) ++ public void multipleInfoHeadersAndTrailersNoEOSThrows() throws Exception { ++ infoHeadersAndTrailersAllowed(false, 10); ++ } ++ ++ private void infoHeadersAndTrailersAllowed(boolean eos, int infoHeaderCount) throws Exception { ++ for (int i = 0; i < infoHeaderCount; ++i) { ++ decode().onHeadersRead(ctx, STREAM_ID, informationalHeaders(), 0, false); ++ } ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false); ++ decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, eos); ++ } ++ ++ @Test() + public void headersReadForPromisedStreamShouldCloseStream() throws Exception { + when(stream.state()).thenReturn(RESERVED_REMOTE); + decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, true); +diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java +index ac63f6dce9..4c5482ba9e 100644 +--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java ++++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java +@@ -24,6 +24,7 @@ import io.netty.channel.ChannelHandlerContext; + import io.netty.channel.ChannelPipeline; + import io.netty.channel.ChannelPromise; + import io.netty.channel.DefaultChannelPromise; ++import io.netty.handler.codec.http.HttpResponseStatus; + import io.netty.handler.codec.http2.Http2RemoteFlowController.FlowControlled; + import io.netty.util.concurrent.ImmediateEventExecutor; + import junit.framework.AssertionFailedError; +@@ -59,11 +60,12 @@ import static org.mockito.Mockito.anyBoolean; + import static org.mockito.Mockito.anyInt; + import static org.mockito.Mockito.anyLong; + import static org.mockito.Mockito.anyShort; +-import static org.mockito.Mockito.eq; + import static org.mockito.Mockito.doAnswer; + import static org.mockito.Mockito.doNothing; ++import static org.mockito.Mockito.eq; + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.never; ++import static org.mockito.Mockito.times; + import static org.mockito.Mockito.verify; + import static org.mockito.Mockito.verifyNoMoreInteractions; + import static org.mockito.Mockito.when; +@@ -342,11 +344,208 @@ public class DefaultHttp2ConnectionEncoderTest { + eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise)); + } + ++ @Test ++ public void trailersDoNotEndStreamThrows() { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ ChannelPromise promise = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise); ++ ++ ChannelPromise promise2 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2); ++ assertTrue(future.isDone()); ++ assertFalse(future.isSuccess()); ++ ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise)); ++ } ++ ++ @Test ++ public void trailersDoNotEndStreamWithDataThrows() { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ ChannelPromise promise = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise); ++ ++ Http2Stream stream = connection.stream(streamId); ++ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true); ++ ++ ChannelPromise promise2 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2); ++ assertTrue(future.isDone()); ++ assertFalse(future.isSuccess()); ++ ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise)); ++ } ++ ++ @Test ++ public void tooManyHeadersNoEOSThrows() { ++ tooManyHeadersThrows(false); ++ } ++ ++ @Test ++ public void tooManyHeadersEOSThrows() { ++ tooManyHeadersThrows(true); ++ } ++ ++ private void tooManyHeadersThrows(boolean eos) { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ ChannelPromise promise = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise); ++ ChannelPromise promise2 = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, true, promise2); ++ ++ ChannelPromise promise3 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3); ++ assertTrue(future.isDone()); ++ assertFalse(future.isSuccess()); ++ ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise)); ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise2)); ++ } ++ ++ @Test ++ public void infoHeadersAndTrailersAllowed() throws Exception { ++ infoHeadersAndTrailers(true, 1); ++ } ++ ++ @Test ++ public void multipleInfoHeadersAndTrailersAllowed() throws Exception { ++ infoHeadersAndTrailers(true, 10); ++ } ++ ++ @Test ++ public void infoHeadersAndTrailersNoEOSThrows() throws Exception { ++ infoHeadersAndTrailers(false, 1); ++ } ++ ++ @Test ++ public void multipleInfoHeadersAndTrailersNoEOSThrows() throws Exception { ++ infoHeadersAndTrailers(false, 10); ++ } ++ ++ private void infoHeadersAndTrailers(boolean eos, int infoHeaderCount) { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ Http2Headers infoHeaders = informationalHeaders(); ++ for (int i = 0; i < infoHeaderCount; ++i) { ++ encoder.writeHeaders(ctx, streamId, infoHeaders, 0, false, newPromise()); ++ } ++ ChannelPromise promise2 = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2); ++ ++ ChannelPromise promise3 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3); ++ assertTrue(future.isDone()); ++ assertEquals(eos, future.isSuccess()); ++ ++ verify(writer, times(infoHeaderCount)).writeHeaders(eq(ctx), eq(streamId), eq(infoHeaders), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), any(ChannelPromise.class)); ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise2)); ++ if (eos) { ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise3)); ++ } ++ } ++ ++ private static Http2Headers informationalHeaders() { ++ Http2Headers headers = new DefaultHttp2Headers(); ++ headers.status(HttpResponseStatus.CONTINUE.codeAsText()); ++ return headers; ++ } ++ ++ @Test ++ public void tooManyHeadersWithDataNoEOSThrows() { ++ tooManyHeadersWithDataThrows(false); ++ } ++ ++ @Test ++ public void tooManyHeadersWithDataEOSThrows() { ++ tooManyHeadersWithDataThrows(true); ++ } ++ ++ private void tooManyHeadersWithDataThrows(boolean eos) { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ ChannelPromise promise = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise); ++ ++ Http2Stream stream = connection.stream(streamId); ++ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true); ++ ++ ChannelPromise promise2 = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, true, promise2); ++ ++ ChannelPromise promise3 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3); ++ assertTrue(future.isDone()); ++ assertFalse(future.isSuccess()); ++ ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise)); ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise2)); ++ } ++ ++ @Test ++ public void infoHeadersAndTrailersWithDataAllowed() { ++ infoHeadersAndTrailersWithData(true, 1); ++ } ++ ++ @Test ++ public void multipleInfoHeadersAndTrailersWithDataAllowed() { ++ infoHeadersAndTrailersWithData(true, 10); ++ } ++ ++ @Test ++ public void infoHeadersAndTrailersWithDataNoEOSThrows() { ++ infoHeadersAndTrailersWithData(false, 1); ++ } ++ ++ @Test ++ public void multipleInfoHeadersAndTrailersWithDataNoEOSThrows() { ++ infoHeadersAndTrailersWithData(false, 10); ++ } ++ ++ private void infoHeadersAndTrailersWithData(boolean eos, int infoHeaderCount) { ++ writeAllFlowControlledFrames(); ++ final int streamId = 6; ++ Http2Headers infoHeaders = informationalHeaders(); ++ for (int i = 0; i < infoHeaderCount; ++i) { ++ encoder.writeHeaders(ctx, streamId, infoHeaders, 0, false, newPromise()); ++ } ++ ++ Http2Stream stream = connection.stream(streamId); ++ when(remoteFlow.hasFlowControlled(eq(stream))).thenReturn(true); ++ ++ ChannelPromise promise2 = newPromise(); ++ encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, false, promise2); ++ ++ ChannelPromise promise3 = newPromise(); ++ ChannelFuture future = encoder.writeHeaders(ctx, streamId, EmptyHttp2Headers.INSTANCE, 0, eos, promise3); ++ assertTrue(future.isDone()); ++ assertEquals(eos, future.isSuccess()); ++ ++ verify(writer, times(infoHeaderCount)).writeHeaders(eq(ctx), eq(streamId), eq(infoHeaders), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), any(ChannelPromise.class)); ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(false), eq(promise2)); ++ if (eos) { ++ verify(writer, times(1)).writeHeaders(eq(ctx), eq(streamId), eq(EmptyHttp2Headers.INSTANCE), eq(0), ++ eq(DEFAULT_PRIORITY_WEIGHT), eq(false), eq(0), eq(true), eq(promise3)); ++ } ++ } ++ + @Test + public void pushPromiseWriteAfterGoAwayReceivedShouldFail() throws Exception { + createStream(STREAM_ID, false); + goAwayReceived(0); +- ChannelFuture future = encoder.writePushPromise(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, ++ ChannelFuture future = encoder.writePushPromise(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, + newPromise()); + assertTrue(future.isDone()); + assertFalse(future.isSuccess()); +diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java +index 28a5c5b44b..33393afc0a 100644 +--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java ++++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java +@@ -360,49 +360,6 @@ public class InboundHttp2ToHttpAdapterTest { + } + } + +- @Test +- public void clientRequestMultipleHeaders() throws Exception { +- boostrapEnv(1, 1, 1); +- // writeHeaders will implicitly add an END_HEADERS tag each time and so this test does not follow the HTTP +- // message flow. We currently accept this message flow and just add the second headers to the trailing headers. +- final String text = ""; +- final ByteBuf content = Unpooled.copiedBuffer(text.getBytes()); +- final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, +- "/some/path/resource2", content, true); +- try { +- HttpHeaders httpHeaders = request.headers(); +- httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3); +- httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, text.length()); +- httpHeaders.setShort(HttpConversionUtil.ExtensionHeaderNames.STREAM_WEIGHT.text(), (short) 16); +- HttpHeaders trailingHeaders = request.trailingHeaders(); +- trailingHeaders.set(of("FoO"), of("goo")); +- trailingHeaders.set(of("foO2"), of("goo2")); +- trailingHeaders.add(of("fOo2"), of("goo3")); +- final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).path( +- new AsciiString("/some/path/resource2")); +- final Http2Headers http2Headers2 = new DefaultHttp2Headers() +- .set(new AsciiString("foo"), new AsciiString("goo")) +- .set(new AsciiString("foo2"), new AsciiString("goo2")) +- .add(new AsciiString("foo2"), new AsciiString("goo3")); +- runInChannel(clientChannel, new Http2Runnable() { +- @Override +- public void run() throws Http2Exception { +- clientHandler.encoder().writeHeaders(ctxClient(), 3, http2Headers, 0, false, newPromiseClient()); +- clientHandler.encoder().writeHeaders(ctxClient(), 3, http2Headers2, 0, false, newPromiseClient()); +- clientHandler.encoder().writeData(ctxClient(), 3, content.retain(), 0, true, newPromiseClient()); +- clientChannel.flush(); +- } +- }); +- awaitRequests(); +- ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class); +- verify(serverListener).messageReceived(requestCaptor.capture()); +- capturedRequests = requestCaptor.getAllValues(); +- assertEquals(request, capturedRequests.get(0)); +- } finally { +- request.release(); +- } +- } +- + @Test + public void clientRequestTrailingHeaders() throws Exception { + boostrapEnv(1, 1, 1); +-- +2.23.0 + diff --git a/CVE-2021-21295.patch b/CVE-2021-21295.patch new file mode 100644 index 0000000..9a28be4 --- /dev/null +++ b/CVE-2021-21295.patch @@ -0,0 +1,547 @@ +From 89c241e3b1795ff257af4ad6eadc616cb2fb3dc4 Mon Sep 17 00:00:00 2001 +From: Norman Maurer +Date: Tue, 9 Mar 2021 08:20:09 +0100 +Subject: [PATCH] Merge pull request from GHSA-wm47-8v5p-wjpj + +Motivation: + +As stated by https://tools.ietf.org/html/rfc7540#section-8.1.2.6 we should report a stream error if the content-length does not match the sum of all data frames. + +Modifications: + +- Verify that the sum of data frames match if a content-length header was send. +- Handle multiple content-length headers and also handle negative values +- Add io.netty.http2.validateContentLength system property which allows to disable the more strict validation +- Add unit tests + +Result: + +Correctly handle the case when the content-length header was included but not match what is send and also when content-length header is invalid +--- + .../handler/codec/http/HttpObjectDecoder.java | 48 +------ + .../io/netty/handler/codec/http/HttpUtil.java | 87 ++++++++++++ + .../http2/DefaultHttp2ConnectionDecoder.java | 100 ++++++++++++-- + .../DefaultHttp2ConnectionDecoderTest.java | 128 ++++++++++++++++++ + 4 files changed, 313 insertions(+), 50 deletions(-) + +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java +index b52e36ac92..82b0c365c1 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java +@@ -16,7 +16,6 @@ + package io.netty.handler.codec.http; + + import static io.netty.util.internal.ObjectUtil.checkPositive; +-import static io.netty.util.internal.StringUtil.COMMA; + + import io.netty.buffer.ByteBuf; + import io.netty.buffer.Unpooled; +@@ -638,49 +637,16 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder { + value = null; + + List contentLengthFields = headers.getAll(HttpHeaderNames.CONTENT_LENGTH); +- + if (!contentLengthFields.isEmpty()) { ++ HttpVersion version = message.protocolVersion(); ++ boolean isHttp10OrEarlier = version.majorVersion() < 1 || (version.majorVersion() == 1 ++ && version.minorVersion() == 0); + // Guard against multiple Content-Length headers as stated in + // https://tools.ietf.org/html/rfc7230#section-3.3.2: +- // +- // If a message is received that has multiple Content-Length header +- // fields with field-values consisting of the same decimal value, or a +- // single Content-Length header field with a field value containing a +- // list of identical decimal values (e.g., "Content-Length: 42, 42"), +- // indicating that duplicate Content-Length header fields have been +- // generated or combined by an upstream message processor, then the +- // recipient MUST either reject the message as invalid or replace the +- // duplicated field-values with a single valid Content-Length field +- // containing that decimal value prior to determining the message body +- // length or forwarding the message. +- boolean multipleContentLengths = +- contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0; +- if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) { +- if (allowDuplicateContentLengths) { +- // Find and enforce that all Content-Length values are the same +- String firstValue = null; +- for (String field : contentLengthFields) { +- String[] tokens = COMMA_PATTERN.split(field, -1); +- for (String token : tokens) { +- String trimmed = token.trim(); +- if (firstValue == null) { +- firstValue = trimmed; +- } else if (!trimmed.equals(firstValue)) { +- throw new IllegalArgumentException( +- "Multiple Content-Length values found: " + contentLengthFields); +- } +- } +- } +- // Replace the duplicated field-values with a single valid Content-Length field +- headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue); +- contentLength = Long.parseLong(firstValue); +- } else { +- // Reject the message as invalid +- throw new IllegalArgumentException( +- "Multiple Content-Length values found: " + contentLengthFields); +- } +- } else { +- contentLength = Long.parseLong(contentLengthFields.get(0)); ++ contentLength = HttpUtil.normalizeAndGetContentLength(contentLengthFields, ++ isHttp10OrEarlier, allowDuplicateContentLengths); ++ if (contentLength != -1) { ++ headers.set(HttpHeaderNames.CONTENT_LENGTH, contentLength); + } + } + +diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java +index 035cd754cf..3d1c57f5ac 100644 +--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java ++++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java +@@ -25,6 +25,11 @@ import java.nio.charset.UnsupportedCharsetException; + import java.util.Iterator; + import java.util.List; + ++import io.netty.handler.codec.Headers; ++import io.netty.util.internal.UnstableApi; ++ ++import static io.netty.util.internal.StringUtil.COMMA; ++ + /** + * Utility methods useful in the HTTP context. + */ +@@ -40,6 +45,7 @@ public final class HttpUtil { + static final EmptyHttpHeaders EMPTY_HEADERS = new EmptyHttpHeaders(); + private static final AsciiString CHARSET_EQUALS = AsciiString.of(HttpHeaderValues.CHARSET + "="); + private static final AsciiString SEMICOLON = AsciiString.of(";"); ++ private static final String COMMA_STRING = String.valueOf(COMMA); + + private HttpUtil() { } + +@@ -519,4 +525,85 @@ public final class HttpUtil { + return contentTypeValue.length() > 0 ? contentTypeValue : null; + } + } ++ ++ /** ++ * Validates, and optionally extracts the content length from headers. This method is not intended for ++ * general use, but is here to be shared between HTTP/1 and HTTP/2 parsing. ++ * ++ * @param contentLengthFields the content-length header fields. ++ * @param isHttp10OrEarlier {@code true} if we are handling HTTP/1.0 or earlier ++ * @param allowDuplicateContentLengths {@code true} if multiple, identical-value content lengths should be allowed. ++ * @return the normalized content length from the headers or {@code -1} if the fields were empty. ++ * @throws IllegalArgumentException if the content-length fields are not valid ++ */ ++ @UnstableApi ++ public static long normalizeAndGetContentLength( ++ List contentLengthFields, boolean isHttp10OrEarlier, ++ boolean allowDuplicateContentLengths) { ++ if (contentLengthFields.isEmpty()) { ++ return -1; ++ } ++ ++ // Guard against multiple Content-Length headers as stated in ++ // https://tools.ietf.org/html/rfc7230#section-3.3.2: ++ // ++ // If a message is received that has multiple Content-Length header ++ // fields with field-values consisting of the same decimal value, or a ++ // single Content-Length header field with a field value containing a ++ // list of identical decimal values (e.g., "Content-Length: 42, 42"), ++ // indicating that duplicate Content-Length header fields have been ++ // generated or combined by an upstream message processor, then the ++ // recipient MUST either reject the message as invalid or replace the ++ // duplicated field-values with a single valid Content-Length field ++ // containing that decimal value prior to determining the message body ++ // length or forwarding the message. ++ String firstField = contentLengthFields.get(0).toString(); ++ boolean multipleContentLengths = ++ contentLengthFields.size() > 1 || firstField.indexOf(COMMA) >= 0; ++ ++ if (multipleContentLengths && !isHttp10OrEarlier) { ++ if (allowDuplicateContentLengths) { ++ // Find and enforce that all Content-Length values are the same ++ String firstValue = null; ++ for (CharSequence field : contentLengthFields) { ++ String[] tokens = field.toString().split(COMMA_STRING, -1); ++ for (String token : tokens) { ++ String trimmed = token.trim(); ++ if (firstValue == null) { ++ firstValue = trimmed; ++ } else if (!trimmed.equals(firstValue)) { ++ throw new IllegalArgumentException( ++ "Multiple Content-Length values found: " + contentLengthFields); ++ } ++ } ++ } ++ // Replace the duplicated field-values with a single valid Content-Length field ++ firstField = firstValue; ++ } else { ++ // Reject the message as invalid ++ throw new IllegalArgumentException( ++ "Multiple Content-Length values found: " + contentLengthFields); ++ } ++ } ++ // Ensure we not allow sign as part of the content-length: ++ // See https://github.com/squid-cache/squid/security/advisories/GHSA-qf3v-rc95-96j5 ++ if (!Character.isDigit(firstField.charAt(0))) { ++ // Reject the message as invalid ++ throw new IllegalArgumentException( ++ "Content-Length value is not a number: " + firstField); ++ } ++ try { ++ final long value = Long.parseLong(firstField); ++ if (value < 0) { ++ // Reject the message as invalid ++ throw new IllegalArgumentException( ++ "Content-Length value must be >=0: " + value); ++ } ++ return value; ++ } catch (NumberFormatException e) { ++ // Reject the message as invalid ++ throw new IllegalArgumentException( ++ "Content-Length value is not a number: " + firstField, e); ++ } ++ } + } +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +index 4027978651..f04a0b5a69 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +@@ -16,8 +16,11 @@ package io.netty.handler.codec.http2; + + import io.netty.buffer.ByteBuf; + import io.netty.channel.ChannelHandlerContext; ++import io.netty.handler.codec.http.HttpHeaderNames; + import io.netty.handler.codec.http.HttpStatusClass; ++import io.netty.handler.codec.http.HttpUtil; + import io.netty.handler.codec.http2.Http2Connection.Endpoint; ++import io.netty.util.internal.SystemPropertyUtil; + import io.netty.util.internal.UnstableApi; + import io.netty.util.internal.logging.InternalLogger; + import io.netty.util.internal.logging.InternalLoggerFactory; +@@ -49,6 +52,8 @@ import static java.lang.Math.min; + */ + @UnstableApi + public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { ++ private static final boolean VALIDATE_CONTENT_LENGTH = ++ SystemPropertyUtil.getBoolean("io.netty.http2.validateContentLength", true); + private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultHttp2ConnectionDecoder.class); + private Http2FrameListener internalFrameListener = new PrefaceFrameListener(); + private final Http2Connection connection; +@@ -57,6 +62,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + private final Http2FrameReader frameReader; + private Http2FrameListener listener; + private final Http2PromisedRequestVerifier requestVerifier; ++ private final Http2Connection.PropertyKey contentLengthKey; + + public DefaultHttp2ConnectionDecoder(Http2Connection connection, + Http2ConnectionEncoder encoder, +@@ -69,6 +75,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + Http2FrameReader frameReader, + Http2PromisedRequestVerifier requestVerifier) { + this.connection = checkNotNull(connection, "connection"); ++ contentLengthKey = this.connection.newKey(); + this.frameReader = checkNotNull(frameReader, "frameReader"); + this.encoder = checkNotNull(encoder, "encoder"); + this.requestVerifier = checkNotNull(requestVerifier, "requestVerifier"); +@@ -171,6 +178,23 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + listener.onUnknownFrame(ctx, frameType, streamId, flags, payload); + } + ++ // See https://tools.ietf.org/html/rfc7540#section-8.1.2.6 ++ private void verifyContentLength(Http2Stream stream, int data, boolean isEnd) throws Http2Exception { ++ if (!VALIDATE_CONTENT_LENGTH) { ++ return; ++ } ++ ContentLength contentLength = stream.getProperty(contentLengthKey); ++ if (contentLength != null) { ++ try { ++ contentLength.increaseReceivedBytes(connection.isServer(), stream.id(), data, isEnd); ++ } finally { ++ if (isEnd) { ++ stream.removeProperty(contentLengthKey); ++ } ++ } ++ } ++ } ++ + /** + * Handles all inbound frames from the network. + */ +@@ -180,7 +204,8 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + boolean endOfStream) throws Http2Exception { + Http2Stream stream = connection.stream(streamId); + Http2LocalFlowController flowController = flowController(); +- int bytesToReturn = data.readableBytes() + padding; ++ int readable = data.readableBytes(); ++ int bytesToReturn = readable + padding; + + final boolean shouldIgnore; + try { +@@ -207,7 +232,6 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + // All bytes have been consumed. + return bytesToReturn; + } +- + Http2Exception error = null; + switch (stream.state()) { + case OPEN: +@@ -235,6 +259,8 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + throw error; + } + ++ verifyContentLength(stream, readable, endOfStream); ++ + // Call back the application and retrieve the number of bytes that have been + // immediately processed. + bytesToReturn = listener.onDataRead(ctx, streamId, data, padding, endOfStream); +@@ -315,14 +341,34 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + stream.state()); + } + +- stream.headersReceived(isInformational); +- encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive); +- +- listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream); ++ if (!stream.isHeadersReceived()) { ++ // extract the content-length header ++ List contentLength = headers.getAll(HttpHeaderNames.CONTENT_LENGTH); ++ if (contentLength != null && !contentLength.isEmpty()) { ++ try { ++ long cLength = HttpUtil.normalizeAndGetContentLength(contentLength, false, true); ++ if (cLength != -1) { ++ headers.setLong(HttpHeaderNames.CONTENT_LENGTH, cLength); ++ stream.setProperty(contentLengthKey, new ContentLength(cLength)); ++ } ++ } catch (IllegalArgumentException e) { ++ throw streamError(stream.id(), PROTOCOL_ERROR, ++ "Multiple content-length headers received", e); ++ } ++ } ++ } + +- // If the headers completes this stream, close it. +- if (endOfStream) { +- lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); ++ stream.headersReceived(isInformational); ++ try { ++ verifyContentLength(stream, 0, endOfStream); ++ encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive); ++ listener.onHeadersRead(ctx, streamId, headers, streamDependency, ++ weight, exclusive, padding, endOfStream); ++ } finally { ++ // If the headers completes this stream, close it. ++ if (endOfStream) { ++ lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture()); ++ } + } + } + +@@ -673,4 +719,40 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + onUnknownFrame0(ctx, frameType, streamId, flags, payload); + } + } ++ ++ private static final class ContentLength { ++ private final long expected; ++ private long seen; ++ ++ ContentLength(long expected) { ++ this.expected = expected; ++ } ++ ++ void increaseReceivedBytes(boolean server, int streamId, int bytes, boolean isEnd) throws Http2Exception { ++ seen += bytes; ++ // Check for overflow ++ if (seen < 0) { ++ throw streamError(streamId, PROTOCOL_ERROR, ++ "Received amount of data did overflow and so not match content-length header %d", expected); ++ } ++ // Check if we received more data then what was advertised via the content-length header. ++ if (seen > expected) { ++ throw streamError(streamId, PROTOCOL_ERROR, ++ "Received amount of data %d does not match content-length header %d", seen, expected); ++ } ++ ++ if (isEnd) { ++ if (seen == 0 && !server) { ++ // This may be a response to a HEAD request, let's just allow it. ++ return; ++ } ++ ++ // Check that we really saw what was told via the content-length header. ++ if (expected > seen) { ++ throw streamError(streamId, PROTOCOL_ERROR, ++ "Received amount of data %d does not match content-length header %d", seen, expected); ++ } ++ } ++ } ++ } + } +diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +index 3fcf560eff..467c6a9904 100644 +--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java ++++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java +@@ -21,17 +21,21 @@ import io.netty.channel.ChannelFuture; + import io.netty.channel.ChannelHandlerContext; + import io.netty.channel.ChannelPromise; + import io.netty.channel.DefaultChannelPromise; ++import io.netty.handler.codec.http.HttpHeaderNames; + import io.netty.handler.codec.http.HttpResponseStatus; + import junit.framework.AssertionFailedError; + import org.junit.Before; + import org.junit.Test; + import org.mockito.ArgumentCaptor; ++import org.mockito.ArgumentMatchers; + import org.mockito.Mock; + import org.mockito.MockitoAnnotations; + import org.mockito.invocation.InvocationOnMock; + import org.mockito.stubbing.Answer; + + import java.util.Collections; ++import java.util.IdentityHashMap; ++import java.util.Map; + import java.util.concurrent.atomic.AtomicInteger; + + import static io.netty.buffer.Unpooled.EMPTY_BUFFER; +@@ -130,6 +134,21 @@ public class DefaultHttp2ConnectionDecoderTest { + when(stream.id()).thenReturn(STREAM_ID); + when(stream.state()).thenReturn(OPEN); + when(stream.open(anyBoolean())).thenReturn(stream); ++ ++ final Map properties = new IdentityHashMap(); ++ when(stream.getProperty(ArgumentMatchers.any())).thenAnswer(new Answer() { ++ @Override ++ public Object answer(InvocationOnMock invocationOnMock) { ++ return properties.get(invocationOnMock.getArgument(0)); ++ } ++ }); ++ when(stream.setProperty(ArgumentMatchers.any(), any())).then(new Answer() { ++ @Override ++ public Object answer(InvocationOnMock invocationOnMock) { ++ return properties.put(invocationOnMock.getArgument(0), invocationOnMock.getArgument(1)); ++ } ++ }); ++ + when(pushStream.id()).thenReturn(PUSH_STREAM_ID); + doAnswer(new Answer() { + @Override +@@ -751,6 +770,115 @@ public class DefaultHttp2ConnectionDecoderTest { + verify(listener).onGoAwayRead(eq(ctx), eq(1), eq(2L), eq(EMPTY_BUFFER)); + } + ++ @Test(expected = Http2Exception.StreamException.class) ++ public void dataContentLengthMissmatch() throws Exception { ++ dataContentLengthInvalid(false); ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void dataContentLengthInvalid() throws Exception { ++ dataContentLengthInvalid(true); ++ } ++ ++ private void dataContentLengthInvalid(boolean negative) throws Exception { ++ final ByteBuf data = dummyData(); ++ int padding = 10; ++ int processedBytes = data.readableBytes() + padding; ++ mockFlowControl(processedBytes); ++ try { ++ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers() ++ .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, false); ++ decode().onDataRead(ctx, STREAM_ID, data, padding, true); ++ verify(localFlow).receiveFlowControlledFrame(eq(stream), eq(data), eq(padding), eq(true)); ++ verify(localFlow).consumeBytes(eq(stream), eq(processedBytes)); ++ ++ verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(), ++ any(Http2Headers.class), eq(0), eq(DEFAULT_PRIORITY_WEIGHT), eq(false), ++ eq(padding), eq(false)); ++ // Verify that the event was absorbed and not propagated to the observer. ++ verify(listener, never()).onDataRead(eq(ctx), anyInt(), any(ByteBuf.class), anyInt(), anyBoolean()); ++ } finally { ++ data.release(); ++ } ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void headersContentLengthPositiveSign() throws Exception { ++ headersContentLengthSign("+1"); ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void headersContentLengthNegativeSign() throws Exception { ++ headersContentLengthSign("-1"); ++ } ++ ++ private void headersContentLengthSign(String length) throws Exception { ++ int padding = 10; ++ when(connection.isServer()).thenReturn(true); ++ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers() ++ .set(HttpHeaderNames.CONTENT_LENGTH, length), padding, false); ++ ++ // Verify that the event was absorbed and not propagated to the observer. ++ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(), ++ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean()); ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void headersContentLengthMissmatch() throws Exception { ++ headersContentLength(false); ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void headersContentLengthInvalid() throws Exception { ++ headersContentLength(true); ++ } ++ ++ private void headersContentLength(boolean negative) throws Exception { ++ int padding = 10; ++ when(connection.isServer()).thenReturn(true); ++ decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers() ++ .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, true); ++ ++ // Verify that the event was absorbed and not propagated to the observer. ++ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(), ++ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean()); ++ } ++ ++ @Test ++ public void multipleHeadersContentLengthSame() throws Exception { ++ multipleHeadersContentLength(true); ++ } ++ ++ @Test(expected = Http2Exception.StreamException.class) ++ public void multipleHeadersContentLengthDifferent() throws Exception { ++ multipleHeadersContentLength(false); ++ } ++ ++ private void multipleHeadersContentLength(boolean same) throws Exception { ++ int padding = 10; ++ when(connection.isServer()).thenReturn(true); ++ Http2Headers headers = new DefaultHttp2Headers(); ++ if (same) { ++ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0); ++ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0); ++ } else { ++ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0); ++ headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 1); ++ } ++ ++ decode().onHeadersRead(ctx, STREAM_ID, headers, padding, true); ++ ++ if (same) { ++ verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(), ++ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean()); ++ assertEquals(1, headers.getAll(HttpHeaderNames.CONTENT_LENGTH).size()); ++ } else { ++ // Verify that the event was absorbed and not propagated to the observer. ++ verify(listener, never()).onHeadersRead(eq(ctx), anyInt(), ++ any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean()); ++ } ++ } ++ + private static ByteBuf dummyData() { + // The buffer is purposely 8 bytes so it will even work for a ping frame. + return wrappedBuffer("abcdefgh".getBytes(UTF_8)); +-- +2.23.0 + diff --git a/CVE-2021-21409.patch b/CVE-2021-21409.patch new file mode 100644 index 0000000..889de61 --- /dev/null +++ b/CVE-2021-21409.patch @@ -0,0 +1,51 @@ +From b0fa4d5aab4215f3c22ce6123dd8dd5f38dc0432 Mon Sep 17 00:00:00 2001 +From: Norman Maurer +Date: Tue, 30 Mar 2021 09:40:47 +0200 +Subject: [PATCH] Merge pull request from GHSA-f256-j965-7f32 + +Motivation: + +We also need to ensure that all the header validation is done when a single header with the endStream flag is received + +Modifications: + +- Adjust code to always enforce the validation +- Add more unit tests + +Result: + +Always correctly validate +--- + .../handler/codec/http2/DefaultHttp2ConnectionDecoder.java | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +index f04a0b5a69..097ac8cdad 100644 +--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java ++++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java +@@ -300,10 +300,13 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + short weight, boolean exclusive, int padding, boolean endOfStream) throws Http2Exception { + Http2Stream stream = connection.stream(streamId); + boolean allowHalfClosedRemote = false; ++ boolean isTrailers = false; + if (stream == null && !connection.streamMayHaveExisted(streamId)) { + stream = connection.remote().createStream(streamId, endOfStream); + // Allow the state to be HALF_CLOSE_REMOTE if we're creating it in that state. + allowHalfClosedRemote = stream.state() == HALF_CLOSED_REMOTE; ++ } else if (stream != null) { ++ isTrailers = stream.isHeadersReceived(); + } + + if (shouldIgnoreHeadersOrDataFrame(ctx, streamId, stream, "HEADERS")) { +@@ -341,7 +344,7 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder { + stream.state()); + } + +- if (!stream.isHeadersReceived()) { ++ if (!isTrailers) { + // extract the content-length header + List contentLength = headers.getAll(HttpHeaderNames.CONTENT_LENGTH); + if (contentLength != null && !contentLength.isEmpty()) { +-- +2.23.0 + diff --git a/netty.spec b/netty.spec index d70c8cc..644da23 100644 --- a/netty.spec +++ b/netty.spec @@ -2,7 +2,7 @@ Name: netty Version: 4.1.13 -Release: 10 +Release: 12 Summary: Asynchronous event-driven network application Java framework License: ASL 2.0 URL: https://netty.io/ @@ -18,6 +18,12 @@ Patch0006: CVE-2019-20445-2.patch Patch0007: CVE-2019-20445-3.patch Patch0008: CVE-2020-11612.patch Patch0009: CVE-2021-21290.patch +Patch0010: CVE-2021-21295-pre1.patch +Patch0011: CVE-2021-21295-pre2.patch +Patch0012: CVE-2021-21295-pre3.patch +Patch0013: CVE-2021-21295-pre4.patch +Patch0014: CVE-2021-21295.patch +Patch0015: CVE-2021-21409.patch BuildRequires: maven-local mvn(ant-contrib:ant-contrib) BuildRequires: mvn(com.jcraft:jzlib) mvn(commons-logging:commons-logging) @@ -135,7 +141,10 @@ export CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_LD_FLAGS" %changelog -* Tue Mar 09 2021 wangyue - 4.1.13-10 +* Tue Apr 06 2021 wangxiao - 4.1.13-12 +- Fix CVE-2021-21295 CVE-2021-21409 + +* Tue Mar 09 2021 wangyue - 4.1.13-11 - fix CVE-2021-21290 * Sun Sep 20 2020 wangyue - 4.1.13-9 -- Gitee