From b8986d581266fea56fc20953d1460f6b01d1f5bc Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 10 Jan 2017 15:05:40 +0100 Subject: [PATCH 1/2] Fixes #1228 - Internal error during SSL handshake. Added a guard to avoid unwrapping during handshake if SSLEngine is already closed. --- .../jetty/client/ssl/SslConnectionTest.java | 94 +++++++++++++++++++ .../eclipse/jetty/io/ssl/SslConnection.java | 3 + 2 files changed, 97 insertions(+) create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslConnectionTest.java diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslConnectionTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslConnectionTest.java new file mode 100644 index 00000000000..fccb80d9972 --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslConnectionTest.java @@ -0,0 +1,94 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 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.client.ssl; + +import java.io.File; +import java.nio.ByteBuffer; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLHandshakeException; + +import org.eclipse.jetty.io.AbstractConnection; +import org.eclipse.jetty.io.ByteArrayEndPoint; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.io.MappedByteBufferPool; +import org.eclipse.jetty.io.ssl.SslConnection; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.Assert; +import org.junit.Test; + +public class SslConnectionTest +{ + @Test + public void testSslConnectionClosedBeforeFill() throws Exception + { + File keyStore = MavenTestingUtils.getTestResourceFile("keystore.jks"); + SslContextFactory sslContextFactory = new SslContextFactory(); + sslContextFactory.setKeyStorePath(keyStore.getAbsolutePath()); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.start(); + + ByteBufferPool byteBufferPool = new MappedByteBufferPool(); + QueuedThreadPool threadPool = new QueuedThreadPool(); + threadPool.start(); + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(); + SSLEngine sslEngine = sslContextFactory.newSSLEngine(); + sslEngine.setUseClientMode(false); + SslConnection sslConnection = new SslConnection(byteBufferPool, threadPool, endPoint, sslEngine); + EndPoint sslEndPoint = sslConnection.getDecryptedEndPoint(); + sslEndPoint.setConnection(new AbstractConnection(sslEndPoint, threadPool) + { + @Override + public void onFillable() + { + } + }); + + // There are no bytes in the endPoint, so we fill zero. + // However, this will trigger state changes in SSLEngine + // that will later cause it to throw ISE("Internal error"). + sslEndPoint.fill(BufferUtil.EMPTY_BUFFER); + + // Close the connection before filling. + sslEndPoint.shutdownOutput(); + + // Put some bytes in the endPoint to trigger + // the required state changes in SSLEngine. + byte[] bytes = new byte[]{0x16, 0x03, 0x03, 0x00, 0x00}; + endPoint.addInput(ByteBuffer.wrap(bytes)); + + // This attempt to read, if not guarded, throws ISE("Internal error"). + // We want SSLHandshakeException to be thrown instead, because it is + // handled better (it is an IOException) by the Connection code that + // reads from the EndPoint. + try + { + sslEndPoint.fill(BufferUtil.EMPTY_BUFFER); + Assert.fail(); + } + catch (SSLHandshakeException x) + { + // Expected. + } + } +} 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 3c9c85809d3..5ba743e9e2a 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 @@ -542,6 +542,9 @@ public class SslConnection extends AbstractConnection // Let's try reading some encrypted data... even if we have some already. int net_filled = getEndPoint().fill(_encryptedInput); + if (net_filled > 0 && !_handshaken && _sslEngine.isOutboundDone()) + throw new SSLHandshakeException("Closed during handshake"); + decryption: while (true) { // Let's unwrap even if we have no net data because in that From 25a1864de7036e0d09b5ce858e095926c80a78a5 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 10 Jan 2017 11:12:14 -0400 Subject: [PATCH 2/2] Issue #1229 - Adding testcase for WAR (expanded) with WEB-INF/lib/jetty-http.jar --- .../jetty/websocket/server/WSServer.java | 35 ++++++++++++++++--- .../server/WebSocketUpgradeFilterTest.java | 27 ++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java index 9ca3a1d7f4a..22aefa61bd8 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java @@ -18,12 +18,15 @@ package org.eclipse.jetty.websocket.server; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertThat; import java.io.File; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import org.eclipse.jetty.annotations.AnnotationConfiguration; @@ -35,6 +38,7 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.IO; +import org.eclipse.jetty.toolchain.test.JAR; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.OS; import org.eclipse.jetty.toolchain.test.TestingDir; @@ -47,7 +51,6 @@ import org.eclipse.jetty.webapp.MetaInfConfiguration; import org.eclipse.jetty.webapp.WebAppContext; import org.eclipse.jetty.webapp.WebInfConfiguration; import org.eclipse.jetty.webapp.WebXmlConfiguration; -import org.junit.Assert; /** * Utility to build out exploded directory WebApps, in the /target/tests/ directory, for testing out servers that use javax.websocket endpoints. @@ -82,7 +85,7 @@ public class WSServer ClassLoader cl = Thread.currentThread().getContextClassLoader(); String endpointPath = clazz.getName().replace('.','/') + ".class"; URL classUrl = cl.getResource(endpointPath); - Assert.assertThat("Class URL for: " + clazz,classUrl,notNullValue()); + assertThat("Class URL for: " + clazz,classUrl,notNullValue()); File destFile = new File(classesDir,OS.separators(endpointPath)); FS.ensureDirExists(destFile.getParentFile()); File srcFile = new File(classUrl.toURI()); @@ -93,7 +96,31 @@ public class WSServer { copyClass(endpointClass); } - + + public void copyLib(Class clazz, String jarFileName) throws URISyntaxException, IOException + { + webinf = new File(contextDir,"WEB-INF"); + FS.ensureDirExists(webinf); + File libDir = new File(webinf,"lib"); + FS.ensureDirExists(libDir); + File jarFile = new File(libDir, jarFileName); + + URL codeSourceURL = clazz.getProtectionDomain().getCodeSource().getLocation(); + assertThat("Class CodeSource URL is file scheme", codeSourceURL.getProtocol(), is("file")); + + File sourceCodeSourceFile = new File(codeSourceURL.toURI()); + if (sourceCodeSourceFile.isDirectory()) + { + LOG.info("Creating " + jarFile + " from " + sourceCodeSourceFile); + JAR.create(sourceCodeSourceFile, jarFile); + } + else + { + LOG.info("Copying " + sourceCodeSourceFile + " to " + jarFile); + IO.copy(sourceCodeSourceFile, jarFile); + } + } + public void copyWebInf(String testResourceName) throws IOException { webinf = new File(contextDir,"WEB-INF"); @@ -173,7 +200,7 @@ public class WSServer contexts = new ContextHandlerCollection(); handlers.addHandler(contexts); server.setHandler(handlers); - + server.start(); String host = connector.getHost(); diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java index 131b289a9b6..4e0884c1ed8 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java @@ -252,6 +252,33 @@ public class WebSocketUpgradeFilterTest } }}); + // WSUF from web.xml, SCI active, apply app-ws configuration via ServletContextListener with WEB-INF/lib/jetty-http.jar + + cases.add(new Object[]{"wsuf/WebAppContext/web.xml/ServletContextListener/jetty-http.jar", new ServerProvider() + { + @Override + public Server newServer() throws Exception + { + File testDir = MavenTestingUtils.getTargetTestingDir("WSUF-webxml"); + + WSServer server = new WSServer(testDir, "/"); + + server.copyWebInf("wsuf-config-via-listener.xml"); + server.copyClass(InfoSocket.class); + server.copyClass(InfoContextAttributeListener.class); + // Add a jetty-http.jar to ensure that the classloader constraints + // and the WebAppClassloader setup is sane and correct + // The odd version string is present to capture bad regex behavior in Jetty + server.copyLib(org.eclipse.jetty.http.pathmap.PathSpec.class, "jetty-http-9.99.999.jar"); + server.start(); + + WebAppContext webapp = server.createWebAppContext(); + server.deployWebapp(webapp); + + return server.getServer(); + } + }}); + // WSUF from web.xml, SCI active, apply app-ws configuration via Servlet.init cases.add(new Object[]{"wsuf/WebAppContext/web.xml/Servlet.init", new ServerProvider()