Skip to content

Commit

Permalink
ISPN-15069 CVE-2023-4586 Hot Rod TLS Hostname validation
Browse files Browse the repository at this point in the history 
* Add TLS hostname validation
* Per-cluster SNI
* Implict SNI if not set
* Also add a method to return current cluster
* JMX equivalent of above
  • Loading branch information
@tristantarrant @ryanemerson
tristantarrant authored and ryanemerson committed Sep 12, 2023
1 parent bbb1a79 commit c5d945b
Show file tree
Hide file tree
Showing 53 changed files with 310 additions and 77 deletions.
11 changes: 9 additions & 2 deletions client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheContainer.java
View file
@@ -1,12 +1,12 @@
package org.infinispan.client.hotrod;

import jakarta.transaction.TransactionManager;

import org.infinispan.client.hotrod.configuration.Configuration;
import org.infinispan.client.hotrod.configuration.TransactionMode;
import org.infinispan.commons.api.BasicCacheContainer;
import org.infinispan.commons.marshall.Marshaller;

import jakarta.transaction.TransactionManager;

public interface RemoteCacheContainer extends BasicCacheContainer {

/**
Expand Down Expand Up @@ -152,6 +152,13 @@ <K, V> RemoteCache<K, V> getCache(String cacheName, boolean forceReturnValue, Tr
*/
boolean switchToDefaultCluster();

/**
* Returns the name of the currently active cluster.
*
* @return the name of the active cluster
*/
String getCurrentClusterName();

Marshaller getMarshaller();

/**
Expand Down
5 changes: 5 additions & 0 deletions client/hotrod-client/src/main/java/org/infinispan/client/hotrod/RemoteCacheManager.java
View file
Expand Up @@ -469,6 +469,11 @@ public boolean switchToDefaultCluster() {
return channelFactory.manualSwitchToCluster(ChannelFactory.DEFAULT_CLUSTER_NAME);
}

@Override
public String getCurrentClusterName() {
return channelFactory.getCurrentClusterName();
}

private Properties loadFromStream(InputStream stream) {
Properties properties = new Properties();
try {
Expand Down
9 changes: 8 additions & 1 deletion ...client/src/main/java/org/infinispan/client/hotrod/configuration/ClusterConfiguration.java
View file
Expand Up @@ -11,11 +11,13 @@ public class ClusterConfiguration {
private final List<ServerConfiguration> serverCluster;
private final String clusterName;
private final ClientIntelligence intelligence;
private final String sniHostName;

public ClusterConfiguration(List<ServerConfiguration> serverCluster, String clusterName, ClientIntelligence intelligence) {
public ClusterConfiguration(List<ServerConfiguration> serverCluster, String clusterName, ClientIntelligence intelligence, String sniHostName) {
this.serverCluster = serverCluster;
this.clusterName = clusterName;
this.intelligence = intelligence;
this.sniHostName = sniHostName;
}

public List<ServerConfiguration> getCluster() {
Expand All @@ -30,12 +32,17 @@ public ClientIntelligence getClientIntelligence() {
return intelligence;
}

public String sniHostName() {
return sniHostName;
}

@Override
public String toString() {
return "ClusterConfiguration{" +
"serverCluster=" + Util.toStr(serverCluster) +
", clusterName='" + clusterName + '\'' +
", intelligence=" + intelligence +
", sniHostName=" + sniHostName +
'}';
}
}
8 changes: 7 additions & 1 deletion ...src/main/java/org/infinispan/client/hotrod/configuration/ClusterConfigurationBuilder.java
View file
Expand Up @@ -18,6 +18,7 @@ public class ClusterConfigurationBuilder extends AbstractConfigurationChildBuild
private final List<ServerConfigurationBuilder> servers = new ArrayList<>();
private final String clusterName;
private ClientIntelligence intelligence;
private String sniHostName;

protected ClusterConfigurationBuilder(ConfigurationBuilder builder, String clusterName) {
super(builder);
Expand Down Expand Up @@ -53,6 +54,11 @@ public ClusterConfigurationBuilder clusterClientIntelligence(ClientIntelligence
return this;
}

public ClusterConfigurationBuilder clusterSniHostName(String clusterSniHostName) {
this.sniHostName = clusterSniHostName;
return this;
}

@Override
public void validate() {
if (clusterName == null || clusterName.isEmpty()) {
Expand All @@ -70,7 +76,7 @@ public void validate() {
public ClusterConfiguration create() {
List<ServerConfiguration> serverCluster = servers.stream()
.map(ServerConfigurationBuilder::create).collect(Collectors.toList());
return new ClusterConfiguration(serverCluster, clusterName, intelligence);
return new ClusterConfiguration(serverCluster, clusterName, intelligence, sniHostName);
}

@Override
Expand Down
10 changes: 9 additions & 1 deletion ...rod-client/src/main/java/org/infinispan/client/hotrod/configuration/SslConfiguration.java
View file
Expand Up @@ -26,10 +26,12 @@ public class SslConfiguration {
private final String protocol;
private final Collection<String> ciphers;
private final String provider;
private final boolean hostnameValidation;

SslConfiguration(boolean enabled, String keyStoreFileName, String keyStoreType, char[] keyStorePassword, char[] keyStoreCertificatePassword, String keyAlias,
SSLContext sslContext,
String trustStoreFileName, String trustStorePath, String trustStoreType, char[] trustStorePassword, String sniHostName, String provider, String protocol, Collection<String> ciphers) {
String trustStoreFileName, String trustStorePath, String trustStoreType, char[] trustStorePassword, String sniHostName, String provider, String protocol, Collection<String> ciphers,
boolean hostnameValidation) {
this.enabled = enabled;
this.keyStoreFileName = keyStoreFileName;
this.keyStoreType = keyStoreType;
Expand All @@ -45,6 +47,7 @@ public class SslConfiguration {
this.provider = provider;
this.protocol = protocol;
this.ciphers = ciphers;
this.hostnameValidation = hostnameValidation;
}

public boolean enabled() {
Expand Down Expand Up @@ -107,6 +110,10 @@ public String provider() {
return provider;
}

public boolean hostnameValidation() {
return hostnameValidation;
}

@Override
public String toString() {
return "SslConfiguration{" +
Expand All @@ -121,6 +128,7 @@ public String toString() {
", provider='" + provider +'\'' +
", protocol='" + protocol + '\'' +
", ciphers='" + ciphers + '\'' +
", hostnameValidation=" + hostnameValidation +
'}';
}
}
21 changes: 20 additions & 1 deletion ...ent/src/main/java/org/infinispan/client/hotrod/configuration/SslConfigurationBuilder.java
View file
Expand Up @@ -39,6 +39,7 @@ public class SslConfigurationBuilder extends AbstractSecurityConfigurationChildB
private String protocol;
private Collection<String> ciphers;
private String provider;
private boolean hostnameValidation = true;

protected SslConfigurationBuilder(SecurityConfigurationBuilder builder) {
super(builder);
Expand Down Expand Up @@ -223,6 +224,17 @@ public SslConfigurationBuilder ciphers(String... ciphers) {
return enable();
}

/**
* Configures whether to enable hostname validation according to <a href="https://datatracker.ietf.org/doc/html/rfc2818">RFC 2818</a>.
* This is enabled by default and requires that the server certificate includes a subjectAltName extension of type dNSName or iPAddress.
*
* @param hostnameValidation whether to enable hostname validation
*/
public SslConfigurationBuilder hostnameValidation(boolean hostnameValidation) {
this.hostnameValidation = hostnameValidation;
return enable();
}

@Override
public void validate() {
if (enabled) {
Expand All @@ -244,6 +256,9 @@ public void validate() {
throw HOTROD.xorSSLContext();
}
}
if (hostnameValidation && sniHostName == null) {
throw HOTROD.missingSniHostName();
}
}
}

Expand All @@ -253,7 +268,7 @@ public SslConfiguration create() {
keyStoreFileName, keyStoreType, keyStorePassword, keyStoreCertificatePassword, keyAlias,
sslContext,
trustStoreFileName, trustStorePath, trustStoreType, trustStorePassword,
sniHostName, provider, protocol, ciphers);
sniHostName, provider, protocol, ciphers, hostnameValidation);
}

@Override
Expand All @@ -273,6 +288,7 @@ public SslConfigurationBuilder read(SslConfiguration template, Combine combine)
this.provider = template.provider();
this.protocol = template.protocol();
this.ciphers = template.ciphers() != null ? new ArrayList<>(template.ciphers()) : null;
this.hostnameValidation = template.hostnameValidation();
return this;
}

Expand Down Expand Up @@ -326,6 +342,9 @@ public ConfigurationBuilder withProperties(Properties properties) {
if (typed.containsKey(ConfigurationProperties.USE_SSL))
this.enabled(typed.getBooleanProperty(ConfigurationProperties.USE_SSL, enabled, true));

if (typed.containsKey(ConfigurationProperties.SSL_HOSTNAME_VALIDATION))
this.hostnameValidation(typed.getBooleanProperty(ConfigurationProperties.SSL_HOSTNAME_VALIDATION, hostnameValidation, true));

return builder.getBuilder();
}
}
6 changes: 6 additions & 0 deletions .../hotrod-client/src/main/java/org/infinispan/client/hotrod/configuration/package-info.java
View file
Expand Up @@ -305,6 +305,12 @@
* For details about cipher lists and possible values, refer to OpenSSL documentation at <a href="https://www.openssl.org/docs/man1.1.1/man1/ciphers">https://www.openssl.org/docs/man1.1.1/man1/ciphers</a></td>
* </tr>
* <tr>
* <td><b>infinispan.client.hotrod.ssl_hostname_validation</b></td>
* <td>boolean</td>
* <td>true</td>
* <td>Whether to enable TLS hostname validation using the rules defined in RFC 2818.</td>
* </tr>
* <tr>
* <td><b>infinispan.client.hotrod.ssl_protocol</b></td>
* <td>String</td>
* <td>N/A</td>
Expand Down
1 change: 1 addition & 0 deletions ...otrod-client/src/main/java/org/infinispan/client/hotrod/impl/ConfigurationProperties.java
View file
Expand Up @@ -69,6 +69,7 @@ public class ConfigurationProperties {
public static final String SSL_PROTOCOL = ICH + "ssl_protocol";
public static final String SSL_CIPHERS = ICH + "ssl_ciphers";
public static final String SSL_CONTEXT = ICH + "ssl_context";
public static final String SSL_HOSTNAME_VALIDATION = ICH + "ssl_hostname_validation";
public static final String MAX_RETRIES = ICH + "max_retries";
// Authentication properties
public static final String USE_AUTH = ICH + "use_auth";
Expand Down
15 changes: 11 additions & 4 deletions ...t/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/topology/ClusterInfo.java
View file
Expand Up @@ -21,20 +21,22 @@ public class ClusterInfo {
// updates won't be allowed to apply since they refer to older views.
private final int topologyAge;
private final ClientIntelligence intelligence;
private final String sniHostName;

public ClusterInfo(String clusterName, List<InetSocketAddress> servers, ClientIntelligence intelligence) {
this(clusterName, servers, -1, intelligence);
public ClusterInfo(String clusterName, List<InetSocketAddress> servers, ClientIntelligence intelligence, String sniHostName) {
this(clusterName, servers, -1, intelligence, sniHostName);
}

private ClusterInfo(String clusterName, List<InetSocketAddress> servers, int topologyAge, ClientIntelligence intelligence) {
private ClusterInfo(String clusterName, List<InetSocketAddress> servers, int topologyAge, ClientIntelligence intelligence, String sniHostName) {
this.clusterName = clusterName;
this.servers = Immutables.immutableListCopy(servers);
this.topologyAge = topologyAge;
this.intelligence = Objects.requireNonNull(intelligence);
this.sniHostName = sniHostName;
}

public ClusterInfo withTopologyAge(int topologyAge) {
return new ClusterInfo(clusterName, servers, topologyAge, intelligence);
return new ClusterInfo(clusterName, servers, topologyAge, intelligence, sniHostName);
}

public String getName() {
Expand All @@ -53,13 +55,18 @@ public ClientIntelligence getIntelligence() {
return intelligence;
}

public String getSniHostName() {
return sniHostName;
}

@Override
public String toString() {
return "ClusterInfo{" +
"name='" + clusterName + '\'' +
", servers=" + servers +
", age=" + topologyAge +
", intelligence=" + intelligence +
", sniHostname=" + sniHostName +
'}';
}
}
21 changes: 12 additions & 9 deletions ...lient/src/main/java/org/infinispan/client/hotrod/impl/transport/netty/ChannelFactory.java
View file
Expand Up @@ -102,6 +102,7 @@ public class ChannelFactory {
@GuardedBy("lock")
private final Set<SocketAddress> failedServers = new HashSet<>();
private final CodecHolder codecHolder;
private AddressResolverGroup<?> dnsResolver;

public ChannelFactory(CodecHolder codecHolder) {
this.codecHolder = codecHolder;
Expand All @@ -123,12 +124,17 @@ public void start(Configuration configuration, Marshaller marshaller, ExecutorSe
// Note that each event loop opens a selector which counts
int maxExecutors = Math.min(asyncThreads, eventLoopThreads);
this.eventLoopGroup = configuration.transportFactory().createEventLoopGroup(maxExecutors, executorService);
DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
.channelType(configuration.transportFactory().datagramChannelClass())
.ttl(configuration.dnsResolverMinTTL(), configuration.dnsResolverMaxTTL())
.negativeTtl(configuration.dnsResolverNegativeTTL());
this.dnsResolver = new RoundRobinDnsAddressResolverGroup(builder);

List<InetSocketAddress> initialServers = new ArrayList<>();
for (ServerConfiguration server : configuration.servers()) {
initialServers.add(InetSocketAddress.createUnresolved(server.host(), server.port()));
}
ClusterInfo mainCluster = new ClusterInfo(DEFAULT_CLUSTER_NAME, initialServers, configuration.clientIntelligence());
ClusterInfo mainCluster = new ClusterInfo(DEFAULT_CLUSTER_NAME, initialServers, configuration.clientIntelligence(), configuration.security().ssl().sniHostName());
List<ClusterInfo> clustersDefinitions = new ArrayList<>();
if (log.isDebugEnabled()) {
log.debugf("Statically configured servers: %s", initialServers);
Expand All @@ -145,8 +151,10 @@ public void start(Configuration configuration, Marshaller marshaller, ExecutorSe
ClientIntelligence intelligence = clusterConfiguration.getClientIntelligence() != null ?
clusterConfiguration.getClientIntelligence() :
configuration.clientIntelligence();

String sniHostName = clusterConfiguration.sniHostName() != null ? clusterConfiguration.sniHostName() : configuration.security().ssl().sniHostName();
ClusterInfo alternateCluster =
new ClusterInfo(clusterConfiguration.getClusterName(), alternateServers, intelligence);
new ClusterInfo(clusterConfiguration.getClusterName(), alternateServers, intelligence, sniHostName);
log.debugf("Add secondary cluster: %s", alternateCluster);
clustersDefinitions.add(alternateCluster);
}
Expand Down Expand Up @@ -187,12 +195,7 @@ public MarshallerRegistry getMarshallerRegistry() {
}

private ChannelPool newPool(SocketAddress address) {
log.debugf("Creating new channel pool for %s", address);
DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
.channelType(configuration.transportFactory().datagramChannelClass())
.ttl(configuration.dnsResolverMinTTL(), configuration.dnsResolverMaxTTL())
.negativeTtl(configuration.dnsResolverNegativeTTL());
AddressResolverGroup<?> dnsResolver = new RoundRobinDnsAddressResolverGroup(builder);
log.debugf("Creating new channel pool for %s", address);
Bootstrap bootstrap = new Bootstrap()
.group(eventLoopGroup)
.channel(configuration.transportFactory().socketChannelClass())
Expand All @@ -210,7 +213,7 @@ private ChannelPool newPool(SocketAddress address) {
}

public ChannelInitializer createChannelInitializer(SocketAddress address, Bootstrap bootstrap) {
return new ChannelInitializer(bootstrap, address, operationsFactory, configuration, this);
return new ChannelInitializer(bootstrap, address, operationsFactory, configuration, this, topologyInfo.getCluster());
}

protected ChannelPool createChannelPool(Bootstrap bootstrap, ChannelInitializer channelInitializer, SocketAddress address) {
Expand Down

0 comments on commit c5d945b

Please sign in to comment.