430951 Support SNI with ExtendedSslContextFactory

refactored common code in SniX509ExtendedKeyManager
added sniHostCheck code to ensure request host is the same as SNI host

8 files changed, 97 insertions, 63 deletions

diff --git a/jetty-server/src/main/config/etc/jetty-ssl.xml b/jetty-server/src/main/config/etc/jetty-ssl.xml
index 0b8bb17054..257c83689c 100644
--- a/jetty-server/src/main/config/etc/jetty-ssl.xml
+++ b/jetty-server/src/main/config/etc/jetty-ssl.xml
@@ -70,7 +70,11 @@
   <New id="sslHttpConfig" class="org.eclipse.jetty.server.HttpConfiguration">
     <Arg><Ref refid="httpConfig"/></Arg>
     <Call name="addCustomizer">
-      <Arg><New class="org.eclipse.jetty.server.SecureRequestCustomizer"/></Arg>
+      <Arg>
+        <New class="org.eclipse.jetty.server.SecureRequestCustomizer">
+          <Property name="jetty.ssl.sniHostCheck" default="true"/>
+        </New>
+      </Arg>
     </Call>
   </New>
 
diff --git a/jetty-server/src/main/config/modules/ssl.mod b/jetty-server/src/main/config/modules/ssl.mod
index e99ed7e3d7..36cc2fa068 100644
--- a/jetty-server/src/main/config/modules/ssl.mod
+++ b/jetty-server/src/main/config/modules/ssl.mod
@@ -38,6 +38,9 @@ http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/plain/jetty-server/
 ## Thread priority delta to give to acceptor threads
 # jetty.ssl.acceptorPriorityDelta=0
 
+## Whether request host names are checked to match any SNI names
+# jetty.ssl.sniHostCheck=true
+
 ### SslContextFactory Configuration
 
 ## Keystore file path (relative to $jetty.base)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index c010a5fc31..fbd82f70a3 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -30,6 +30,7 @@ import javax.servlet.DispatcherType;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.http.HttpServletRequest;
 
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpGenerator;
 import org.eclipse.jetty.http.HttpHeader;
@@ -353,7 +354,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
 
                     }
                 }
-                catch (EofException|QuietServletException e)
+                catch (EofException|QuietServletException|BadMessageException e)
                 {
                     error=true;
                     LOG.debug(e);
@@ -465,7 +466,14 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
             else
             {
                 _response.setHeader(HttpHeader.CONNECTION.asString(),HttpHeaderValue.CLOSE.asString());
-                _response.sendError(500, x.getMessage());
+
+                if (x instanceof BadMessageException)
+                {
+                    BadMessageException bme = (BadMessageException)x;
+                    _response.sendError(bme.getCode(), bme.getReason());
+                }
+                else
+                    _response.sendError(500, x.getClass().toString());
             }
         }
         catch (IOException e)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
index 2ebfbaf92a..4a3695350e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
@@ -25,6 +25,7 @@ import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLSession;
 import javax.servlet.ServletRequest;
 
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.io.ssl.SslConnection;
 import org.eclipse.jetty.io.ssl.SslConnection.DecryptedEndPoint;
@@ -48,6 +49,19 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
      */
     public static final String CACHED_INFO_ATTR = CachedInfo.class.getName();
 
+    private boolean _sniHostCheck;
+    
+    
+    public SecureRequestCustomizer()
+    {
+        this(true);
+    }
+    
+    public SecureRequestCustomizer(boolean sniHostCheck)
+    {
+        _sniHostCheck=sniHostCheck;
+    }
+    
     @Override
     public void customize(Connector connector, HttpConfiguration channelConfig, Request request)
     {
@@ -88,6 +102,16 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
         request.setScheme(HttpScheme.HTTPS.asString());
         SSLSession sslSession = sslEngine.getSession();
 
+        if (_sniHostCheck)
+        {
+            String sniName = (String)sslSession.getValue("org.eclipse.jetty.util.ssl.sniname");
+            if (sniName!=null && !sniName.equalsIgnoreCase(request.getServerName()))
+            {
+                LOG.warn("Host does not match SNI Name: {}!={}",sniName,request.getServerName());
+                throw new BadMessageException(400,"Host does not match SNI");
+            }
+        }
+        
         try
         {
             String cipherSuite=sslSession.getCipherSuite();
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
index f266b9609c..96eba76a1b 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
@@ -1261,7 +1261,7 @@ public class RequestTest
         {
             long start=System.currentTimeMillis();
             String response = _connector.getResponses(request);
-            assertThat(response,Matchers.containsString("Form too many keys"));
+            assertThat(response,Matchers.containsString("IllegalStateException"));
             long now=System.currentTimeMillis();
             assertTrue((now-start)<5000);
         }
@@ -1307,7 +1307,7 @@ public class RequestTest
         {
             long start=System.currentTimeMillis();
             String response = _connector.getResponses(request);
-            assertTrue(response.contains("Form too large:"));
+            assertTrue(response.contains("IllegalStateException"));
             long now=System.currentTimeMillis();
             assertTrue((now-start)<5000);
         }
@@ -1414,7 +1414,7 @@ public class RequestTest
                 request.setAttribute(Request.__MULTIPART_CONFIG_ELEMENT, mpce);
 
                 //We should get an error when we getParams if there was a problem parsing the multipart
-                String field1 = request.getParameter("xxx");
+                request.getParameter("xxx");
                 //A 200 response is actually wrong here
             }
             catch (RuntimeException e)
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
index 20fc3b8934..96d0f9315a 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
@@ -125,7 +125,9 @@ public class SslConnectionFactoryTest
     @Test
     public void testSNIConnect() throws Exception
     {
-        String response= getResponse("jetty.eclipse.org","jetty.eclipse.org");
+        String response;
+        
+        response= getResponse("jetty.eclipse.org","jetty.eclipse.org");
         Assert.assertThat(response,Matchers.containsString("host=jetty.eclipse.org"));
         
         response= getResponse("www.example.com","www.example.com");
@@ -139,11 +141,30 @@ public class SslConnectionFactoryTest
         
         response= getResponse("www.san.com","san example");
         Assert.assertThat(response,Matchers.containsString("host=www.san.com"));
+      
     }
-
     
+    @Test
+    public void testBadSNIConnect() throws Exception
+    {
+        String response;
+        
+        response= getResponse("www.example.com","some.other.com","www.example.com");
+        Assert.assertThat(response,Matchers.containsString("HTTP/1.1 400 "));
+        Assert.assertThat(response,Matchers.containsString("Host does not match SNI"));
+    }
+
+
     private String getResponse(String host,String cn) throws Exception
     {
+        String response = getResponse(host,host,cn);
+        Assert.assertThat(response,Matchers.startsWith("HTTP/1.1 200 OK"));
+        Assert.assertThat(response,Matchers.containsString("url=/ctx/path"));
+        return response;
+    }
+    
+    private String getResponse(String sniHost,String reqHost, String cn) throws Exception
+    {
         SslContextFactory clientContextFactory = new SslContextFactory(true);
         clientContextFactory.start();
         SSLSocketFactory factory = clientContextFactory.getSslContext().getSocketFactory();
@@ -152,7 +173,7 @@ public class SslConnectionFactoryTest
 
         if (cn!=null)
         {        
-            SNIHostName serverName = new SNIHostName(host);
+            SNIHostName serverName = new SNIHostName(sniHost);
             List<SNIServerName> serverNames = new ArrayList<>();
             serverNames.add(serverName);
 
@@ -170,10 +191,8 @@ public class SslConnectionFactoryTest
             Assert.assertThat(cert.getSubjectX500Principal().getName("CANONICAL"), Matchers.startsWith("cn="+cn));
         }
 
-        sslSocket.getOutputStream().write(("GET /ctx/path HTTP/1.0\r\nHost: "+host+":"+_port+"\r\n\r\n").getBytes(StandardCharsets.ISO_8859_1));
+        sslSocket.getOutputStream().write(("GET /ctx/path HTTP/1.0\r\nHost: "+reqHost+":"+_port+"\r\n\r\n").getBytes(StandardCharsets.ISO_8859_1));
         String response = IO.toString(sslSocket.getInputStream());
-        Assert.assertThat(response,Matchers.startsWith("HTTP/1.1 200 OK"));
-        Assert.assertThat(response,Matchers.containsString("url=/ctx/path"));
         
         sslSocket.close();
         clientContextFactory.stop();
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/ExtendedSslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/ExtendedSslContextFactory.java
index 5fc615f7c1..f3cee8618d 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/ExtendedSslContextFactory.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/ExtendedSslContextFactory.java
@@ -223,9 +223,9 @@ public class ExtendedSslContextFactory extends SslContextFactory
             return _alias;
         }
         
-        public SNIHostName getServerName()
+        public String getServerName()
         {
-            return _name;
+            return _name==null?null:_name.getAsciiName();
         }
     }
 }
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java
index 8f117923ec..9730e3caab 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java
@@ -27,6 +27,7 @@ import java.util.Collection;
 
 import javax.net.ssl.SNIMatcher;
 import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.X509ExtendedKeyManager;
 
@@ -42,6 +43,8 @@ import org.eclipse.jetty.util.log.Logger;
 public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
 {
     static final Logger LOG = Log.getLogger(SniX509ExtendedKeyManager.class);
+    public final static String SNI_NAME = "org.eclipse.jetty.util.ssl.sniname";
+    public final static String NO_MATCHERS="No Matchers";
     private final X509ExtendedKeyManager _delegate;
 
     /* ------------------------------------------------------------ */
@@ -55,7 +58,6 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
     {
         _delegate = keyManager;
     }
-    
 
     @Override
     public String chooseClientAlias(String[] keyType, Principal[] issuers, Socket socket)
@@ -68,31 +70,31 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
     {
         return _delegate.chooseEngineClientAlias(keyType,issuers,engine);
     }
-    
-    @Override
-    public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket)
+
+    protected String chooseServerAlias(String keyType, Principal[] issuers, Collection<SNIMatcher> matchers, SSLSession session)
     {
         // Look for the aliases that are suitable for the keytype and issuers
         String[] aliases = _delegate.getServerAliases(keyType,issuers);
-        
         if (aliases==null || aliases.length==0)
             return null;
              
         // Look for an SNI alias
         String alias=null;
-        Collection<SNIMatcher> matchers = ((SSLSocket)socket).getSSLParameters().getSNIMatchers();
+        String host=null;
         if (matchers!=null)
         {
             for (SNIMatcher m : matchers)
             {
                 if (m instanceof ExtendedSslContextFactory.AliasSNIMatcher)
                 {
-                    alias=((ExtendedSslContextFactory.AliasSNIMatcher)m).getAlias();
+                    ExtendedSslContextFactory.AliasSNIMatcher matcher = (ExtendedSslContextFactory.AliasSNIMatcher)m;
+                    alias=matcher.getAlias();
+                    host=matcher.getServerName();
                     break;
                 }
             }
         }
-        
+
         if (LOG.isDebugEnabled())
             LOG.debug("choose {} from {}",alias,Arrays.asList(aliases));
         
@@ -102,54 +104,30 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
             for (String a:aliases)
             {
                 if (a.equals(alias))
+                {
+                    session.putValue(SNI_NAME,host);
                     return alias;
+                }
             }
-            
             return null;
         }
-        
-        return _delegate.chooseServerAlias(keyType,issuers,socket);
+        return NO_MATCHERS;
     }
-
+    
     @Override
-    public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine)
+    public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket)
     {
-        // Look for the aliases that are suitable for the keytype and issuers
-        String[] aliases = _delegate.getServerAliases(keyType,issuers);
-        if (aliases==null || aliases.length==0)
-            return null;
-             
-        // Look for an SNI alias
-        String alias=null;
-        Collection<SNIMatcher> matchers = engine.getSSLParameters().getSNIMatchers();
-        if (matchers!=null)
-        {
-            for (SNIMatcher m : matchers)
-            {
-                if (m instanceof ExtendedSslContextFactory.AliasSNIMatcher)
-                {
-                    alias=((ExtendedSslContextFactory.AliasSNIMatcher)m).getAlias();
-                    break;
-                }
-            }
-        }
-
-        if (LOG.isDebugEnabled())
-            LOG.debug("choose {} from {}",alias,Arrays.asList(aliases));
+        SSLSocket sslSocket = (SSLSocket)socket;
         
-        // Check if the SNI selected alias is allowable
-        if (alias!=null)
-        {
-            for (String a:aliases)
-            {
-                if (a.equals(alias))
-                    return alias;
-            }
-            
-            return null;
-        }
+        String alias = chooseServerAlias(keyType,issuers,sslSocket.getSSLParameters().getSNIMatchers(),sslSocket.getHandshakeSession());
+        return alias==NO_MATCHERS?_delegate.chooseServerAlias(keyType,issuers,socket):alias;
+    }
         
-        return _delegate.chooseEngineServerAlias(keyType,issuers,engine);
+    @Override
+    public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine)
+    {
+        String alias = chooseServerAlias(keyType,issuers,engine.getSSLParameters().getSNIMatchers(),engine.getHandshakeSession());
+        return alias==NO_MATCHERS?_delegate.chooseEngineServerAlias(keyType,issuers,engine):alias;
     }   
     
     @Override
@@ -175,6 +153,4 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
     {
         return _delegate.getServerAliases(keyType,issuers);
     }
-
-    
 }