This commit is contained in:
wangxiao65 2020-09-16 10:29:45 +08:00
parent e3734d8568
commit 0c32b165cc
5 changed files with 309 additions and 1 deletions

100
CVE-2020-11996.patch Normal file
View File

@ -0,0 +1,100 @@
From 9a0231683a77e2957cea0fdee88b193b30b0c976 Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Fri, 22 May 2020 11:27:49 +0100
Subject: [PATCH] Fix BZ 64467. Improve performance of closing idle streams
---
.../coyote/http2/Http2UpgradeHandler.java | 10 +++---
.../coyote/http2/TestHttp2Section_5_1.java | 31 ++++++++++++++++---
webapps/docs/changelog.xml | 4 +++
3 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index bd836940fb..f0d5f27bda 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1343,11 +1343,11 @@ public HeaderEmitter headersStart(int streamId, boolean headersEndStream)
}
- private void closeIdleStreams(int newMaxActiveRemoteStreamId) throws Http2Exception {
- for (int i = maxActiveRemoteStreamId + 2; i < newMaxActiveRemoteStreamId; i += 2) {
- Stream stream = getStream(i, false);
- if (stream != null) {
- stream.closeIfIdle();
+ private void closeIdleStreams(int newMaxActiveRemoteStreamId) {
+ for (Entry<Integer,Stream> entry : streams.entrySet()) {
+ if (entry.getKey().intValue() > maxActiveRemoteStreamId &&
+ entry.getKey().intValue() < newMaxActiveRemoteStreamId) {
+ entry.getValue().closeIfIdle();
}
}
maxActiveRemoteStreamId = newMaxActiveRemoteStreamId;
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
index 2a466814e1..f878653ecf 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
@@ -147,21 +147,44 @@ public void testClientSendOldStream() throws Exception {
@Test
public void testImplicitClose() throws Exception {
+ doTestImplicitClose(5);
+ }
+
+
+ // https://bz.apache.org/bugzilla/show_bug.cgi?id=64467
+ @Test
+ public void testImplicitCloseLargeId() throws Exception {
+ doTestImplicitClose(Integer.MAX_VALUE - 8);
+ }
+
+
+ private void doTestImplicitClose(int lastStreamId) throws Exception {
+
+ long startFirst = System.nanoTime();
http2Connect();
+ long durationFirst = System.nanoTime() - startFirst;
sendPriority(3, 0, 16);
- sendPriority(5, 0, 16);
+ sendPriority(lastStreamId, 0, 16);
- sendSimpleGetRequest(5);
+ long startSecond = System.nanoTime();
+ sendSimpleGetRequest(lastStreamId);
readSimpleGetResponse();
- Assert.assertEquals(getSimpleResponseTrace(5), output.getTrace());
+ long durationSecond = System.nanoTime() - startSecond;
+
+ Assert.assertEquals(getSimpleResponseTrace(lastStreamId), output.getTrace());
output.clearTrace();
+ // Allow second request to take up to 5 times first request or up to 1 second - whichever is the larger - mainly
+ // to allow for CI systems under load that can exhibit significant timing variation.
+ Assert.assertTrue("First request took [" + durationFirst/1000000 + "ms], second request took [" +
+ durationSecond/1000000 + "ms]", durationSecond < 1000000000 || durationSecond < durationFirst * 3);
+
// Should trigger an error since stream 3 should have been implicitly
// closed.
sendSimpleGetRequest(3);
- handleGoAwayResponse(5);
+ handleGoAwayResponse(lastStreamId);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5665df4..7b81937 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -1803,6 +1803,10 @@
HTTP 205 responses. Additional fix to r1795278. Based on a patch
provided by Alexandr Saperov. (violetagg)
</fix>
+ <fix>
+ <bug>64467</bug>: Improve performance of closing idle HTTP/2 streams.
+ (markt)
+ </fix>
<update>
<bug>61345</bug>: Add a server listener that can be used to do system
property replacement from the property source configured in the

53
CVE-2020-13934.patch Normal file
View File

@ -0,0 +1,53 @@
From 172977f04a5215128f1e278a688983dcd230f399 Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Fri, 26 Jun 2020 12:49:50 +0100
Subject: [PATCH] Ensure HTTP/1.1 processor is recycled after a direct h2c
connection
---
java/org/apache/coyote/AbstractProtocol.java | 9 ++++++---
webapps/docs/changelog.xml | 4 ++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java
index cb326dc12e..5bc2212549 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -772,8 +772,10 @@ public SocketState process(SocketWrapperBase<S> wrapper, SocketEvent status) {
// Assume direct HTTP/2 connection
UpgradeProtocol upgradeProtocol = getProtocol().getUpgradeProtocol("h2c");
if (upgradeProtocol != null) {
- processor = upgradeProtocol.getProcessor(
- wrapper, getProtocol().getAdapter());
+ // Release the Http11 processor to be re-used
+ release(processor);
+ // Create the upgrade processor
+ processor = upgradeProtocol.getProcessor(wrapper, getProtocol().getAdapter());
wrapper.unRead(leftOverInput);
// Associate with the processor with the connection
connections.put(socket, processor);
@@ -785,7 +785,8 @@ public SocketState process(SocketWrapperBase<S> wrapper, SocketEvent status) {
"abstractConnectionHandler.negotiatedProcessor.fail",
"h2c"));
}
- return SocketState.CLOSED;
+ // Exit loop and trigger appropriate clean-up
+ state = SocketState.CLOSED;
}
} else {
HttpUpgradeHandler httpUpgradeHandler = upgradeToken.getHttpUpgradeHandler();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5665df4..60cd317 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -232,6 +236,10 @@
<fix>
Avoid unnecessary processing of async timeouts. (markt)
</fix>
+ <fix>
+ Ensure that the HTTP/1.1 processor is correctly recycled when a direct
+ connection to h2c is made. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">

61
CVE-2020-13935.patch Normal file
View File

@ -0,0 +1,61 @@
From 40fa74c74822711ab878079d0a69f7357926723d Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Mon, 29 Jun 2020 14:02:59 +0100
Subject: [PATCH] Fix BZ 64563 - additional payload length validation
https://bz.apache.org/bugzilla/show_bug.cgi?id=64563
---
java/org/apache/tomcat/websocket/LocalStrings.properties | 1 +
java/org/apache/tomcat/websocket/WsFrameBase.java | 7 +++++++
webapps/docs/changelog.xml | 8 ++++++++
3 files changed, 16 insertions(+)
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index 9412ffeb61..929822d94c 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -70,6 +70,7 @@ wsFrame.noContinuation=A new message was started when a continuation frame was e
wsFrame.notMasked=The client frame was not masked but all client frames must be masked
wsFrame.oneByteCloseCode=The client sent a close frame with a single byte payload which is not valid
wsFrame.partialHeaderComplete=WebSocket frame received. fin [{0}], rsv [{1}], OpCode [{2}], payload length [{3}]
+wsFrame.payloadMsbInvalid=An invalid WebSocket frame was received - the most significant bit of a 64-bit payload was illegally set
wsFrame.sessionClosed=The client data cannot be processed because the session has already been closed
wsFrame.suspendRequested=Suspend of the message receiving has already been requested.
wsFrame.textMessageTooBig=The decoded text message was too big for the output buffer and the endpoint does not support partial messages
diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java b/java/org/apache/tomcat/websocket/WsFrameBase.java
index 28cdc30036..4afad67534 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -261,6 +261,13 @@ private boolean processRemainingHeader() throws IOException {
} else if (payloadLength == 127) {
payloadLength = byteArrayToLong(inputBuffer.array(),
inputBuffer.arrayOffset() + inputBuffer.position(), 8);
+ // The most significant bit of those 8 bytes is required to be zero
+ // (see RFC 6455, section 5.2). If the most significant bit is set,
+ // the resulting payload length will be negative so test for that.
+ if (payloadLength < 0) {
+ throw new WsIOException(
+ new CloseReason(CloseCodes.PROTOCOL_ERROR, sm.getString("wsFrame.payloadMsbInvalid")));
+ }
inputBuffer.position(inputBuffer.position() + 8);
}
if (Util.isControl(opCode)) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e75f367171..1d1a735c7e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -127,6 +127,14 @@
</fix>
</changelog>
</subsection>
+ <subsection name="WebSocket">
+ <changelog>
+ <fix>
+ <bug>64563</bug>: Add additional validation of payload length for
+ WebSocket messages. (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<fix>

84
CVE-2020-9484.patch Normal file
View File

@ -0,0 +1,84 @@
From 3aa8f28db7efb311cdd1b6fe15a9cd3b167a2222 Mon Sep 17 00:00:00 2001
From: Mark Thomas <markt@apache.org>
Date: Tue, 5 May 2020 15:50:15 +0100
Subject: [PATCH] Improve validation of storage location when using FileStore.
---
.../apache/catalina/session/FileStore.java | 19 +++++++++++++++++--
.../catalina/session/LocalStrings.properties | 1 +
webapps/docs/changelog.xml | 3 +++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java
index 066d6035f1..cf3ea880fa 100644
--- a/java/org/apache/catalina/session/FileStore.java
+++ b/java/org/apache/catalina/session/FileStore.java
@@ -33,6 +33,8 @@
import org.apache.catalina.Globals;
import org.apache.catalina.Session;
import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
/**
* Concrete implementation of the <b>Store</b> interface that utilizes
@@ -43,6 +45,10 @@
*/
public final class FileStore extends StoreBase {
+ private static final Log log = LogFactory.getLog(FileStore.class);
+ private static final StringManager sm = StringManager.getManager(FileStore.class);
+
+
// ----------------------------------------------------- Constants
/**
@@ -341,11 +347,20 @@ private File directory() throws IOException {
* used in the file naming.
*/
private File file(String id) throws IOException {
- if (this.directory == null) {
+ File storageDir = directory();
+ if (storageDir == null) {
return null;
}
+
String filename = id + FILE_EXT;
- File file = new File(directory(), filename);
+ File file = new File(storageDir, filename);
+
+ // Check the file is within the storage directory
+ if (!file.getCanonicalPath().startsWith(storageDir.getCanonicalPath())) {
+ log.warn(sm.getString("fileStore.invalid", file.getPath(), id));
+ return null;
+ }
+
return file;
}
}
diff --git a/java/org/apache/catalina/session/LocalStrings.properties b/java/org/apache/catalina/session/LocalStrings.properties
index 5815915..d72bee4 100644
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -16,6 +16,7 @@
fileStore.saving=Saving Session [{0}] to file [{1}]
fileStore.loading=Loading Session [{0}] from file [{1}]
fileStore.removing=Removing Session [{0}] at file [{1}]
+fileStore.invalid=Invalid persistence file [{0}] for session ID [{1}]
fileStore.createFailed=Unable to create directory [{0}] for the storage of session data
fileStore.deleteFailed=Unable to delete file [{0}] which is preventing the creation of the session storage location
fileStore.deleteSessionFailed=Unable to delete file [{0}] which is no longer required
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5665df4..a384d62 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -171,6 +171,9 @@
When generating a redirect to a directory in the Default Servlet, avoid
generating a protocol relative redirect. (markt)
</fix>
+ <add>
+ Improve validation of storage location when using FileStore. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Coyote">

View File

@ -13,7 +13,7 @@
Name: tomcat
Epoch: 1
Version: %{major_version}.%{minor_version}.%{micro_version}
Release: 13
Release: 14
Summary: Implementation of the Java Servlet, JavaServer Pages, Java Expression Language and Java WebSocket technologies
License: ASL 2.0
URL: http://tomcat.apache.org/
@ -66,6 +66,10 @@ Patch6020: CVE-2020-1938-3.patch
Patch6021: CVE-2020-1938-4.patch
Patch6022: CVE-2020-1938-5.patch
Patch6023: CVE-2020-1935.patch
Patch6024: CVE-2020-9484.patch
Patch6025: CVE-2020-11996.patch
Patch6026: CVE-2020-13934.patch
Patch6027: CVE-2020-13935.patch
BuildRequires: ecj >= 1:4.6.1 findutils apache-commons-collections apache-commons-daemon
BuildRequires: apache-commons-dbcp apache-commons-pool tomcat-taglibs-standard ant
@ -467,6 +471,12 @@ fi
%{_javadocdir}/%{name}
%changelog
* Wed Sep 9 2020 wangxiao <wangxiao65@huawei.com> - 1:9.0.10-14
- Type:cves
- ID: CVE-2020-9484 CVE-2020-11996 CVE-2020-13934 CVE-2020-13935
- SUG:restart
- DESC: fix CVE-2020-9484 CVE-2020-11996 CVE-2020-13934 CVE-2020-13935
* Tue May 19 2020 huanghaitao <huanghaitao8@huawei.com> - 1:9.0.10-13
- Type:cves
- ID: CVE-2019-17563 CVE-2019-12418 CVE-2020-1935 CVE-2020-1938