Merge pull request #4309 from eclipse/jetty-9.4.x-4305-no_application…

…_protocol

Fixes #4305 - Jetty server shall alert fatal no_application_protocol …

jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java

@@ -55,13 +55,13 @@ public void configure(SSLEngine sslEngine, Connection connection)
         ALPNClientConnection alpn = (ALPNClientConnection)connection;
         SSLParameters sslParameters = sslEngine.getSSLParameters();
         List<String> 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;
 

jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java

@@ -55,7 +55,7 @@ public void configure(SSLEngine sslEngine, Connection connection)
         sslEngine.setHandshakeApplicationProtocolSelector(new ALPNCallback((ALPNServerConnection)connection));
     }
 
-    private final class ALPNCallback implements BiFunction<SSLEngine, List<String>, String>, SslHandshakeListener
+    private static final class ALPNCallback implements BiFunction<SSLEngine, List<String>, String>, SslHandshakeListener
     {
         private final ALPNServerConnection alpnConnection;
 
@@ -68,10 +68,19 @@ private ALPNCallback(ALPNServerConnection connection)
         @Override
         public String apply(SSLEngine engine, List<String> 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

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.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 void startServer(Handler handler) throws Exception
         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 void testClientNotSupportingALPNServerSpeaksDefaultProtocol() throws Exce
         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 void testClientSupportingALPNServerSpeaksNegotiatedProtocol() throws Exce
         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 void handle(String target, Request baseRequest, HttpServletRequest reques
             }
         }
     }
+
+    @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);
+        }
+    }
 }