From 02c55e638fc14d696fdef42ce0ec9316713a5917 Mon Sep 17 00:00:00 2001
From: Simone Bordet <simone.bordet@gmail.com>
Date: Wed, 18 Jul 2018 10:07:14 +0200
Subject: [PATCH 2/2] Fixes #2722 - Improve configurability for SETTINGS
frames. (#2723)
* Fixes #2722 - Improve configurability for SETTINGS frames.
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
---
.../jetty/http2/client/HTTP2Client.java | 13 +++
.../client/HTTP2ClientConnectionFactory.java | 1 +
.../jetty/http2/frames/SettingsFrame.java | 2 +
.../eclipse/jetty/http2/parser/Parser.java | 13 ++-
.../http2/parser/SettingsBodyParser.java | 29 ++++++-
.../frames/SettingsGenerateParseTest.java | 84 +++++++++++++++++--
.../AbstractHTTP2ServerConnectionFactory.java | 14 ++++
7 files changed, 145 insertions(+), 11 deletions(-)
diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java
index 83ff1c487e..865379c68a 100644
--- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java
+++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java
@@ -34,6 +34,7 @@ import org.eclipse.jetty.alpn.client.ALPNClientConnectionFactory;
import org.eclipse.jetty.http2.BufferingFlowControlStrategy;
import org.eclipse.jetty.http2.FlowControlStrategy;
import org.eclipse.jetty.http2.api.Session;
+import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.ClientConnectionFactory;
import org.eclipse.jetty.io.Connection;
@@ -129,6 +130,7 @@ public class HTTP2Client extends ContainerLifeCycle
private List<String> protocols = Arrays.asList("h2", "h2-17", "h2-16", "h2-15", "h2-14");
private int initialSessionRecvWindow = 16 * 1024 * 1024;
private int initialStreamRecvWindow = 8 * 1024 * 1024;
+ private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
@Override
@@ -334,6 +336,17 @@ public class HTTP2Client extends ContainerLifeCycle
this.initialStreamRecvWindow = initialStreamRecvWindow;
}
+ @ManagedAttribute("The max number of keys in all SETTINGS frames")
+ public int getMaxSettingsKeys()
+ {
+ return maxSettingsKeys;
+ }
+
+ public void setMaxSettingsKeys(int maxSettingsKeys)
+ {
+ this.maxSettingsKeys = maxSettingsKeys;
+ }
+
public void connect(InetSocketAddress address, Session.Listener listener, Promise<Session> promise)
{
connect(null, address, listener, promise);
diff --git a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java
index 9875d60252..b07e4c2f2a 100644
--- a/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java
+++ b/jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java
@@ -67,6 +67,7 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
FlowControlStrategy flowControl = client.getFlowControlStrategyFactory().newFlowControlStrategy();
HTTP2ClientSession session = new HTTP2ClientSession(scheduler, endPoint, generator, listener, flowControl);
Parser parser = new Parser(byteBufferPool, session, 4096, 8192);
+ parser.setMaxSettingsKeys(client.getMaxSettingsKeys());
HTTP2ClientConnection connection = new HTTP2ClientConnection(client, byteBufferPool, executor, endPoint,
parser, session, client.getInputBufferSize(), promise, listener);
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/SettingsFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/SettingsFrame.java
index b2ffa40ec2..76eed25b4b 100644
--- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/SettingsFrame.java
+++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/SettingsFrame.java
@@ -22,6 +22,8 @@ import java.util.Map;
public class SettingsFrame extends Frame
{
+ public static final int DEFAULT_MAX_KEYS = 64;
+
public static final int HEADER_TABLE_SIZE = 1;
public static final int ENABLE_PUSH = 2;
public static final int MAX_CONCURRENT_STREAMS = 3;
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java
index 4187211512..016bd0ef74 100644
--- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java
+++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java
@@ -52,6 +52,7 @@ public class Parser
private final HeaderParser headerParser;
private final HeaderBlockParser headerBlockParser;
private final BodyParser[] bodyParsers;
+ private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private boolean continuation;
private State state = State.HEADER;
@@ -71,7 +72,7 @@ public class Parser
bodyParsers[FrameType.HEADERS.getType()] = new HeadersBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments);
bodyParsers[FrameType.PRIORITY.getType()] = new PriorityBodyParser(headerParser, listener);
bodyParsers[FrameType.RST_STREAM.getType()] = new ResetBodyParser(headerParser, listener);
- bodyParsers[FrameType.SETTINGS.getType()] = new SettingsBodyParser(headerParser, listener);
+ bodyParsers[FrameType.SETTINGS.getType()] = new SettingsBodyParser(headerParser, listener, getMaxSettingsKeys());
bodyParsers[FrameType.PUSH_PROMISE.getType()] = new PushPromiseBodyParser(headerParser, listener, headerBlockParser);
bodyParsers[FrameType.PING.getType()] = new PingBodyParser(headerParser, listener);
bodyParsers[FrameType.GO_AWAY.getType()] = new GoAwayBodyParser(headerParser, listener);
@@ -203,6 +204,16 @@ public class Parser
return headerParser.hasFlag(bit);
}
+ public int getMaxSettingsKeys()
+ {
+ return maxSettingsKeys;
+ }
+
+ public void setMaxSettingsKeys(int maxSettingsKeys)
+ {
+ this.maxSettingsKeys = maxSettingsKeys;
+ }
+
protected void notifyConnectionFailure(int error, String reason)
{
try
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java
index 7452f2bd17..1d2cf892ab 100644
--- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java
+++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java
@@ -32,16 +32,25 @@ import org.eclipse.jetty.util.log.Logger;
public class SettingsBodyParser extends BodyParser
{
private static final Logger LOG = Log.getLogger(SettingsBodyParser.class);
+
+ private final int maxKeys;
private State state = State.PREPARE;
private int cursor;
private int length;
private int settingId;
private int settingValue;
+ private int keys;
private Map<Integer, Integer> settings;
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener)
+ {
+ this(headerParser, listener, SettingsFrame.DEFAULT_MAX_KEYS);
+ }
+
+ public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener, int maxKeys)
{
super(headerParser, listener);
+ this.maxKeys = maxKeys;
}
protected void reset()
@@ -54,6 +63,11 @@ public class SettingsBodyParser extends BodyParser
settings = null;
}
+ public int getMaxKeys()
+ {
+ return maxKeys;
+ }
+
@Override
protected void emptyBody(ByteBuffer buffer)
{
@@ -116,7 +130,8 @@ public class SettingsBodyParser extends BodyParser
settingValue = buffer.getInt();
if (LOG.isDebugEnabled())
LOG.debug(String.format("setting %d=%d",settingId, settingValue));
- settings.put(settingId, settingValue);
+ if (!onSetting(buffer, settings, settingId, settingValue))
+ return false;
state = State.SETTING_ID;
length -= 4;
if (length == 0)
@@ -142,7 +157,8 @@ public class SettingsBodyParser extends BodyParser
{
if (LOG.isDebugEnabled())
LOG.debug(String.format("setting %d=%d",settingId, settingValue));
- settings.put(settingId, settingValue);
+ if (!onSetting(buffer, settings, settingId, settingValue))
+ return false;
state = State.SETTING_ID;
if (length == 0)
return onSettings(settings);
@@ -158,6 +174,15 @@ public class SettingsBodyParser extends BodyParser
return false;
}
+ protected boolean onSetting(ByteBuffer buffer, Map<Integer, Integer> settings, int key, int value)
+ {
+ ++keys;
+ if (keys > getMaxKeys())
+ return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_settings_frame");
+ settings.put(key, value);
+ return true;
+ }
+
protected boolean onSettings(Map<Integer, Integer> settings)
{
SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK));
diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/SettingsGenerateParseTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/SettingsGenerateParseTest.java
index dc65254586..ccf3c70483 100644
--- a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/SettingsGenerateParseTest.java
+++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/SettingsGenerateParseTest.java
@@ -41,9 +41,9 @@ public class SettingsGenerateParseTest
private final ByteBufferPool byteBufferPool = new MappedByteBufferPool();
@Test
- public void testGenerateParseNoSettings() throws Exception
+ public void testGenerateParseNoSettings()
{
- List<SettingsFrame> frames = testGenerateParse(Collections.<Integer, Integer>emptyMap());
+ List<SettingsFrame> frames = testGenerateParse(Collections.emptyMap());
Assert.assertEquals(1, frames.size());
SettingsFrame frame = frames.get(0);
Assert.assertEquals(0, frame.getSettings().size());
@@ -51,7 +51,7 @@ public class SettingsGenerateParseTest
}
@Test
- public void testGenerateParseSettings() throws Exception
+ public void testGenerateParseSettings()
{
Map<Integer, Integer> settings1 = new HashMap<>();
int key1 = 13;
@@ -73,7 +73,7 @@ public class SettingsGenerateParseTest
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
- final List<SettingsFrame> frames = new ArrayList<>();
+ List<SettingsFrame> frames = new ArrayList<>();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@@ -104,11 +104,11 @@ public class SettingsGenerateParseTest
}
@Test
- public void testGenerateParseInvalidSettings() throws Exception
+ public void testGenerateParseInvalidSettings()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
- final AtomicInteger errorRef = new AtomicInteger();
+ AtomicInteger errorRef = new AtomicInteger();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@@ -139,11 +139,11 @@ public class SettingsGenerateParseTest
}
@Test
- public void testGenerateParseOneByteAtATime() throws Exception
+ public void testGenerateParseOneByteAtATime()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
- final List<SettingsFrame> frames = new ArrayList<>();
+ List<SettingsFrame> frames = new ArrayList<>();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@@ -182,4 +182,72 @@ public class SettingsGenerateParseTest
Assert.assertTrue(frame.isReply());
}
}
+
+ @Test
+ public void testGenerateParseTooManySettingsInOneFrame()
+ {
+ SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
+
+ AtomicInteger errorRef = new AtomicInteger();
+ Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
+ {
+ @Override
+ public void onConnectionFailure(int error, String reason)
+ {
+ errorRef.set(error);
+ }
+ }, 4096, 8192);
+ int maxSettingsKeys = 32;
+ parser.setMaxSettingsKeys(maxSettingsKeys);
+ parser.init(UnaryOperator.identity());
+
+ Map<Integer, Integer> settings = new HashMap<>();
+ for (int i = 0; i < maxSettingsKeys + 1; ++i)
+ settings.put(i + 10, i);
+
+ ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
+ generator.generateSettings(lease, settings, false);
+
+ for (ByteBuffer buffer : lease.getByteBuffers())
+ {
+ while (buffer.hasRemaining())
+ parser.parse(buffer);
+ }
+
+ Assert.assertEquals(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, errorRef.get());
+ }
+
+ @Test
+ public void testGenerateParseTooManySettingsInMultipleFrames()
+ {
+ SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
+
+ AtomicInteger errorRef = new AtomicInteger();
+ Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
+ {
+ @Override
+ public void onConnectionFailure(int error, String reason)
+ {
+ errorRef.set(error);
+ }
+ }, 4096, 8192);
+ int maxSettingsKeys = 32;
+ parser.setMaxSettingsKeys(maxSettingsKeys);
+ parser.init(UnaryOperator.identity());
+
+ Map<Integer, Integer> settings = new HashMap<>();
+ settings.put(13, 17);
+
+ ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
+ for (int i = 0; i < maxSettingsKeys + 1; ++i)
+ generator.generateSettings(lease, settings, false);
+
+ for (ByteBuffer buffer : lease.getByteBuffers())
+ {
+ while (buffer.hasRemaining())
+ parser.parse(buffer);
+ }
+
+ Assert.assertEquals(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, errorRef.get());
+ }
}
diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java
index b2cc1ad504..317b7db7b9 100644
--- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java
+++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java
@@ -50,6 +50,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
private int initialStreamRecvWindow = 512 * 1024;
private int maxConcurrentStreams = 128;
private int maxHeaderBlockFragment = 0;
+ private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
private long streamIdleTimeout;
@@ -145,6 +146,17 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
this.streamIdleTimeout = streamIdleTimeout;
}
+ @ManagedAttribute("The max number of keys in all SETTINGS frames")
+ public int getMaxSettingsKeys()
+ {
+ return maxSettingsKeys;
+ }
+
+ public void setMaxSettingsKeys(int maxSettingsKeys)
+ {
+ this.maxSettingsKeys = maxSettingsKeys;
+ }
+
/**
* @return -1
* @deprecated feature removed, no replacement
@@ -205,6 +217,8 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize());
ServerParser parser = newServerParser(connector, session);
+ parser.setMaxSettingsKeys(getMaxSettingsKeys());
+
HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(),
endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener);
connection.addListener(connectionListener);
--
2.20.1