This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 76bb4bfbfeae827dce896f650655bbf6e251ed49 Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Oct 5 20:54:14 2023 +0100 Improvements to HTTP/2 overhead protection. --- java/org/apache/coyote/http2/Http2AsyncParser.java | 51 ++++++++++++---------- java/org/apache/coyote/http2/Http2Protocol.java | 19 +++++++- .../apache/coyote/http2/Http2UpgradeHandler.java | 9 +++- webapps/docs/changelog.xml | 3 ++ webapps/docs/config/http2.xml | 9 +++- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java index 1965123bc4..e1c8170765 100644 --- a/java/org/apache/coyote/http2/Http2AsyncParser.java +++ b/java/org/apache/coyote/http2/Http2AsyncParser.java @@ -284,36 +284,39 @@ class Http2AsyncParser extends Http2Parser { 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 index 484d5d200a..8287af0e69 100644 --- 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 class Http2Protocol implements UpgradeProtocol { } + 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 index 53b75b5457..cfba750566 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -378,7 +378,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH stream.close(se); } } finally { - if (overheadCount.get() > 0) { + if (isOverheadLimitExceeded()) { throw new ConnectionException( sm.getString("upgradeHandler.tooMuchOverhead", connectionId), Http2Error.ENHANCE_YOUR_CALM); @@ -1428,6 +1428,11 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH } + boolean isOverheadLimitExceeded() { + return overheadCount.get() > 0; + } + + // ----------------------------------------------- Http2Parser.Input methods @Override @@ -1707,6 +1712,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH 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) { @@ -1840,6 +1846,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH @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 index 20ef9f669a..37b7774513 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -167,6 +167,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 index 629b135cc2..f78aa03538 100644 --- a/webapps/docs/config/http2.xml +++ b/webapps/docs/config/http2.xml @@ -139,7 +139,7 @@ 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 @@ -147,6 +147,13 @@ <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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org