From 0ddb95b4505122b1f971e0f1cd2b06c742e10ce8 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Fri, 12 Jul 2024 08:13:43 -0600 Subject: [PATCH 1/2] Backporte 9fec9a8 - to address CVE-2024-34750 --- .../coyote/http2/Http2UpgradeHandler.java | 20 +++++++++---------- java/org/apache/coyote/http2/Stream.java | 16 +++++++++++++++ webapps/docs/changelog.xml | 4 ++++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 728edc6c2f8e..b84c08ef27fb 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -283,8 +283,8 @@ private void processStreamOnContainerThread(Stream stream) { } - protected void decrementActiveRemoteStreamCount() { - setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet()); + protected void decrementActiveRemoteStreamCount(Stream stream) { + setConnectionTimeoutForStreamCount(stream.decrementAndGetActiveRemoteStreamCount()); } @@ -591,7 +591,7 @@ void sendStreamReset(StreamStateMachine state, StreamException se) throws IOExce boolean active = state.isActive(); state.sendReset(); if (active) { - decrementActiveRemoteStreamCount(); + decrementActiveRemoteStreamCount(getStream(se.getStreamId())); } } socketWrapper.write(true, rstFrame, 0, rstFrame.length); @@ -835,7 +835,7 @@ void writeBody(Stream stream, ByteBuffer data, int len, boolean finished) throws protected void sentEndOfStream(Stream stream) { stream.sentEndOfStream(); if (!stream.isActive()) { - decrementActiveRemoteStreamCount(); + decrementActiveRemoteStreamCount(stream); } } @@ -1216,7 +1216,7 @@ private int allocate(AbstractStream stream, int allocation) { } - private Stream getStream(int streamId) { + Stream getStream(int streamId) { Integer key = Integer.valueOf(streamId); AbstractStream result = streams.get(key); if (result instanceof Stream) { @@ -1696,6 +1696,7 @@ public HeaderEmitter headersStart(int streamId, boolean headersEndStream) throws Stream stream = getStream(streamId, false); if (stream == null) { stream = createRemoteStream(streamId); + activeRemoteStreamCount.incrementAndGet(); } if (streamId < maxActiveRemoteStreamId) { throw new ConnectionException(sm.getString("upgradeHandler.stream.old", Integer.valueOf(streamId), @@ -1774,9 +1775,8 @@ public void headersEnd(int streamId, boolean endOfStream) throws Http2Exception Stream stream = (Stream) abstractNonZeroStream; if (stream.isActive()) { if (stream.receivedEndOfHeaders()) { - - if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.incrementAndGet()) { - decrementActiveRemoteStreamCount(); + if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.get()) { + decrementActiveRemoteStreamCount(stream); // Ignoring maxConcurrentStreams increases the overhead count increaseOverheadCount(FrameType.HEADERS); throw new StreamException( @@ -1820,7 +1820,7 @@ public void receivedEndOfStream(int streamId) throws ConnectionException { private void receivedEndOfStream(Stream stream) throws ConnectionException { stream.receivedEndOfStream(); if (!stream.isActive()) { - decrementActiveRemoteStreamCount(); + decrementActiveRemoteStreamCount(stream); } } @@ -1846,7 +1846,7 @@ public void reset(int streamId, long errorCode) throws Http2Exception { boolean active = stream.isActive(); stream.receiveReset(errorCode); if (active) { - decrementActiveRemoteStreamCount(); + decrementActiveRemoteStreamCount(stream); } } } diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index d09506dcc402..1ec1f80b9e5b 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -26,6 +26,7 @@ import java.util.HashSet; import java.util.Locale; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -89,6 +90,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { private final StreamInputBuffer inputBuffer; private final StreamOutputBuffer streamOutputBuffer = new StreamOutputBuffer(); private final Http2OutputBuffer http2OutputBuffer = new Http2OutputBuffer(coyoteResponse, streamOutputBuffer); + private final AtomicBoolean removedFromActiveCount = new AtomicBoolean(false); // State machine would be too much overhead private int headerState = HEADER_STATE_START; @@ -838,6 +840,20 @@ public void setIncremental(boolean incremental) { } + int decrementAndGetActiveRemoteStreamCount() { + /* + * Protect against mis-counting of active streams. This method should only be called once per stream but since + * the count of active streams is used to enforce the maximum concurrent streams limit, make sure each stream is + * only removed from the active count exactly once. + */ + if (removedFromActiveCount.compareAndSet(false, true)) { + return handler.activeRemoteStreamCount.decrementAndGet(); + } else { + return handler.activeRemoteStreamCount.get(); + } + } + + private static void push(final Http2UpgradeHandler handler, final Request request, final Stream stream) throws IOException { if (org.apache.coyote.Constants.IS_SECURITY_ENABLED) { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 1317ec9d3c5f..383aecc45a70 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -161,6 +161,10 @@ org.apache.catalina.security.TLSCertificateReloadListener. (markt) + + Make counting of active HTTP/2 streams per connection more robust. + (markt) + From b9396f6d3abb8fe1456493e6b204bead77c8ccbf Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Fri, 12 Jul 2024 08:15:59 -0600 Subject: [PATCH 2/2] Prepare for release 8.5.100-TT.2 --- build.properties.default | 2 +- res/maven/mvn.properties.default | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build.properties.default b/build.properties.default index a8f0a0bd9fd7..11b9867ace3f 100644 --- a/build.properties.default +++ b/build.properties.default @@ -33,7 +33,7 @@ version.major=8 version.minor=5 version.build=100 version.patch=0 -version.suffix=-TT.1 +version.suffix=-TT.2 version.dev= # ----- Build tools ----- diff --git a/res/maven/mvn.properties.default b/res/maven/mvn.properties.default index d3de933542bd..70445a0c9181 100644 --- a/res/maven/mvn.properties.default +++ b/res/maven/mvn.properties.default @@ -39,7 +39,7 @@ maven.asf.release.repo.url=https://repository.apache.org/service/local/staging/d maven.asf.release.repo.repositoryId=apache.releases.https # Release version info -maven.asf.release.deploy.version=8.5.101 +maven.asf.release.deploy.version=8.5.100-TT.2 #Where do we load the libraries from tomcat.lib.path=../../output/build/lib