Skip to content

Commit

Permalink
Merge pull request #1612 from fl4via/2.2.x_backport-bug-fixes
Browse files Browse the repository at this point in the history
[UNDERTOW-2405][UNDERTOW-2391][UNDERTOW-2407][UNDERTOW-2408][UNDERTOW-2334] CVE-2024-27316 CVE-2023-5685 CVE-2024-6162 Backport bug fixes to 2.2.x
  • Loading branch information
fl4via authored Jun 22, 2024
2 parents 303ccdf + a28ac53 commit ed4266f
Show file tree
Hide file tree
Showing 14 changed files with 364 additions and 39 deletions.
95 changes: 94 additions & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,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 @@ -437,6 +440,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 @@ -452,7 +458,6 @@
<reportsDirectory>${project.build.directory}/surefire-h2c-reports</reportsDirectory>
</configuration>
</execution>

<execution>
<id>proxy-h2c-upgrade</id>
<phase>test</phase>
Expand All @@ -462,6 +467,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 @@ -473,6 +565,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, -1));
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 @@ -108,7 +108,9 @@ public boolean handle(final ByteBuffer byteBuffer) throws IOException {
throw UndertowMessages.MESSAGES.http2ContinuationFrameNotExpected();
}
parser = continuationParser;
continuationParser.moreData(length);
if (!continuationParser.moreData(length)) {
http2Channel.sendGoAway(Http2Channel.ERROR_PROTOCOL_ERROR);
}
break;
}
case FRAME_TYPE_PUSH_PROMISE: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ abstract class Http2HeaderBlockParser extends Http2PushBackParser implements Hpa

private final HpackDecoder decoder;
private int frameRemaining = -1;
private int totalHeaderLength = 0;
private boolean invalid = false;
private boolean processingPseudoHeaders = true;
private final boolean client;
Expand Down Expand Up @@ -81,7 +82,7 @@ abstract class Http2HeaderBlockParser extends Http2PushBackParser implements Hpa
protected void handleData(ByteBuffer resource, Http2FrameHeaderParser header) throws IOException {
boolean continuationFramesComing = Bits.anyAreClear(header.flags, Http2Channel.HEADERS_FLAG_END_HEADERS);
if (frameRemaining == -1) {
frameRemaining = header.length;
frameRemaining = totalHeaderLength = header.length;
}
final boolean moreDataThisFrame = resource.remaining() < frameRemaining;
final int pos = resource.position();
Expand Down Expand Up @@ -156,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 @@ -200,9 +201,15 @@ public void emitHeader(HttpString name, String value, boolean neverIndex) throws
}
protected abstract int getPaddingLength();
@Override
protected void moreData(int data) {
super.moreData(data);
protected boolean moreData(int data) {
boolean acceptMoreData = super.moreData(data);
frameRemaining += data;
totalHeaderLength += data;
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,11 +18,11 @@

package io.undertow.protocols.http2;

import io.undertow.UndertowMessages;

import java.io.IOException;
import java.nio.ByteBuffer;

import io.undertow.UndertowMessages;

/**
* Parser that supports push back when not all data can be read.
*
Expand Down Expand Up @@ -123,9 +123,10 @@ protected void finish() {
finished = true;
}

protected void moreData(int data) {
protected boolean moreData(int data) {
finished = false;
this.remainingData += data;
return true;
}

public int getFrameLength() {
Expand Down
Loading

0 comments on commit ed4266f

Please sign in to comment.