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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org