Skip to content

Commit

Permalink
Merge pull request #1604 from fl4via/UNDERTOW-2405
Browse files Browse the repository at this point in the history 
[UNDERTOW-2405] CVE-2024-27316 Add system property io.undertow.http2-max-header-size (default value set to 20000)
  • Loading branch information
@fl4via
fl4via authored Jun 20, 2024
2 parents 5ed2b4f + 603f276 commit 62aef6a
Show file tree
Hide file tree
Showing 12 changed files with 356 additions and 49 deletions.
95 changes: 94 additions & 1 deletion core/pom.xml
View file
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
View file
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
View file
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
3 changes: 1 addition & 2 deletions core/src/main/java/io/undertow/protocols/http2/Http2FrameHeaderParser.java
View file
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
8 changes: 6 additions & 2 deletions core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java
View file
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
33 changes: 13 additions & 20 deletions core/src/main/java/io/undertow/server/protocol/http/HttpOpenListener.java
View file
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

0 comments on commit 62aef6a

Please sign in to comment.