diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index 363bb90cc86..a2a3ffaaf8a 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -59,6 +59,7 @@ import org.eclipse.jetty.server.SecureRequestCustomizer; 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; @@ -75,6 +76,7 @@ import static org.hamcrest.Matchers.instanceOf; 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; @@ -924,4 +926,81 @@ public class HttpClientTLSTest // 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()); + } + } } diff --git a/jetty-client/src/test/resources/jetty-logging.properties b/jetty-client/src/test/resources/jetty-logging.properties index f74a4da98d1..fba2f665548 100644 --- a/jetty-client/src/test/resources/jetty-logging.properties +++ b/jetty-client/src/test/resources/jetty-logging.properties @@ -4,3 +4,4 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog #org.eclipse.jetty.io.ChannelEndPoint.LEVEL=DEBUG #org.eclipse.jetty.io.ssl.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG +#org.eclipse.jetty.util.ssl.LEVEL=DEBUG diff --git a/jetty-client/src/test/resources/keystore_sni_non_domain.p12 b/jetty-client/src/test/resources/keystore_sni_non_domain.p12 new file mode 100644 index 00000000000..0ded97ee3f3 Binary files /dev/null and b/jetty-client/src/test/resources/keystore_sni_non_domain.p12 differ 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 c178ad1b6ba..dcd742aec1c 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 @@ -191,7 +191,7 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager 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; } @@ -205,7 +205,7 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager 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; } 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 0da182e7cbf..d623ea5627d 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 @@ -23,6 +23,7 @@ import java.io.IOException; 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; @@ -2179,6 +2180,8 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable public static class Client extends SslContextFactory { + private SniProvider sniProvider = (sslEngine, serverNames) -> serverNames; + public Client() { this(false); @@ -2203,6 +2206,89 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable // Client has no SNI functionality. return keyManager; } + + @Override + public void customize(SSLEngine sslEngine) + { + SSLParameters sslParameters = sslEngine.getSSLParameters(); + List serverNames = sslParameters.getServerNames(); + if (serverNames == null) + serverNames = Collections.emptyList(); + List 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); + } + + /** + *

A provider for SNI names to send to the server during the TLS handshake.

+ *

By default, the OpenJDK TLS implementation does not send SNI names when + * they are IP addresses, following what currently specified in + * TLS 1.3, + * or when they are non-domain strings such as {@code "localhost"}.

+ *

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}.

+ */ + @FunctionalInterface + public interface SniProvider + { + /** + *

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.

+ *

This allows to send non-domain SNI such as {@code "localhost"} or + * IP addresses.

+ */ + public static final SniProvider NON_DOMAIN_SNI_PROVIDER = Client::getSniServerNames; + + /** + *

Provides the SNI names to send to the server.

+ *

Currently, RFC 6066 allows for different types of server names, + * but defines only one of type "host_name".

+ *

As such, the input {@code serverNames} list and the list to be returned + * contain at most one element.

+ * + * @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 apply(SSLEngine sslEngine, List serverNames); + } + + private static List getSniServerNames(SSLEngine sslEngine, List 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 Collections.singletonList(new SNIHostName(host.getBytes(StandardCharsets.US_ASCII))); + } + } + return serverNames; + } } @ManagedObject @@ -2304,10 +2390,16 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable @Override public String sniSelect(String keyType, Principal[] issuers, SSLSession session, String sniHost, Collection 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 { @@ -2323,19 +2415,26 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable // 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; - - // Prefer strict matches over wildcard matches. - return matching.stream() - .min(Comparator.comparingInt(cert -> cert.getWilds().size())) - .map(X509::getAlias) - .orElse(alias); } + + if (LOG.isDebugEnabled()) + LOG.debug("Selected alias={}", String.valueOf(alias)); + + return alias; } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java index 1c2139f3707..9484cf99493 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/X509.java @@ -18,14 +18,13 @@ package org.eclipse.jetty.util.ssl; -import java.security.cert.CertificateParsingException; +import java.net.InetAddress; import java.security.cert.X509Certificate; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import javax.naming.InvalidNameException; import javax.naming.ldap.LdapName; import javax.naming.ldap.Rdn; import javax.security.auth.x500.X500Principal; @@ -37,17 +36,15 @@ import org.eclipse.jetty.util.log.Logger; public class X509 { private static final Logger LOG = Log.getLogger(X509.class); - /* * @see {@link X509Certificate#getKeyUsage()} */ private static final int KEY_USAGE__KEY_CERT_SIGN = 5; - /* - * * @see {@link X509Certificate#getSubjectAlternativeNames()} */ private static final int SUBJECT_ALTERNATIVE_NAMES__DNS_NAME = 2; + private static final int SUBJECT_ALTERNATIVE_NAMES__IP_ADDRESS = 7; public static boolean isCertSign(X509Certificate x509) { @@ -63,51 +60,97 @@ public class X509 private final String _alias; private final Set _hosts = new LinkedHashSet<>(); private final Set _wilds = new LinkedHashSet<>(); + private final Set _addresses = new LinkedHashSet<>(); - public X509(String alias, X509Certificate x509) throws CertificateParsingException, InvalidNameException + public X509(String alias, X509Certificate x509) { _alias = alias; _x509 = x509; - // Look for alternative name extensions - Collection> altNames = x509.getSubjectAlternativeNames(); - if (altNames != null) + try { - for (List list : altNames) + // Look for alternative name extensions + Collection> altNames = x509.getSubjectAlternativeNames(); + if (altNames != null) { - if (((Number)list.get(0)).intValue() == SUBJECT_ALTERNATIVE_NAMES__DNS_NAME) + for (List list : altNames) { - String cn = list.get(1).toString(); + int nameType = ((Number)list.get(0)).intValue(); + switch (nameType) + { + case SUBJECT_ALTERNATIVE_NAMES__DNS_NAME: + { + String name = list.get(1).toString(); + if (LOG.isDebugEnabled()) + LOG.debug("Certificate alias={} SAN dns={} in {}", alias, name, this); + addName(name); + break; + } + case SUBJECT_ALTERNATIVE_NAMES__IP_ADDRESS: + { + String address = list.get(1).toString(); + if (LOG.isDebugEnabled()) + LOG.debug("Certificate alias={} SAN ip={} in {}", alias, address, this); + addAddress(address); + break; + } + default: + break; + } + } + } + + // If no names found, look up the CN from the subject + LdapName name = new LdapName(x509.getSubjectX500Principal().getName(X500Principal.RFC2253)); + for (Rdn rdn : name.getRdns()) + { + if (rdn.getType().equalsIgnoreCase("CN")) + { + String cn = rdn.getValue().toString(); if (LOG.isDebugEnabled()) - LOG.debug("Certificate SAN alias={} CN={} in {}", alias, cn, this); - if (cn != null) - addName(cn); + LOG.debug("Certificate CN alias={} CN={} in {}", alias, cn, this); + addName(cn); } } } - - // If no names found, look up the CN from the subject - LdapName name = new LdapName(x509.getSubjectX500Principal().getName(X500Principal.RFC2253)); - for (Rdn rdn : name.getRdns()) + catch (Exception x) { - if (rdn.getType().equalsIgnoreCase("CN")) - { - String cn = rdn.getValue().toString(); - if (LOG.isDebugEnabled()) - LOG.debug("Certificate CN alias={} CN={} in {}", alias, cn, this); - if (cn != null && cn.contains(".") && !cn.contains(" ")) - addName(cn); - } + throw new IllegalArgumentException(x); } } protected void addName(String cn) { - cn = StringUtil.asciiToLowerCase(cn); - if (cn.startsWith("*.")) - _wilds.add(cn.substring(2)); - else - _hosts.add(cn); + if (cn != null) + { + cn = StringUtil.asciiToLowerCase(cn); + if (cn.startsWith("*.")) + _wilds.add(cn.substring(2)); + else + _hosts.add(cn); + } + } + + private void addAddress(String host) + { + // Class InetAddress handles IPV6 brackets and IPv6 short forms, so that [::1] + // would match 0:0:0:0:0:0:0:1 as well as 0000:0000:0000:0000:0000:0000:0000:0001. + InetAddress address = toInetAddress(host); + if (address != null) + _addresses.add(address); + } + + private InetAddress toInetAddress(String address) + { + try + { + return InetAddress.getByName(address); + } + catch (Throwable x) + { + LOG.ignore(x); + return null; + } } public String getAlias() @@ -140,19 +183,49 @@ public class X509 if (dot >= 0) { String domain = host.substring(dot + 1); - return _wilds.contains(domain); + if (_wilds.contains(domain)) + return true; } + + // 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); } } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/SslContextFactoryTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/SslContextFactoryTest.java index 751b124e606..011d12e1609 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/SslContextFactoryTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/SslContextFactoryTest.java @@ -361,7 +361,7 @@ public class SslContextFactoryTest assertTrue(cf.getX509("other").matches("www.example.com")); assertFalse(cf.getX509("other").matches("eclipse.org")); - assertThat(cf.getX509("san").getHosts(), containsInAnyOrder("www.san.com", "m.san.com")); + assertThat(cf.getX509("san").getHosts(), containsInAnyOrder("san example", "www.san.com", "m.san.com")); assertTrue(cf.getX509("san").getWilds().isEmpty()); assertTrue(cf.getX509("san").matches("www.san.com")); assertTrue(cf.getX509("san").matches("m.san.com"));