From 6283a78e4f2505e110ea186623572152fbff3cb4 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 13 Aug 2019 19:07:10 +0200 Subject: [PATCH] HTTP2: Guard against empty DATA frames (without end_of_stream flag) set (#9461) Motivation: It is possible for a remote peer to flood the server / client with empty DATA frames (without end_of_stream flag) set and so cause high CPU usage without the possibility to ever hit a limit. We need to guard against this. See CVE-2019-9518 Modifications: - Add a new config option to AbstractHttp2ConnectionBuilder and sub-classes which allows to set the max number of consecutive empty DATA frames (without end_of_stream flag). After this limit is hit we will close the connection. A limit of 10 is used by default. - Add unit tests Result: Guards against CVE-2019-9518 --- ...AbstractHttp2ConnectionHandlerBuilder.java | 30 ++++ .../Http2EmptyDataFrameConnectionDecoder.java | 41 +++++ .../http2/Http2EmptyDataFrameListener.java | 65 ++++++++ .../codec/http2/Http2FrameCodecBuilder.java | 15 +- .../http2/Http2MultiplexCodecBuilder.java | 15 ++ ...p2EmptyDataFrameConnectionDecoderTest.java | 58 +++++++ .../Http2EmptyDataFrameListenerTest.java | 142 ++++++++++++++++++ 7 files changed, 365 insertions(+), 1 deletion(-) create mode 100644 codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoder.java create mode 100644 codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListener.java create mode 100644 codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoderTest.java create mode 100644 codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListenerTest.java diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java index 0a81012ef9..de2adf40dd 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/AbstractHttp2ConnectionHandlerBuilder.java @@ -108,6 +108,7 @@ public abstract class AbstractHttp2ConnectionHandlerBuilder 0) { + decoder = new Http2EmptyDataFrameConnectionDecoder(decoder, maxConsecutiveEmptyDataFrames); + } final T handler; try { // Call the abstract build method diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoder.java new file mode 100644 index 0000000000..93957c7909 --- /dev/null +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoder.java @@ -0,0 +1,41 @@ +/* + * Copyright 2019 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.http2; + +import io.netty.util.internal.ObjectUtil; + +/** + * Enforce a limit on the maximum number of consecutive empty DATA frames (without end_of_stream flag) that are allowed + * before the connection will be closed. + */ +final class Http2EmptyDataFrameConnectionDecoder extends DecoratingHttp2ConnectionDecoder { + + private final int maxConsecutiveEmptyFrames; + + Http2EmptyDataFrameConnectionDecoder(Http2ConnectionDecoder delegate, int maxConsecutiveEmptyFrames) { + super(delegate); + this.maxConsecutiveEmptyFrames = ObjectUtil.checkPositive( + maxConsecutiveEmptyFrames, "maxConsecutiveEmptyFrames"); + } + + @Override + public void frameListener(Http2FrameListener listener) { + if (listener != null) { + super.frameListener(new Http2EmptyDataFrameListener(listener, maxConsecutiveEmptyFrames)); + } else { + super.frameListener(null); + } + } +} diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListener.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListener.java new file mode 100644 index 0000000000..dcbd987ca4 --- /dev/null +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListener.java @@ -0,0 +1,65 @@ +/* + * Copyright 2019 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.http2; + +import io.netty.buffer.ByteBuf; +import io.netty.channel.ChannelHandlerContext; +import io.netty.util.internal.ObjectUtil; + +/** + * Enforce a limit on the maximum number of consecutive empty DATA frames (without end_of_stream flag) that are allowed + * before the connection will be closed. + */ +final class Http2EmptyDataFrameListener extends Http2FrameListenerDecorator { + private final int maxConsecutiveEmptyFrames; + + private boolean violationDetected; + private int emptyDataFrames; + + Http2EmptyDataFrameListener(Http2FrameListener listener, int maxConsecutiveEmptyFrames) { + super(listener); + this.maxConsecutiveEmptyFrames = ObjectUtil.checkPositive( + maxConsecutiveEmptyFrames, "maxConsecutiveEmptyFrames"); + } + + @Override + public int onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding, boolean endOfStream) + throws Http2Exception { + if (endOfStream || data.isReadable()) { + emptyDataFrames = 0; + } else if (emptyDataFrames++ == maxConsecutiveEmptyFrames && !violationDetected) { + violationDetected = true; + throw Http2Exception.connectionError(Http2Error.ENHANCE_YOUR_CALM, + "Maximum number %d of empty data frames without end_of_stream flag received", + maxConsecutiveEmptyFrames); + } + + return super.onDataRead(ctx, streamId, data, padding, endOfStream); + } + + @Override + public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers, + int padding, boolean endStream) throws Http2Exception { + emptyDataFrames = 0; + super.onHeadersRead(ctx, streamId, headers, padding, endStream); + } + + @Override + public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers, int streamDependency, + short weight, boolean exclusive, int padding, boolean endStream) throws Http2Exception { + emptyDataFrames = 0; + super.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endStream); + } +} diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java index 069d737a24..8fc0d9631a 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodecBuilder.java @@ -167,6 +167,16 @@ public class Http2FrameCodecBuilder extends return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); } + @Override + public int decoderEnforceMaxConsecutiveEmptyDataFrames() { + return super.decoderEnforceMaxConsecutiveEmptyDataFrames(); + } + + @Override + public Http2FrameCodecBuilder decoderEnforceMaxConsecutiveEmptyDataFrames(int maxConsecutiveEmptyFrames) { + return super.decoderEnforceMaxConsecutiveEmptyDataFrames(maxConsecutiveEmptyFrames); + } + /** * Build a {@link Http2FrameCodec} object. */ @@ -192,7 +202,10 @@ public class Http2FrameCodecBuilder extends } Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader, promisedRequestVerifier(), isAutoAckSettingsFrame(), isAutoAckPingFrame()); - + int maxConsecutiveEmptyDataFrames = decoderEnforceMaxConsecutiveEmptyDataFrames(); + if (maxConsecutiveEmptyDataFrames > 0) { + decoder = new Http2EmptyDataFrameConnectionDecoder(decoder, maxConsecutiveEmptyDataFrames); + } return build(decoder, encoder, initialSettings()); } return super.build(); diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java index fb95637832..099e4a0418 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodecBuilder.java @@ -196,6 +196,16 @@ public class Http2MultiplexCodecBuilder return super.decoupleCloseAndGoAway(decoupleCloseAndGoAway); } + @Override + public int decoderEnforceMaxConsecutiveEmptyDataFrames() { + return super.decoderEnforceMaxConsecutiveEmptyDataFrames(); + } + + @Override + public Http2MultiplexCodecBuilder decoderEnforceMaxConsecutiveEmptyDataFrames(int maxConsecutiveEmptyFrames) { + return super.decoderEnforceMaxConsecutiveEmptyDataFrames(maxConsecutiveEmptyFrames); + } + @Override public Http2MultiplexCodec build() { Http2FrameWriter frameWriter = this.frameWriter; @@ -219,6 +229,11 @@ public class Http2MultiplexCodecBuilder Http2ConnectionDecoder decoder = new DefaultHttp2ConnectionDecoder(connection, encoder, frameReader, promisedRequestVerifier(), isAutoAckSettingsFrame(), isAutoAckPingFrame()); + int maxConsecutiveEmptyDataFrames = decoderEnforceMaxConsecutiveEmptyDataFrames(); + if (maxConsecutiveEmptyDataFrames > 0) { + decoder = new Http2EmptyDataFrameConnectionDecoder(decoder, maxConsecutiveEmptyDataFrames); + } + return build(decoder, encoder, initialSettings()); } return super.build(); diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoderTest.java new file mode 100644 index 0000000000..2241c23f6d --- /dev/null +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameConnectionDecoderTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2019 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.http2; + +import org.hamcrest.CoreMatchers; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class Http2EmptyDataFrameConnectionDecoderTest { + + @Test + public void testDecoration() { + Http2ConnectionDecoder delegate = mock(Http2ConnectionDecoder.class); + final ArgumentCaptor listenerArgumentCaptor = + ArgumentCaptor.forClass(Http2FrameListener.class); + when(delegate.frameListener()).then(new Answer() { + @Override + public Http2FrameListener answer(InvocationOnMock invocationOnMock) { + return listenerArgumentCaptor.getValue(); + } + }); + Http2FrameListener listener = mock(Http2FrameListener.class); + Http2EmptyDataFrameConnectionDecoder decoder = new Http2EmptyDataFrameConnectionDecoder(delegate, 2); + decoder.frameListener(listener); + verify(delegate).frameListener(listenerArgumentCaptor.capture()); + + assertThat(decoder.frameListener(), CoreMatchers.instanceOf(Http2EmptyDataFrameListener.class)); + } + + @Test + public void testDecorationWithNull() { + Http2ConnectionDecoder delegate = mock(Http2ConnectionDecoder.class); + + Http2EmptyDataFrameConnectionDecoder decoder = new Http2EmptyDataFrameConnectionDecoder(delegate, 2); + decoder.frameListener(null); + assertNull(decoder.frameListener()); + } +} diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListenerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListenerTest.java new file mode 100644 index 0000000000..e7b2ac4296 --- /dev/null +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2EmptyDataFrameListenerTest.java @@ -0,0 +1,142 @@ +/* + * Copyright 2019 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.http2; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.ChannelHandlerContext; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; + +import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +public class Http2EmptyDataFrameListenerTest { + + @Mock + private Http2FrameListener frameListener; + @Mock + private ChannelHandlerContext ctx; + + @Mock + private ByteBuf nonEmpty; + + private Http2EmptyDataFrameListener listener; + + @Before + public void setUp() { + initMocks(this); + when(nonEmpty.isReadable()).thenReturn(true); + listener = new Http2EmptyDataFrameListener(frameListener, 2); + } + + @Test + public void testEmptyDataFrames() throws Http2Exception { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + + try { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + fail(); + } catch (Http2Exception expected) { + // expected + } + verify(frameListener, times(2)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(false)); + } + + @Test + public void testEmptyDataFramesWithNonEmptyInBetween() throws Http2Exception { + Http2EmptyDataFrameListener listener = new Http2EmptyDataFrameListener(frameListener, 2); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, nonEmpty, 0, false); + + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + + try { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + fail(); + } catch (Http2Exception expected) { + // expected + } + verify(frameListener, times(4)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(false)); + } + + @Test + public void testEmptyDataFramesWithEndOfStreamInBetween() throws Http2Exception { + Http2EmptyDataFrameListener listener = new Http2EmptyDataFrameListener(frameListener, 2); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, true); + + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + + try { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + fail(); + } catch (Http2Exception expected) { + // expected + } + verify(frameListener, times(1)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(true)); + verify(frameListener, times(3)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(false)); + } + + @Test + public void testEmptyDataFramesWithHeaderFrameInBetween() throws Http2Exception { + Http2EmptyDataFrameListener listener = new Http2EmptyDataFrameListener(frameListener, 2); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onHeadersRead(ctx, 1, EmptyHttp2Headers.INSTANCE, 0, true); + + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + + try { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + fail(); + } catch (Http2Exception expected) { + // expected + } + + verify(frameListener, times(1)).onHeadersRead(eq(ctx), eq(1), eq(EmptyHttp2Headers.INSTANCE), eq(0), eq(true)); + verify(frameListener, times(3)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(false)); + } + + @Test + public void testEmptyDataFramesWithHeaderFrameInBetween2() throws Http2Exception { + Http2EmptyDataFrameListener listener = new Http2EmptyDataFrameListener(frameListener, 2); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onHeadersRead(ctx, 1, EmptyHttp2Headers.INSTANCE, 0, (short) 0, false, 0, true); + + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + + try { + listener.onDataRead(ctx, 1, Unpooled.EMPTY_BUFFER, 0, false); + fail(); + } catch (Http2Exception expected) { + // expected + } + + verify(frameListener, times(1)).onHeadersRead(eq(ctx), eq(1), + eq(EmptyHttp2Headers.INSTANCE), eq(0), eq((short) 0), eq(false), eq(0), eq(true)); + verify(frameListener, times(3)).onDataRead(eq(ctx), eq(1), any(ByteBuf.class), eq(0), eq(false)); + } +}