Fixes #6034 - SslContextFactory may select a wildcard certificate during SNI selection when a more specific SSL certificate is present.

Now matching certificates are sorted, non-wildcard first, so that a more specific alias is returned.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java

@@ -28,6 +28,7 @@ import java.util.Queue;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import javax.net.ssl.SNIHostName;
 import javax.net.ssl.SNIServerName;
 import javax.net.ssl.SSLEngine;
@@ -58,6 +59,7 @@ import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.ssl.SniX509ExtendedKeyManager;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
+import org.eclipse.jetty.util.ssl.X509;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
@@ -163,7 +165,27 @@ public class SniSslConnectionFactoryTest
     @Test
     public void testSNIConnect() throws Exception
     {
-        start("src/test/resources/keystore_sni.p12");
+        start(ssl ->
+        {
+            ssl.setKeyStorePath("src/test/resources/keystore_sni.p12");
+            ssl.setSNISelector((keyType, issuers, session, sniHost, certificates) ->
+            {
+                // Make sure the *.domain.com comes before sub.domain.com
+                // to test that we prefer more specific domains.
+                List<X509> sortedCertificates = certificates.stream()
+                    // As sorted() sorts ascending, make *.domain.com the smallest.
+                    .sorted((x509a, x509b) ->
+                    {
+                        if (x509a.matches("domain.com"))
+                            return -1;
+                        if (x509b.matches("domain.com"))
+                            return 1;
+                        return 0;
+                    })
+                    .collect(Collectors.toList());
+                return ssl.sniSelect(keyType, issuers, session, sniHost, sortedCertificates);
+            });
+        });
 
         String response = getResponse("jetty.eclipse.org", "jetty.eclipse.org");
         assertThat(response, Matchers.containsString("X-HOST: jetty.eclipse.org"));
@@ -174,6 +196,9 @@ public class SniSslConnectionFactoryTest
         response = getResponse("foo.domain.com", "*.domain.com");
         assertThat(response, Matchers.containsString("X-HOST: foo.domain.com"));
 
+        response = getResponse("sub.domain.com", "sub.domain.com");
+        assertThat(response, Matchers.containsString("X-HOST: sub.domain.com"));
+
         response = getResponse("m.san.com", "san example");
         assertThat(response, Matchers.containsString("X-HOST: m.san.com"));
 

jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java

@@ -49,6 +49,7 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 import javax.net.ssl.CertPathTrustManagerParameters;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.KeyManager;
@@ -58,7 +59,6 @@ import javax.net.ssl.SNIMatcher;
 import javax.net.ssl.SNIServerName;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
-import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLServerSocket;
@@ -2230,7 +2230,7 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum
         }
 
         @Override
-        public String sniSelect(String keyType, Principal[] issuers, SSLSession session, String sniHost, Collection<X509> certificates) throws SSLHandshakeException
+        public String sniSelect(String keyType, Principal[] issuers, SSLSession session, String sniHost, Collection<X509> certificates)
         {
             if (sniHost == null)
             {
@@ -2239,12 +2239,24 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum
             }
             else
             {
-                // Match the SNI host, or let the JDK decide unless unmatched SNIs are rejected.
+                // Match the SNI host.
-                return certificates.stream()
+                List<X509> matching = certificates.stream()
                     .filter(x509 -> x509.matches(sniHost))
-                    .findFirst()
+                    .collect(Collectors.toList());
+
+                // No match, let the JDK decide unless unmatched SNIs are rejected.
+                if (matching.isEmpty())
+                    return isSniRequired() ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE;
+
+                String alias = matching.get(0).getAlias();
+                if (matching.size() == 1)
+                    return alias;
+
+                // Prefer strict matches over wildcard matches.
+                return matching.stream()
+                    .min(Comparator.comparingInt(cert -> cert.getWilds().size()))
                     .map(X509::getAlias)
-                    .orElse(_sniRequired ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE);
+                    .orElse(alias);
             }
         }