475927 - SecureRequestCustomizer fails to match host.

Fixed by storing in the SSLSession the SNI names correspondent to the
alias that was selected when the TLS connection was initiated.

3 files changed, 126 insertions, 113 deletions

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java
index 0b870e9f6b..674ff5fc57 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java
@@ -43,10 +43,10 @@ import org.eclipse.jetty.util.log.Logger;
  * Provides stream handling utilities in
  * singleton Threadpool implementation accessed by static members.
  */
-public class IO 
+public class IO
 {
     private static final Logger LOG = Log.getLogger(IO.class);
-    
+
     /* ------------------------------------------------------------------- */
     public final static String
         CRLF      = "\015\012";
@@ -80,9 +80,9 @@ public class IO
             this.read=read;
             this.write=write;
         }
-        
+
         /* ------------------------------------------------------------ */
-        /* 
+        /*
          * @see java.lang.Runnable#run()
          */
         public void run()
@@ -109,19 +109,19 @@ public class IO
             }
         }
     }
-    
+
     /* ------------------------------------------------------------------- */
     /** Copy Stream in to Stream out until EOF or exception.
      * @param in the input stream to read from (until EOF)
      * @param out the output stream to write to
-     * @throws IOException if unable to copy streams 
+     * @throws IOException if unable to copy streams
      */
     public static void copy(InputStream in, OutputStream out)
          throws IOException
     {
         copy(in,out,-1);
     }
-    
+
     /* ------------------------------------------------------------------- */
     /** Copy Reader to Writer out until EOF or exception.
      * @param in the read to read from (until EOF)
@@ -133,7 +133,7 @@ public class IO
     {
         copy(in,out,-1);
     }
-    
+
     /* ------------------------------------------------------------------- */
     /** Copy Stream in to Stream for byteCount bytes or until EOF or exception.
      * @param in the stream to read from
@@ -145,20 +145,20 @@ public class IO
                             OutputStream out,
                             long byteCount)
          throws IOException
-    {     
+    {
         byte buffer[] = new byte[bufferSize];
         int len=bufferSize;
-        
+
         if (byteCount>=0)
         {
             while (byteCount>0)
             {
                 int max = byteCount<bufferSize?(int)byteCount:bufferSize;
                 len=in.read(buffer,0,max);
-                
+
                 if (len==-1)
                     break;
-                
+
                 byteCount -= len;
                 out.write(buffer,0,len);
             }
@@ -173,8 +173,8 @@ public class IO
                 out.write(buffer,0,len);
             }
         }
-    }  
-    
+    }
+
     /* ------------------------------------------------------------------- */
     /** Copy Reader to Writer for byteCount bytes or until EOF or exception.
      * @param in the Reader to read from
@@ -186,10 +186,10 @@ public class IO
                             Writer out,
                             long byteCount)
          throws IOException
-    {  
+    {
         char buffer[] = new char[bufferSize];
         int len=bufferSize;
-        
+
         if (byteCount>=0)
         {
             while (byteCount>0)
@@ -197,11 +197,11 @@ public class IO
                 if (byteCount<bufferSize)
                     len=in.read(buffer,0,(int)byteCount);
                 else
-                    len=in.read(buffer,0,bufferSize);                   
-                
+                    len=in.read(buffer,0,bufferSize);
+
                 if (len==-1)
                     break;
-                
+
                 byteCount -= len;
                 out.write(buffer,0,len);
             }
@@ -253,7 +253,7 @@ public class IO
         }
         else
             to.mkdirs();
-        
+
         File[] files = from.listFiles();
         if (files!=null)
         {
@@ -266,7 +266,7 @@ public class IO
             }
         }
     }
-    
+
     /* ------------------------------------------------------------ */
     public static void copyFile(File from,File to) throws IOException
     {
@@ -276,7 +276,7 @@ public class IO
             copy(in,out);
         }
     }
-    
+
     /* ------------------------------------------------------------ */
     /** Read input stream to string.
      * @param in the stream to read from (until EOF)
@@ -288,13 +288,13 @@ public class IO
     {
         return toString(in,(Charset)null);
     }
-    
+
     /* ------------------------------------------------------------ */
     /** Read input stream to string.
      * @param in the stream to read from (until EOF)
      * @param encoding the encoding to use (can be null to use default Charset)
      * @return the String parsed from the stream
-     * @throws IOException if unable to read the stream (or handle the charset) 
+     * @throws IOException if unable to read the stream (or handle the charset)
      */
     public static String toString(InputStream in,String encoding)
         throws IOException
@@ -306,7 +306,7 @@ public class IO
      * @param in the stream to read from (until EOF)
      * @param encoding the Charset to use (can be null to use default Charset)
      * @return the String parsed from the stream
-     * @throws IOException if unable to read the stream (or handle the charset) 
+     * @throws IOException if unable to read the stream (or handle the charset)
      */
     public static String toString(InputStream in, Charset encoding)
             throws IOException
@@ -322,7 +322,7 @@ public class IO
     /** Read input stream to string.
      * @param in the reader to read from (until EOF)
      * @return the String parsed from the reader
-     * @throws IOException if unable to read the stream (or handle the charset) 
+     * @throws IOException if unable to read the stream (or handle the charset)
      */
     public static String toString(Reader in)
         throws IOException
@@ -351,7 +351,7 @@ public class IO
         }
         return file.delete();
     }
-    
+
     /**
      * Closes an arbitrary closable, and logs exceptions at ignore level
      *
@@ -379,7 +379,7 @@ public class IO
     {
         close((Closeable)is);
     }
-    
+
     /**
      * closes an output stream, and logs exceptions
      *
@@ -392,7 +392,7 @@ public class IO
 
     /**
      * closes a reader, and logs exceptions
-     * 
+     *
      * @param reader the reader to close
      */
     public static void close(Reader reader)
@@ -402,14 +402,14 @@ public class IO
 
     /**
      * closes a writer, and logs exceptions
-     * 
+     *
      * @param writer the writer to close
      */
     public static void close(Writer writer)
     {
         close((Closeable)writer);
     }
-    
+
     /* ------------------------------------------------------------ */
     public static byte[] readBytes(InputStream in)
         throws IOException
@@ -426,7 +426,7 @@ public class IO
      * This method wraps a gather write with a loop that handles the limitations of some operating systems that have a
      * limit on the number of buffers written. The method loops on the write until either all the content is written or
      * no progress is made.
-     * 
+     *
      * @param out
      *            The GatheringByteChannel to write to
      * @param buffers
@@ -446,14 +446,14 @@ public class IO
         {
             // Write as much as we can
             long wrote=out.write(buffers,offset,length);
-            
+
             // If we can't write any more, give up
             if (wrote==0)
                 break;
-            
+
             // count the total
             total+=wrote;
-            
+
             // Look for unwritten content
             for (int i=offset;i<buffers.length;i++)
             {
@@ -467,27 +467,12 @@ public class IO
             }
             length=0;
         }
-        
+
         return total;
     }
 
     /* ------------------------------------------------------------ */
     /**
-     * @param name  A host name like www.foo.com
-     * @param domain A domain name like foo.com
-     * @return True if the host name is in the domain name
-     */
-    public static boolean isInDomain(String name, String domain)
-    {
-        if (!name.endsWith(domain))
-            return false;
-        if (name.length()==domain.length())
-            return true;
-        return name.charAt(name.length()-domain.length()-1)=='.';
-    }
-    
-    /* ------------------------------------------------------------ */
-    /**
      * @return An outputstream to nowhere
      */
     public static OutputStream getNullStream()
@@ -496,17 +481,17 @@ public class IO
     }
 
     /* ------------------------------------------------------------ */
-    /** 
+    /**
      * @return An outputstream to nowhere
      */
     public static InputStream getClosedStream()
     {
         return __closedStream;
     }
-    
+
     /* ------------------------------------------------------------ */
     /* ------------------------------------------------------------ */
-    private static class NullOS extends OutputStream                                    
+    private static class NullOS extends OutputStream
     {
         @Override
         public void close(){}
@@ -521,10 +506,10 @@ public class IO
     }
     private static NullOS __nullStream = new NullOS();
 
-    
+
     /* ------------------------------------------------------------ */
     /* ------------------------------------------------------------ */
-    private static class ClosedIS extends InputStream                                    
+    private static class ClosedIS extends InputStream
     {
         @Override
         public int read() throws IOException
@@ -533,28 +518,28 @@ public class IO
         }
     }
     private static ClosedIS __closedStream = new ClosedIS();
-    
+
     /* ------------------------------------------------------------ */
-    /** 
+    /**
      * @return An writer to nowhere
      */
     public static Writer getNullWriter()
     {
         return __nullWriter;
     }
-    
+
     /* ------------------------------------------------------------ */
-    /** 
+    /**
      * @return An writer to nowhere
      */
     public static PrintWriter getNullPrintWriter()
     {
         return __nullPrintWriter;
     }
-    
+
     /* ------------------------------------------------------------ */
     /* ------------------------------------------------------------ */
-    private static class NullWrite extends Writer                                    
+    private static class NullWrite extends Writer
     {
         @Override
         public void close(){}
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 bac7a23939..823fbc75a3 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
@@ -24,6 +24,7 @@ import java.security.PrivateKey;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.List;
 
 import javax.net.ssl.SNIMatcher;
 import javax.net.ssl.SSLEngine;
@@ -41,9 +42,9 @@ import org.eclipse.jetty.util.log.Logger;
  */
 public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
 {
-    public static final String SNI_NAME = "org.eclipse.jetty.util.ssl.sniname";
-    public static final String SNI_WILD = "org.eclipse.jetty.util.ssl.sniwild";
-    public static final String NO_MATCHERS="No Matchers";
+    public static final String SNI_HOSTS = "org.eclipse.jetty.util.ssl.snihosts";
+    public static final String SNI_WILDS = "org.eclipse.jetty.util.ssl.sniwilds";
+    private static final String NO_MATCHERS = "no_matchers";
     private static final Logger LOG = Log.getLogger(SniX509ExtendedKeyManager.class);
 
     private final X509ExtendedKeyManager _delegate;
@@ -72,10 +73,11 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
         if (aliases==null || aliases.length==0)
             return null;
 
-        // Look for an SNI alias
-        String alias=null;
+        // Look for the SNI information.
         String host=null;
-        String wild=null;
+        String alias=null;
+        List<String> hosts=null;
+        List<String> wilds=null;
         if (matchers!=null)
         {
             for (SNIMatcher m : matchers)
@@ -83,16 +85,17 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
                 if (m instanceof SslContextFactory.AliasSNIMatcher)
                 {
                     SslContextFactory.AliasSNIMatcher matcher = (SslContextFactory.AliasSNIMatcher)m;
+                    host=matcher.getHost();
                     alias=matcher.getAlias();
-                    host=matcher.getServerName();
-                    wild=matcher.getWildDomain();
+                    hosts=matcher.getHosts();
+                    wilds=matcher.getWilds();
                     break;
                 }
             }
         }
 
         if (LOG.isDebugEnabled())
-            LOG.debug("matched {}/{} from {}",alias,host,Arrays.asList(aliases));
+            LOG.debug("Matched {} with alias {}/{}/{} from {}",host,alias,hosts,wilds,Arrays.asList(aliases));
 
         // Check if the SNI selected alias is allowable
         if (alias!=null)
@@ -101,9 +104,8 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
             {
                 if (a.equals(alias))
                 {
-                    session.putValue(SNI_NAME,host);
-                    if (wild!=null)
-                        session.putValue(SNI_WILD,wild);
+                    session.putValue(SNI_HOSTS,hosts);
+                    session.putValue(SNI_WILDS,wilds);
                     return alias;
                 }
             }
@@ -121,7 +123,7 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
         if (alias==NO_MATCHERS)
             alias=_delegate.chooseServerAlias(keyType,issuers,socket);
         if (LOG.isDebugEnabled())
-            LOG.debug("chose {}/{} on {}",alias,keyType,socket);
+            LOG.debug("Chose alias {}/{} on {}",alias,keyType,socket);
         return alias;
     }
 
@@ -132,7 +134,7 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager
         if (alias==NO_MATCHERS)
             alias=_delegate.chooseEngineServerAlias(keyType,issuers,engine);
         if (LOG.isDebugEnabled())
-            LOG.debug("chose {}/{} on {}",alias,keyType,engine);
+            LOG.debug("Chose alias {}/{} on {}",alias,keyType,engine);
         return alias;
     }
 
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
index 86c11565a3..faef8465f3 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
@@ -47,6 +47,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import javax.naming.ldap.LdapName;
 import javax.naming.ldap.Rdn;
@@ -149,7 +150,7 @@ public class SslContextFactory extends AbstractLifeCycle
     private final Set<String> _excludeCipherSuites = new LinkedHashSet<>();
 
     /** Included cipher suites. */
-    private final List<String> _includeCipherSuites = new ArrayList<String>();
+    private final List<String> _includeCipherSuites = new ArrayList<>();
     private boolean _useCipherSuitesOrder=true;
 
     /** Cipher comparator for ordering ciphers */
@@ -167,7 +168,7 @@ public class SslContextFactory extends AbstractLifeCycle
 
     /** SSL certificate alias */
     private String _certAlias;
-    private final Map<String,String> _certAliases = new HashMap<>();
+    private final Map<String,String> _certHosts = new HashMap<>();
     private final Map<String,String> _certWilds = new HashMap<>();
 
     /** Truststore path */
@@ -371,7 +372,7 @@ public class SslContextFactory extends AbstractLifeCycle
                 }
 
                 // Look for X.509 certificates to create alias map
-                _certAliases.clear();
+                _certHosts.clear();
                 if (keyStore!=null)
                 {
                     for (String alias : Collections.list(keyStore.aliases()))
@@ -404,7 +405,7 @@ public class SslContextFactory extends AbstractLifeCycle
                                         if (cn!=null)
                                         {
                                             named=true;
-                                            _certAliases.put(cn,alias);
+                                            _certHosts.put(cn,alias);
                                         }
                                     }
                                 }
@@ -422,7 +423,7 @@ public class SslContextFactory extends AbstractLifeCycle
                                         if (LOG.isDebugEnabled())
                                             LOG.debug("Certificate cn alias={} cn={} in {}",alias,cn,this);
                                         if (cn!=null && cn.contains(".") && !cn.contains(" "))
-                                            _certAliases.put(cn,alias);
+                                            _certHosts.put(cn,alias);
                                     }
                                 }
                             }
@@ -432,11 +433,18 @@ public class SslContextFactory extends AbstractLifeCycle
 
                 // find wild aliases
                 _certWilds.clear();
-                for (String name : _certAliases.keySet())
-                    if (name.startsWith("*."))
-                        _certWilds.put(name.substring(2),_certAliases.get(name));
+                for (Iterator<Map.Entry<String, String>> iterator = _certHosts.entrySet().iterator(); iterator.hasNext();)
+                {
+                    Map.Entry<String, String> entry = iterator.next();
+                    String host = entry.getKey();
+                    if (host.startsWith("*."))
+                    {
+                        _certWilds.put(host.substring(2), entry.getValue());
+                        iterator.remove();
+                    }
+                }
 
-                LOG.info("x509={} wild={} alias={} for {}",_certAliases,_certWilds,_certAlias,this);
+                LOG.info("x509={} wild={} alias={} for {}",_certHosts,_certWilds,_certAlias,this);
 
                 // Instantiate key and trust managers
                 KeyManager[] keyManagers = getKeyManagers(keyStore);
@@ -470,7 +478,7 @@ public class SslContextFactory extends AbstractLifeCycle
     {
         _factory = null;
         super.doStop();
-        _certAliases.clear();
+        _certHosts.clear();
         _certWilds.clear();
     }
 
@@ -1140,7 +1148,7 @@ public class SslContextFactory extends AbstractLifeCycle
                     }
                 }
 
-                if (!_certAliases.isEmpty() || !_certWilds.isEmpty())
+                if (!_certHosts.isEmpty() || !_certWilds.isEmpty())
                 {
                     for (int idx = 0; idx < managers.length; idx++)
                     {
@@ -1615,7 +1623,7 @@ public class SslContextFactory extends AbstractLifeCycle
         SSLParameters sslParams = sslEngine.getSSLParameters();
         sslParams.setEndpointIdentificationAlgorithm(_endpointIdentificationAlgorithm);
         sslParams.setUseCipherSuitesOrder(_useCipherSuitesOrder);
-        if (!_certAliases.isEmpty() || !_certWilds.isEmpty())
+        if (!_certHosts.isEmpty() || !_certWilds.isEmpty())
         {
             if (LOG.isDebugEnabled())
                 LOG.debug("Enable SNI matching {}",sslEngine);
@@ -1752,78 +1760,96 @@ public class SslContextFactory extends AbstractLifeCycle
 
     class AliasSNIMatcher extends SNIMatcher
     {
+        private String _host;
         private String _alias;
-        private String _wild;
-        private SNIHostName _name;
+        private List<String> _hosts;
+        private List<String> _wilds;
 
-        protected AliasSNIMatcher()
+        AliasSNIMatcher()
         {
             super(StandardConstants.SNI_HOST_NAME);
+            _hosts = Collections.emptyList();
+            _wilds = Collections.emptyList();
         }
 
         @Override
         public boolean matches(SNIServerName serverName)
         {
-            LOG.debug("matches={} for {}",serverName,this);
+            if (LOG.isDebugEnabled())
+                LOG.debug("SNI matching for {}",serverName);
 
             if (serverName instanceof SNIHostName)
             {
-                _name=(SNIHostName)serverName;
+                String host = _host = ((SNIHostName)serverName).getAsciiName();
 
-                // If we don't have a SNI name, or didn't see any certificate aliases,
-                // just say true as it will either somehow work or fail elsewhere
-                if (_certAliases.size()==0)
+                // If we don't have a SNI host, or didn't see any certificate aliases,
+                // just say true as it will either somehow work or fail elsewhere.
+                if (_certHosts.isEmpty())
                     return true;
 
                 // Try an exact match
-                _alias = _certAliases.get(_name.getAsciiName());
+                _alias = _certHosts.get(host);
                 if (_alias!=null)
                 {
+                    _hosts = _certHosts.entrySet().stream()
+                            .filter(entry -> _alias.equals(entry.getValue()))
+                            .map(Map.Entry::getKey)
+                            .collect(Collectors.toList());
                     if (LOG.isDebugEnabled())
-                        LOG.debug("matched {}->{}",_name.getAsciiName(),_alias);
-                    return true;
+                        LOG.debug("SNI matched {}->{}->{}",host,_alias,_hosts);
                 }
 
-                // Try wild card matches
-                String domain = _name.getAsciiName();
-                _alias = _certWilds.get(domain);
+                // Try wildcard matches.
                 if (_alias==null)
                 {
-                    int dot=domain.indexOf('.');
-                    if (dot>=0)
+                    _alias = _certWilds.get(host);
+                    if (_alias==null)
                     {
-                        domain=domain.substring(dot+1);
-                        _alias = _certWilds.get(domain);
+                        int dot=host.indexOf('.');
+                        if (dot>=0)
+                        {
+                            host=host.substring(dot+1);
+                            _alias = _certWilds.get(host);
+                        }
                     }
                 }
                 if (_alias!=null)
                 {
-                    _wild=domain;
+                    _wilds = _certWilds.entrySet().stream()
+                            .filter(entry -> _alias.equals(entry.getValue()))
+                            .map(Map.Entry::getKey)
+                            .collect(Collectors.toList());
                     if (LOG.isDebugEnabled())
-                        LOG.debug("wild match {}->{}",_name.getAsciiName(),_alias);
+                        LOG.debug("SNI wild match {}->{}->{}",host,_alias,_wilds);
                     return true;
                 }
             }
+
             if (LOG.isDebugEnabled())
-                LOG.debug("No match for {}",_name.getAsciiName());
+                LOG.debug("SNI no match for {}", serverName);
 
             // Return true and allow the KeyManager to accept or reject when choosing a certificate.
             return true;
         }
 
+        public String getHost()
+        {
+            return _host;
+        }
+
         public String getAlias()
         {
             return _alias;
         }
 
-        public String getWildDomain()
+        public List<String> getHosts()
         {
-            return _wild;
+            return _hosts;
         }
 
-        public String getServerName()
+        public List<String> getWilds()
         {
-            return _name==null?null:_name.getAsciiName();
+            return _wilds;
         }
     }
 }