Skip to content

Commit

Permalink
[UNDERTOW-2405] CVE-2024-27316 Create system property io.undertow.htt…
Browse files Browse the repository at this point in the history
…p2-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 <frainone@redhat.com>
  • Loading branch information
fl4via committed Jun 20, 2024
1 parent d798de6 commit 296636d
Show file tree
Hide file tree
Showing 9 changed files with 340 additions and 19 deletions.
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 @@ -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
Loading

0 comments on commit 296636d

Please sign in to comment.