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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org