This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6d1a9fd6642387969e4410b9989c85856b74917a
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 3785962f7d..2a6a2b47a6 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 ff7b7e3860..e9095489a9 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -63,8 +63,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.
@@ -98,6 +100,7 @@ public class Http2Protocol implements UpgradeProtocol {
private int maxTrailerCount = Constants.DEFAULT_MAX_TRAILER_COUNT;
private int maxTrailerSize = Constants.DEFAULT_MAX_TRAILER_SIZE;
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;
@@ -344,6 +347,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 22472e87d5..5f92b983b4 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -375,7 +375,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);
@@ -1422,6 +1422,11 @@ class Http2UpgradeHandler extends AbstractStream
implements InternalHttpUpgradeH
}
+ boolean isOverheadLimitExceeded() {
+ return overheadCount.get() > 0;
+ }
+
+
// ----------------------------------------------- Http2Parser.Input
methods
@Override
@@ -1701,6 +1706,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) {
@@ -1834,6 +1840,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 2f4d6510b6..a589223fd8 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 472359637b..45b1b6a750 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -222,7 +222,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
@@ -230,6 +230,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
