Issue #3978 HTTP2 Vulnerabilities

Reduce the number of RateControl fields, instead using common field in
HeaderParser.

Avoid null checking rateControl by having a NO_RATE_CONTROL static

HPack does not emit field with empty header name.

Apply rate control to any header parsing issue resulting in
session/stream failure

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-08-19 10:16:40 +10:00
parent 47fb8f4dea
commit 5fc83c3d0c
19 changed files with 120 additions and 66 deletions

View File

@ -46,6 +46,7 @@ import org.eclipse.jetty.http2.frames.DataFrame;
import org.eclipse.jetty.http2.frames.GoAwayFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.http2.parser.RateControl;
import org.eclipse.jetty.http2.parser.ServerParser;
import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory;
import org.eclipse.jetty.server.Connector;
@ -747,7 +748,7 @@ public class HTTP2Test extends AbstractTest
RawHTTP2ServerConnectionFactory connectionFactory = new RawHTTP2ServerConnectionFactory(new HttpConfiguration(), serverListener)
{
@Override
protected ServerParser newServerParser(Connector connector, ServerParser.Listener listener)
protected ServerParser newServerParser(Connector connector, ServerParser.Listener listener, RateControl rateControl)
{
return super.newServerParser(connector, new ServerParser.Listener.Wrapper(listener)
{
@ -757,7 +758,7 @@ public class HTTP2Test extends AbstractTest
super.onGoAway(frame);
goAwayLatch.countDown();
}
});
}, rateControl);
}
};
prepareServer(connectionFactory);

View File

@ -245,4 +245,9 @@ public abstract class BodyParser
LOG.info("Failure while notifying listener " + listener, x);
}
}
protected boolean rateControlOnEvent(Object o)
{
return headerParser.getRateControl().onEvent(o);
}
}

View File

@ -30,16 +30,14 @@ public class ContinuationBodyParser extends BodyParser
{
private final HeaderBlockParser headerBlockParser;
private final HeaderBlockFragments headerBlockFragments;
private final RateControl rateControl;
private State state = State.PREPARE;
private int length;
public ContinuationBodyParser(HeaderParser headerParser, Parser.Listener listener, HeaderBlockParser headerBlockParser, HeaderBlockFragments headerBlockFragments, RateControl rateControl)
public ContinuationBodyParser(HeaderParser headerParser, Parser.Listener listener, HeaderBlockParser headerBlockParser, HeaderBlockFragments headerBlockFragments)
{
super(headerParser, listener);
this.headerBlockParser = headerBlockParser;
this.headerBlockFragments = headerBlockFragments;
this.rateControl = rateControl;
}
@Override
@ -52,7 +50,7 @@ public class ContinuationBodyParser extends BodyParser
else
{
ContinuationFrame frame = new ContinuationFrame(getStreamId(), hasFlag(Flags.END_HEADERS));
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_continuation_frame_rate");
}
}

View File

@ -26,16 +26,14 @@ import org.eclipse.jetty.util.BufferUtil;
public class DataBodyParser extends BodyParser
{
private final RateControl rateControl;
private State state = State.PREPARE;
private int padding;
private int paddingLength;
private int length;
public DataBodyParser(HeaderParser headerParser, Parser.Listener listener, RateControl rateControl)
public DataBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
super(headerParser, listener);
this.rateControl = rateControl;
}
private void reset()
@ -56,7 +54,7 @@ public class DataBodyParser extends BodyParser
else
{
DataFrame frame = new DataFrame(getStreamId(), BufferUtil.EMPTY_BUFFER, isEndStream());
if (!isEndStream() && rateControl != null && !rateControl.onEvent(frame))
if (!isEndStream() && !rateControlOnEvent(frame))
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_data_frame_rate");
else
onData(frame);

View File

@ -103,6 +103,11 @@ public class HeaderBlockParser
{
if (LOG.isDebugEnabled())
LOG.debug(x);
if (!headerParser.getRateControl().onEvent(x))
{
notifier.connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_header_frame_rate");
return SESSION_FAILURE;
}
notifier.streamFailure(headerParser.getStreamId(), ErrorCode.PROTOCOL_ERROR.code, "invalid_hpack_block");
return STREAM_FAILURE;
}

View File

@ -30,6 +30,7 @@ import org.eclipse.jetty.http2.frames.FrameType;
*/
public class HeaderParser
{
private final RateControl rateControl;
private State state = State.LENGTH;
private int cursor;
@ -38,6 +39,16 @@ public class HeaderParser
private int flags;
private int streamId;
HeaderParser(RateControl rateControl)
{
this.rateControl = rateControl;
}
public RateControl getRateControl()
{
return rateControl;
}
protected void reset()
{
state = State.LENGTH;

View File

@ -32,7 +32,6 @@ public class HeadersBodyParser extends BodyParser
{
private final HeaderBlockParser headerBlockParser;
private final HeaderBlockFragments headerBlockFragments;
private final RateControl rateControl;
private State state = State.PREPARE;
private int cursor;
private int length;
@ -41,12 +40,11 @@ public class HeadersBodyParser extends BodyParser
private int parentStreamId;
private int weight;
public HeadersBodyParser(HeaderParser headerParser, Parser.Listener listener, HeaderBlockParser headerBlockParser, HeaderBlockFragments headerBlockFragments, RateControl rateControl)
public HeadersBodyParser(HeaderParser headerParser, Parser.Listener listener, HeaderBlockParser headerBlockParser, HeaderBlockFragments headerBlockFragments)
{
super(headerParser, listener);
this.headerBlockParser = headerBlockParser;
this.headerBlockFragments = headerBlockFragments;
this.rateControl = rateControl;
}
private void reset()
@ -71,7 +69,7 @@ public class HeadersBodyParser extends BodyParser
{
MetaData metaData = headerBlockParser.parse(BufferUtil.EMPTY_BUFFER, 0);
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, null, isEndStream());
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
else
onHeaders(frame);
@ -193,7 +191,7 @@ public class HeadersBodyParser extends BodyParser
else
{
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, null, isEndStream());
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
}
}

View File

@ -56,15 +56,19 @@ public class Parser
private UnknownBodyParser unknownBodyParser;
private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH;
private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private RateControl rateControl;
private boolean continuation;
private State state = State.HEADER;
public Parser(ByteBufferPool byteBufferPool, Listener listener, int maxDynamicTableSize, int maxHeaderSize)
{
this (byteBufferPool, listener, maxDynamicTableSize, maxHeaderSize, RateControl.NO_RATE_CONTROL);
}
public Parser(ByteBufferPool byteBufferPool, Listener listener, int maxDynamicTableSize, int maxHeaderSize, RateControl rateControl)
{
this.byteBufferPool = byteBufferPool;
this.listener = listener;
this.headerParser = new HeaderParser();
this.headerParser = new HeaderParser(rateControl == null ? RateControl.NO_RATE_CONTROL : rateControl);
this.hpackDecoder = new HpackDecoder(maxDynamicTableSize, maxHeaderSize);
this.bodyParsers = new BodyParser[FrameType.values().length];
}
@ -73,19 +77,19 @@ public class Parser
{
Listener listener = wrapper.apply(this.listener);
RateControl rateControl = getRateControl();
unknownBodyParser = new UnknownBodyParser(headerParser, listener, rateControl);
unknownBodyParser = new UnknownBodyParser(headerParser, listener);
HeaderBlockParser headerBlockParser = new HeaderBlockParser(headerParser, byteBufferPool, hpackDecoder, unknownBodyParser);
HeaderBlockFragments headerBlockFragments = new HeaderBlockFragments();
bodyParsers[FrameType.DATA.getType()] = new DataBodyParser(headerParser, listener, rateControl);
bodyParsers[FrameType.HEADERS.getType()] = new HeadersBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments, rateControl);
bodyParsers[FrameType.PRIORITY.getType()] = new PriorityBodyParser(headerParser, listener, rateControl);
bodyParsers[FrameType.DATA.getType()] = new DataBodyParser(headerParser, listener);
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, getMaxSettingsKeys(), rateControl);
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, rateControl);
bodyParsers[FrameType.PING.getType()] = new PingBodyParser(headerParser, listener);
bodyParsers[FrameType.GO_AWAY.getType()] = new GoAwayBodyParser(headerParser, listener);
bodyParsers[FrameType.WINDOW_UPDATE.getType()] = new WindowUpdateBodyParser(headerParser, listener);
bodyParsers[FrameType.CONTINUATION.getType()] = new ContinuationBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments, rateControl);
bodyParsers[FrameType.CONTINUATION.getType()] = new ContinuationBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments);
}
private void reset()
@ -238,12 +242,7 @@ public class Parser
public RateControl getRateControl()
{
return rateControl;
}
public void setRateControl(RateControl rateControl)
{
this.rateControl = rateControl;
return headerParser.getRateControl();
}
protected void notifyConnectionFailure(int error, String reason)

View File

@ -26,15 +26,13 @@ import org.eclipse.jetty.http2.frames.PingFrame;
public class PingBodyParser extends BodyParser
{
private final RateControl rateControl;
private State state = State.PREPARE;
private int cursor;
private byte[] payload;
public PingBodyParser(HeaderParser headerParser, Parser.Listener listener, RateControl rateControl)
public PingBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
super(headerParser, listener);
this.rateControl = rateControl;
}
private void reset()
@ -97,7 +95,7 @@ public class PingBodyParser extends BodyParser
private boolean onPing(ByteBuffer buffer, byte[] payload)
{
PingFrame frame = new PingFrame(payload, hasFlag(Flags.ACK));
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_ping_frame_rate");
reset();
notifyPing(frame);

View File

@ -25,16 +25,14 @@ import org.eclipse.jetty.http2.frames.PriorityFrame;
public class PriorityBodyParser extends BodyParser
{
private final RateControl rateControl;
private State state = State.PREPARE;
private int cursor;
private boolean exclusive;
private int parentStreamId;
public PriorityBodyParser(HeaderParser headerParser, Parser.Listener listener, RateControl rateControl)
public PriorityBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
super(headerParser, listener);
this.rateControl = rateControl;
}
private void reset()
@ -119,7 +117,7 @@ public class PriorityBodyParser extends BodyParser
private boolean onPriority(ByteBuffer buffer, int parentStreamId, int weight, boolean exclusive)
{
PriorityFrame frame = new PriorityFrame(getStreamId(), parentStreamId, weight, exclusive);
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_priority_frame_rate");
reset();
notifyPriority(frame);

View File

@ -37,9 +37,9 @@ public class ServerParser extends Parser
private State state = State.PREFACE;
private boolean notifyPreface = true;
public ServerParser(ByteBufferPool byteBufferPool, Listener listener, int maxDynamicTableSize, int maxHeaderSize)
public ServerParser(ByteBufferPool byteBufferPool, Listener listener, int maxDynamicTableSize, int maxHeaderSize, RateControl rateControl)
{
super(byteBufferPool, listener, maxDynamicTableSize, maxHeaderSize);
super(byteBufferPool, listener, maxDynamicTableSize, maxHeaderSize, rateControl);
this.listener = listener;
this.prefaceParser = new PrefaceParser(listener);
}

View File

@ -36,7 +36,6 @@ public class SettingsBodyParser extends BodyParser
private static final Logger LOG = Log.getLogger(SettingsBodyParser.class);
private final int maxKeys;
private final RateControl rateControl;
private State state = State.PREPARE;
private int cursor;
private int length;
@ -47,14 +46,13 @@ public class SettingsBodyParser extends BodyParser
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
this(headerParser, listener, SettingsFrame.DEFAULT_MAX_KEYS, null);
this(headerParser, listener, SettingsFrame.DEFAULT_MAX_KEYS);
}
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener, int maxKeys, RateControl rateControl)
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener, int maxKeys)
{
super(headerParser, listener);
this.maxKeys = maxKeys;
this.rateControl = rateControl;
}
protected void reset()
@ -76,7 +74,7 @@ public class SettingsBodyParser extends BodyParser
protected void emptyBody(ByteBuffer buffer)
{
SettingsFrame frame = new SettingsFrame(Collections.emptyMap(), hasFlag(Flags.ACK));
if (rateControl != null && !rateControl.onEvent(frame))
if (!rateControlOnEvent(frame))
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_settings_frame");
else
onSettings(frame);
@ -220,7 +218,8 @@ public class SettingsBodyParser extends BodyParser
public static SettingsFrame parseBody(final ByteBuffer buffer)
{
AtomicReference<SettingsFrame> frameRef = new AtomicReference<>();
SettingsBodyParser parser = new SettingsBodyParser(new HeaderParser(), new Parser.Listener.Adapter()
// TODO should we do rate control here?
SettingsBodyParser parser = new SettingsBodyParser(new HeaderParser(RateControl.NO_RATE_CONTROL), new Parser.Listener.Adapter()
{
@Override
public void onSettings(SettingsFrame frame)

View File

@ -21,18 +21,15 @@ package org.eclipse.jetty.http2.parser;
import java.nio.ByteBuffer;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.frames.Frame;
import org.eclipse.jetty.http2.frames.UnknownFrame;
public class UnknownBodyParser extends BodyParser
{
private final RateControl rateControl;
private int cursor;
public UnknownBodyParser(HeaderParser headerParser, Parser.Listener listener, RateControl rateControl)
public UnknownBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
super(headerParser, listener);
this.rateControl = rateControl;
}
@Override
@ -41,12 +38,9 @@ public class UnknownBodyParser extends BodyParser
int length = cursor == 0 ? getBodyLength() : cursor;
cursor = consume(buffer, length);
boolean parsed = cursor == 0;
if (parsed && rateControl != null)
{
Frame frame = new UnknownFrame(getFrameType());
if (!rateControl.onEvent(frame))
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_unknown_frame");
}
if (parsed && !rateControlOnEvent(new UnknownFrame(getFrameType())))
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_unknown_frame");
return parsed;
}

View File

@ -135,8 +135,7 @@ public class FrameFloodTest
{
failed.set(true);
}
}, 4096, 8192);
parser.setRateControl(new WindowRateControl(8, Duration.ofSeconds(1)));
}, 4096, 8192, new WindowRateControl(8, Duration.ofSeconds(1)));
parser.init(UnaryOperator.identity());
if (preamble != null)

View File

@ -175,7 +175,7 @@ public class HpackDecoder
name = Huffman.decode(buffer, length);
else
name = toASCIIString(buffer, length);
for (int i = 0; i < name.length(); i++)
for (int i = name.length(); i-- > 0;)
{
char c = name.charAt(i);
if (c >= 'A' && c <= 'Z')

View File

@ -74,11 +74,13 @@ public class MetaDataBuilder
{
HttpHeader header = field.getHeader();
String name = field.getName();
if (name == null || name.length() == 0)
throw new HpackException.SessionException("Header size 0");
String value = field.getValue();
int fieldSize = name.length() + (value == null ? 0 : value.length());
_size += fieldSize + 32;
if (_size > _maxSize)
throw new HpackException.SessionException("Header Size %d > %d", _size, _maxSize);
throw new HpackException.SessionException("Header size %d > %d", _size, _maxSize);
if (field instanceof StaticTableHttpField)
{

View File

@ -26,6 +26,7 @@ import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http2.hpack.HpackException.CompressionException;
import org.eclipse.jetty.http2.hpack.HpackException.SessionException;
import org.eclipse.jetty.http2.hpack.HpackException.StreamException;
import org.eclipse.jetty.util.TypeUtil;
import org.hamcrest.Matchers;
@ -43,6 +44,21 @@ import static org.junit.jupiter.api.Assertions.fail;
public class HpackDecoderTest
{
/*
0 1 2 3 4 5 6 7
+---+---+---+---+---+---+---+---+
| 0 | 0 | 0 | 0 | 0 |
+---+---+-----------------------+
| H | Name Length (7+) |
+---+---------------------------+
| Name String (Length octets) |
+---+---------------------------+
| H | Value Length (7+) |
+---+---------------------------+
| Value String (Length octets) |
+-------------------------------+
*/
@Test
public void testDecodeD_3() throws Exception
{
@ -253,7 +269,7 @@ public class HpackDecoderTest
decoder.decode(buffer);
fail();
}
catch (HpackException.SessionException e)
catch (SessionException e)
{
assertThat(e.getMessage(), Matchers.startsWith("Unknown index"));
}
@ -506,4 +522,38 @@ public class HpackDecoderTest
CompressionException ex = assertThrows(CompressionException.class, () -> decoder.decode(buffer));
assertThat(ex.getMessage(), Matchers.containsString("Bad termination"));
}
@Test()
public void testZeroLengthName() throws Exception
{
HpackDecoder decoder = new HpackDecoder(4096, 8192);
String encoded = "00000130";
ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
SessionException ex = assertThrows(SessionException.class, () -> decoder.decode(buffer));
assertThat(ex.getMessage(), Matchers.containsString("Header size 0"));
}
@Test()
public void testZeroLengthValue() throws Exception
{
HpackDecoder decoder = new HpackDecoder(4096, 8192);
String encoded = "00016800";
ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
MetaData metaData = decoder.decode(buffer);
assertThat(metaData.getFields().size(), is(1));
assertThat(metaData.getFields().get("h"), is(""));
}
@Test()
public void testUpperCaseName() throws Exception
{
HpackDecoder decoder = new HpackDecoder(4096, 8192);
String encoded = "0001480130";
ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
StreamException ex = assertThrows(StreamException.class, () -> decoder.decode(buffer));
assertThat(ex.getMessage(), Matchers.containsString("Uppercase header"));
}
}

View File

@ -501,7 +501,7 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest
x.printStackTrace();
}
}
}, 4096, 8192);
}, 4096, 8192, null);
parser.init(UnaryOperator.identity());
byte[] bytes = new byte[1024];

View File

@ -251,10 +251,9 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
session.setInitialSessionRecvWindow(getInitialSessionRecvWindow());
session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize());
ServerParser parser = newServerParser(connector, session);
ServerParser parser = newServerParser(connector, session, getRateControl());
parser.setMaxFrameLength(getMaxFrameLength());
parser.setMaxSettingsKeys(getMaxSettingsKeys());
parser.setRateControl(getRateControl());
HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(),
endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener);
@ -264,9 +263,9 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
protected abstract ServerSessionListener newSessionListener(Connector connector, EndPoint endPoint);
protected ServerParser newServerParser(Connector connector, ServerParser.Listener listener)
protected ServerParser newServerParser(Connector connector, ServerParser.Listener listener, RateControl rateControl)
{
return new ServerParser(connector.getByteBufferPool(), listener, getMaxDynamicTableSize(), getHttpConfiguration().getRequestHeaderSize());
return new ServerParser(connector.getByteBufferPool(), listener, getMaxDynamicTableSize(), getHttpConfiguration().getRequestHeaderSize(), rateControl);
}
@ManagedObject("The container of HTTP/2 sessions")