Skip to content

Commit

Permalink
Fixes #6276 - Support non-standard domains in SNI and X509.
Browse files Browse the repository at this point in the history 
Backported support for IP addresses in X509 (from #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.

Signed-off-by: Simone Bordet <[email protected]>
  • Loading branch information
@sbordet
sbordet committed May 18, 2021
1 parent 97a2a99 commit 3a9c34e
Show file tree
Hide file tree
Showing 7 changed files with 279 additions and 47 deletions.
79 changes: 79 additions & 0 deletions jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.SNIHostName;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLException;
Expand Down Expand Up @@ -75,6 +77,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;

Expand Down Expand Up @@ -924,4 +927,80 @@ 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();
// Force to send a non-domain SNI.
clientTLS.setSNIProvider((sslEngine, serverNames) -> Collections.singletonList(new SNIHostName("127.0.0.1")));
// 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);

// 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());

// 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());
}
}
1 change: 1 addition & 0 deletions jetty-client/src/test/resources/jetty-logging.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Binary file added jetty-client/src/test/resources/keystore_sni_non_domain.p12
View file
Binary file not shown.
4 changes: 2 additions & 2 deletions jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,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;
}

Expand All @@ -205,7 +205,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;
}

Expand Down
124 changes: 113 additions & 11 deletions jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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;
Expand Down Expand Up @@ -2179,6 +2180,8 @@ public String getHost()

public static class Client extends SslContextFactory
{
private SniProvider sniProvider = (sslEngine, serverNames) -> serverNames;

public Client()
{
this(false);
Expand All @@ -2203,6 +2206,92 @@ protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyMan
// Client has no SNI functionality.
return keyManager;
}

@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 #IP_ADDRESS_SNI_PROVIDER}.</p>
*/
@FunctionalInterface
public interface SniProvider
{
/**
* <p>An SNI provider that sends, if the given {@code serverNames} list is empty,
* the host address retrieved via
* {@link InetAddress#getHostAddress() InetAddress.getLocalHost().getHostAddress()}.</p>
*/
public static final SniProvider IP_ADDRESS_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())
{
try
{
String address = InetAddress.getLocalHost().getHostAddress();
// 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(address.getBytes(StandardCharsets.US_ASCII)));
}
catch (Throwable x)
{
if (LOG.isDebugEnabled())
LOG.debug("Could not retrieve localhost address", x);
}
}
return serverNames;
}
}

@ManagedObject
Expand Down Expand Up @@ -2304,10 +2393,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
{
Expand All @@ -2323,19 +2418,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;
}

@Override
Expand Down
Loading

0 comments on commit 3a9c34e

Please sign in to comment.