Improvements to HTTP/2 overhead protection.

apache · Oct 9, 2023 · 9cdfe25 · 9cdfe25
1 parent d6db22e
commit 9cdfe25
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 27 deletions.
diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -284,36 +284,39 @@ public void completed(Long result, Void attachment) {
                                     readUnknownFrame(streamId, frameTypeId, flags, payloadSize, payload);
                             }
                         }
-                        // See if there is a new 9 byte header and continue parsing if possible
-                        if (payload.remaining() >= 9) {
-                            int position = payload.position();
-                            payloadSize = ByteUtil.getThreeBytes(payload, position);
-                            frameTypeId = ByteUtil.getOneByte(payload, position + 3);
-                            frameType = FrameType.valueOf(frameTypeId);
-                            flags = ByteUtil.getOneByte(payload, position + 4);
-                            streamId = ByteUtil.get31Bits(payload, position + 5);
-                            streamException = false;
-                            if (payload.remaining() - 9 >= payloadSize) {
-                                continueParsing = true;
-                                // Now go over frame header
-                                payload.position(payload.position() + 9);
-                                try {
-                                    validateFrame(null, frameType, streamId, flags, payloadSize);
-                                } catch (StreamException e) {
-                                    error = e;
-                                    streamException = true;
-                                } catch (Http2Exception e) {
-                                    error = e;
-                                    continueParsing = false;
+                        if (!upgradeHandler.isOverheadLimitExceeded()) {
+                            // See if there is a new 9 byte header and continue parsing if possible
+                            if (payload.remaining() >= 9) {
+                                int position = payload.position();
+                                payloadSize = ByteUtil.getThreeBytes(payload, position);
+                                frameTypeId = ByteUtil.getOneByte(payload, position + 3);
+                                frameType = FrameType.valueOf(frameTypeId);
+                                flags = ByteUtil.getOneByte(payload, position + 4);
+                                streamId = ByteUtil.get31Bits(payload, position + 5);
+                                streamException = false;
+                                if (payload.remaining() - 9 >= payloadSize) {
+                                    continueParsing = true;
+                                    // Now go over frame header
+                                    payload.position(payload.position() + 9);
+                                    try {
+                                        validateFrame(null, frameType, streamId, flags, payloadSize);
+                                    } catch (StreamException e) {
+                                        error = e;
+                                        streamException = true;
+                                    } catch (Http2Exception e) {
+                                        error = e;
+                                        continueParsing = false;
+                                    }
                                 }
                             }
                         }
                     } while (continueParsing);
                 } catch (RuntimeException | IOException | Http2Exception e) {
                     error = e;
-                }
-                if (payload.hasRemaining()) {
-                    socketWrapper.unRead(payload);
+                } finally {
+                    if (payload.hasRemaining()) {
+                        socketWrapper.unRead(payload);
+                    }
                 }
             }
             if (state == CompletionState.DONE) {

diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -53,8 +53,10 @@ public class Http2Protocol implements UpgradeProtocol {
     // Maximum amount of streams which can be concurrently executed over
     // a single connection
     static final int DEFAULT_MAX_CONCURRENT_STREAM_EXECUTION = 20;
-
+    // Default factor used when adjusting overhead count for overhead frames
     static final int DEFAULT_OVERHEAD_COUNT_FACTOR = 10;
+    // Default factor used when adjusting overhead count for reset frames
+    static final int DEFAULT_OVERHEAD_RESET_FACTOR = 50;
     // Not currently configurable. This makes the practical limit for
     // overheadCountFactor to be ~20. The exact limit will vary with traffic
     // patterns.
@@ -85,6 +87,7 @@ public class Http2Protocol implements UpgradeProtocol {
     private int maxHeaderCount = Constants.DEFAULT_MAX_HEADER_COUNT;
     private int maxTrailerCount = Constants.DEFAULT_MAX_TRAILER_COUNT;
     private int overheadCountFactor = DEFAULT_OVERHEAD_COUNT_FACTOR;
+    private int overheadResetFactor = DEFAULT_OVERHEAD_RESET_FACTOR;
     private int overheadContinuationThreshold = DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD;
     private int overheadDataThreshold = DEFAULT_OVERHEAD_DATA_THRESHOLD;
     private int overheadWindowUpdateThreshold = DEFAULT_OVERHEAD_WINDOW_UPDATE_THRESHOLD;
@@ -290,6 +293,20 @@ public void setOverheadCountFactor(int overheadCountFactor) {
     }
 
 
+    public int getOverheadResetFactor() {
+        return overheadResetFactor;
+    }
+
+
+    public void setOverheadResetFactor(int overheadResetFactor) {
+        if (overheadResetFactor < 0) {
+            this.overheadResetFactor = 0;
+        } else {
+            this.overheadResetFactor = overheadResetFactor;
+        }
+    }
+
+
     public int getOverheadContinuationThreshold() {
         return overheadContinuationThreshold;
     }

diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -377,7 +377,7 @@ public SocketState upgradeDispatch(SocketEvent status) {
                                     stream.close(se);
                                 }
                             } finally {
-                                if (overheadCount.get() > 0) {
+                                if (isOverheadLimitExceeded()) {
                                     throw new ConnectionException(
                                             sm.getString("upgradeHandler.tooMuchOverhead", connectionId),
                                             Http2Error.ENHANCE_YOUR_CALM);
@@ -1368,6 +1368,11 @@ private void updateOverheadCount(FrameType frameType, int increment) {
     }
 
 
+    boolean isOverheadLimitExceeded() {
+        return overheadCount.get() > 0;
+    }
+
+
     // ----------------------------------------------- Http2Parser.Input methods
 
     @Override
@@ -1630,6 +1635,7 @@ public void reset(int streamId, long errorCode) throws Http2Exception {
             log.debug(sm.getString("upgradeHandler.reset.receive", getConnectionId(), Integer.toString(streamId),
                     Long.toString(errorCode)));
         }
+        increaseOverheadCount(FrameType.RST, getProtocol().getOverheadResetFactor());
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
         abstractNonZeroStream.checkState(FrameType.RST);
         if (abstractNonZeroStream instanceof Stream) {
@@ -1763,6 +1769,7 @@ public void incrementWindowSize(int streamId, int increment) throws Http2Excepti
 
     @Override
     public void priorityUpdate(int prioritizedStreamID, Priority p) throws Http2Exception {
+        increaseOverheadCount(FrameType.PRIORITY_UPDATE);
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(prioritizedStreamID, true);
         if (abstractNonZeroStream instanceof Stream) {
             Stream stream = (Stream) abstractNonZeroStream;

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -177,6 +177,9 @@
       <fix>
         Align validation of HTTP trailer fields with standard fields. (markt)
       </fix>
+      <fix>
+        Improvements to HTTP/2 overhead protection. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
@@ -139,14 +139,21 @@
       count starts at <code>-10 * overheadCountFactor</code>. The count is
       decreased by 20 for each data frame sent or received and each headers frame
       received. The count is increased by the <code>overheadCountFactor</code>
-      for each setting received, priority frame received and ping received. If
+      for each setting, priority, priority update and ping frame received. If
       the overhead count exceeds zero, the connection is closed. A value of less
       than <code>1</code> disables this protection. In normal usage a value of
       approximately <code>20</code> or higher will close the connection before
       any streams can complete. If not specified, a default value of
       <code>10</code> will be used.</p>
     </attribute>
 
+    <attribute name="overheadResetFactor" required="false">
+      <p>The amount by which the overhead count (see
+      <strong>overheadCountFactor</strong>) will be increased for each reset
+      frame received. If not specified, a default value of <code>50</code> will
+      be used. A value of less than zero will be treated as zero.</p>
+    </attribute>
+
     <attribute name="overheadDataThreshold" required="false">
       <p>The threshold below which the average payload size of the current and
       previous non-final <code>DATA</code> frames will trigger an increase in