This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new d5bd2de5 Expand the HTTP/2 excessive overhead protection
d5bd2de5 is described below
commit d5bd2de5faca467d0f4279934ae4755c01978746
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Aug 13 19:23:03 2019 +0100
Expand the HTTP/2 excessive overhead protection
Various abusive behaviours described in
https://github.com/Netflix/security-bulletins/blob/master/advisories/third-party/2019-002.md
that don't trigger a DoS in Tomcat but never the less should be blocked.
Close the connection if detected.
Detection thresholds are configurable.
---
java/org/apache/coyote/http2/Http2Parser.java | 18 ++++--
java/org/apache/coyote/http2/Http2Protocol.java | 36 +++++++++++
.../apache/coyote/http2/Http2UpgradeHandler.java | 70 +++++++++++++++++++++-
test/org/apache/coyote/http2/Http2TestBase.java | 8 ++-
.../apache/coyote/http2/TestHttp2Section_5_2.java | 4 ++
.../apache/coyote/http2/TestHttp2Section_5_3.java | 4 ++
webapps/docs/changelog.xml | 5 ++
webapps/docs/config/http2.xml | 34 +++++++++++
8 files changed, 171 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Parser.java
b/java/org/apache/coyote/http2/Http2Parser.java
index 3980094..bf01b33 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -164,7 +164,7 @@ class Http2Parser {
Integer.toString(streamId), Integer.toString(dataLength),
padding));
}
- ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize);
+ ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize,
endOfStream);
if (dest == null) {
swallow(streamId, dataLength, false);
// Process padding before sending any notifications in case padding
@@ -298,7 +298,10 @@ class Http2Parser {
Http2Error.FRAME_SIZE_ERROR);
}
- if (payloadSize != 0) {
+ if (payloadSize == 0 && !ack) {
+ // Ensure empty SETTINGS frame increments the overhead count
+ output.setting(null, 0);
+ } else {
// Process the settings
byte[] setting = new byte[6];
for (int i = 0; i < payloadSize / 6; i++) {
@@ -376,9 +379,15 @@ class Http2Parser {
Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
}
+ boolean endOfHeaders = Flags.isEndOfHeaders(flags);
+
+ // Used to detect abusive clients sending large numbers of small
+ // continuation frames
+ output.headersContinue(payloadSize, endOfHeaders);
+
readHeaderPayload(streamId, payloadSize);
- if (Flags.isEndOfHeaders(flags)) {
+ if (endOfHeaders) {
headersCurrentStream = -1;
onHeadersComplete(streamId);
}
@@ -629,7 +638,7 @@ class Http2Parser {
HpackDecoder getHpackDecoder();
// Data frames
- ByteBuffer startRequestBodyFrame(int streamId, int payloadSize) throws
Http2Exception;
+ ByteBuffer startRequestBodyFrame(int streamId, int payloadSize,
boolean endOfStream) throws Http2Exception;
void endRequestBodyFrame(int streamId) throws Http2Exception;
void receivedEndOfStream(int streamId) throws ConnectionException;
void swallowedPadding(int streamId, int paddingLength) throws
ConnectionException, IOException;
@@ -637,6 +646,7 @@ class Http2Parser {
// Header frames
HeaderEmitter headersStart(int streamId, boolean headersEndStream)
throws Http2Exception, IOException;
+ void headersContinue(int payloadSize, boolean endOfHeaders);
void headersEnd(int streamId) throws ConnectionException;
// Priority frames (also headers)
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java
b/java/org/apache/coyote/http2/Http2Protocol.java
index 27449d4..a8556f6 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -55,6 +55,9 @@ public class Http2Protocol implements UpgradeProtocol {
static final int DEFAULT_INITIAL_WINDOW_SIZE = (1 << 16) - 1;
static final int DEFAULT_OVERHEAD_COUNT_FACTOR = 1;
+ static final int DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD = 1024;
+ static final int DEFAULT_OVERHEAD_DATA_THRESHOLD = 1024;
+ static final int DEFAULT_OVERHEAD_WINDOW_UPDATE_THRESHOLD = 1024;
private static final String HTTP_UPGRADE_NAME = "h2c";
private static final String ALPN_NAME = "h2";
@@ -82,6 +85,9 @@ 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 overheadContinuationThreshold =
DEFAULT_OVERHEAD_CONTINUATION_THRESHOLD;
+ private int overheadDataThreadhold = DEFAULT_OVERHEAD_DATA_THRESHOLD;
+ private int overheadWindowUpdateThreadhold =
DEFAULT_OVERHEAD_WINDOW_UPDATE_THRESHOLD;
private boolean initiatePingDisabled = false;
// Compression
@@ -316,6 +322,36 @@ public class Http2Protocol implements UpgradeProtocol {
}
+ public int getOverheadContinuationThreshhold() {
+ return overheadContinuationThreshold;
+ }
+
+
+ public void setOverheadContinuationThreshhold(int
overheadContinuationThreshold) {
+ this.overheadContinuationThreshold = overheadContinuationThreshold;
+ }
+
+
+ public int getOverheadDataThreadhold() {
+ return overheadDataThreadhold;
+ }
+
+
+ public void setOverheadDataThreadhold(int overheadDataThreadhold) {
+ this.overheadDataThreadhold = overheadDataThreadhold;
+ }
+
+
+ public int getOverheadWindowUpdateThreadhold() {
+ return overheadWindowUpdateThreadhold;
+ }
+
+
+ public void setOverheadWindowUpdateThreadhold(int
overheadWindowUpdateThreadhold) {
+ this.overheadWindowUpdateThreadhold = overheadWindowUpdateThreadhold;
+ }
+
+
public void setInitiatePingDisabled(boolean initiatePingDisabled) {
this.initiatePingDisabled = initiatePingDisabled;
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 50365a9..383417b 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1452,8 +1452,25 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
@Override
- public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize)
throws Http2Exception {
+ public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize,
boolean endOfStream) throws Http2Exception {
+ // DATA frames reduce the overhead count ...
reduceOverheadCount();
+
+ // .. but lots of small payloads are inefficient so that will increase
+ // the overhead count unless it is the final DATA frame where small
+ // payloads are expected.
+ if (!endOfStream) {
+ int overheadThreshold = protocol.getOverheadDataThreadhold();
+ if (payloadSize < overheadThreshold) {
+ if (payloadSize == 0) {
+ // Avoid division by zero
+ overheadCount.addAndGet(overheadThreshold);
+ } else {
+ overheadCount.addAndGet(overheadThreshold / payloadSize);
+ }
+ }
+ }
+
Stream stream = getStream(streamId, true);
stream.checkState(FrameType.DATA);
stream.receivedData(payloadSize);
@@ -1497,8 +1514,6 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
// determines if a new stream is created or if this stream is ignored.
checkPauseState();
- reduceOverheadCount();
-
if (connectionState.get().isNewStreamAllowed()) {
Stream stream = getStream(streamId, false);
if (stream == null) {
@@ -1514,16 +1529,21 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
closeIdleStreams(streamId);
if (localSettings.getMaxConcurrentStreams() <
activeRemoteStreamCount.incrementAndGet()) {
setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+ // Ignoring maxConcurrentStreams increases the overhead count
+ increaseOverheadCount();
throw new
StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
Long.toString(localSettings.getMaxConcurrentStreams())),
Http2Error.REFUSED_STREAM, streamId);
}
+ // Valid new stream reduces the overhead count
+ reduceOverheadCount();
return stream;
} else {
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.noNewStreams",
connectionId, Integer.toString(streamId)));
}
+ reduceOverheadCount();
// Stateless so a static can be used to save on GC
return HEADER_SINK;
}
@@ -1565,6 +1585,25 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
@Override
+ public void headersContinue(int payloadSize, boolean endOfHeaders) {
+ // Generally, continuation frames don't impact the overhead count but
if
+ // they are small and the frame isn't the final header frame then that
+ // is indicative of an abusive client
+ if (!endOfHeaders) {
+ int overheadThreshold =
getProtocol().getOverheadContinuationThreshhold();
+ if (payloadSize < overheadThreshold) {
+ if (payloadSize == 0) {
+ // Avoid division by zero
+ overheadCount.addAndGet(overheadThreshold);
+ } else {
+ overheadCount.addAndGet(overheadThreshold / payloadSize);
+ }
+ }
+ }
+ }
+
+
+ @Override
public void headersEnd(int streamId) throws ConnectionException {
Stream stream = getStream(streamId,
connectionState.get().isNewStreamAllowed());
if (stream != null) {
@@ -1598,6 +1637,11 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
increaseOverheadCount();
+ // Possible with empty settings frame
+ if (setting == null) {
+ return;
+ }
+
// Special handling required
if (setting == Setting.INITIAL_WINDOW_SIZE) {
long oldValue = remoteSettings.getInitialWindowSize();
@@ -1658,10 +1702,30 @@ public class Http2UpgradeHandler extends AbstractStream
implements InternalHttpU
@Override
public void incrementWindowSize(int streamId, int increment) throws
Http2Exception {
+ int overheadThreshold = protocol.getOverheadWindowUpdateThreadhold();
+
if (streamId == 0) {
+ // Check for small increments which are inefficient
+ if (increment < overheadThreshold) {
+ // The smaller the increment, the larger the overhead
+ overheadCount.addAndGet(overheadThreshold / increment);
+ }
+
incrementWindowSize(increment);
} else {
Stream stream = getStream(streamId, true);
+
+ // Check for small increments which are inefficient
+ if (increment < overheadThreshold) {
+ // For Streams, client might only release the minimum so check
+ // against current demand
+ BacklogTracker tracker = backLogStreams.get(stream);
+ if (tracker == null || increment <
tracker.getRemainingReservation()) {
+ // The smaller the increment, the larger the overhead
+ overheadCount.addAndGet(overheadThreshold / increment);
+ }
+ }
+
stream.checkState(FrameType.WINDOW_UPDATE);
stream.incrementWindowSize(increment);
}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java
b/test/org/apache/coyote/http2/Http2TestBase.java
index 0361296..4630804 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -972,7 +972,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
@Override
- public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize)
{
+ public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize,
boolean endOfStream) {
lastStreamId = Integer.toString(streamId);
bytesRead += payloadSize;
if (traceBody) {
@@ -1051,6 +1051,12 @@ public abstract class Http2TestBase extends
TomcatBaseTest {
@Override
+ public void headersContinue(int payloadSize, boolean endOfHeaders) {
+ // NO-OP: Logging occurs per header
+ }
+
+
+ @Override
public void headersEnd(int streamId) {
trace.append(streamId + "-HeadersEnd\n");
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_2.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_2.java
index 545825f..ad559807 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_2.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_2.java
@@ -40,6 +40,10 @@ public class TestHttp2Section_5_2 extends Http2TestBase {
http2Connect();
+ // This test uses small window updates that will trigger the excessive
+ // overhead protection so disable it.
+ http2Protocol.setOverheadWindowUpdateThreadhold(0);
+
// Set the default window size to 1024 bytes
sendSettings(0, false, new SettingValue(4, 1024));
// Wait for the ack
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
b/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
index a0bcbaf..3b14890 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_3.java
@@ -52,6 +52,10 @@ public class TestHttp2Section_5_3 extends Http2TestBase {
http2Connect();
+ // This test uses small window updates that will trigger the excessive
+ // overhead protection so disable it.
+ http2Protocol.setOverheadWindowUpdateThreadhold(0);
+
// Default connection window size is 64k - 1. Initial request will have
// used 8k (56k -1). Increase it to 57k
sendWindowUpdate(0, 1 + 1024);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index bfcdbab..8f8e6a5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -151,6 +151,11 @@
Timeouts for HTTP/2 connections were not always correctly handled
leaving some connections open for longer than expected. (markt)
</fix>
+ <add>
+ Expand the HTTP/2 excessive overhead protection to cover various forms
+ of abusive client behaviour and close the connection if any such
+ behaviour is detected. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Web applications">
diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml
index 28579f0..8a8ae70 100644
--- a/webapps/docs/config/http2.xml
+++ b/webapps/docs/config/http2.xml
@@ -188,6 +188,17 @@
The default value is an empty String (regexp matching disabled).</p>
</attribute>
+ <attribute name="overheadContinuationThreshold" required="false">
+ <p>The threshold below which the payload size of a non-final
+ <code>CONTINUATION</code> frame will trigger an increase in the overhead
+ count (see <strong>overheadCountFactor</strong>). The overhead count will
+ be increased by <code>overheadContinuationThreshold/payloadSize</code> so
+ that the smaller the <code>CONTINUATION</code> frame, the greater the
+ increase in the overhead count. A value of zero or less disables the
+ checking of non-final <code>CONTINUATION</code> frames. If not specified,
+ a default value of <code>1024</code> will be used.</p>
+ </attribute>
+
<attribute name="overheadCountFactor" required="false">
<p>The factor to apply when counting overhead frames to determine if a
connection has too high an overhead and should be closed. The overhead
@@ -202,6 +213,29 @@
used.</p>
</attribute>
+ <attribute name="overheadDataThreadhold" required="false">
+ <p>The threshold below which the payload size of a non-final
+ <code>DATA</code> frame will trigger an increase in the overhead count
+ (see <strong>overheadCountFactor</strong>). The overhead count will be
+ increased by <code>overheadDataThreadhold/payloadSize</code> so that the
+ smaller the <code>DATA</code> frame, the greater the increase in the
+ overhead count. A value of zero or less disables the checking of
non-final
+ <code>DATA</code> frames. If not specified, a default value of
+ <code>1024</code> will be used.</p>
+ </attribute>
+
+ <attribute name="overheadWindowUpdateThreadhold" required="false">
+ <p>The threshold below which the size of a <code>WINDOW_UPDATE</code>
+ frame will trigger an increase in the overhead count (see
+ <strong>overheadCountFactor</strong>). The overhead count will be
+ increased by <code>overheadWindowUpdateThreadhold/updateSize</code> so
+ that the smaller the <code>WINDOW_UPDATE</code>, the greater the increase
+ in the overhead count. A value of zero or less disables the checking of
+ <code>WINDOW_UPDATE</code> frames. If not specified, a default value of
+ <code>1024</code> will be used.</p>
+ </attribute>
+
+
<attribute name="readTimeout" required="false">
<p>The time, in milliseconds, that Tomcat will wait for additional data
when a partial HTTP/2 frame has been received. Negative values will be
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
