This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 81712d2dd73b1460a709c1171019eef71572c1f8 Author: Mark Thomas <[email protected]> AuthorDate: Thu Apr 23 11:04:48 2026 +0100 Add validation that HTTP/2 :scheme is consistent with TLS usage --- .../apache/catalina/connector/CoyoteAdapter.java | 7 +- .../apache/coyote/http2/LocalStrings.properties | 1 + java/org/apache/coyote/http2/Stream.java | 7 ++ test/org/apache/coyote/http2/Http2TestBase.java | 6 +- .../apache/coyote/http2/TestHttp2Section_8_3.java | 74 ++++++++++++++++++++++ webapps/docs/changelog.xml | 4 ++ 6 files changed, 95 insertions(+), 4 deletions(-) diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 4c9fa7dfd4..6ef80a2535 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -555,9 +555,10 @@ public class CoyoteAdapter implements Adapter { protected boolean postParseRequest(org.apache.coyote.Request req, Request request, org.apache.coyote.Response res, Response response) throws IOException, ServletException { - // If the processor has set the scheme (AJP does this, HTTP does this if - // SSL is enabled) use this to set the secure flag as well. If the - // processor hasn't set it, use the settings from the connector + /* + * If the processor has set the scheme (AJP and HTTP/2 do this, HTTP/1.x does this if SSL is enabled), use this + * to set the secure flag as well. If the processor hasn't set it, use the settings from the connector. + */ if (req.scheme().isNull()) { // Use connector scheme and secure configuration, (defaults to // "http" and false respectively) diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties index 838538fe57..61c0d733fb 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -95,6 +95,7 @@ stream.header.contentLength=Connection [{0}], Stream [{1}], The content length h stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value [{3}] stream.header.duplicate=Connection [{0}], Stream [{1}], received multiple [{2}] headers stream.header.empty=Connection [{0}], Stream [{1}], Invalid empty header name +stream.header.inconsistentScheme=Connection [{0}], Stream [{1}], The scheme [{2}] is not consistent with the TLS enabled setting of [{3}] stream.header.invalid=Connection [{0}], Stream [{1}], The header [{2}] contained invalid value [{3}] stream.header.noPath=Connection [{0}], Stream [{1}], The [:path] pseudo header was empty stream.header.required=Connection [{0}], Stream [{1}], One or more required headers was missing diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index 2e83a441b5..c30b8f3460 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -395,6 +395,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { case ":scheme": { if (coyoteRequest.scheme().isNull()) { coyoteRequest.scheme().setString(value); + // Check scheme is consistent with TLS usage + if ("https".equals(value) != handler.getProtocol().getHttp11Protocol().isSSLEnabled()) { + headerException = new StreamException( + sm.getString("stream.header.inconsistentScheme", getConnectionId(), getIdAsString(), + value, Boolean.toString(handler.getProtocol().getHttp11Protocol().isSSLEnabled())), + Http2Error.PROTOCOL_ERROR, getIdAsInt()); + } } else { headerException = new StreamException( sm.getString("stream.header.duplicate", getConnectionId(), getIdAsString(), ":scheme"), diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java index 902c8d747d..5ef72be778 100644 --- a/test/org/apache/coyote/http2/Http2TestBase.java +++ b/test/org/apache/coyote/http2/Http2TestBase.java @@ -377,7 +377,11 @@ public abstract class Http2TestBase extends TomcatBaseTest { MimeHeaders headers = new MimeHeaders(); headers.addValue(":method").setString(Method.POST); - headers.addValue(":scheme").setString("http"); + if (getTomcatInstance().getConnector().getSecure()) { + headers.addValue(":scheme").setString("https"); + } else { + headers.addValue(":scheme").setString("http"); + } headers.addValue(":path").setString(path); headers.addValue(":authority").setString("localhost:" + getPort()); if (useExpectation) { diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_3.java b/test/org/apache/coyote/http2/TestHttp2Section_8_3.java new file mode 100644 index 0000000000..84ae06ad4d --- /dev/null +++ b/test/org/apache/coyote/http2/TestHttp2Section_8_3.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.coyote.http2; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.http.Method; + +/** + * Unit tests for Section 8.3 of <a href="https://tools.ietf.org/html/rfc9113#section-8.3";>RFC 9113</a>. + */ +public class TestHttp2Section_8_3 extends Http2TestBase { + + /* + * Not explicitly specified in section 8.3 but closely aligned to it. + */ + + @Test + public void testSchemeInconsistencyNonTLS() throws Exception { + testSchemeInconsistenct(false); + } + + + @Test + public void testSchemeInconsistencyTLS() throws Exception { + testSchemeInconsistenct(true); + } + + + private void testSchemeInconsistenct(boolean connectionUsesTls) throws Exception { + // Start HTTP/2 over non-TLS connection + http2Connect(connectionUsesTls); + + byte[] frameHeader = new byte[9]; + ByteBuffer headersPayload = ByteBuffer.allocate(128); + + List<Header> headers = new ArrayList<>(4); + headers.add(new Header(":method", Method.GET)); + if (connectionUsesTls) { + headers.add(new Header(":scheme", "http")); + } else { + headers.add(new Header(":scheme", "https")); + } + headers.add(new Header(":path", "/simple")); + headers.add(new Header(":authority", "localhost:" + getPort())); + + buildGetRequest(frameHeader, headersPayload, null, headers, 3); + + writeFrame(frameHeader, headersPayload); + + parser.readFrame(); + + Assert.assertEquals("3-RST-[1]\n", output.getTrace()); + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index fa3214b054..f78277e4db 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -295,6 +295,10 @@ <fix> Add TLS 1.3 groups added in OpenSSL 4.0. (remm) </fix> + <fix> + Add validation that the HTTP/2 <code>:scheme</code> pseudo-header is + consistent with the use (or not) of TLS. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
