diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 3c46539ec347..7920a6c96503 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -612,8 +612,17 @@ public void onWindowUpdate(WindowUpdateFrame frame) } else { - if (!isStreamClosed(streamId)) + if (isStreamClosed(streamId)) + { + // SPEC: this case must not be treated as an error. + // However, we want to rate control it. + if (!rateControlOnEvent(frame)) + onConnectionFailure(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_window_update_frame_rate"); + } + else + { onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "unexpected_window_update_frame"); + } } } else @@ -815,14 +824,26 @@ public void ping(PingFrame frame, Callback callback) void reset(HTTP2Stream stream, ResetFrame frame, Callback callback) { - control(stream, Callback.from(() -> + if (rateControlOnEvent(frame)) { - if (stream != null) + control(stream, Callback.from(() -> { - stream.close(); - removeStream(stream); - } - }, callback), frame); + if (stream != null) + { + stream.close(); + removeStream(stream); + } + }, callback), frame); + } + else + { + onConnectionFailure(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_rst_stream_frame_rate"); + } + } + + private boolean rateControlOnEvent(Object event) + { + return getParser().rateControlOnEvent(event); } /** diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java index 02da72d84840..cab182a455a0 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/BodyParser.java @@ -226,7 +226,7 @@ private void notifyConnectionFailure(int error, String reason) protected boolean streamFailure(int streamId, int error, String reason) { notifyStreamFailure(streamId, error, reason); - return false; + return true; } private void notifyStreamFailure(int streamId, int error, String reason) @@ -243,6 +243,6 @@ private void notifyStreamFailure(int streamId, int error, String reason) protected boolean rateControlOnEvent(Object o) { - return headerParser.getRateControl().onEvent(o); + return headerParser.rateControlOnEvent(o); } } diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java index 273afdd8d7ac..0ebe562e45e8 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java @@ -45,6 +45,11 @@ public RateControl getRateControl() return rateControl; } + boolean rateControlOnEvent(Object event) + { + return getRateControl().onEvent(event); + } + protected void reset() { state = State.LENGTH; diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java index 61c4feaaddfe..d90e81c106bd 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java @@ -90,6 +90,11 @@ public void init(Listener listener) bodyParsers[FrameType.CONTINUATION.getType()] = new ContinuationBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments); } + public boolean rateControlOnEvent(Object event) + { + return headerParser.rateControlOnEvent(event); + } + protected Listener getListener() { return listener; diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowUpdateBodyParser.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowUpdateBodyParser.java index 8a64cbac0d1d..509263e27c1c 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowUpdateBodyParser.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowUpdateBodyParser.java @@ -89,15 +89,16 @@ public boolean parse(ByteBuffer buffer) private boolean onWindowUpdate(ByteBuffer buffer, int windowDelta) { int streamId = getStreamId(); + WindowUpdateFrame frame = new WindowUpdateFrame(streamId, windowDelta); + reset(); if (windowDelta == 0) { if (streamId == 0) return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_window_update_frame"); - else + if (rateControlOnEvent(frame)) return streamFailure(streamId, ErrorCode.PROTOCOL_ERROR.code, "invalid_window_update_frame"); + return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_window_update_frame_rate"); } - WindowUpdateFrame frame = new WindowUpdateFrame(streamId, windowDelta); - reset(); notifyWindowUpdate(frame); return true; } diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java b/jetty-core/jetty-http2/jetty-http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java index 013fbe6ff1ea..6626d01bcc7d 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/test/java/org/eclipse/jetty/http2/frames/FrameFloodTest.java @@ -15,10 +15,11 @@ import java.nio.ByteBuffer; import java.time.Duration; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.Flags; import org.eclipse.jetty.http2.WindowRateControl; import org.eclipse.jetty.http2.hpack.HpackEncoder; @@ -29,6 +30,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.lessThan; +import static org.junit.jupiter.api.Assertions.assertEquals; public class FrameFloodTest { @@ -144,6 +146,13 @@ public void testResetStreamFrameFlood() testFrameFlood(null, frameFrom(payload.length, FrameType.RST_STREAM.getType(), 0, 13, payload)); } + @Test + public void testWindowUpdateFrameFlood() + { + byte[] payload = {0, 0, 0, 0}; + testFrameFlood(null, frameFrom(payload.length, FrameType.WINDOW_UPDATE.getType(), 0, 13, payload)); + } + @Test public void testUnknownFrameFlood() { @@ -153,14 +162,14 @@ public void testUnknownFrameFlood() private void testFrameFlood(byte[] preamble, byte[] bytes) { - AtomicBoolean failed = new AtomicBoolean(); + AtomicInteger failed = new AtomicInteger(); Parser parser = new Parser(bufferPool, 8192, new WindowRateControl(8, Duration.ofSeconds(1))); parser.init(new Parser.Listener() { @Override public void onConnectionFailure(int error, String reason) { - failed.set(true); + failed.set(error); } }); @@ -174,7 +183,7 @@ public void onConnectionFailure(int error, String reason) } int count = 0; - while (!failed.get()) + while (failed.get() == 0) { ByteBuffer buffer = ByteBuffer.wrap(bytes); while (buffer.hasRemaining()) @@ -183,5 +192,7 @@ public void onConnectionFailure(int error, String reason) } assertThat("too many frames allowed", ++count, lessThan(1024)); } + + assertEquals(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, failed.get()); } } diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2.xml b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2.xml index a185f7fadca7..cafea3dad890 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2.xml +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2.xml @@ -12,7 +12,7 @@ - + diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2c.xml b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2c.xml index 71020ac9c881..cf52d701c9d5 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2c.xml +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/etc/jetty-http2c.xml @@ -12,7 +12,7 @@ - + diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2.mod b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2.mod index 34081544e885..f0ede3ca8b38 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2.mod +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2.mod @@ -35,5 +35,5 @@ etc/jetty-http2.xml ## Specifies the maximum number of bad frames and pings per second, ## after which a session is closed to avoid denial of service attacks. -# jetty.http2.rateControl.maxEventsPerSecond=50 +# jetty.http2.rateControl.maxEventsPerSecond=128 # end::documentation[] diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2c.mod b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2c.mod index 573d4158a50a..20d3ea4af31c 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2c.mod +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/config/modules/http2c.mod @@ -33,5 +33,5 @@ etc/jetty-http2c.xml ## Specifies the maximum number of bad frames and pings per second, ## after which a session is closed to avoid denial of service attacks. -# jetty.http2c.rateControl.maxEventsPerSecond=50 +# jetty.http2c.rateControl.maxEventsPerSecond=128 # end::documentation[] diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SmallThreadPoolLoadTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SmallThreadPoolLoadTest.java index 45fea7e4cd9d..04a95048f29d 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SmallThreadPoolLoadTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/SmallThreadPoolLoadTest.java @@ -24,6 +24,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.RateControl; import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.DataFrame; @@ -69,6 +70,8 @@ protected void prepareServer(ConnectionFactory... connectionFactories) public void testConcurrentWithSmallServerThreadPool() throws Exception { start(new LoadHandler()); + AbstractHTTP2ServerConnectionFactory h2 = connector.getConnectionFactory(AbstractHTTP2ServerConnectionFactory.class); + h2.setRateControlFactory(new RateControl.Factory() {}); // Only one connection to the server. Session session = newClientSession(new Session.Listener() {}); diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/TriggeredResetTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/TriggeredResetTest.java new file mode 100644 index 000000000000..32e2ff74f234 --- /dev/null +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/TriggeredResetTest.java @@ -0,0 +1,171 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http2.tests; + +import java.nio.ByteBuffer; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.HTTP2Session; +import org.eclipse.jetty.http2.WindowRateControl; +import org.eclipse.jetty.http2.api.Session; +import org.eclipse.jetty.http2.api.Stream; +import org.eclipse.jetty.http2.api.server.ServerSessionListener; +import org.eclipse.jetty.http2.frames.GoAwayFrame; +import org.eclipse.jetty.http2.frames.HeadersFrame; +import org.eclipse.jetty.http2.frames.SettingsFrame; +import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class TriggeredResetTest extends AbstractTest +{ + @Test + public void testClosedDataFrameTriggersReset() throws Exception + { + CountDownLatch settingsLatch = new CountDownLatch(2); + start(new ServerSessionListener() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + MetaData.Response response = new MetaData.Response(HttpStatus.OK_200, null, HttpVersion.HTTP_2, HttpFields.EMPTY); + stream.headers(new HeadersFrame(stream.getId(), response, null, true)); + return null; + } + }); + AbstractHTTP2ServerConnectionFactory h2 = connector.getBean(AbstractHTTP2ServerConnectionFactory.class); + int maxEventRate = 5; + h2.setRateControlFactory(new WindowRateControl.Factory(maxEventRate)); + + AtomicInteger errorRef = new AtomicInteger(); + Session session = newClientSession(new Session.Listener() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + + @Override + public void onGoAway(Session session, GoAwayFrame frame) + { + errorRef.set(frame.getError()); + } + }); + + assertTrue(settingsLatch.await(5, TimeUnit.SECONDS)); + + CountDownLatch responseLatch = new CountDownLatch(1); + Stream stream = session.newStream(new HeadersFrame(newRequest("GET", HttpFields.EMPTY), null, true), new Stream.Listener() + { + @Override + public void onHeaders(Stream stream, HeadersFrame frame) + { + responseLatch.countDown(); + } + }) + .get(5, TimeUnit.SECONDS); + + assertThat(stream.getId(), equalTo(1)); + assertTrue(responseLatch.await(5, TimeUnit.SECONDS)); + + // Send many DATA frames for that stream to trigger a reset from the server. + // DATA format in bytes: LEN(3) TYPE(1) FLAGS(1) STREAM_ID(4) + // Empty DATA frame with END_STREAM=true for STREAM_ID=1. + byte[] dataFrameBytes = {0, 0, 0, 0, 1, 0, 0, 0, 1}; + int dataFrameCount = 2 * maxEventRate; + ByteBuffer byteBuffer = ByteBuffer.allocate(dataFrameBytes.length * dataFrameCount); + for (int i = 0; i < dataFrameCount; ++i) + { + byteBuffer.put(dataFrameBytes); + } + byteBuffer.flip(); + ((HTTP2Session)session).getEndPoint().write(Callback.NOOP, byteBuffer); + + await().atMost(5, TimeUnit.SECONDS).until(errorRef::get, equalTo(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code)); + } + + @Test + public void testWindowUpdateExceededTriggersReset() throws Exception + { + CountDownLatch settingsLatch = new CountDownLatch(2); + start(new ServerSessionListener() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + }); + AbstractHTTP2ServerConnectionFactory h2 = connector.getBean(AbstractHTTP2ServerConnectionFactory.class); + int maxEventRate = 5; + h2.setRateControlFactory(new WindowRateControl.Factory(maxEventRate)); + + AtomicInteger errorRef = new AtomicInteger(); + Session session = newClientSession(new Session.Listener() + { + @Override + public void onSettings(Session session, SettingsFrame frame) + { + settingsLatch.countDown(); + } + + @Override + public void onGoAway(Session session, GoAwayFrame frame) + { + errorRef.set(frame.getError()); + } + }); + + assertTrue(settingsLatch.await(5, TimeUnit.SECONDS)); + + // The request is not responded, so the stream remains alive. + Stream stream = session.newStream(new HeadersFrame(newRequest("GET", HttpFields.EMPTY), null, true), new Stream.Listener() {}) + .get(5, TimeUnit.SECONDS); + + assertThat(stream.getId(), equalTo(1)); + + // Now send WINDOW_UPDATE frames to overflow 2^31-1. + // WINDOW_UPDATE format in bytes: LEN(3) TYPE(1) FLAGS(1) STREAM_ID(4) WINDOW(4) + byte[] dataFrameBytes = {0, 0, 4, 8, 0, 0, 0, 0, 1, -1, -1, -1, -1}; + int dataFrameCount = 2 * maxEventRate; + ByteBuffer byteBuffer = ByteBuffer.allocate(dataFrameBytes.length * dataFrameCount); + for (int i = 0; i < dataFrameCount; ++i) + { + byteBuffer.put(dataFrameBytes); + } + byteBuffer.flip(); + ((HTTP2Session)session).getEndPoint().write(Callback.NOOP, byteBuffer); + + await().atMost(5, TimeUnit.SECONDS).until(errorRef::get, equalTo(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code)); + } +}