Fix denial of service security vulnerability

Resolves: CVE-2018-12545
2019-04-11 16:05:27 +02:00
3 changed files with 386 additions and 5 deletions
--- a/0001-Fedora-jetty.home.patch
+++ b/0001-Fedora-jetty.home.patch
@ -1,14 +1,14 @@
-From e1f6d2f9749b6a04a3b7d163d17df41751fb8977 Mon Sep 17 00:00:00 2001
+From 9ec63a25b98d2ff5b5815e9ae6c5998a75c1d031 Mon Sep 17 00:00:00 2001
 From: Michael Simacek <msimacek@redhat.com>
 Date: Tue, 2 Jun 2015 22:52:21 +0200
-Subject: [PATCH] Fedora jetty.home
+Subject: [PATCH 1/2] Fedora jetty.home

 ---
 .../eclipse/jetty/start/config/CommandLineConfigSource.java    | 3 +++
 1 file changed, 3 insertions(+)

 diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/config/CommandLineConfigSource.java b/jetty-start/src/main/java/org/eclipse/jetty/start/config/CommandLineConfigSource.java
-index f0c9e07..7d2ee3e 100644
+index f0c9e07b2a..7d2ee3e0d1 100644
 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/config/CommandLineConfigSource.java
 +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/config/CommandLineConfigSource.java
@@ -120,6 +120,9 @@ public class CommandLineConfigSource implements ConfigSource
@ -22,5 +22,5 @@ index f0c9e07..7d2ee3e 100644
                     return home;
                 }
 -- 
-2.17.1
+2.20.1

--- a/0002-Fixes-2722-Improve-configurability-for-SETTINGS-fram.patch
+++ b/0002-Fixes-2722-Improve-configurability-for-SETTINGS-fram.patch
@ -0,0 +1,375 @@
+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
+
--- a/jetty.spec
+++ b/jetty.spec
@ -50,7 +50,7 @@

 Name:           jetty
 Version:        9.4.11
-Release:        2%{addver}%{?dist}
+Release:        3%{addver}%{?dist}
 Summary:        Java Webserver and Servlet Container

 # Jetty is dual licensed under both ASL 2.0 and EPL 1.0, see NOTICE.txt
@ -64,6 +64,7 @@ Source5:        %{name}.service
 Source6:        LICENSE-MIT

 Patch1:         0001-Fedora-jetty.home.patch
+Patch2:         0002-Fixes-2722-Improve-configurability-for-SETTINGS-fram.patch

 BuildRequires:  maven-local
 BuildRequires:  mvn(javax.servlet:javax.servlet-api)
@ -602,6 +603,7 @@ License:        (ASL 2.0 or EPL) and MIT
 %setup -q -n %{name}.project-%{name}-%{version}%{addver}

 %patch1 -p1
+%patch2 -p1

 find . -name "*.?ar" -exec rm {} \;
 find . -name "*.class" -exec rm {} \;
@ -968,6 +970,10 @@ exit 0
 %license LICENSE-eplv10-aslv20.html LICENSE-MIT

 %changelog
+* Thu Apr 11 2019 Mikolaj Izdebski <mizdebsk@redhat.com> - 9.4.11-3.v20180605
+- Fix denial of service security vulnerability
+- Resolves: CVE-2018-12545
+
 * Mon Jul 02 2018 Michael Simacek <msimacek@redhat.com> - 9.4.11-2.v20180605
 - Fix missing classes in start.jar