You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/02/23 15:23:11 UTC
[tomcat] branch 9.0.x updated: Validate the scheme pseudo-header in HTTP/2
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 9c38aac33a Validate the scheme pseudo-header in HTTP/2
9c38aac33a is described below
commit 9c38aac33ae02da550d459d011d3ffb030a46147
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 23 15:22:10 2023 +0000
Validate the scheme pseudo-header in HTTP/2
---
java/org/apache/coyote/http2/Stream.java | 7 ++
java/org/apache/coyote/http2/StreamProcessor.java | 6 ++
.../apache/tomcat/util/http/parser/HttpParser.java | 51 ++++++++++++++
.../apache/coyote/http2/TestHttp2Section_8_1.java | 80 +++++++++++++++++++++-
webapps/docs/changelog.xml | 8 +++
5 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 1b8202871f..593c05cdf7 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -148,6 +148,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
private void prepareRequest() {
+ if (coyoteRequest.scheme().isNull()) {
+ if (handler.getProtocol().getHttp11Protocol().isSSLEnabled()) {
+ coyoteRequest.scheme().setString("https");
+ } else {
+ coyoteRequest.scheme().setString("http");
+ }
+ }
MessageBytes hostValueMB = coyoteRequest.getMimeHeaders().getUniqueValue("host");
if (hostValueMB == null) {
throw new IllegalArgumentException();
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java
index 3e969cc5ea..675a62e18d 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -478,6 +478,12 @@ class StreamProcessor extends AbstractProcessor {
return false;
}
+ // Scheme must adhere to RFC 3986
+ String scheme = request.scheme().toString();
+ if (!HttpParser.isScheme(scheme)) {
+ return false;
+ }
+
// Invalid character in request target
// (other checks such as valid %nn happen later)
ByteChunk bc = request.requestURI().getByteChunk();
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 0758dd5915..6975e3b53f 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -47,6 +47,7 @@ public class HttpParser {
private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE];
private static final boolean[] IS_ALPHA = new boolean[ARRAY_SIZE];
private static final boolean[] IS_NUMERIC = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_SCHEME = new boolean[ARRAY_SIZE];
private static final boolean[] IS_UNRESERVED = new boolean[ARRAY_SIZE];
private static final boolean[] IS_SUBDELIM = new boolean[ARRAY_SIZE];
private static final boolean[] IS_USERINFO = new boolean[ARRAY_SIZE];
@@ -94,6 +95,10 @@ public class HttpParser {
IS_ALPHA[i] = true;
}
+ if (IS_ALPHA[i] || IS_NUMERIC[i] || i == '+' || i == '-' || i == '.') {
+ IS_SCHEME[i] = true;
+ }
+
if (IS_ALPHA[i] || IS_NUMERIC[i] || i == '-' || i == '.' || i == '_' || i == '~') {
IS_UNRESERVED[i] = true;
}
@@ -319,6 +324,52 @@ public class HttpParser {
}
+ public static boolean isScheme(int c) {
+ // Fast for valid scheme characters, slower for some incorrect
+ // ones
+ try {
+ return IS_SCHEME[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
+ /**
+ * Is the provided String a scheme as per RFC 3986?
+ * <br>
+ * Note: scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+ * <br>
+ * Since a scheme requires at least 1 ALPHA, {@code null} and the empty
+ * string ({@code ""}) are not considered to be valid tokens.
+ *
+ * @param s The string to test
+ *
+ * @return {@code true} if the string is a valid scheme, otherwise
+ * {@code false}
+ */
+ public static boolean isScheme(String s) {
+ if (s == null) {
+ return false;
+ }
+ if (s.isEmpty()) {
+ return false;
+ }
+ char[] chars = s.toCharArray();
+ if (!isAlpha(chars[0])) {
+ return false;
+ }
+ if (chars.length > 1) {
+ for (int i = 1; i < chars.length; i++) {
+ if (!isScheme(chars[i])) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+
public static boolean isUserInfo(int c) {
// Fast for valid user info characters, slower for some incorrect
// ones
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index e23693d6f4..620e5aeaa1 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -410,6 +410,11 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
private void doInvalidPseudoHeaderTest(List<Header> headers) throws Exception {
+ doInvalidPseudoHeaderTest(headers, "3-RST-[1]\n");
+ }
+
+ private void doInvalidPseudoHeaderTest(List<Header> headers, String expected) throws Exception {
+
byte[] headersFrameHeader = new byte[9];
ByteBuffer headersPayload = ByteBuffer.allocate(128);
@@ -420,6 +425,79 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
parser.readFrame();
- Assert.assertEquals("3-RST-[1]\n", output.getTrace());
+ String trace = output.getTrace();
+ if (trace.length() > expected.length()) {
+ trace = trace.substring(0, expected.length());
+ }
+ Assert.assertEquals(output.getTrace(), expected, trace);
+ }
+
+
+ @Test
+ public void testSchemeHeaderValid() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "abcd"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame();
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testSchemeHeaderInvalid01() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "ab!cd"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "3-HeadersStart\n3-Header-[:status]-[400]\n");
+ }
+
+
+ @Test
+ public void testSchemeHeaderInvalid02() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", ""));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "3-HeadersStart\n3-Header-[:status]-[400]\n");
+ }
+
+
+ @Test
+ public void testSchemeHeaderMissing() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "0-Goaway-[3]-[1]-");
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5438c9ee38..7d82d735bf 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -114,6 +114,14 @@
</update>
</changelog>
</subsection>
+ <subsection name="Coyote">
+ <changelog>
+ <add>
+ Add a check for the validity of the scheme pseudo-header in HTTP/2.
+ (markt)
+ </add>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 9.0.72 (remm)" rtext="2023-02-23">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org