Skip to content

Commit 96817a0

Browse files
committed
Add validation that HTTP/2 :scheme is consistent with TLS usage
1 parent 11478ae commit 96817a0

6 files changed

Lines changed: 95 additions & 4 deletions

File tree

java/org/apache/catalina/connector/CoyoteAdapter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,9 +555,10 @@ public String getDomain() {
555555
protected boolean postParseRequest(org.apache.coyote.Request req, Request request, org.apache.coyote.Response res,
556556
Response response) throws IOException, ServletException {
557557

558-
// If the processor has set the scheme (AJP does this, HTTP does this if
559-
// SSL is enabled) use this to set the secure flag as well. If the
560-
// processor hasn't set it, use the settings from the connector
558+
/*
559+
* If the processor has set the scheme (AJP and HTTP/2 do this, HTTP/1.x does this if SSL is enabled), use this
560+
* to set the secure flag as well. If the processor hasn't set it, use the settings from the connector.
561+
*/
561562
if (req.scheme().isNull()) {
562563
// Use connector scheme and secure configuration, (defaults to
563564
// "http" and false respectively)

java/org/apache/coyote/http2/LocalStrings.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ stream.header.contentLength=Connection [{0}], Stream [{1}], The content length h
9898
stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value [{3}]
9999
stream.header.duplicate=Connection [{0}], Stream [{1}], received multiple [{2}] headers
100100
stream.header.empty=Connection [{0}], Stream [{1}], Invalid empty header name
101+
stream.header.inconsistentScheme=Connection [{0}], Stream [{1}], The scheme [{2}] is not consistent with the TLS enabled setting of [{3}]
101102
stream.header.invalid=Connection [{0}], Stream [{1}], The header [{2}] contained invalid value [{3}]
102103
stream.header.noPath=Connection [{0}], Stream [{1}], The [:path] pseudo header was empty
103104
stream.header.required=Connection [{0}], Stream [{1}], One or more required headers was missing

java/org/apache/coyote/http2/Stream.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,13 @@ public final void emitHeader(String name, String value) throws HpackException {
394394
case ":scheme": {
395395
if (coyoteRequest.scheme().isNull()) {
396396
coyoteRequest.scheme().setString(value);
397+
// Check scheme is consistent with TLS usage
398+
if ("https".equals(value) != handler.getProtocol().getHttp11Protocol().isSSLEnabled()) {
399+
headerException = new StreamException(
400+
sm.getString("stream.header.inconsistentScheme", getConnectionId(), getIdAsString(),
401+
value, Boolean.toString(handler.getProtocol().getHttp11Protocol().isSSLEnabled())),
402+
Http2Error.PROTOCOL_ERROR, getIdAsInt());
403+
}
397404
} else {
398405
headerException = new StreamException(
399406
sm.getString("stream.header.duplicate", getConnectionId(), getIdAsString(), ":scheme"),

test/org/apache/coyote/http2/Http2TestBase.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,11 @@ protected void buildPostRequest(byte[] headersFrameHeader, ByteBuffer headersPay
377377

378378
MimeHeaders headers = new MimeHeaders();
379379
headers.addValue(":method").setString(Method.POST);
380-
headers.addValue(":scheme").setString("http");
380+
if (getTomcatInstance().getConnector().getSecure()) {
381+
headers.addValue(":scheme").setString("https");
382+
} else {
383+
headers.addValue(":scheme").setString("http");
384+
}
381385
headers.addValue(":path").setString(path);
382386
headers.addValue(":authority").setString("localhost:" + getPort());
383387
if (useExpectation) {
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.coyote.http2;
18+
19+
import java.nio.ByteBuffer;
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
23+
import org.junit.Assert;
24+
import org.junit.Test;
25+
26+
import org.apache.tomcat.util.http.Method;
27+
28+
/**
29+
* Unit tests for Section 8.3 of <a href="https://tools.ietf.org/html/rfc9113#section-8.3">RFC 9113</a>.
30+
*/
31+
public class TestHttp2Section_8_3 extends Http2TestBase {
32+
33+
/*
34+
* Not explicitly specified in section 8.3 but closely aligned to it.
35+
*/
36+
37+
@Test
38+
public void testSchemeInconsistencyNonTLS() throws Exception {
39+
testSchemeInconsistenct(false);
40+
}
41+
42+
43+
@Test
44+
public void testSchemeInconsistencyTLS() throws Exception {
45+
testSchemeInconsistenct(true);
46+
}
47+
48+
49+
private void testSchemeInconsistenct(boolean connectionUsesTls) throws Exception {
50+
// Start HTTP/2 over non-TLS connection
51+
http2Connect(connectionUsesTls);
52+
53+
byte[] frameHeader = new byte[9];
54+
ByteBuffer headersPayload = ByteBuffer.allocate(128);
55+
56+
List<Header> headers = new ArrayList<>(4);
57+
headers.add(new Header(":method", Method.GET));
58+
if (connectionUsesTls) {
59+
headers.add(new Header(":scheme", "http"));
60+
} else {
61+
headers.add(new Header(":scheme", "https"));
62+
}
63+
headers.add(new Header(":path", "/simple"));
64+
headers.add(new Header(":authority", "localhost:" + getPort()));
65+
66+
buildGetRequest(frameHeader, headersPayload, null, headers, 3);
67+
68+
writeFrame(frameHeader, headersPayload);
69+
70+
parser.readFrame();
71+
72+
Assert.assertEquals("3-RST-[1]\n", output.getTrace());
73+
}
74+
}

webapps/docs/changelog.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@
188188
<fix>
189189
Add TLS 1.3 groups added in OpenSSL 4.0. (remm)
190190
</fix>
191+
<fix>
192+
Add validation that the HTTP/2 <code>:scheme</code> pseudo-header is
193+
consistent with the use (or not) of TLS. (markt)
194+
</fix>
191195
</changelog>
192196
</subsection>
193197
<subsection name="Other">

0 commit comments

Comments
 (0)