[Git][java-team/tomcat10][bookworm] Fix CVE-2024-34750: Denial of Service caused by excessive HTTP headers
Emmanuel Bourg (@ebourg)
gitlab at salsa.debian.org
Thu Jul 4 00:53:31 BST 2024
Emmanuel Bourg pushed to branch bookworm at Debian Java Maintainers / tomcat10
Commits:
05f08e52 by Emmanuel Bourg at 2024-07-04T01:49:03+02:00
Fix CVE-2024-34750: Denial of Service caused by excessive HTTP headers
- - - - -
3 changed files:
- debian/changelog
- + debian/patches/CVE-2024-34750.patch
- debian/patches/series
Changes:
=====================================
debian/changelog
=====================================
@@ -1,3 +1,13 @@
+tomcat10 (10.1.6-1+deb12u3) bookworm-security; urgency=medium
+
+ * Fix CVE-2024-34750: Denial of Service. When processing an HTTP/2 stream,
+ Tomcat did not handle some cases of excessive HTTP headers correctly. This
+ led to a miscounting of active HTTP/2 streams which in turn led to the use
+ of an incorrect infinite timeout which allowed connections to remain open
+ which should have been closed.
+
+ -- Emmanuel Bourg <ebourg at apache.org> Thu, 04 Jul 2024 01:28:48 +0200
+
tomcat10 (10.1.6-1+deb12u2) bookworm-security; urgency=high
* Team upload.
=====================================
debian/patches/CVE-2024-34750.patch
=====================================
@@ -0,0 +1,329 @@
+Description: Make counting of active streams more robust
+Origin: upstream, https://github.com/apache/tomcat/commit/aa7ff731ec25f915c124592c8d1d1f6ee069cc22
+ https://github.com/apache/tomcat/commit/b2e1f5b0bb0b6646efaf875cd45b9eb65a103a28
+ https://github.com/apache/tomcat/commit/8d2ddd76ee0b47c51cbae88d0d85de86a7daca4c
+ https://github.com/apache/tomcat/commit/2afae300c9ac9c0e516e2e9de580847d925365c3
+--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
++++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+@@ -154,7 +154,7 @@
+ boolean active = state.isActive();
+ state.sendReset();
+ if (active) {
+- activeRemoteStreamCount.decrementAndGet();
++ decrementActiveRemoteStreamCount(getStream(se.getStreamId()));
+ }
+ }
+
+@@ -203,7 +203,7 @@
+ }
+ }
+ if (endOfStream) {
+- stream.sentEndOfStream();
++ sentEndOfStream(stream);
+ }
+ }
+
+@@ -227,10 +227,7 @@
+ header[3] = FrameType.DATA.getIdByte();
+ if (finished) {
+ header[4] = FLAG_END_OF_STREAM;
+- stream.sentEndOfStream();
+- if (!stream.isActive()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+- }
++ sentEndOfStream(stream);
+ }
+ if (writable) {
+ ByteUtil.set31Bits(header, 5, stream.getIdAsInt());
+@@ -350,10 +347,7 @@
+ header[3] = FrameType.DATA.getIdByte();
+ if (finished) {
+ header[4] = FLAG_END_OF_STREAM;
+- sendfile.stream.sentEndOfStream();
+- if (!sendfile.stream.isActive()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+- }
++ sentEndOfStream(sendfile.stream);
+ }
+ if (writable) {
+ if (log.isDebugEnabled()) {
+@@ -434,10 +428,7 @@
+ header[3] = FrameType.DATA.getIdByte();
+ if (finished) {
+ header[4] = FLAG_END_OF_STREAM;
+- sendfile.stream.sentEndOfStream();
+- if (!sendfile.stream.isActive()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+- }
++ sentEndOfStream(sendfile.stream);
+ }
+ if (writable) {
+ if (log.isDebugEnabled()) {
+--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
++++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+@@ -291,6 +291,11 @@
+ }
+
+
++ protected void decrementActiveRemoteStreamCount(Stream stream) {
++ setConnectionTimeoutForStreamCount(stream.decrementAndGetActiveRemoteStreamCount());
++ }
++
++
+ void processStreamOnContainerThread(StreamProcessor streamProcessor, SocketEvent event) {
+ StreamRunnable streamRunnable = new StreamRunnable(streamProcessor, event);
+ if (streamConcurrency == null) {
+@@ -594,7 +599,7 @@
+ boolean active = state.isActive();
+ state.sendReset();
+ if (active) {
+- activeRemoteStreamCount.decrementAndGet();
++ decrementActiveRemoteStreamCount(getStream(se.getStreamId()));
+ }
+ }
+ socketWrapper.write(true, rstFrame, 0, rstFrame.length);
+@@ -708,7 +713,7 @@
+ }
+ stream.sentHeaders();
+ if (endOfStream) {
+- stream.sentEndOfStream();
++ sentEndOfStream(stream);
+ }
+ }
+
+@@ -813,10 +818,7 @@
+ header[3] = FrameType.DATA.getIdByte();
+ if (finished) {
+ header[4] = FLAG_END_OF_STREAM;
+- stream.sentEndOfStream();
+- if (!stream.isActive()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+- }
++ sentEndOfStream(stream);
+ }
+ if (writable) {
+ ByteUtil.set31Bits(header, 5, stream.getIdAsInt());
+@@ -837,6 +839,14 @@
+ }
+
+
++ protected void sentEndOfStream(Stream stream) {
++ stream.sentEndOfStream();
++ if (!stream.isActive()) {
++ decrementActiveRemoteStreamCount(stream);
++ }
++ }
++
++
+ /*
+ * Handles an I/O error on the socket underlying the HTTP/2 connection when it is triggered by application code
+ * (usually reading the request or writing the response). Such I/O errors are fatal so the connection is closed. The
+@@ -1169,7 +1179,7 @@
+ }
+
+
+- private Stream getStream(int streamId) {
++ Stream getStream(int streamId) {
+ Integer key = Integer.valueOf(streamId);
+ AbstractStream result = streams.get(key);
+ if (result instanceof Stream) {
+@@ -1625,20 +1635,6 @@
+
+
+ @Override
+- public void receivedEndOfStream(int streamId) throws ConnectionException {
+- AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId,
+- connectionState.get().isNewStreamAllowed());
+- if (abstractNonZeroStream instanceof Stream) {
+- Stream stream = (Stream) abstractNonZeroStream;
+- stream.receivedEndOfStream();
+- if (!stream.isActive()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
+- }
+- }
+- }
+-
+-
+- @Override
+ public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws IOException {
+ AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId);
+ writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount, false);
+@@ -1656,6 +1652,7 @@
+ 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),
+@@ -1736,17 +1733,17 @@
+
+
+ @Override
+- public void headersEnd(int streamId) throws Http2Exception {
++ public void headersEnd(int streamId, boolean endOfStream) throws Http2Exception {
+ AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId,
+ connectionState.get().isNewStreamAllowed());
+ if (abstractNonZeroStream instanceof Stream) {
++ boolean processStream = false;
+ setMaxProcessedStream(streamId);
+ Stream stream = (Stream) abstractNonZeroStream;
+ if (stream.isActive()) {
+ if (stream.receivedEndOfHeaders()) {
+-
+- if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.incrementAndGet()) {
+- setConnectionTimeoutForStreamCount(activeRemoteStreamCount.decrementAndGet());
++ if (localSettings.getMaxConcurrentStreams() < activeRemoteStreamCount.get()) {
++ decrementActiveRemoteStreamCount(stream);
+ // Ignoring maxConcurrentStreams increases the overhead count
+ increaseOverheadCount(FrameType.HEADERS);
+ throw new StreamException(
+@@ -1757,9 +1754,40 @@
+ // Valid new stream reduces the overhead count
+ reduceOverheadCount(FrameType.HEADERS);
+
+- processStreamOnContainerThread(stream);
++ processStream = true;
+ }
+ }
++ /*
++ * Need to process end of stream before calling processStreamOnContainerThread to avoid a race condition
++ * where the container thread finishes before end of stream is processed, thinks the request hasn't been
++ * fully read so issues a RST with error code 0 (NO_ERROR) to tell the client not to send the request body,
++ * if any. This breaks tests and generates unnecessary RST messages for standard clients.
++ */
++ if (endOfStream) {
++ receivedEndOfStream(stream);
++ }
++ if (processStream) {
++ processStreamOnContainerThread(stream);
++ }
++ }
++ }
++
++
++ @Override
++ public void receivedEndOfStream(int streamId) throws ConnectionException {
++ AbstractNonZeroStream abstractNonZeroStream =
++ getAbstractNonZeroStream(streamId, connectionState.get().isNewStreamAllowed());
++ if (abstractNonZeroStream instanceof Stream) {
++ Stream stream = (Stream) abstractNonZeroStream;
++ receivedEndOfStream(stream);
++ }
++ }
++
++
++ private void receivedEndOfStream(Stream stream) throws ConnectionException {
++ stream.receivedEndOfStream();
++ if (!stream.isActive()) {
++ decrementActiveRemoteStreamCount(stream);
+ }
+ }
+
+@@ -1785,7 +1813,7 @@
+ boolean active = stream.isActive();
+ stream.receiveReset(errorCode);
+ if (active) {
+- activeRemoteStreamCount.decrementAndGet();
++ decrementActiveRemoteStreamCount(stream);
+ }
+ }
+ }
+--- a/java/org/apache/coyote/http2/Http2Parser.java
++++ b/java/org/apache/coyote/http2/Http2Parser.java
+@@ -620,10 +620,9 @@
+ }
+
+ synchronized (output) {
+- output.headersEnd(streamId);
++ output.headersEnd(streamId, headersEndStream);
+
+ if (headersEndStream) {
+- output.receivedEndOfStream(streamId);
+ headersEndStream = false;
+ }
+ }
+@@ -771,7 +770,7 @@
+
+ void headersContinue(int payloadSize, boolean endOfHeaders);
+
+- void headersEnd(int streamId) throws Http2Exception;
++ void headersEnd(int streamId, boolean endOfStream) throws Http2Exception;
+
+ // Priority frames (also headers)
+ void reprioritise(int streamId, int parentStreamId, boolean exclusive, int weight) throws Http2Exception;
+--- a/test/org/apache/coyote/http2/Http2TestBase.java
++++ b/test/org/apache/coyote/http2/Http2TestBase.java
+@@ -1116,13 +1116,6 @@
+
+
+ @Override
+- public void receivedEndOfStream(int streamId) {
+- lastStreamId = Integer.toString(streamId);
+- trace.append(lastStreamId + "-EndOfStream\n");
+- }
+-
+-
+- @Override
+ public HeaderEmitter headersStart(int streamId, boolean headersEndStream) {
+ lastStreamId = Integer.toString(streamId);
+ trace.append(lastStreamId + "-HeadersStart\n");
+@@ -1177,8 +1170,18 @@
+
+
+ @Override
+- public void headersEnd(int streamId) {
++ public void headersEnd(int streamId, boolean endOfStream) {
+ trace.append(streamId + "-HeadersEnd\n");
++ if (endOfStream) {
++ receivedEndOfStream(streamId) ;
++ }
++ }
++
++
++ @Override
++ public void receivedEndOfStream(int streamId) {
++ lastStreamId = Integer.toString(streamId);
++ trace.append(lastStreamId + "-EndOfStream\n");
+ }
+
+
+--- a/java/org/apache/coyote/http2/Stream.java
++++ b/java/org/apache/coyote/http2/Stream.java
+@@ -27,6 +27,7 @@
+ import java.util.Locale;
+ import java.util.Map;
+ import java.util.Set;
++import java.util.concurrent.atomic.AtomicBoolean;
+ import java.util.function.Supplier;
+
+ import org.apache.coyote.ActionCode;
+@@ -87,6 +88,7 @@
+ 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;
+@@ -829,6 +831,20 @@
+ }
+
+
++ 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) {
=====================================
debian/patches/series
=====================================
@@ -20,3 +20,4 @@ CVE-2023-45648.patch
CVE-2024-24549.patch
CVE-2024-23672.patch
CVE-2023-46589.patch
+CVE-2024-34750.patch
View it on GitLab: https://salsa.debian.org/java-team/tomcat10/-/commit/05f08e52005250027f4c41a389c27dff4ede0d0b
--
This project does not include diff previews in email notifications.
View it on GitLab: https://salsa.debian.org/java-team/tomcat10/-/commit/05f08e52005250027f4c41a389c27dff4ede0d0b
You're receiving this email because of your account on salsa.debian.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-java-commits/attachments/20240703/6b3e3113/attachment.htm>
More information about the pkg-java-commits
mailing list