From d798de663e834450acec1041e44bae938a7b45b6 Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Mon, 17 Jun 2024 01:02:20 -0300 Subject: [PATCH 1/3] [UNDERTOW-2405] CVE-2024-27316 Remove new max connections per listener config, we can use high and low watermarks for this This reverts commit c27c1e40c945c11f13b210fd72fadf0ae641f3d0. Signed-off-by: Flavia Rainone --- .../protocol/http/HttpOpenListener.java | 33 ++++++++----------- .../protocol/http2/Http2OpenListener.java | 10 ++---- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpOpenListener.java b/core/src/main/java/io/undertow/server/protocol/http/HttpOpenListener.java index 8b9368f1de..27ede4cb48 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpOpenListener.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpOpenListener.java @@ -18,6 +18,19 @@ package io.undertow.server.protocol.http; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.xnio.ChannelListener; +import org.xnio.IoUtils; +import org.xnio.OptionMap; +import org.xnio.Options; +import org.xnio.Pool; +import org.xnio.StreamConnection; + import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; import io.undertow.UndertowOptions; @@ -34,18 +47,6 @@ import io.undertow.server.HttpHandler; import io.undertow.server.ServerConnection; import io.undertow.server.XnioByteBufferPool; -import org.xnio.ChannelListener; -import org.xnio.IoUtils; -import org.xnio.OptionMap; -import org.xnio.Options; -import org.xnio.Pool; -import org.xnio.StreamConnection; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.util.Collections; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; /** * Open listener for HTTP server. XNIO should be set up to chain the accept handler to post-accept open @@ -55,8 +56,6 @@ */ public final class HttpOpenListener implements ChannelListener, DelegateOpenListener { - private static final int DEFAULT_MAX_CONNECTIONS_PER_LISTENER = 100; - private static final int MAX_CONNECTIONS_PER_LISTENER = Integer.getInteger("io.undertow.max-connections-per-listener", DEFAULT_MAX_CONNECTIONS_PER_LISTENER); private final Set connections = Collections.newSetFromMap(new ConcurrentHashMap<>()); private final ByteBufferPool bufferPool; @@ -103,12 +102,6 @@ public void handleEvent(StreamConnection channel) { @Override public void handleEvent(final StreamConnection channel, PooledByteBuffer buffer) { - if (connections.size() >= MAX_CONNECTIONS_PER_LISTENER) { - UndertowLogger.REQUEST_IO_LOGGER.debugf("Reached maximum number of connections %d per listener; closing open connection request from %s", - MAX_CONNECTIONS_PER_LISTENER, channel.getPeerAddress()); - IoUtils.safeClose(channel); - return; - } if (UndertowLogger.REQUEST_LOGGER.isTraceEnabled()) { UndertowLogger.REQUEST_LOGGER.tracef("Opened connection with %s", channel.getPeerAddress()); } diff --git a/core/src/main/java/io/undertow/server/protocol/http2/Http2OpenListener.java b/core/src/main/java/io/undertow/server/protocol/http2/Http2OpenListener.java index 15e6082c8c..5d0015fda8 100644 --- a/core/src/main/java/io/undertow/server/protocol/http2/Http2OpenListener.java +++ b/core/src/main/java/io/undertow/server/protocol/http2/Http2OpenListener.java @@ -51,8 +51,7 @@ */ public final class Http2OpenListener implements ChannelListener, DelegateOpenListener { - private static final int DEFAULT_MAX_CONNECTIONS_PER_LISTENER = 100; - private static final int MAX_CONNECTIONS_PER_LISTENER = Integer.getInteger("io.undertow.max-connections-per-listener", DEFAULT_MAX_CONNECTIONS_PER_LISTENER); + private final Set connections = Collections.newSetFromMap(new ConcurrentHashMap<>()); @@ -114,12 +113,7 @@ public void handleEvent(final StreamConnection channel, PooledByteBuffer buffer) if (UndertowLogger.REQUEST_LOGGER.isTraceEnabled()) { UndertowLogger.REQUEST_LOGGER.tracef("Opened HTTP/2 connection with %s", channel.getPeerAddress()); } - if (connections.size() >= MAX_CONNECTIONS_PER_LISTENER) { - UndertowLogger.REQUEST_IO_LOGGER.debugf("Reached maximum number of connections %d per listener; closing open connection request from %s", - MAX_CONNECTIONS_PER_LISTENER, channel.getPeerAddress()); - IoUtils.safeClose(channel); - return; - } + //cool, we have a Http2 connection. Http2Channel http2Channel = new Http2Channel(channel, protocol, bufferPool, buffer, false, false, undertowOptions); Integer idleTimeout = undertowOptions.get(UndertowOptions.IDLE_TIMEOUT); From 296636d341dd8c9ff60dae017500c61f051bc42a Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Mon, 17 Jun 2024 01:13:08 -0300 Subject: [PATCH 2/3] [UNDERTOW-2405] CVE-2024-27316 Create system property io.undertow.http2-max-header-size, for configuring the maximum size of HTTP2 header sizes, default value set to 20000 Add a test for that new configuration and update affected tests accordingly. Also: add TODO place holders for new config Undertow.HTTP2_MAX_HEADER_SIZE to be added in Undertow 2.4.0.Final. Signed-off-by: Flavia Rainone --- core/pom.xml | 95 ++++++++++++++- .../java/io/undertow/UndertowMessages.java | 4 +- .../protocols/http2/Http2Channel.java | 66 +++++++++- .../http2/Http2HeaderBlockParser.java | 8 +- .../handlers/DefaultMaxHeaderTestCase.java | 114 ++++++++++++++++++ .../server/handlers/LongURLTestCase.java | 31 +++-- .../LotsOfHeadersResponseTestCase.java | 4 + .../LotsOfQueryParametersTestCase.java | 20 ++- .../io/undertow/testutils/DefaultServer.java | 17 ++- 9 files changed, 340 insertions(+), 19 deletions(-) create mode 100644 core/src/test/java/io/undertow/server/handlers/DefaultMaxHeaderTestCase.java diff --git a/core/pom.xml b/core/pom.xml index 429a31a2aa..d208965baf 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -372,6 +372,9 @@ true reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + true ${dump} @@ -396,6 +399,9 @@ true reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + true ${dump} @@ -411,7 +417,6 @@ ${project.build.directory}/surefire-h2c-reports - proxy-h2c-upgrade test @@ -421,6 +426,93 @@ true reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + + + true + ${dump} + ${bufferSize} + localhost + 7777 + org.jboss.logmanager.LogManager + + ${test.level} + ${test.ipv6} + false + + ${project.build.directory}/surefire-h2c-upgrade-reports + + + + + proxy-h2-big-http2-max-header-size + test + + test + + + true + reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + + + true + ${dump} + ${bufferSize} + localhost + 7777 + org.jboss.logmanager.LogManager + + ${test.level} + ${test.ipv6} + false + 600000 + + ${project.build.directory}/surefire-h2-reports + + + + proxy-h2c-big-http2-max-header-size + test + + test + + + true + reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + + + true + ${dump} + ${bufferSize} + localhost + 7777 + org.jboss.logmanager.LogManager + + ${test.level} + ${test.ipv6} + false + 600000 + + ${project.build.directory}/surefire-h2c-reports + + + + proxy-h2c-upgrade-big-http2-max-header-size + test + + test + + + true + reversealphabetical + + *LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase + true ${dump} @@ -432,6 +524,7 @@ ${test.level} ${test.ipv6} false + 600000 ${project.build.directory}/surefire-h2c-upgrade-reports diff --git a/core/src/main/java/io/undertow/UndertowMessages.java b/core/src/main/java/io/undertow/UndertowMessages.java index 93dffd598b..14eb1b3fd3 100644 --- a/core/src/main/java/io/undertow/UndertowMessages.java +++ b/core/src/main/java/io/undertow/UndertowMessages.java @@ -518,8 +518,8 @@ public interface UndertowMessages { // @Message(id = 159, value = "Max size must be larger than one") // IllegalArgumentException maxSizeMustBeLargerThanOne(); - @Message(id = 161, value = "HTTP/2 header block is too large") - String headerBlockTooLarge(); + @Message(id = 161, value = "HTTP/2 header block is too large, maximum header size is %s") + String headerBlockTooLarge(int maxHeaderSize); @Message(id = 162, value = "An invalid SameSite attribute [%s] is specified. It must be one of %s") IllegalArgumentException invalidSameSiteMode(String mode, String validAttributes); diff --git a/core/src/main/java/io/undertow/protocols/http2/Http2Channel.java b/core/src/main/java/io/undertow/protocols/http2/Http2Channel.java index c8663ac2bb..4241d2439f 100644 --- a/core/src/main/java/io/undertow/protocols/http2/Http2Channel.java +++ b/core/src/main/java/io/undertow/protocols/http2/Http2Channel.java @@ -70,6 +70,29 @@ */ public class Http2Channel extends AbstractFramedChannel implements Attachable { + // HTTP2 MAX HEADER SIZE constants, to be properly replaced by UndertowOptions + /** + * Name of the system property for configuring HTTP2 max header size: {@code "io.undertow.http2-max-header-size"}. + *

This system property will be replaced by an {@link UndertowOptions Undertow option}.

+ */ + public static final String HTTP2_MAX_HEADER_SIZE_PROPERTY = "io.undertow.http2-max-header-size"; + /** + * Default value of HTTP2 max header size. If the system property {@code "io.undertow.http2-max-header-size"} is left + * undefined, this value will be used for the corresponding {@link #HTTP2_MAX_HEADER_SIZE system property configuration}. + */ + private static final int DEFAULT_HTTP2_MAX_HEADER_SIZE = 20000; + /** + * System property {@code "io.undertow.http2-max-header-size"} for configuring the max header size configuration for HTTP2 + * messages. + *

+ * The effective maximum size for headers to be used by this channel will be the minimum of the three: this system property, + * {@link UndertowOptions#MAX_HEADER_SIZE}, and {@link UndertowOptions#HTTP2_SETTINGS_HEADER_TABLE_SIZE}. When calculating + * the minimum, only positive values are taken into account, as values of -1 indicate the absence of a limit. + *

+ * To be replaced by an {@link UndertowOptions Undertow option} in a future release. + */ + private static final int HTTP2_MAX_HEADER_SIZE = Integer.getInteger(HTTP2_MAX_HEADER_SIZE_PROPERTY, DEFAULT_HTTP2_MAX_HEADER_SIZE); + public static final String CLEARTEXT_UPGRADE_STRING = "h2c"; @@ -240,7 +263,7 @@ public Http2Channel(StreamConnection connectedStreamChannel, String protocol, By encoderHeaderTableSize = settings.get(UndertowOptions.HTTP2_SETTINGS_HEADER_TABLE_SIZE, Hpack.DEFAULT_TABLE_SIZE); receiveMaxFrameSize = settings.get(UndertowOptions.HTTP2_SETTINGS_MAX_FRAME_SIZE, DEFAULT_MAX_FRAME_SIZE); maxPadding = settings.get(UndertowOptions.HTTP2_PADDING_SIZE, 0); - maxHeaderListSize = settings.get(UndertowOptions.HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, settings.get(UndertowOptions.MAX_HEADER_SIZE, UndertowOptions.DEFAULT_MAX_HEADER_SIZE)); + maxHeaderListSize = getMaxHeaderSize(settings); if(maxPadding > 0) { paddingRandom = new SecureRandom(); } else { @@ -311,6 +334,42 @@ public void handleEvent(Http2Channel channel) { } } + private static int getMaxHeaderSize(OptionMap settings) { + final int maxHeaderSizeConfig = settings.get(UndertowOptions.MAX_HEADER_SIZE, HTTP2_MAX_HEADER_SIZE); + // soon to be removed http2 settings max header + final int http2SettingsMaxHeaderListSize = settings.get(UndertowOptions.HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, HTTP2_MAX_HEADER_SIZE); + if (HTTP2_MAX_HEADER_SIZE > 0) { + if (maxHeaderSizeConfig > 0) { + if (http2SettingsMaxHeaderListSize > 0) { + return Math.min(Math.min(HTTP2_MAX_HEADER_SIZE, maxHeaderSizeConfig), http2SettingsMaxHeaderListSize); + } else { + return Math.min(HTTP2_MAX_HEADER_SIZE, maxHeaderSizeConfig); + } + } else { + if (http2SettingsMaxHeaderListSize > 0) { + return Math.min(HTTP2_MAX_HEADER_SIZE, http2SettingsMaxHeaderListSize); + } else { + return HTTP2_MAX_HEADER_SIZE; + } + } + } else { + if (maxHeaderSizeConfig > 0) { + if (http2SettingsMaxHeaderListSize > 0) { + return Math.min(maxHeaderSizeConfig, http2SettingsMaxHeaderListSize); + } else { + return maxHeaderSizeConfig; + } + } else { + if (http2SettingsMaxHeaderListSize > 0) { + return http2SettingsMaxHeaderListSize; + } else { + // replace any value <= 0 by -1 + return -1; + } + } + } + } + private void sendSettings() { List settings = new ArrayList<>(); settings.add(new Http2Setting(Http2Setting.SETTINGS_HEADER_TABLE_SIZE, encoderHeaderTableSize)); @@ -690,7 +749,10 @@ protected Collection> channels = new ArrayList<>(currentStreams.size()); for(Map.Entry entry : currentStreams.entrySet()) { if(!entry.getValue().sourceClosed) { - channels.add(entry.getValue().sourceChannel); + final Http2StreamSourceChannel sourceChannel = entry.getValue().sourceChannel; + if (sourceChannel != null) { + channels.add(sourceChannel); + } } } return channels; diff --git a/core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java b/core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java index 9294d36682..088fcc561f 100644 --- a/core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java +++ b/core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java @@ -157,7 +157,7 @@ public void emitHeader(HttpString name, String value, boolean neverIndex) throws if(maxHeaderListSize > 0) { headerSize += (name.length() + value.length() + 32); if (headerSize > maxHeaderListSize) { - throw new HpackException(UndertowMessages.MESSAGES.headerBlockTooLarge(), Http2Channel.ERROR_PROTOCOL_ERROR); + throw new HpackException(UndertowMessages.MESSAGES.headerBlockTooLarge(maxHeaderListSize), Http2Channel.ERROR_PROTOCOL_ERROR); } } if(maxHeaders > 0 && headerMap.size() > maxHeaders) { @@ -205,7 +205,11 @@ protected boolean moreData(int data) { boolean acceptMoreData = super.moreData(data); frameRemaining += data; totalHeaderLength += data; - return acceptMoreData && totalHeaderLength < maxHeaderListSize; + if (maxHeaderListSize > 0 && totalHeaderLength > maxHeaderListSize) { + UndertowLogger.REQUEST_LOGGER.debug(UndertowMessages.MESSAGES.headerBlockTooLarge(maxHeaderListSize)); + return false; + } + return acceptMoreData; } public boolean isInvalid() { diff --git a/core/src/test/java/io/undertow/server/handlers/DefaultMaxHeaderTestCase.java b/core/src/test/java/io/undertow/server/handlers/DefaultMaxHeaderTestCase.java new file mode 100644 index 0000000000..ec66af11f6 --- /dev/null +++ b/core/src/test/java/io/undertow/server/handlers/DefaultMaxHeaderTestCase.java @@ -0,0 +1,114 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2024 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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 io.undertow.server.handlers; + +import io.undertow.UndertowOptions; +import io.undertow.protocols.http2.Http2Channel; +import io.undertow.testutils.AjpIgnore; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.TestHttpClient; +import io.undertow.util.StatusCodes; +import org.apache.http.client.methods.HttpGet; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.xnio.OptionMap; + +import java.io.IOException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + +/** + * Default max header config test case. + * + * @author Flavia Rainone + */ +@RunWith(DefaultServer.class) +@AjpIgnore +public class DefaultMaxHeaderTestCase { + + private static final String MESSAGE = "HelloUrl"; + private static final int COUNT = 10000; + + @BeforeClass + public static void setup() { + // skip this test if we are running in a scenario with max header size property configured + Assume.assumeTrue(System.getProperty(Http2Channel.HTTP2_MAX_HEADER_SIZE_PROPERTY) == null); + final BlockingHandler blockingHandler = new BlockingHandler(); + DefaultServer.setRootHandler(blockingHandler); + blockingHandler.setRootHandler(exchange -> exchange.getResponseSender().send(exchange.getRelativePath())); + } + + @Test + public void testLargeURL() throws IOException { + final String message = MESSAGE.repeat(COUNT); + try (TestHttpClient client = new TestHttpClient()) { + try { + final HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/" + message); + assertStatus(client.execute(get).getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + } + + @Test + public void testLotsOfQueryParameters_MaxParameters_Ok() throws IOException { + final OptionMap existing = DefaultServer.getUndertowOptions(); + try (TestHttpClient client = new TestHttpClient()) { + try { + final StringBuilder qs = new StringBuilder(); + for (int i = 0; i < UndertowOptions.DEFAULT_MAX_PARAMETERS; ++i) { + qs.append("QUERY").append(i); + qs.append("="); + qs.append(URLEncoder.encode("Hello Query" + i, StandardCharsets.UTF_8)); + qs.append("&"); + } + qs.deleteCharAt(qs.length() - 1); // delete last useless '&' + final HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/path?" + qs); + assertStatus(client.execute(get).getStatusLine().getStatusCode()); + } finally { + DefaultServer.setUndertowOptions(existing); + client.getConnectionManager().shutdown(); + } + } + } + + private void assertStatus(int statusCode) { + if (DefaultServer.isH2()) { + if (DefaultServer.isH2upgrade()) { + Assert.assertTrue(statusCode == StatusCodes.BAD_REQUEST || + // on proxy, it might happen that it gets a ClosedChannelException before actually reading the 400 + // response in that case, the client will receive a 503 + (statusCode == StatusCodes.SERVICE_UNAVAILABLE && DefaultServer.isProxy()) || + // most times test is run it is ok, because the header is processed before the upgrade + statusCode == StatusCodes.OK); + } else { + Assert.assertTrue(statusCode == StatusCodes.BAD_REQUEST || + // on proxy, it might happen that it gets a ClosedChannelException before actually reading the 400 + // response in that case, the client will receive a 503 + (statusCode == StatusCodes.SERVICE_UNAVAILABLE && DefaultServer.isProxy())); + } + } else { + Assert.assertEquals(StatusCodes.OK, statusCode); + } + } +} diff --git a/core/src/test/java/io/undertow/server/handlers/LongURLTestCase.java b/core/src/test/java/io/undertow/server/handlers/LongURLTestCase.java index a8a10c28de..74cd66a01c 100644 --- a/core/src/test/java/io/undertow/server/handlers/LongURLTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/LongURLTestCase.java @@ -18,20 +18,22 @@ package io.undertow.server.handlers; -import java.io.IOException; - -import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.junit.Assert; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; +import io.undertow.protocols.http2.Http2Channel; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.testutils.AjpIgnore; import io.undertow.testutils.DefaultServer; import io.undertow.testutils.HttpClientUtils; import io.undertow.testutils.TestHttpClient; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.IOException; /** * @author Stuart Douglas @@ -43,8 +45,15 @@ public class LongURLTestCase { private static final String MESSAGE = "HelloUrl"; private static final int COUNT = 10000; + // TODO private static OptionMap EXISTING_OPTIONS = DefaultServer.getUndertowOptions(); + @BeforeClass public static void setup() { + // skip this test if we are running in a scenario with default max header size property + Assume.assumeNotNull(System.getProperty(Http2Channel.HTTP2_MAX_HEADER_SIZE_PROPERTY)); + // TODO + // DefaultServer.setUndertowOptions(OptionMap.builder().set(UndertowOptions.HTTP2_MAX_HEADER_SIZE, 100000).getMap()); + final BlockingHandler blockingHandler = new BlockingHandler(); DefaultServer.setRootHandler(blockingHandler); blockingHandler.setRootHandler(new HttpHandler() { @@ -55,6 +64,12 @@ public void handleRequest(final HttpServerExchange exchange) { }); } + // TODO + /*@AfterClass + public static void teardown() { + DefaultServer.setUndertowOptions(EXISTING_OPTIONS); + }*/ + @Test public void testLargeURL() throws IOException { diff --git a/core/src/test/java/io/undertow/server/handlers/LotsOfHeadersResponseTestCase.java b/core/src/test/java/io/undertow/server/handlers/LotsOfHeadersResponseTestCase.java index 2e58d3eca2..1d7d3d3b12 100644 --- a/core/src/test/java/io/undertow/server/handlers/LotsOfHeadersResponseTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/LotsOfHeadersResponseTestCase.java @@ -18,6 +18,7 @@ package io.undertow.server.handlers; +import io.undertow.protocols.http2.Http2Channel; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.testutils.AjpIgnore; @@ -49,6 +50,9 @@ public class LotsOfHeadersResponseTestCase { @BeforeClass public static void setup() { + // TODO replace by new UndertowOptions.HTTP2_MAX_HEADER_SIZE + // skip this test if we are running in a scenario with default max header size property + Assume.assumeNotNull(System.getProperty(Http2Channel.HTTP2_MAX_HEADER_SIZE_PROPERTY)); final BlockingHandler blockingHandler = new BlockingHandler(); DefaultServer.setRootHandler(blockingHandler); blockingHandler.setRootHandler(new HttpHandler() { diff --git a/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java b/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java index c599ab07bc..0d2d5edffb 100644 --- a/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java @@ -18,6 +18,8 @@ package io.undertow.server.handlers; +import io.undertow.UndertowOptions; +import io.undertow.protocols.http2.Http2Channel; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.testutils.AjpIgnore; @@ -29,16 +31,16 @@ import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; import org.junit.Assert; +import org.junit.Assume; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.xnio.OptionMap; import java.io.IOException; import java.net.URLEncoder; import java.util.Deque; import java.util.Map; -import io.undertow.UndertowOptions; -import org.xnio.OptionMap; /** * @author Stuart Douglas @@ -52,11 +54,19 @@ public class LotsOfQueryParametersTestCase { private static final int DEFAULT_MAX_PARAMETERS = 1000; private static final int TEST_MAX_PARAMETERS = 10; + // TODO private static OptionMap EXISTING_OPTIONS = DefaultServer.getUndertowOptions(); @BeforeClass public static void setup() { + // skip this test if we are running in a scenario with default max header size property + Assume.assumeNotNull(System.getProperty(Http2Channel.HTTP2_MAX_HEADER_SIZE_PROPERTY)); final BlockingHandler blockingHandler = new BlockingHandler(); DefaultServer.setRootHandler(blockingHandler); + // TODO replace by UndertowOption.HTTP2_MAX_HEADER_SIZE + //final OptionMap bigMaxHeaderOptions = OptionMap.builder().set(UndertowOptions.HTTP2_MAX_HEADER_SIZE, 100000).getMap(); + //DefaultServer.setUndertowOptions(bigMaxHeaderOptions); + //DefaultServer.setProxyOptions(bigMaxHeaderOptions); + blockingHandler.setRootHandler(new HttpHandler() { @Override public void handleRequest(final HttpServerExchange exchange) { @@ -67,6 +77,12 @@ public void handleRequest(final HttpServerExchange exchange) { }); } + // TODO + /* @AfterClass + public static void teardown() { + DefaultServer.setUndertowOptions(EXISTING_OPTIONS); + } */ + @Test @AjpIgnore public void testLotsOfQueryParameters_Default_Ok() throws IOException { TestHttpClient client = new TestHttpClient(); diff --git a/core/src/test/java/io/undertow/testutils/DefaultServer.java b/core/src/test/java/io/undertow/testutils/DefaultServer.java index 2c66675d63..a04bd949a0 100644 --- a/core/src/test/java/io/undertow/testutils/DefaultServer.java +++ b/core/src/test/java/io/undertow/testutils/DefaultServer.java @@ -471,7 +471,13 @@ public static boolean startServer() { proxyServer = worker.createStreamConnectionServer(new InetSocketAddress(Inet4Address.getByName(getHostAddress(DEFAULT)), getHostPort(DEFAULT)), proxyAcceptListener, serverOptions); loadBalancingProxyClient = new LoadBalancingProxyClient(GSSAPIAuthenticationMechanism.EXCLUSIVITY_CHECKER) .setMaxQueueSize(20) - .addHost(new URI("h2", null, getHostAddress(DEFAULT), getHostPort(DEFAULT) + PROXY_OFFSET, "/", null, null), null, new UndertowXnioSsl(xnio, OptionMap.EMPTY, SSL_BUFFER_POOL, clientContext), OptionMap.create(UndertowOptions.ENABLE_HTTP2, true)); + .addHost(new URI("h2", null, getHostAddress(DEFAULT), getHostPort(DEFAULT) + PROXY_OFFSET, "/", null, null), null, + new UndertowXnioSsl(xnio, OptionMap.EMPTY, SSL_BUFFER_POOL, clientContext), + // TODO config UndertowOptions.HTTP2_MAX_HEADER_SIZE here + // for testing purposes, we will disable the max header size for the proxy client, because + // this cannot be changed per test (the host is not available after this code is run, and + // it doesn't have a way of setting options afterward either + OptionMap.create(UndertowOptions.ENABLE_HTTP2, true/*, UndertowOptions.MAX_HEADER_SIZE, -1*/)); ProxyHandler proxyHandler = ProxyHandler.builder() .setProxyClient(loadBalancingProxyClient) .setMaxRequestTime(60000) @@ -493,7 +499,14 @@ public static boolean startServer() { proxyServer = worker.createStreamConnectionServer(new InetSocketAddress(Inet4Address.getByName(getHostAddress(DEFAULT)), getHostPort(DEFAULT)), proxyAcceptListener, serverOptions); loadBalancingProxyClient = new LoadBalancingProxyClient(GSSAPIAuthenticationMechanism.EXCLUSIVITY_CHECKER) .setMaxQueueSize(20) - .addHost(new URI(h2cUpgrade ? "http" : "h2c-prior", null, getHostAddress(DEFAULT), getHostPort(DEFAULT) + PROXY_OFFSET, "/", null, null), null, null, OptionMap.create(UndertowOptions.ENABLE_HTTP2, true)); + .addHost(new URI(h2cUpgrade ? "http" : "h2c-prior", null, getHostAddress(DEFAULT), + getHostPort(DEFAULT) + PROXY_OFFSET, "/", null, null), + null, null, + // TODO add UndertowOptions.HTTP2_MAX_HEADER_SIZE config here + // for testing purposes, we will disable the max header size for the proxy client, because + // this cannot be changed per test (the host is not available after this code is run, and + // it doesn't have a way of setting options afterward either + OptionMap.create(UndertowOptions.ENABLE_HTTP2, true/*, UndertowOptions.HTTP2_MAX_HEADER_SIZE, -1*/)); ProxyHandler proxyHandler = ProxyHandler.builder() .setProxyClient(loadBalancingProxyClient) .setMaxRequestTime(60000) From 603f2766384d3daa34ff11a784f512d61b3873ef Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Tue, 18 Jun 2024 08:33:16 -0300 Subject: [PATCH 3/3] [UNDERTOW-2405] The correct error code to be sent if frame parser refuses more data is PROTOCOL_ERROR This makes this code consistent with the handling of headers that surpass the max header size limit elsewhere in HTTP2 (see Http2HeaderBlockParser.emitHeader), and the justification is that the max header size must have been handshaken with the peer as part of settings frame, via the SETTINGS_HEADER_TABLE_SIZE parameter. Signed-off-by: Flavia Rainone --- .../io/undertow/protocols/http2/Http2FrameHeaderParser.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/io/undertow/protocols/http2/Http2FrameHeaderParser.java b/core/src/main/java/io/undertow/protocols/http2/Http2FrameHeaderParser.java index 9a10f359c6..461fb9cd35 100644 --- a/core/src/main/java/io/undertow/protocols/http2/Http2FrameHeaderParser.java +++ b/core/src/main/java/io/undertow/protocols/http2/Http2FrameHeaderParser.java @@ -26,7 +26,6 @@ import java.nio.ByteBuffer; import static io.undertow.protocols.http2.Http2Channel.DATA_FLAG_END_STREAM; -import static io.undertow.protocols.http2.Http2Channel.ERROR_ENHANCE_YOUR_CALM; import static io.undertow.protocols.http2.Http2Channel.FRAME_TYPE_CONTINUATION; import static io.undertow.protocols.http2.Http2Channel.FRAME_TYPE_DATA; import static io.undertow.protocols.http2.Http2Channel.FRAME_TYPE_GOAWAY; @@ -110,7 +109,7 @@ public boolean handle(final ByteBuffer byteBuffer) throws IOException { } parser = continuationParser; if (!continuationParser.moreData(length)) { - http2Channel.sendGoAway(ERROR_ENHANCE_YOUR_CALM); + http2Channel.sendGoAway(Http2Channel.ERROR_PROTOCOL_ERROR); } break; }