You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/06/15 07:47:54 UTC
[tomcat] branch 9.0.x updated: Add debug logging for overhead count
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
The following commit(s) were added to refs/heads/9.0.x by this push:
new 30cae12 Add debug logging for overhead count
30cae12 is described below
commit 30cae120a61f075b1712f2e8da4daa23f1135c83
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jun 14 21:48:21 2021 +0100
Add debug logging for overhead count
---
java/org/apache/coyote/http2/Http2Protocol.java | 3 +
.../apache/coyote/http2/Http2UpgradeHandler.java | 70 ++++++++++++++++------
.../apache/coyote/http2/LocalStrings.properties | 1 +
webapps/docs/changelog.xml | 5 ++
4 files changed, 62 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java
index 4d34c80..52fdfc6 100644
--- a/java/org/apache/coyote/http2/Http2Protocol.java
+++ b/java/org/apache/coyote/http2/Http2Protocol.java
@@ -66,6 +66,9 @@ public class Http2Protocol implements UpgradeProtocol {
static final int DEFAULT_MAX_CONCURRENT_STREAM_EXECUTION = 20;
static final int DEFAULT_OVERHEAD_COUNT_FACTOR = 1;
+ // Not currently configurable. This makes the practical limit for
+ // overheadCountFactor to be 2.
+ static final int DEFAULT_OVERHEAD_REDUCTION_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;
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index cce501b..79f7531 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -750,7 +750,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
Integer.toString(len), Boolean.valueOf(finished)));
}
- reduceOverheadCount();
+ reduceOverheadCount(FrameType.DATA);
// Need to check this now since sending end of stream will change this.
boolean writeable = stream.canWrite();
@@ -1391,13 +1391,49 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
}
- private void reduceOverheadCount() {
- overheadCount.decrementAndGet();
+ private void reduceOverheadCount(FrameType frameType) {
+ // A non-overhead frame reduces the overhead count by
+ // Http2Protocol.DEFAULT_OVERHEAD_REDUCTION_FACTOR. A simple browser
+ // request is likely to have one non-overhead frame (HEADERS) and one
+ // overhead frame (REPRIORITISE). With the default settings the overhead
+ // count will remain unchanged for each simple request.
+ // Requests and responses with bodies will create additional
+ // non-overhead frames, further reducing the overhead count.
+ updateOverheadCount(frameType, Http2Protocol.DEFAULT_OVERHEAD_REDUCTION_FACTOR);
}
- private void increaseOverheadCount() {
- overheadCount.addAndGet(getProtocol().getOverheadCountFactor());
+ private void increaseOverheadCount(FrameType frameType) {
+ // An overhead frame increases the overhead count by
+ // overheadCountFactor. By default, this means an overhead frame
+ // increases the overhead count by 1. A simple browser request is likely
+ // to have one non-overhead frame (HEADERS) and one overhead frame
+ // (REPRIORITISE). With the default settings the overhead count will
+ // remain unchanged for each simple request.
+ updateOverheadCount(frameType, getProtocol().getOverheadCountFactor());
+ }
+
+
+ private void increaseOverheadCount(FrameType frameType, int increment) {
+ // Overhead frames that indicate inefficient (and potentially malicious)
+ // use of small frames trigger an increase that is inversely
+ // proportional to size. The default threshold for all three potential
+ // areas for abuse (HEADERS, DATA, WINDOW_UPDATE) is 1024 bytes. Frames
+ // with sizes smaller than this will trigger an increase of
+ // threshold/size.
+ // DATA and WINDOW_UPDATE take an average over the last two non-final
+ // frames to allow for client buffering schemes that can result in some
+ // small DATA payloads.
+ updateOverheadCount(frameType, increment);
+ }
+
+
+ private void updateOverheadCount(FrameType frameType, int increment) {
+ long newOverheadCount = overheadCount.addAndGet(increment);
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("upgradeHandler.overheadChange",
+ connectionId, getIdAsString(), frameType.name(), Long.valueOf(newOverheadCount)));
+ }
}
@@ -1456,7 +1492,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
public ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, boolean endOfStream) throws Http2Exception {
// DATA frames reduce the overhead count ...
- reduceOverheadCount();
+ reduceOverheadCount(FrameType.DATA);
// .. but lots of small payloads are inefficient so that will increase
// the overhead count unless it is the final DATA frame where small
@@ -1475,7 +1511,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
average = 1;
}
if (average < overheadThreshold) {
- overheadCount.addAndGet(overheadThreshold / average);
+ increaseOverheadCount(FrameType.DATA, overheadThreshold / average);
}
}
@@ -1557,7 +1593,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
log.debug(sm.getString("upgradeHandler.noNewStreams",
connectionId, Integer.toString(streamId)));
}
- reduceOverheadCount();
+ reduceOverheadCount(FrameType.HEADERS);
// Stateless so a static can be used to save on GC
return HEADER_SINK;
}
@@ -1585,7 +1621,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
getConnectionId(), Integer.valueOf(streamId)), Http2Error.PROTOCOL_ERROR);
}
- increaseOverheadCount();
+ increaseOverheadCount(FrameType.PRIORITY);
AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId);
if (abstractNonZeroStream == null) {
@@ -1611,9 +1647,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
if (payloadSize < overheadThreshold) {
if (payloadSize == 0) {
// Avoid division by zero
- overheadCount.addAndGet(overheadThreshold);
+ increaseOverheadCount(FrameType.HEADERS, overheadThreshold);
} else {
- overheadCount.addAndGet(overheadThreshold / payloadSize);
+ increaseOverheadCount(FrameType.HEADERS, overheadThreshold / payloadSize);
}
}
}
@@ -1633,13 +1669,13 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.incrementAndGet()) {
setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
// Ignoring maxConcurrentStreams increases the overhead count
- increaseOverheadCount();
+ increaseOverheadCount(FrameType.HEADERS);
throw new StreamException(sm.getString("upgradeHandler.tooManyRemoteStreams",
Long.toString(localSettings.getMaxConcurrentStreams())),
Http2Error.REFUSED_STREAM, streamId);
}
// Valid new stream reduces the overhead count
- reduceOverheadCount();
+ reduceOverheadCount(FrameType.HEADERS);
processStreamOnContainerThread(stream);
}
@@ -1677,7 +1713,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
public void setting(Setting setting, long value) throws ConnectionException {
- increaseOverheadCount();
+ increaseOverheadCount(FrameType.SETTINGS);
// Possible with empty settings frame
if (setting == null) {
@@ -1726,7 +1762,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
public void pingReceive(byte[] payload, boolean ack) throws IOException {
if (!ack) {
- increaseOverheadCount();
+ increaseOverheadCount(FrameType.PING);
}
pingManager.receivePing(payload, ack);
}
@@ -1762,7 +1798,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
// Check for small increments which are inefficient
if (average < overheadThreshold) {
// The smaller the increment, the larger the overhead
- overheadCount.addAndGet(overheadThreshold / average);
+ increaseOverheadCount(FrameType.WINDOW_UPDATE, overheadThreshold / average);
}
incrementWindowSize(increment);
@@ -1776,7 +1812,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
BacklogTracker tracker = backLogStreams.get(stream);
if (tracker == null || increment < tracker.getRemainingReservation()) {
// The smaller the increment, the larger the overhead
- overheadCount.addAndGet(overheadThreshold / average);
+ increaseOverheadCount(FrameType.WINDOW_UPDATE, overheadThreshold / average);
}
}
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index 25858cb..3b971a7 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -132,6 +132,7 @@ upgradeHandler.invalidPreface=Connection [{0}], Invalid connection preface
upgradeHandler.ioerror=Connection [{0}]
upgradeHandler.noAllocation=Connection [{0}], Stream [{1}], Timeout waiting for allocation
upgradeHandler.noNewStreams=Connection [{0}], Stream [{1}], Stream ignored as no new streams are permitted on this connection
+upgradeHandler.overheadChange=Connection [{0}], Stream [{1}], Frame type [{2}] resulted in new overhead count of [{3}]
upgradeHandler.pause.entry=Connection [{0}] Pausing
upgradeHandler.pingFailed=Connection [{0}] Failed to send ping to client
upgradeHandler.prefaceReceived=Connection [{0}], Connection preface received from client
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 328f1ea..2ea1c48 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -122,6 +122,11 @@
from Tomcat are larger than <code>overheadWindowUpdateThreshold</code>.
(markt)
</fix>
+ <add>
+ Add additional debug logging to track the current state of the HTTP/2
+ overhead count that Tomcat uses to detect and close potentially
+ malicious connections. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org