From 5836c50a20f813a1346bdd5c83bebc7ba68e0f2c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Jul 2018 10:39:43 +0200 Subject: [PATCH] Issue #2679 - HTTP/2 Spec Compliance. Fixed max frame length handling. Signed-off-by: Simone Bordet --- .../jetty/http2/client/HTTP2Client.java | 13 ++++ .../client/HTTP2ClientConnectionFactory.java | 5 ++ .../jetty/http2/parser/HeaderParser.java | 7 ++ .../eclipse/jetty/http2/parser/Parser.java | 27 +++++++- .../http2/frames/MaxFrameSizeParseTest.java | 66 +++++++++++++++++++ .../AbstractHTTP2ServerConnectionFactory.java | 13 ++++ 6 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java 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 83ff1c487e7..da00dc0094c 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.Frame; 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 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 maxFrameLength = Frame.DEFAULT_MAX_LENGTH; private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); @Override @@ -334,6 +336,17 @@ public class HTTP2Client extends ContainerLifeCycle this.initialStreamRecvWindow = initialStreamRecvWindow; } + @ManagedAttribute("The max frame length in bytes") + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + public void connect(InetSocketAddress address, Session.Listener listener, Promise 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 9875d602526..9d5ec06954b 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.setMaxFrameLength(client.getMaxFrameLength()); HTTP2ClientConnection connection = new HTTP2ClientConnection(client, byteBufferPool, executor, endPoint, parser, session, client.getInputBufferSize(), promise, listener); @@ -110,6 +111,10 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory settings = new HashMap<>(); settings.computeIfAbsent(SettingsFrame.INITIAL_WINDOW_SIZE, k -> client.getInitialStreamRecvWindow()); + Integer maxFrameLength = settings.get(SettingsFrame.MAX_FRAME_SIZE); + if (maxFrameLength != null) + getParser().setMaxFrameLength(maxFrameLength); + PrefaceFrame prefaceFrame = new PrefaceFrame(); SettingsFrame settingsFrame = new SettingsFrame(settings, false); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java index a9d3b9e2771..df717084674 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.http2.parser; import java.nio.ByteBuffer; import org.eclipse.jetty.http2.frames.Frame; +import org.eclipse.jetty.http2.frames.FrameType; /** *

The parser for the frame header of HTTP/2 frames.

@@ -144,6 +145,12 @@ public class HeaderParser return streamId; } + @Override + public String toString() + { + return String.format("[%s|%d|%d|%d]", FrameType.from(getFrameType()), getLength(), flags, getStreamId()); + } + private enum State { LENGTH, TYPE, FLAGS, STREAM_ID, STREAM_ID_BYTES 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 60ffd501313..029e1170574 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 @@ -24,6 +24,7 @@ import java.util.function.UnaryOperator; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.Flags; import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; @@ -53,6 +54,7 @@ public class Parser private final HeaderBlockParser headerBlockParser; private final BodyParser[] bodyParsers; private final UnknownBodyParser unknownBodyParser; + private int maxFrameLength; private boolean continuation; private State state = State.HEADER; @@ -62,6 +64,7 @@ public class Parser this.headerParser = new HeaderParser(); this.headerBlockParser = new HeaderBlockParser(byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize)); this.unknownBodyParser = new UnknownBodyParser(headerParser, listener); + this.maxFrameLength = Frame.DEFAULT_MAX_LENGTH; this.bodyParsers = new BodyParser[FrameType.values().length]; } @@ -139,10 +142,13 @@ public class Parser if (!headerParser.parse(buffer)) return false; - FrameType frameType = FrameType.from(getFrameType()); if (LOG.isDebugEnabled()) - LOG.debug("Parsed {} frame header from {}", frameType, buffer); + LOG.debug("Parsed {} frame header from {}", headerParser, buffer); + if (headerParser.getLength() > getMaxFrameLength()) + return handleFrameTooLarge(buffer); + + FrameType frameType = FrameType.from(getFrameType()); if (continuation) { if (frameType != FrameType.CONTINUATION) @@ -169,6 +175,13 @@ public class Parser return true; } + private boolean handleFrameTooLarge(ByteBuffer buffer) + { + BufferUtil.clear(buffer); + notifyConnectionFailure(ErrorCode.FRAME_SIZE_ERROR.code, "invalid_frame_length"); + return false; + } + protected boolean parseBody(ByteBuffer buffer) { int type = getFrameType(); @@ -209,6 +222,16 @@ public class Parser return headerParser.hasFlag(bit); } + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + protected void notifyConnectionFailure(int error, String reason) { try diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java new file mode 100644 index 00000000000..b902c1d925c --- /dev/null +++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java @@ -0,0 +1,66 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.frames; + +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.UnaryOperator; + +import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.parser.Parser; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.MappedByteBufferPool; +import org.junit.Assert; +import org.junit.Test; + +public class MaxFrameSizeParseTest +{ + private final ByteBufferPool byteBufferPool = new MappedByteBufferPool(); + + @Test + public void testMaxFrameSize() + { + int maxFrameLength = Frame.DEFAULT_MAX_LENGTH + 16; + + AtomicInteger failure = new AtomicInteger(); + Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter() + { + @Override + public void onConnectionFailure(int error, String reason) + { + failure.set(error); + } + }, 4096, 8192); + parser.setMaxFrameLength(maxFrameLength); + parser.init(UnaryOperator.identity()); + + // Iterate a few times to be sure the parser is properly reset. + for (int i = 0; i < 2; ++i) + { + byte[] bytes = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0}; + ByteBuffer buffer = ByteBuffer.wrap(bytes); + buffer.putInt(0, maxFrameLength + 1); + buffer.position(1); + while (buffer.hasRemaining()) + parser.parse(buffer); + } + + Assert.assertEquals(ErrorCode.FRAME_SIZE_ERROR.code, failure.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 b2cc1ad504e..753da94ef57 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 maxFrameLength = Frame.DEFAULT_MAX_LENGTH; 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 frame length in bytes") + public int getMaxFrameLength() + { + return maxFrameLength; + } + + public void setMaxFrameLength(int maxFrameLength) + { + this.maxFrameLength = maxFrameLength; + } + /** * @return -1 * @deprecated feature removed, no replacement @@ -205,6 +217,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize()); ServerParser parser = newServerParser(connector, session); + parser.setMaxFrameLength(getMaxFrameLength()); HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener); connection.addListener(connectionListener);