Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UNDERTOW-2405] CVE-2024-27316 Add system property io.undertow.http2-max-header-size (default value set to 20000) #1604

Merged
merged 3 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 94 additions & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,9 @@
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<excludes>
<exclude>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</exclude>
</excludes>
<systemPropertyVariables>
<test.h2>true</test.h2>
<test.dump>${dump}</test.dump>
Expand All @@ -396,6 +399,9 @@
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<excludes>
<exclude>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</exclude>
</excludes>
<systemPropertyVariables>
<test.h2c>true</test.h2c>
<test.dump>${dump}</test.dump>
Expand All @@ -411,7 +417,6 @@
<reportsDirectory>${project.build.directory}/surefire-h2c-reports</reportsDirectory>
</configuration>
</execution>

<execution>
<id>proxy-h2c-upgrade</id>
<phase>test</phase>
Expand All @@ -421,6 +426,93 @@
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<excludes>
<exclude>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</exclude>
</excludes>
<systemPropertyVariables>
<test.h2c-upgrade>true</test.h2c-upgrade>
<test.dump>${dump}</test.dump>
<test.bufferSize>${bufferSize}</test.bufferSize>
<default.server.address>localhost</default.server.address>
<default.server.port>7777</default.server.port>
<java.util.logging.manager>org.jboss.logmanager.LogManager
</java.util.logging.manager>
<test.level>${test.level}</test.level>
<java.net.preferIPv6Addresses>${test.ipv6}</java.net.preferIPv6Addresses>
<sun.net.useExclusiveBind>false</sun.net.useExclusiveBind>
</systemPropertyVariables>
<reportsDirectory>${project.build.directory}/surefire-h2c-upgrade-reports</reportsDirectory>
</configuration>
</execution>
<!-- TODO: remove these executions as soon as UndertowOptions.MAX_HTTP2_HEADER_SIZE is created -->
<execution>
<id>proxy-h2-big-http2-max-header-size</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<includes>
<include>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</include>
</includes>
<systemPropertyVariables>
<test.h2>true</test.h2>
<test.dump>${dump}</test.dump>
<test.bufferSize>${bufferSize}</test.bufferSize>
<default.server.address>localhost</default.server.address>
<default.server.port>7777</default.server.port>
<java.util.logging.manager>org.jboss.logmanager.LogManager
</java.util.logging.manager>
<test.level>${test.level}</test.level>
<java.net.preferIPv6Addresses>${test.ipv6}</java.net.preferIPv6Addresses>
<sun.net.useExclusiveBind>false</sun.net.useExclusiveBind>
<io.undertow.http2-max-header-size>600000</io.undertow.http2-max-header-size>
</systemPropertyVariables>
<reportsDirectory>${project.build.directory}/surefire-h2-reports</reportsDirectory>
</configuration>
</execution>
<execution>
<id>proxy-h2c-big-http2-max-header-size</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<includes>
<include>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</include>
</includes>
<systemPropertyVariables>
<test.h2c>true</test.h2c>
<test.dump>${dump}</test.dump>
<test.bufferSize>${bufferSize}</test.bufferSize>
<default.server.address>localhost</default.server.address>
<default.server.port>7777</default.server.port>
<java.util.logging.manager>org.jboss.logmanager.LogManager
</java.util.logging.manager>
<test.level>${test.level}</test.level>
<java.net.preferIPv6Addresses>${test.ipv6}</java.net.preferIPv6Addresses>
<sun.net.useExclusiveBind>false</sun.net.useExclusiveBind>
<io.undertow.http2-max-header-size>600000</io.undertow.http2-max-header-size>
</systemPropertyVariables>
<reportsDirectory>${project.build.directory}/surefire-h2c-reports</reportsDirectory>
</configuration>
</execution>
<execution>
<id>proxy-h2c-upgrade-big-http2-max-header-size</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<enableAssertions>true</enableAssertions>
<runOrder>reversealphabetical</runOrder>
<includes>
<include>*LongURLTestCase, *LotsOfQueryParametersTestCase, *LotsOfHeadersResponseTestCase</include>
</includes>
<systemPropertyVariables>
<test.h2c-upgrade>true</test.h2c-upgrade>
<test.dump>${dump}</test.dump>
Expand All @@ -432,6 +524,7 @@
<test.level>${test.level}</test.level>
<java.net.preferIPv6Addresses>${test.ipv6}</java.net.preferIPv6Addresses>
<sun.net.useExclusiveBind>false</sun.net.useExclusiveBind>
<io.undertow.http2-max-header-size>600000</io.undertow.http2-max-header-size>
</systemPropertyVariables>
<reportsDirectory>${project.build.directory}/surefire-h2c-upgrade-reports</reportsDirectory>
</configuration>
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/undertow/UndertowMessages.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
66 changes: 64 additions & 2 deletions core/src/main/java/io/undertow/protocols/http2/Http2Channel.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@
*/
public class Http2Channel extends AbstractFramedChannel<Http2Channel, AbstractHttp2StreamSourceChannel, AbstractHttp2StreamSinkChannel> 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"}.
* <p>This system property will be replaced by an {@link UndertowOptions Undertow option}.</p>
*/
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.
* <p>
* 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.
* <p>
* 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";


Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Http2Setting> settings = new ArrayList<>();
settings.add(new Http2Setting(Http2Setting.SETTINGS_HEADER_TABLE_SIZE, encoderHeaderTableSize));
Expand Down Expand Up @@ -690,7 +749,10 @@ protected Collection<AbstractFramedStreamSourceChannel<Http2Channel, AbstractHtt
List<AbstractFramedStreamSourceChannel<Http2Channel, AbstractHttp2StreamSourceChannel, AbstractHttp2StreamSinkChannel>> channels = new ArrayList<>(currentStreams.size());
for(Map.Entry<Integer, StreamHolder> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -55,8 +56,6 @@
*/
public final class HttpOpenListener implements ChannelListener<StreamConnection>, 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<HttpServerConnection> connections = Collections.newSetFromMap(new ConcurrentHashMap<>());

private final ByteBufferPool bufferPool;
Expand Down Expand Up @@ -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());
}
Expand Down
Loading
Loading