Fixes #6276 - Support non-standard domains in SNI and X509. (#6296)

Improved support for IP addresses in X509 (after #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.
Skip X509 matching over IP addresses when the host does
not look like an IP address, to avoid reverse DNS lookup.

Signed-off-by: Simone Bordet <[email protected]>
(cherry picked from commit 04df6d4)

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java

@@ -57,6 +57,7 @@
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.SslConnectionFactory;
+import org.eclipse.jetty.toolchain.test.Net;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.util.thread.ExecutorThreadPool;
@@ -73,6 +74,7 @@
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -940,4 +942,81 @@ protected SSLEngineResult wrap(SSLEngine sslEngine, ByteBuffer[] input, ByteBuff
         // The HTTP request will resume and be forced to handle the TLS buffer expansion.
         assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
     }
+
+    @Test
+    public void testDefaultNonDomainSNI() throws Exception
+    {
+        SslContextFactory.Server serverTLS = new SslContextFactory.Server();
+        serverTLS.setKeyStorePath("src/test/resources/keystore_sni_non_domain.p12");
+        serverTLS.setKeyStorePassword("storepwd");
+        serverTLS.setSNISelector((keyType, issuers, session, sniHost, certificates) ->
+        {
+            // Java clients don't send SNI by default if it's not a domain.
+            assertNull(sniHost);
+            return serverTLS.sniSelect(keyType, issuers, session, sniHost, certificates);
+        });
+        startServer(serverTLS, new EmptyServerHandler());
+
+        SslContextFactory.Client clientTLS = new SslContextFactory.Client();
+        // Trust any certificate received by the server.
+        clientTLS.setTrustStorePath("src/test/resources/keystore_sni_non_domain.p12");
+        clientTLS.setTrustStorePassword("storepwd");
+        // Disable TLS-level hostName verification, as we may receive a random certificate.
+        clientTLS.setEndpointIdentificationAlgorithm(null);
+        startClient(clientTLS);
+
+        // Host is "localhost" which is not a domain, so the JDK won't send SNI.
+        ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
+            .scheme(HttpScheme.HTTPS.asString())
+            .send();
+
+        assertEquals(HttpStatus.OK_200, response.getStatus());
+    }
+
+    @Test
+    public void testForcedNonDomainSNI() throws Exception
+    {
+        SslContextFactory.Server serverTLS = new SslContextFactory.Server();
+        serverTLS.setKeyStorePath("src/test/resources/keystore_sni_non_domain.p12");
+        serverTLS.setKeyStorePassword("storepwd");
+        serverTLS.setSNISelector((keyType, issuers, session, sniHost, certificates) ->
+        {
+            // We have forced the client to send the non-domain SNI.
+            assertNotNull(sniHost);
+            return serverTLS.sniSelect(keyType, issuers, session, sniHost, certificates);
+        });
+        startServer(serverTLS, new EmptyServerHandler());
+
+        SslContextFactory.Client clientTLS = new SslContextFactory.Client();
+        // Trust any certificate received by the server.
+        clientTLS.setTrustStorePath("src/test/resources/keystore_sni_non_domain.p12");
+        clientTLS.setTrustStorePassword("storepwd");
+        // Force TLS-level hostName verification, as we want to receive the correspondent certificate.
+        clientTLS.setEndpointIdentificationAlgorithm("HTTPS");
+        startClient(clientTLS);
+
+        clientTLS.setSNIProvider(SslContextFactory.Client.SniProvider.NON_DOMAIN_SNI_PROVIDER);
+
+        // Send a request with SNI "localhost", we should get the certificate at alias=localhost.
+        ContentResponse response1 = client.newRequest("localhost", connector.getLocalPort())
+            .scheme(HttpScheme.HTTPS.asString())
+            .send();
+        assertEquals(HttpStatus.OK_200, response1.getStatus());
+
+        // Send a request with SNI "127.0.0.1", we should get the certificate at alias=ip.
+        ContentResponse response2 = client.newRequest("127.0.0.1", connector.getLocalPort())
+            .scheme(HttpScheme.HTTPS.asString())
+            .send();
+        assertEquals(HttpStatus.OK_200, response2.getStatus());
+
+        if (Net.isIpv6InterfaceAvailable())
+        {
+            // Send a request with SNI "[::1]", we should get the certificate at alias=ip.
+            ContentResponse response3 = client.newRequest("[::1]", connector.getLocalPort())
+                .scheme(HttpScheme.HTTPS.asString())
+                .send();
+
+            assertEquals(HttpStatus.OK_200, response3.getStatus());
+        }
+    }
 }

jetty-client/src/test/resources/jetty-logging.properties

@@ -4,3 +4,4 @@
 #org.eclipse.jetty.io.SocketChannelEndPoint.LEVEL=DEBUG
 #org.eclipse.jetty.io.ssl.LEVEL=DEBUG
 #org.eclipse.jetty.http.LEVEL=DEBUG
+#org.eclipse.jetty.util.ssl.LEVEL=DEBUG

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

@@ -196,7 +196,7 @@ public String chooseServerAlias(String keyType, Principal[] issuers, Socket sock
         if (delegate)
             alias = _delegate.chooseServerAlias(keyType, issuers, socket);
         if (LOG.isDebugEnabled())
-            LOG.debug("Chose {} alias {}/{} on {}", delegate ? "delegate" : "explicit", alias, keyType, socket);
+            LOG.debug("Chose {} alias={} keyType={} on {}", delegate ? "delegate" : "explicit", String.valueOf(alias), keyType, socket);
         return alias;
     }
 
@@ -210,7 +210,7 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEn
         if (delegate)
             alias = _delegate.chooseEngineServerAlias(keyType, issuers, engine);
         if (LOG.isDebugEnabled())
-            LOG.debug("Chose {} alias {}/{} on {}", delegate ? "delegate" : "explicit", alias, keyType, engine);
+            LOG.debug("Chose {} alias={} keyType={} on {}", delegate ? "delegate" : "explicit", String.valueOf(alias), keyType, engine);
         return alias;
     }
 

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

@@ -18,6 +18,7 @@
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.nio.charset.StandardCharsets;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.KeyStore;
 import java.security.NoSuchAlgorithmException;
@@ -2082,6 +2083,8 @@ public String getHost()
 
     public static class Client extends SslContextFactory
     {
+        private SniProvider sniProvider = (sslEngine, serverNames) -> serverNames;
+
         public Client()
         {
             this(false);
@@ -2099,6 +2102,89 @@ protected void checkConfiguration()
             checkEndPointIdentificationAlgorithm();
             super.checkConfiguration();
         }
+
+        @Override
+        public void customize(SSLEngine sslEngine)
+        {
+            SSLParameters sslParameters = sslEngine.getSSLParameters();
+            List<SNIServerName> serverNames = sslParameters.getServerNames();
+            if (serverNames == null)
+                serverNames = Collections.emptyList();
+            List<SNIServerName> newServerNames = getSNIProvider().apply(sslEngine, serverNames);
+            if (newServerNames != null && newServerNames != serverNames)
+            {
+                sslParameters.setServerNames(newServerNames);
+                sslEngine.setSSLParameters(sslParameters);
+            }
+            super.customize(sslEngine);
+        }
+
+        /**
+         * @return the SNI provider used to customize the SNI
+         */
+        public SniProvider getSNIProvider()
+        {
+            return sniProvider;
+        }
+
+        /**
+         * @param sniProvider the SNI provider used to customize the SNI
+         */
+        public void setSNIProvider(SniProvider sniProvider)
+        {
+            this.sniProvider = Objects.requireNonNull(sniProvider);
+        }
+
+        /**
+         * <p>A provider for SNI names to send to the server during the TLS handshake.</p>
+         * <p>By default, the OpenJDK TLS implementation does not send SNI names when
+         * they are IP addresses, following what currently specified in
+         * <a href="https://datatracker.ietf.org/doc/html/rfc6066#section-3">TLS 1.3</a>,
+         * or when they are non-domain strings such as {@code "localhost"}.</p>
+         * <p>If you need to send custom SNI, such as a non-domain SNI or an IP address SNI,
+         * you can set your own SNI provider or use {@link #NON_DOMAIN_SNI_PROVIDER}.</p>
+         */
+        @FunctionalInterface
+        public interface SniProvider
+        {
+            /**
+             * <p>An SNI provider that, if the given {@code serverNames} list is empty,
+             * retrieves the host via {@link SSLEngine#getPeerHost()}, converts it to
+             * ASCII bytes, and sends it as SNI.</p>
+             * <p>This allows to send non-domain SNI such as {@code "localhost"} or
+             * IP addresses.</p>
+             */
+            public static final SniProvider NON_DOMAIN_SNI_PROVIDER = Client::getSniServerNames;
+
+            /**
+             * <p>Provides the SNI names to send to the server.</p>
+             * <p>Currently, RFC 6066 allows for different types of server names,
+             * but defines only one of type "host_name".</p>
+             * <p>As such, the input {@code serverNames} list and the list to be returned
+             * contain at most one element.</p>
+             *
+             * @param sslEngine the SSLEngine that processes the TLS handshake
+             * @param serverNames the non-null immutable list of server names computed by implementation
+             * @return either the same {@code serverNames} list passed as parameter, or a new list
+             * containing the server names to send to the server
+             */
+            public List<SNIServerName> apply(SSLEngine sslEngine, List<SNIServerName> serverNames);
+        }
+
+        private static List<SNIServerName> getSniServerNames(SSLEngine sslEngine, List<SNIServerName> serverNames)
+        {
+            if (serverNames.isEmpty())
+            {
+                String host = sslEngine.getPeerHost();
+                if (host != null)
+                {
+                    // Must use the byte[] constructor, because the character ':' is forbidden when
+                    // using the String constructor (but typically present in IPv6 addresses).
+                    return List.of(new SNIHostName(host.getBytes(StandardCharsets.US_ASCII)));
+                }
+            }
+            return serverNames;
+        }
     }
 
     @ManagedObject
@@ -2235,10 +2321,16 @@ public void setSNISelector(SniX509ExtendedKeyManager.SniSelector sniSelector)
         @Override
         public String sniSelect(String keyType, Principal[] issuers, SSLSession session, String sniHost, Collection<X509> certificates)
         {
+            boolean sniRequired = isSniRequired();
+
+            if (LOG.isDebugEnabled())
+                LOG.debug("Selecting alias: keyType={}, sni={}, sniRequired={}, certs={}", keyType, String.valueOf(sniHost), sniRequired, certificates);
+
+            String alias;
             if (sniHost == null)
             {
                 // No SNI, so reject or delegate.
-                return isSniRequired() ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE;
+                alias = sniRequired ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE;
             }
             else
             {
@@ -2254,19 +2346,26 @@ public String sniSelect(String keyType, Principal[] issuers, SSLSession session,
                     // SNI, as we will likely be called again with a different keyType.
                     boolean anyMatching = aliasCerts().values().stream()
                         .anyMatch(x509 -> x509.matches(sniHost));
-                    return isSniRequired() || anyMatching ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE;
+                    alias = sniRequired || anyMatching ? null : SniX509ExtendedKeyManager.SniSelector.DELEGATE;
                 }
+                else
+                {
+                    alias = matching.get(0).getAlias();
+                    if (matching.size() > 1)
+                    {
+                        // Prefer strict matches over wildcard matches.
+                        alias = matching.stream()
+                            .min(Comparator.comparingInt(cert -> cert.getWilds().size()))
+                            .map(X509::getAlias)
+                            .orElse(alias);
+                    }
+                }
+            }
 
-                String alias = matching.get(0).getAlias();
-                if (matching.size() == 1)
-                    return alias;
+            if (LOG.isDebugEnabled())
+                LOG.debug("Selected alias={}", String.valueOf(alias));
 
-                // Prefer strict matches over wildcard matches.
-                return matching.stream()
-                    .min(Comparator.comparingInt(cert -> cert.getWilds().size()))
-                    .map(X509::getAlias)
-                    .orElse(alias);
-            }
+            return alias;
         }
 
         protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager)

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

@@ -182,21 +182,45 @@ public boolean matches(String host)
                 return true;
         }
 
-        InetAddress address = toInetAddress(host);
-        if (address != null)
-            return _addresses.contains(address);
+        // Check if the host looks like an IP address to avoid
+        // DNS lookup for host names that did not match.
+        if (seemsIPAddress(host))
+        {
+            InetAddress address = toInetAddress(host);
+            if (address != null)
+                return _addresses.contains(address);
+        }
 
         return false;
     }
 
+    private static boolean seemsIPAddress(String host)
+    {
+        // IPv4 is just numbers and dots.
+        String ipv4RegExp = "[0-9\\.]+";
+        // IPv6 is hex and colons and possibly brackets.
+        String ipv6RegExp = "[0-9a-fA-F:\\[\\]]+";
+        return host.matches(ipv4RegExp) ||
+            (host.matches(ipv6RegExp) && containsAtLeastTwoColons(host));
+    }
+
+    private static boolean containsAtLeastTwoColons(String host)
+    {
+        int index = host.indexOf(':');
+        if (index >= 0)
+            index = host.indexOf(':', index + 1);
+        return index > 0;
+    }
+
     @Override
     public String toString()
     {
-        return String.format("%s@%x(%s,h=%s,w=%s)",
+        return String.format("%s@%x(%s,h=%s,a=%s,w=%s)",
             getClass().getSimpleName(),
             hashCode(),
             _alias,
             _hosts,
+            _addresses,
             _wilds);
     }
 }