From e7e7a30e11736bec148cd185ea6ec4f1796b1dd7 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 13 Nov 2019 10:28:51 +0100 Subject: [PATCH 1/2] Fixes #4305 - Jetty server shall alert fatal no_application_protocol if no client application protocol is supported. Now catching any exception and returning null so JSSE returns the no_application_protocol to the client. Signed-off-by: Simone Bordet --- .../java/client/JDK9ClientALPNProcessor.java | 4 +- .../java/server/JDK9ServerALPNProcessor.java | 19 +++-- .../jetty/alpn/java/server/JDK9ALPNTest.java | 76 ++++++++++++++++++- 3 files changed, 88 insertions(+), 11 deletions(-) diff --git a/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java b/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java index f8e444b051e..96d5e53c10d 100644 --- a/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java @@ -55,13 +55,13 @@ public class JDK9ClientALPNProcessor implements ALPNProcessor.Client ALPNClientConnection alpn = (ALPNClientConnection)connection; SSLParameters sslParameters = sslEngine.getSSLParameters(); List protocols = alpn.getProtocols(); - sslParameters.setApplicationProtocols(protocols.toArray(new String[protocols.size()])); + sslParameters.setApplicationProtocols(protocols.toArray(new String[0])); sslEngine.setSSLParameters(sslParameters); ((DecryptedEndPoint)connection.getEndPoint()).getSslConnection() .addHandshakeListener(new ALPNListener(alpn)); } - private final class ALPNListener implements SslHandshakeListener + private static final class ALPNListener implements SslHandshakeListener { private final ALPNClientConnection alpnConnection; diff --git a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java index ca091a96e15..7ddf0727871 100644 --- a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java @@ -55,7 +55,7 @@ public class JDK9ServerALPNProcessor implements ALPNProcessor.Server, SslHandsha sslEngine.setHandshakeApplicationProtocolSelector(new ALPNCallback((ALPNServerConnection)connection)); } - private final class ALPNCallback implements BiFunction, String>, SslHandshakeListener + private static final class ALPNCallback implements BiFunction, String>, SslHandshakeListener { private final ALPNServerConnection alpnConnection; @@ -68,10 +68,19 @@ public class JDK9ServerALPNProcessor implements ALPNProcessor.Server, SslHandsha @Override public String apply(SSLEngine engine, List protocols) { - if (LOG.isDebugEnabled()) - LOG.debug("apply {} {}", alpnConnection, protocols); - alpnConnection.select(protocols); - return alpnConnection.getProtocol(); + try + { + if (LOG.isDebugEnabled()) + LOG.debug("apply {} {}", alpnConnection, protocols); + alpnConnection.select(protocols); + return alpnConnection.getProtocol(); + } + catch (Throwable x) + { + // Cannot negotiate the protocol, return null to have + // JSSE send Alert.NO_APPLICATION_PROTOCOL to the client. + return null; + } } @Override diff --git a/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java b/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java index 4163a3facec..be9a3e52912 100644 --- a/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java +++ b/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java @@ -19,15 +19,18 @@ package org.eclipse.jetty.alpn.java.server; import java.io.BufferedReader; -import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.SocketChannel; import java.nio.charset.StandardCharsets; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -40,11 +43,16 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; public class JDK9ALPNTest { @@ -65,6 +73,13 @@ public class JDK9ALPNTest server.start(); } + @AfterEach + public void dispose() throws Exception + { + if (server != null) + server.stop(); + } + private SslContextFactory newSslContextFactory() { SslContextFactory sslContextFactory = new SslContextFactory.Server(); @@ -83,7 +98,7 @@ public class JDK9ALPNTest startServer(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) { baseRequest.setHandled(true); } @@ -125,7 +140,7 @@ public class JDK9ALPNTest startServer(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) { baseRequest.setHandled(true); } @@ -163,4 +178,57 @@ public class JDK9ALPNTest } } } + + @Test + public void testClientSupportingALPNCannotNegotiateProtocol() throws Exception + { + startServer(new AbstractHandler() { + @Override + public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) + { + jettyRequest.setHandled(true); + } + }); + + SslContextFactory sslContextFactory = new SslContextFactory.Client(true); + sslContextFactory.start(); + String host = "localhost"; + int port = connector.getLocalPort(); + try (SocketChannel client = SocketChannel.open(new InetSocketAddress(host, port))) + { + client.socket().setSoTimeout(5000); + + SSLEngine sslEngine = sslContextFactory.newSSLEngine(host, port); + sslEngine.setUseClientMode(true); + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setApplicationProtocols(new String[]{"unknown/1.0"}); + sslEngine.setSSLParameters(sslParameters); + sslEngine.beginHandshake(); + assertSame(SSLEngineResult.HandshakeStatus.NEED_WRAP, sslEngine.getHandshakeStatus()); + + ByteBuffer sslBuffer = ByteBuffer.allocate(sslEngine.getSession().getPacketBufferSize()); + + SSLEngineResult result = sslEngine.wrap(BufferUtil.EMPTY_BUFFER, sslBuffer); + assertSame(SSLEngineResult.Status.OK, result.getStatus()); + + sslBuffer.flip(); + client.write(sslBuffer); + + assertSame(SSLEngineResult.HandshakeStatus.NEED_UNWRAP, sslEngine.getHandshakeStatus()); + + sslBuffer.clear(); + int read = client.read(sslBuffer); + assertThat(read, greaterThan(0)); + + sslBuffer.flip(); + // TLS frame layout: record_type, major_version, minor_version, hi_length, lo_length + int recordTypeAlert = 21; + assertEquals(recordTypeAlert, sslBuffer.get(0) & 0xFF); + // Alert record layout: alert_level, alert_code + int alertLevelFatal = 2; + assertEquals(alertLevelFatal, sslBuffer.get(5) & 0xFF); + int alertCodeNoApplicationProtocol = 120; + assertEquals(alertCodeNoApplicationProtocol, sslBuffer.get(6) & 0xFF); + } + } } From e3f31a86a2f91f93f3ef1ea6eb19aea3e6f1730f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 13 Nov 2019 16:46:11 +0100 Subject: [PATCH 2/2] Fixed flaky test testUpload_Multipart(). Signed-off-by: Simone Bordet --- .../src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java index 0687f349878..a3859ffc55d 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java @@ -271,6 +271,7 @@ public class HugeResourceTest multipart.addFilePart(name, filename, new PathContentProvider(inputFile), null); URI destUri = server.getURI().resolve("/multipart"); + client.setIdleTimeout(90_000); Request request = client.newRequest(destUri).method(HttpMethod.POST).content(multipart); ContentResponse response = request.send(); assertThat("HTTP Response Code", response.getStatus(), is(200));