You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rainer Jung <ra...@kippdata.de> on 2023/06/08 03:50:03 UTC
Re: [tomcat] branch 9.0.x updated: Fix BZ 66548 - Add validation of Sec-Websocket-Key header
The new TestKeyHeader fails for me very frequently for TC 9 and TC 8.5,
but not for 10.1 or 11.
For TC 9 it fails roughly 30-50% of the times I run it. It fails for
jsse and for tcnative and for a wide range of JDKs and RHEL/SLES Linux
versions.
The failure happens only for NIO2, not for NIO. When it fails, the test
case takes about 60 seconds longer than when it is OK. The failing test
case is often testValid - the first test case that runs - and sometime
the second one testTooLong01.
It seems the pattern for TC 8.5 is the same but the tests are still running.
Best regards,
Rainer
Am 24.05.23 um 12:29 schrieb markt@apache.org:
> 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 37f979762b Fix BZ 66548 - Add validation of Sec-Websocket-Key header
> 37f979762b is described below
>
> commit 37f979762b6ce28031e8e14a3d258cb1349e05a1
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Tue Apr 11 08:18:43 2023 +0100
>
> Fix BZ 66548 - Add validation of Sec-Websocket-Key header
>
> Note that the validation isn't perfect. It aims to be good enough.
> https://bz.apache.org/bugzilla/show_bug.cgi?id=66548
> ---
> .../apache/tomcat/util/codec/binary/Base64.java | 9 +++
> .../tomcat/websocket/server/UpgradeUtil.java | 39 +++++++++-
> .../tomcat/websocket/server/TestKeyHeader.java | 87 ++++++++++++++++++++++
> .../tomcat/websocket/server/TesterWsClient.java | 18 ++++-
> webapps/docs/changelog.xml | 11 +++
> 5 files changed, 159 insertions(+), 5 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/codec/binary/Base64.java b/java/org/apache/tomcat/util/codec/binary/Base64.java
> index c3b4fa9c16..884c3190d0 100644
> --- a/java/org/apache/tomcat/util/codec/binary/Base64.java
> +++ b/java/org/apache/tomcat/util/codec/binary/Base64.java
> @@ -434,6 +434,15 @@ public class Base64 extends BaseNCodec {
> }
>
>
> + public static boolean isInAlphabet(char c) {
> + // Fast for valid data. May be slow for invalid data.
> + try {
> + return STANDARD_DECODE_TABLE[c] != -1;
> + } catch (ArrayIndexOutOfBoundsException ex) {
> + return false;
> + }
> + }
> +
> /**
> * Encode table to use: either STANDARD or URL_SAFE. Note: the DECODE_TABLE above remains static because it is able
> * to decode both STANDARD and URL_SAFE streams, but the encodeTable must be a member variable so we can switch
> diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
> index eec6698f75..96d7aaf943 100644
> --- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
> +++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
> @@ -95,7 +95,7 @@ public class UpgradeUtil {
> return;
> }
> key = req.getHeader(Constants.WS_KEY_HEADER_NAME);
> - if (key == null) {
> + if (!validateKey(key)) {
> resp.sendError(HttpServletResponse.SC_BAD_REQUEST);
> return;
> }
> @@ -224,6 +224,43 @@ public class UpgradeUtil {
> }
>
>
> + /*
> + * Validate the key. It should be the base64 encoding of a random 16-byte value. 16-bytes are encoded in 24 base64
> + * characters, the last two of which must be ==.
> + *
> + * The validation isn't perfect:
> + *
> + * - it doesn't check the final non-'=' character is valid in the context of the number of bits it is meant to be
> + * encoding.
> + *
> + * - it doesn't check that the value is random and changes for each connection.
> + *
> + * Given that this header is for the benefit of the client, not the server, this should be good enough.
> + */
> + private static boolean validateKey(String key) {
> + if (key == null) {
> + return false;
> + }
> +
> + if (key.length() != 24) {
> + return false;
> + }
> +
> + char[] keyChars = key.toCharArray();
> + if (keyChars[22] != '=' || keyChars[23] != '=') {
> + return false;
> + }
> +
> + for (int i = 0; i < 22; i++) {
> + if (!Base64.isInAlphabet(keyChars[i])) {
> + return false;
> + }
> + }
> +
> + return true;
> + }
> +
> +
> private static List<Transformation> createTransformations(List<Extension> negotiatedExtensions) {
>
> TransformationFactory factory = TransformationFactory.getInstance();
> diff --git a/test/org/apache/tomcat/websocket/server/TestKeyHeader.java b/test/org/apache/tomcat/websocket/server/TestKeyHeader.java
> new file mode 100644
> index 0000000000..fa05e44304
> --- /dev/null
> +++ b/test/org/apache/tomcat/websocket/server/TestKeyHeader.java
> @@ -0,0 +1,87 @@
> +/*
> + * 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.tomcat.websocket.server;
> +
> +import java.nio.charset.StandardCharsets;
> +
> +import javax.servlet.http.HttpServletResponse;
> +import javax.websocket.CloseReason.CloseCodes;
> +
> +import org.junit.Assert;
> +import org.junit.Test;
> +
> +import org.apache.tomcat.websocket.TesterEchoServer;
> +import org.apache.tomcat.websocket.WebSocketBaseTest;
> +
> +public class TestKeyHeader extends WebSocketBaseTest {
> +
> + @Test
> + public void testEmptyString() throws Exception {
> + doTest("", HttpServletResponse.SC_BAD_REQUEST);
> + }
> +
> +
> + @Test
> + public void testValid() throws Exception {
> + // "0123456789012345" encoded with base64
> + doTest("MDEyMzQ1Njc4OTAxMjM0NQ==", HttpServletResponse.SC_SWITCHING_PROTOCOLS);
> + }
> +
> +
> + @Test
> + public void testInvalidCharacter() throws Exception {
> + // "0123456789012345" encoded with base64
> + doTest("MDEy(zQ1Njc4OTAxMjM0NQ==", HttpServletResponse.SC_BAD_REQUEST);
> + }
> +
> +
> + @Test
> + public void testTooShort() throws Exception {
> + // "012345678901234" encoded with base64
> + doTest("MDEyMzQ1Njc4OTAxMjM0", HttpServletResponse.SC_BAD_REQUEST);
> + }
> +
> +
> + @Test
> + public void testTooLong01() throws Exception {
> + // "01234567890123456" encoded with base64
> + doTest("MDEyMzQ1Njc4OTAxMjM0NTY=", HttpServletResponse.SC_BAD_REQUEST);
> + }
> +
> +
> + @Test
> + public void testTooLong02() throws Exception {
> + // "012345678901234678" encoded with base64
> + doTest("MDEyMzQ1Njc4OTAxMjM0NTY3OA==", HttpServletResponse.SC_BAD_REQUEST);
> + }
> +
> + private void doTest(String keyHeaderValue, int expectedStatusCode) throws Exception {
> + startServer(TesterEchoServer.Config.class);
> +
> + TesterWsClient client = new TesterWsClient("localhost", getPort(), keyHeaderValue);
> + String req = client.createUpgradeRequest(TesterEchoServer.Config.PATH_BASIC);
> + client.write(req.getBytes(StandardCharsets.UTF_8));
> + int rc = client.readUpgradeResponse();
> +
> + Assert.assertEquals(expectedStatusCode, rc);
> +
> + if (expectedStatusCode == HttpServletResponse.SC_SWITCHING_PROTOCOLS) {
> + client.sendCloseFrame(CloseCodes.NORMAL_CLOSURE);
> + }
> + client.closeSocket();
> + }
> +}
> diff --git a/test/org/apache/tomcat/websocket/server/TesterWsClient.java b/test/org/apache/tomcat/websocket/server/TesterWsClient.java
> index e060a7168b..7a5c9c4ee0 100644
> --- a/test/org/apache/tomcat/websocket/server/TesterWsClient.java
> +++ b/test/org/apache/tomcat/websocket/server/TesterWsClient.java
> @@ -30,10 +30,16 @@ import javax.websocket.CloseReason.CloseCode;
> public class TesterWsClient {
>
> private static final byte[] maskingKey = new byte[] { 0x12, 0x34, 0x56, 0x78 };
> + private static final String DEFAULT_KEY_HEADER_VALUE = "OEvAoAKn5jsuqv2/YJ1Wfg==";
>
> private final Socket socket;
> + private final String keyHeaderValue;
>
> public TesterWsClient(String host, int port) throws Exception {
> + this(host, port, DEFAULT_KEY_HEADER_VALUE);
> + }
> +
> + public TesterWsClient(String host, int port, String keyHeaderValue) throws Exception {
> this.socket = new Socket(host, port);
> // Set read timeout in case of failure so test doesn't hang
> socket.setSoTimeout(2000);
> @@ -41,6 +47,7 @@ public class TesterWsClient {
> // TODO: Hoping this causes writes to wait for a TCP ACK for TCP RST
> // test cases but I'm not sure?
> socket.setTcpNoDelay(true);
> + this.keyHeaderValue = keyHeaderValue;
> }
>
> public void httpUpgrade(String path) throws IOException {
> @@ -65,12 +72,15 @@ public class TesterWsClient {
> write(createFrame(true, 8, codeBytes));
> }
>
> - private void readUpgradeResponse() throws IOException {
> + public int readUpgradeResponse() throws IOException {
> BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
> String line = in.readLine();
> + // line expected to be "HTTP/1.1 nnn "
> + int result = Integer.parseInt(line.substring(9, 12));
> while (line != null && !line.isEmpty()) {
> line = in.readLine();
> }
> + return result;
> }
>
> public void closeSocket() throws IOException {
> @@ -89,14 +99,14 @@ public class TesterWsClient {
> socket.close();
> }
>
> - private void write(byte[] bytes) throws IOException {
> + public void write(byte[] bytes) throws IOException {
> socket.getOutputStream().write(bytes);
> socket.getOutputStream().flush();
> }
>
> - private static String createUpgradeRequest(String path) {
> + public String createUpgradeRequest(String path) {
> String[] upgradeRequestLines = { "GET " + path + " HTTP/1.1", "Connection: Upgrade", "Host: localhost:8080",
> - "Origin: localhost:8080", "Sec-WebSocket-Key: OEvAoAKn5jsuqv2/YJ1Wfg==", "Sec-WebSocket-Version: 13",
> + "Origin: localhost:8080", "Sec-WebSocket-Key: " + keyHeaderValue, "Sec-WebSocket-Version: 13",
> "Upgrade: websocket" };
> StringBuffer sb = new StringBuffer();
> for (String line : upgradeRequestLines) {
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index fc0075a63a..28cc871d8b 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -139,6 +139,17 @@
> </fix>
> </changelog>
> </subsection>
> + <subsection name="WebSocket">
> + <changelog>
> + <fix>
> + <bug>66548</bug>: Expand the validation of the value of the
> + <code>Sec-Websocket-Key</code> header in the HTTP upgrade request that
> + initiates a WebSocket connection. The value is not decoded but it is
> + checked for the correct length and that only valid characters from the
> + base64 alphabet are used. (markt)
> + </fix>
> + </changelog>
> + </subsection>
> <subsection name="Other">
> <changelog>
> <update>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 9.0.x updated: Fix BZ 66548 - Add validation of Sec-Websocket-Key header
Posted by Mark Thomas <ma...@apache.org>.
On 08/06/2023 09:57, Rémy Maucherat wrote:
> On Thu, Jun 8, 2023 at 6:07 AM Rainer Jung <ra...@kippdata.de> wrote:
>>
>> Am 08.06.23 um 05:50 schrieb Rainer Jung:
>>> The new TestKeyHeader fails for me very frequently for TC 9 and TC 8.5,
>>> but not for 10.1 or 11.
>>>
>>> For TC 9 it fails roughly 30-50% of the times I run it. It fails for
>>> jsse and for tcnative and for a wide range of JDKs and RHEL/SLES Linux
>>> versions.
>>>
>>> The failure happens only for NIO2, not for NIO. When it fails, the test
>>> case takes about 60 seconds longer than when it is OK. The failing test
>>> case is often testValid - the first test case that runs - and sometime
>>> the second one testTooLong01.
>>>
>>> It seems the pattern for TC 8.5 is the same but the tests are still
>>> running.
>>
>> You probably already fixed it for 10.1 and 11, but for 9.0 and 8.5 the
>> backport of 44e7282b54 seems to be missing.
>
> I'm not running into the issue but I added the commit to the two branches.
Thanks.
Arguably, reading and ignoring the response was fixing the symptom not
the cause. The root cause being that:
- Tomcat was stopped while processing an HTTP upgrade to WebSocket
- the upgrade did not complete
- the error was swallowed so the connection wasn't cleaned-up
https://github.com/apache/tomcat/commit/a2c34d6569f2b959413b06852d7a957ff9e9c39d
(back-ported to all versions) should have addressed the root cause.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 9.0.x updated: Fix BZ 66548 - Add validation of Sec-Websocket-Key header
Posted by Rémy Maucherat <re...@apache.org>.
On Thu, Jun 8, 2023 at 6:07 AM Rainer Jung <ra...@kippdata.de> wrote:
>
> Am 08.06.23 um 05:50 schrieb Rainer Jung:
> > The new TestKeyHeader fails for me very frequently for TC 9 and TC 8.5,
> > but not for 10.1 or 11.
> >
> > For TC 9 it fails roughly 30-50% of the times I run it. It fails for
> > jsse and for tcnative and for a wide range of JDKs and RHEL/SLES Linux
> > versions.
> >
> > The failure happens only for NIO2, not for NIO. When it fails, the test
> > case takes about 60 seconds longer than when it is OK. The failing test
> > case is often testValid - the first test case that runs - and sometime
> > the second one testTooLong01.
> >
> > It seems the pattern for TC 8.5 is the same but the tests are still
> > running.
>
> You probably already fixed it for 10.1 and 11, but for 9.0 and 8.5 the
> backport of 44e7282b54 seems to be missing.
I'm not running into the issue but I added the commit to the two branches.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 9.0.x updated: Fix BZ 66548 - Add validation of Sec-Websocket-Key header
Posted by Rainer Jung <ra...@kippdata.de>.
Am 08.06.23 um 05:50 schrieb Rainer Jung:
> The new TestKeyHeader fails for me very frequently for TC 9 and TC 8.5,
> but not for 10.1 or 11.
>
> For TC 9 it fails roughly 30-50% of the times I run it. It fails for
> jsse and for tcnative and for a wide range of JDKs and RHEL/SLES Linux
> versions.
>
> The failure happens only for NIO2, not for NIO. When it fails, the test
> case takes about 60 seconds longer than when it is OK. The failing test
> case is often testValid - the first test case that runs - and sometime
> the second one testTooLong01.
>
> It seems the pattern for TC 8.5 is the same but the tests are still
> running.
You probably already fixed it for 10.1 and 11, but for 9.0 and 8.5 the
backport of 44e7282b54 seems to be missing.
> Best regards,
>
> Rainer
>
> Am 24.05.23 um 12:29 schrieb markt@apache.org:
>> 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 37f979762b Fix BZ 66548 - Add validation of
>> Sec-Websocket-Key header
>> 37f979762b is described below
>>
>> commit 37f979762b6ce28031e8e14a3d258cb1349e05a1
>> Author: Mark Thomas <ma...@apache.org>
>> AuthorDate: Tue Apr 11 08:18:43 2023 +0100
>>
>> Fix BZ 66548 - Add validation of Sec-Websocket-Key header
>> Note that the validation isn't perfect. It aims to be good enough.
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=66548
>> ---
>> .../apache/tomcat/util/codec/binary/Base64.java | 9 +++
>> .../tomcat/websocket/server/UpgradeUtil.java | 39 +++++++++-
>> .../tomcat/websocket/server/TestKeyHeader.java | 87
>> ++++++++++++++++++++++
>> .../tomcat/websocket/server/TesterWsClient.java | 18 ++++-
>> webapps/docs/changelog.xml | 11 +++
>> 5 files changed, 159 insertions(+), 5 deletions(-)
>>
>> diff --git a/java/org/apache/tomcat/util/codec/binary/Base64.java
>> b/java/org/apache/tomcat/util/codec/binary/Base64.java
>> index c3b4fa9c16..884c3190d0 100644
>> --- a/java/org/apache/tomcat/util/codec/binary/Base64.java
>> +++ b/java/org/apache/tomcat/util/codec/binary/Base64.java
>> @@ -434,6 +434,15 @@ public class Base64 extends BaseNCodec {
>> }
>> + public static boolean isInAlphabet(char c) {
>> + // Fast for valid data. May be slow for invalid data.
>> + try {
>> + return STANDARD_DECODE_TABLE[c] != -1;
>> + } catch (ArrayIndexOutOfBoundsException ex) {
>> + return false;
>> + }
>> + }
>> +
>> /**
>> * Encode table to use: either STANDARD or URL_SAFE. Note: the
>> DECODE_TABLE above remains static because it is able
>> * to decode both STANDARD and URL_SAFE streams, but the
>> encodeTable must be a member variable so we can switch
>> diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
>> b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
>> index eec6698f75..96d7aaf943 100644
>> --- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
>> +++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
>> @@ -95,7 +95,7 @@ public class UpgradeUtil {
>> return;
>> }
>> key = req.getHeader(Constants.WS_KEY_HEADER_NAME);
>> - if (key == null) {
>> + if (!validateKey(key)) {
>> resp.sendError(HttpServletResponse.SC_BAD_REQUEST);
>> return;
>> }
>> @@ -224,6 +224,43 @@ public class UpgradeUtil {
>> }
>> + /*
>> + * Validate the key. It should be the base64 encoding of a random
>> 16-byte value. 16-bytes are encoded in 24 base64
>> + * characters, the last two of which must be ==.
>> + *
>> + * The validation isn't perfect:
>> + *
>> + * - it doesn't check the final non-'=' character is valid in the
>> context of the number of bits it is meant to be
>> + * encoding.
>> + *
>> + * - it doesn't check that the value is random and changes for
>> each connection.
>> + *
>> + * Given that this header is for the benefit of the client, not
>> the server, this should be good enough.
>> + */
>> + private static boolean validateKey(String key) {
>> + if (key == null) {
>> + return false;
>> + }
>> +
>> + if (key.length() != 24) {
>> + return false;
>> + }
>> +
>> + char[] keyChars = key.toCharArray();
>> + if (keyChars[22] != '=' || keyChars[23] != '=') {
>> + return false;
>> + }
>> +
>> + for (int i = 0; i < 22; i++) {
>> + if (!Base64.isInAlphabet(keyChars[i])) {
>> + return false;
>> + }
>> + }
>> +
>> + return true;
>> + }
>> +
>> +
>> private static List<Transformation>
>> createTransformations(List<Extension> negotiatedExtensions) {
>> TransformationFactory factory =
>> TransformationFactory.getInstance();
>> diff --git
>> a/test/org/apache/tomcat/websocket/server/TestKeyHeader.java
>> b/test/org/apache/tomcat/websocket/server/TestKeyHeader.java
>> new file mode 100644
>> index 0000000000..fa05e44304
>> --- /dev/null
>> +++ b/test/org/apache/tomcat/websocket/server/TestKeyHeader.java
>> @@ -0,0 +1,87 @@
>> +/*
>> + * 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.tomcat.websocket.server;
>> +
>> +import java.nio.charset.StandardCharsets;
>> +
>> +import javax.servlet.http.HttpServletResponse;
>> +import javax.websocket.CloseReason.CloseCodes;
>> +
>> +import org.junit.Assert;
>> +import org.junit.Test;
>> +
>> +import org.apache.tomcat.websocket.TesterEchoServer;
>> +import org.apache.tomcat.websocket.WebSocketBaseTest;
>> +
>> +public class TestKeyHeader extends WebSocketBaseTest {
>> +
>> + @Test
>> + public void testEmptyString() throws Exception {
>> + doTest("", HttpServletResponse.SC_BAD_REQUEST);
>> + }
>> +
>> +
>> + @Test
>> + public void testValid() throws Exception {
>> + // "0123456789012345" encoded with base64
>> + doTest("MDEyMzQ1Njc4OTAxMjM0NQ==",
>> HttpServletResponse.SC_SWITCHING_PROTOCOLS);
>> + }
>> +
>> +
>> + @Test
>> + public void testInvalidCharacter() throws Exception {
>> + // "0123456789012345" encoded with base64
>> + doTest("MDEy(zQ1Njc4OTAxMjM0NQ==",
>> HttpServletResponse.SC_BAD_REQUEST);
>> + }
>> +
>> +
>> + @Test
>> + public void testTooShort() throws Exception {
>> + // "012345678901234" encoded with base64
>> + doTest("MDEyMzQ1Njc4OTAxMjM0",
>> HttpServletResponse.SC_BAD_REQUEST);
>> + }
>> +
>> +
>> + @Test
>> + public void testTooLong01() throws Exception {
>> + // "01234567890123456" encoded with base64
>> + doTest("MDEyMzQ1Njc4OTAxMjM0NTY=",
>> HttpServletResponse.SC_BAD_REQUEST);
>> + }
>> +
>> +
>> + @Test
>> + public void testTooLong02() throws Exception {
>> + // "012345678901234678" encoded with base64
>> + doTest("MDEyMzQ1Njc4OTAxMjM0NTY3OA==",
>> HttpServletResponse.SC_BAD_REQUEST);
>> + }
>> +
>> + private void doTest(String keyHeaderValue, int
>> expectedStatusCode) throws Exception {
>> + startServer(TesterEchoServer.Config.class);
>> +
>> + TesterWsClient client = new TesterWsClient("localhost",
>> getPort(), keyHeaderValue);
>> + String req =
>> client.createUpgradeRequest(TesterEchoServer.Config.PATH_BASIC);
>> + client.write(req.getBytes(StandardCharsets.UTF_8));
>> + int rc = client.readUpgradeResponse();
>> +
>> + Assert.assertEquals(expectedStatusCode, rc);
>> +
>> + if (expectedStatusCode ==
>> HttpServletResponse.SC_SWITCHING_PROTOCOLS) {
>> + client.sendCloseFrame(CloseCodes.NORMAL_CLOSURE);
>> + }
>> + client.closeSocket();
>> + }
>> +}
>> diff --git
>> a/test/org/apache/tomcat/websocket/server/TesterWsClient.java
>> b/test/org/apache/tomcat/websocket/server/TesterWsClient.java
>> index e060a7168b..7a5c9c4ee0 100644
>> --- a/test/org/apache/tomcat/websocket/server/TesterWsClient.java
>> +++ b/test/org/apache/tomcat/websocket/server/TesterWsClient.java
>> @@ -30,10 +30,16 @@ import javax.websocket.CloseReason.CloseCode;
>> public class TesterWsClient {
>> private static final byte[] maskingKey = new byte[] { 0x12,
>> 0x34, 0x56, 0x78 };
>> + private static final String DEFAULT_KEY_HEADER_VALUE =
>> "OEvAoAKn5jsuqv2/YJ1Wfg==";
>> private final Socket socket;
>> + private final String keyHeaderValue;
>> public TesterWsClient(String host, int port) throws Exception {
>> + this(host, port, DEFAULT_KEY_HEADER_VALUE);
>> + }
>> +
>> + public TesterWsClient(String host, int port, String
>> keyHeaderValue) throws Exception {
>> this.socket = new Socket(host, port);
>> // Set read timeout in case of failure so test doesn't hang
>> socket.setSoTimeout(2000);
>> @@ -41,6 +47,7 @@ public class TesterWsClient {
>> // TODO: Hoping this causes writes to wait for a TCP ACK for
>> TCP RST
>> // test cases but I'm not sure?
>> socket.setTcpNoDelay(true);
>> + this.keyHeaderValue = keyHeaderValue;
>> }
>> public void httpUpgrade(String path) throws IOException {
>> @@ -65,12 +72,15 @@ public class TesterWsClient {
>> write(createFrame(true, 8, codeBytes));
>> }
>> - private void readUpgradeResponse() throws IOException {
>> + public int readUpgradeResponse() throws IOException {
>> BufferedReader in = new BufferedReader(new
>> InputStreamReader(socket.getInputStream()));
>> String line = in.readLine();
>> + // line expected to be "HTTP/1.1 nnn "
>> + int result = Integer.parseInt(line.substring(9, 12));
>> while (line != null && !line.isEmpty()) {
>> line = in.readLine();
>> }
>> + return result;
>> }
>> public void closeSocket() throws IOException {
>> @@ -89,14 +99,14 @@ public class TesterWsClient {
>> socket.close();
>> }
>> - private void write(byte[] bytes) throws IOException {
>> + public void write(byte[] bytes) throws IOException {
>> socket.getOutputStream().write(bytes);
>> socket.getOutputStream().flush();
>> }
>> - private static String createUpgradeRequest(String path) {
>> + public String createUpgradeRequest(String path) {
>> String[] upgradeRequestLines = { "GET " + path + "
>> HTTP/1.1", "Connection: Upgrade", "Host: localhost:8080",
>> - "Origin: localhost:8080", "Sec-WebSocket-Key:
>> OEvAoAKn5jsuqv2/YJ1Wfg==", "Sec-WebSocket-Version: 13",
>> + "Origin: localhost:8080", "Sec-WebSocket-Key: " +
>> keyHeaderValue, "Sec-WebSocket-Version: 13",
>> "Upgrade: websocket" };
>> StringBuffer sb = new StringBuffer();
>> for (String line : upgradeRequestLines) {
>> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
>> index fc0075a63a..28cc871d8b 100644
>> --- a/webapps/docs/changelog.xml
>> +++ b/webapps/docs/changelog.xml
>> @@ -139,6 +139,17 @@
>> </fix>
>> </changelog>
>> </subsection>
>> + <subsection name="WebSocket">
>> + <changelog>
>> + <fix>
>> + <bug>66548</bug>: Expand the validation of the value of the
>> + <code>Sec-Websocket-Key</code> header in the HTTP upgrade
>> request that
>> + initiates a WebSocket connection. The value is not decoded
>> but it is
>> + checked for the correct length and that only valid characters
>> from the
>> + base64 alphabet are used. (markt)
>> + </fix>
>> + </changelog>
>> + </subsection>
>> <subsection name="Other">
>> <changelog>
>> <update>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org