MadeYouReset HTTP/2 vulnerability

@@ -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);
     }
 
     /**

@@ -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);
     }
 }

@@ -45,6 +45,11 @@ public RateControl getRateControl()
         return rateControl;
     }
 
+    boolean rateControlOnEvent(Object event)
+    {
+        return getRateControl().onEvent(event);
+    }
+
     protected void reset()
     {
         state = State.LENGTH;

@@ -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;

@@ -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;
     }

@@ -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());
     }
 }

@@ -12,7 +12,7 @@
         <Set name="maxSettingsKeys" property="jetty.http2c.maxSettingsKeys"/>
         <Set name="rateControlFactory">
           <New class="org.eclipse.jetty.http2.WindowRateControl$Factory">
-            <Arg type="int"><Property name="jetty.http2c.rateControl.maxEventsPerSecond" default="50"/></Arg>
+            <Arg type="int"><Property name="jetty.http2c.rateControl.maxEventsPerSecond" default="128"/></Arg>
           </New>
         </Set>
       </New>

@@ -12,7 +12,7 @@
         <Set name="maxSettingsKeys"><Property name="jetty.http2.maxSettingsKeys" default="64"/></Set>
         <Set name="rateControlFactory">
           <New class="org.eclipse.jetty.http2.WindowRateControl$Factory">
-            <Arg type="int"><Property name="jetty.http2.rateControl.maxEventsPerSecond" default="50"/></Arg>
+            <Arg type="int"><Property name="jetty.http2.rateControl.maxEventsPerSecond" default="128"/></Arg>
           </New>
         </Set>
       </New>

@@ -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[]

@@ -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[]

@@ -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() {});

@@ -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));
+    }
+}