From 00d379c94ba865dced2025c2d1bc3e2e0e41e880 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 18 Mar 2021 08:08:55 -0500 Subject: [PATCH] Fixes #6072 - jetty server high CPU when client send data length > 17408. Avoid spinning if the input buffer is full. Signed-off-by: Simone Bordet Co-authored-by: Joakim Erdfelt --- .../eclipse/jetty/io/ssl/SslConnection.java | 9 ++- .../jetty/server/ssl/SSLEngineTest.java | 71 ++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 9baa82e365b..bd7c6d49de0 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -730,7 +730,12 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr case BUFFER_UNDERFLOW: if (netFilled > 0) - continue; // try filling some more + { + if (BufferUtil.space(_encryptedInput) > 0) + continue; // try filling some more + BufferUtil.clear(_encryptedInput); + throw new SSLHandshakeException("Encrypted buffer max length exceeded"); + } _underflown = true; if (netFilled < 0 && _sslEngine.getUseClientMode()) { @@ -739,7 +744,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { Throwable handshakeFailure = new SSLHandshakeException("Abruptly closed by peer"); if (closeFailure != null) - handshakeFailure.initCause(closeFailure); + handshakeFailure.addSuppressed(closeFailure); throw handshakeFailure; } return filled = -1; diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLEngineTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLEngineTest.java index c3be87edad3..28cee271b7e 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLEngineTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLEngineTest.java @@ -34,19 +34,31 @@ import java.net.Socket; import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URL; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicLong; +import javax.net.SocketFactory; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLSession; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.io.ssl.SslConnection; +import org.eclipse.jetty.server.ConnectionFactory; +import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; @@ -59,6 +71,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -106,12 +119,13 @@ public class SSLEngineTest private Server server; private ServerConnector connector; + private SslContextFactory.Server sslContextFactory; @BeforeEach public void startServer() throws Exception { String keystore = MavenTestingUtils.getTestResourceFile("keystore").getAbsolutePath(); - SslContextFactory sslContextFactory = new SslContextFactory.Server(); + sslContextFactory = new SslContextFactory.Server(); sslContextFactory.setKeyStorePath(keystore); sslContextFactory.setKeyStorePassword("storepwd"); sslContextFactory.setKeyManagerPassword("keypwd"); @@ -191,6 +205,61 @@ public class SSLEngineTest assertThat(response.length(), greaterThan(102400)); } + @Test + public void testInvalidLargeTLSFrame() throws Exception + { + AtomicLong unwraps = new AtomicLong(); + ConnectionFactory http = connector.getConnectionFactory(HttpConnectionFactory.class); + ConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, http.getProtocol()) + { + @Override + protected SslConnection newSslConnection(Connector connector, EndPoint endPoint, SSLEngine engine) + { + return new SslConnection(connector.getByteBufferPool(), connector.getExecutor(), endPoint, engine, isDirectBuffersForEncryption(), isDirectBuffersForDecryption()) + { + @Override + protected SSLEngineResult unwrap(SSLEngine sslEngine, ByteBuffer input, ByteBuffer output) throws SSLException + { + unwraps.incrementAndGet(); + return super.unwrap(sslEngine, input, output); + } + }; + } + }; + ServerConnector tlsConnector = new ServerConnector(server, 1, 1, ssl, http); + server.addConnector(tlsConnector); + server.setHandler(new HelloWorldHandler()); + server.start(); + + // Create raw TLS record. + byte[] bytes = new byte[20005]; + Arrays.fill(bytes, (byte)1); + + bytes[0] = 22; // record type + bytes[1] = 3; // major version + bytes[2] = 3; // minor version + bytes[3] = 78; // record length 2 bytes / 0x4E20 / decimal 20,000 + bytes[4] = 32; // record length + bytes[5] = 1; // message type + bytes[6] = 0; // message length 3 bytes / 0x004E17 / decimal 19,991 + bytes[7] = 78; + bytes[8] = 23; + + SocketFactory socketFactory = SocketFactory.getDefault(); + try (Socket client = socketFactory.createSocket("localhost", tlsConnector.getLocalPort())) + { + client.getOutputStream().write(bytes); + + // Sleep to see if the server spins. + Thread.sleep(1000); + assertThat(unwraps.get(), lessThan(128L)); + + // Read until -1 or read timeout. + client.setSoTimeout(1000); + IO.readBytes(client.getInputStream()); + } + } + @Test public void testRequestJettyHttps() throws Exception {