Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backporte 9fec9a8 - to address CVE-2024-34750 #42

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.properties.default
Original file line number Diff line number Diff line change
Expand Up @@ -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 -----
Expand Down
20 changes: 10 additions & 10 deletions java/org/apache/coyote/http2/Http2UpgradeHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ private void processStreamOnContainerThread(Stream stream) {
}


protected void decrementActiveRemoteStreamCount() {
setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
protected void decrementActiveRemoteStreamCount(Stream stream) {
setConnectionTimeoutForStreamCount(stream.decrementAndGetActiveRemoteStreamCount());
}


Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions java/org/apache/coyote/http2/Stream.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion res/maven/mvn.properties.default
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@
<code>org.apache.catalina.security.TLSCertificateReloadListener</code>.
(markt)
</fix>
<fix>
Make counting of active HTTP/2 streams per connection more robust.
(markt)
</fix>
</changelog>
</subsection>
<subsection name="Jasper">
Expand Down
Loading