Skip to content

Commit 1b6a4c2

Browse files
authored
ZOOKEEPER-4828: Honor ssl.context.supplier.class for client-server TLS
Reviewers: anmolnar, PDavid Author: SvenssonWeb Closes #2433 from SvenssonWeb/ZOOKEEPER-4828
1 parent 53a78e3 commit 1b6a4c2

4 files changed

Lines changed: 164 additions & 23 deletions

File tree

zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,8 @@ protected void initChannel(SocketChannel ch) throws Exception {
443443
// The synchronized is to prevent the race on shared variable "sslContext".
444444
// Basically we only need to create it once.
445445
private synchronized void initSSL(ChannelPipeline pipeline)
446-
throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
446+
throws X509Exception.SSLContextException, X509Exception.KeyManagerException,
447+
X509Exception.TrustManagerException, SSLException {
447448
if (sslContext == null) {
448449
try (ClientX509Util x509Util = new ClientX509Util()) {
449450
sslContext = x509Util.createNettySslContextForClient(clientConfig);

zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@
1919
package org.apache.zookeeper.common;
2020

2121
import io.netty.handler.ssl.DelegatingSslContext;
22+
import io.netty.handler.ssl.IdentityCipherSuiteFilter;
23+
import io.netty.handler.ssl.JdkSslContext;
2224
import io.netty.handler.ssl.OpenSsl;
2325
import io.netty.handler.ssl.SslContext;
2426
import io.netty.handler.ssl.SslContextBuilder;
2527
import io.netty.handler.ssl.SslProvider;
2628
import java.security.Security;
2729
import java.util.Arrays;
2830
import javax.net.ssl.KeyManager;
31+
import javax.net.ssl.SSLContext;
2932
import javax.net.ssl.SSLEngine;
3033
import javax.net.ssl.SSLException;
3134
import javax.net.ssl.SSLParameters;
@@ -62,7 +65,13 @@ public String getSslProviderProperty() {
6265
}
6366

6467
public SslContext createNettySslContextForClient(ZKConfig config)
65-
throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
68+
throws X509Exception.SSLContextException, X509Exception.KeyManagerException,
69+
X509Exception.TrustManagerException, SSLException {
70+
SSLContext suppliedSSLContext = loadSuppliedSSLContext(config);
71+
if (suppliedSSLContext != null) {
72+
return createNettyJdkSslContext(config, suppliedSSLContext, true);
73+
}
74+
6675
SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
6776

6877
KeyManager km = buildKeyManager(config);
@@ -97,6 +106,11 @@ public SslContext createNettySslContextForClient(ZKConfig config)
97106

98107
public SslContext createNettySslContextForServer(ZKConfig config)
99108
throws X509Exception.SSLContextException, X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
109+
SSLContext suppliedSSLContext = loadSuppliedSSLContext(config);
110+
if (suppliedSSLContext != null) {
111+
return createNettyJdkSslContext(config, suppliedSSLContext, false);
112+
}
113+
100114
KeyManager km = buildKeyManager(config);
101115
if (km == null) {
102116
throw new X509Exception.SSLContextException(
@@ -133,6 +147,54 @@ public SslContext createNettySslContextForServer(ZKConfig config, KeyManager key
133147
}
134148
}
135149

150+
/**
151+
* Wraps a user supplied {@link SSLContext} in a Netty {@link SslContext}, applying the configured
152+
* protocols, cipher suites, client auth mode and hostname verification on top of it.
153+
*
154+
* <p>A supplied SSLContext carries its own key and trust managers, so it can only be used with the
155+
* JDK SSL provider: the OpenSSL providers build their own native context and cannot delegate to it.
156+
*
157+
* <p>Unlike the file based path, hostname verification is applied whenever it is enabled. The file
158+
* based path relies on {@link ZKTrustManager} to verify hostnames and only falls back to endpoint
159+
* identification when no trust manager is available, which is never the case for a supplied context.
160+
*
161+
* @param config the configuration to read the SSL options from.
162+
* @param sslContext the user supplied SSLContext.
163+
* @param isClient {@code true} to create a client side context, {@code false} for server side.
164+
* @return the Netty SslContext.
165+
* @throws X509Exception.SSLContextException if a non JDK SSL provider is configured.
166+
*/
167+
private SslContext createNettyJdkSslContext(ZKConfig config, SSLContext sslContext, boolean isClient)
168+
throws X509Exception.SSLContextException {
169+
SslProvider sslProvider = getSslProvider(config);
170+
if (sslProvider != SslProvider.JDK) {
171+
throw new X509Exception.SSLContextException("An SSLContext supplied through "
172+
+ getSslContextSupplierClassProperty()
173+
+ " can only be used with the JDK SSL provider, but "
174+
+ getSslProviderProperty()
175+
+ " is set to "
176+
+ sslProvider);
177+
}
178+
179+
SslContext nettySslContext = new JdkSslContext(
180+
sslContext,
181+
isClient,
182+
getCipherSuites(config),
183+
IdentityCipherSuiteFilter.INSTANCE,
184+
null,
185+
isClient ? X509Util.ClientAuth.NONE.toNettyClientAuth() : getClientAuth(config).toNettyClientAuth(),
186+
getEnabledProtocols(config),
187+
false);
188+
189+
boolean hostnameVerificationEnabled = isClient
190+
? isServerHostnameVerificationEnabled(config)
191+
: isClientHostnameVerificationEnabled(config);
192+
if (hostnameVerificationEnabled) {
193+
return addHostnameVerification(nettySslContext, isClient ? "Server" : "Client");
194+
}
195+
return nettySslContext;
196+
}
197+
136198
private SslContextBuilder handleTcnativeOcspStapling(SslContextBuilder builder, ZKConfig config) {
137199
SslProvider sslProvider = getSslProvider(config);
138200
boolean tcnative = sslProvider == SslProvider.OPENSSL || sslProvider == SslProvider.OPENSSL_REFCNT;

zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -360,30 +360,47 @@ public int getSslHandshakeTimeoutMillis() {
360360
}
361361
}
362362

363-
@SuppressWarnings("unchecked")
364363
public SSLContextAndOptions createSSLContextAndOptions(ZKConfig config) throws SSLContextException {
364+
final SSLContext suppliedSSLContext = loadSuppliedSSLContext(config);
365+
if (suppliedSSLContext != null) {
366+
return new SSLContextAndOptions(this, config, suppliedSSLContext);
367+
}
368+
return createSSLContextAndOptionsFromConfig(config);
369+
}
370+
371+
/**
372+
* Loads an {@link SSLContext} from the {@link Supplier} implementation named by the
373+
* {@link #getSslContextSupplierClassProperty()} property. This allows a user to take full control over
374+
* the construction of the SSLContext, for example to use a hardware key store or an SSLContext obtained
375+
* from a container, rather than having ZooKeeper load key material from files.
376+
*
377+
* @param config the configuration to read the supplier class name from.
378+
* @return the supplied SSLContext, or {@code null} if the property is not set.
379+
* @throws SSLContextException if the supplier class cannot be loaded, instantiated or invoked.
380+
*/
381+
@SuppressWarnings("unchecked")
382+
protected SSLContext loadSuppliedSSLContext(ZKConfig config) throws SSLContextException {
365383
final String supplierContextClassName = config.getProperty(sslContextSupplierClassProperty);
366-
if (supplierContextClassName != null) {
367-
LOG.debug("Loading SSLContext supplier from property '{}'", sslContextSupplierClassProperty);
384+
if (supplierContextClassName == null) {
385+
return null;
386+
}
387+
LOG.debug("Loading SSLContext supplier from property '{}'", sslContextSupplierClassProperty);
368388

369-
try {
370-
Class<?> sslContextClass = Class.forName(supplierContextClassName);
371-
Supplier<SSLContext> sslContextSupplier = (Supplier<SSLContext>) sslContextClass.getConstructor().newInstance();
372-
return new SSLContextAndOptions(this, config, sslContextSupplier.get());
373-
} catch (ClassNotFoundException
374-
| ClassCastException
375-
| NoSuchMethodException
376-
| InvocationTargetException
377-
| InstantiationException
378-
| IllegalAccessException e) {
379-
throw new SSLContextException("Could not retrieve the SSLContext from supplier source '"
380-
+ supplierContextClassName
381-
+ "' provided in the property '"
382-
+ sslContextSupplierClassProperty
383-
+ "'", e);
384-
}
385-
} else {
386-
return createSSLContextAndOptionsFromConfig(config);
389+
try {
390+
Class<?> sslContextClass = Class.forName(supplierContextClassName);
391+
Supplier<SSLContext> sslContextSupplier = (Supplier<SSLContext>) sslContextClass.getConstructor().newInstance();
392+
return sslContextSupplier.get();
393+
} catch (ClassNotFoundException
394+
| ClassCastException
395+
| NoSuchMethodException
396+
| InvocationTargetException
397+
| InstantiationException
398+
| IllegalAccessException e) {
399+
throw new SSLContextException("Could not retrieve the SSLContext from supplier source '"
400+
+ supplierContextClassName
401+
+ "' provided in the property '"
402+
+ sslContextSupplierClassProperty
403+
+ "'", e);
387404
}
388405
}
389406

zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static org.junit.jupiter.api.Assertions.assertThrows;
2727
import static org.junit.jupiter.api.Assertions.assertTrue;
2828
import io.netty.buffer.UnpooledByteBufAllocator;
29+
import io.netty.handler.ssl.JdkSslContext;
2930
import io.netty.handler.ssl.SslContext;
3031
import java.io.IOException;
3132
import java.net.InetAddress;
@@ -91,6 +92,8 @@ public void cleanUp() {
9192
System.clearProperty(x509Util.getCipherSuitesProperty());
9293
System.clearProperty(x509Util.getSslProtocolProperty());
9394
System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
95+
System.clearProperty(x509Util.getSslHostnameVerificationEnabledProperty());
96+
System.clearProperty(x509Util.getSslClientHostnameVerificationEnabledProperty());
9497
System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
9598
System.clearProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET);
9699
System.clearProperty(FIPS_MODE_PROPERTY);
@@ -725,6 +728,64 @@ public void testCreateSSLContext_validCustomSSLContextClass(
725728
assertEquals(SSLContext.getDefault(), sslContext);
726729
}
727730

731+
@ParameterizedTest
732+
@MethodSource("data")
733+
public void testCreateNettySslContextForClient_customSSLContextClass(
734+
X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
735+
throws Exception {
736+
init(caKeyType, certKeyType, keyPassword, paramIndex);
737+
try (ClientX509Util clientX509Util = new ClientX509Util()) {
738+
ZKConfig zkConfig = new ZKConfig();
739+
zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), SslContextSupplier.class.getName());
740+
// Disable hostname verification so the JdkSslContext is not wrapped in a DelegatingSslContext.
741+
zkConfig.setProperty(clientX509Util.getSslHostnameVerificationEnabledProperty(), "false");
742+
743+
SslContext sslContext = clientX509Util.createNettySslContextForClient(zkConfig);
744+
745+
assertTrue(sslContext instanceof JdkSslContext);
746+
assertEquals(SSLContext.getDefault(), ((JdkSslContext) sslContext).context());
747+
assertTrue(sslContext.isClient());
748+
}
749+
}
750+
751+
@ParameterizedTest
752+
@MethodSource("data")
753+
public void testCreateNettySslContextForServer_customSSLContextClass(
754+
X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
755+
throws Exception {
756+
init(caKeyType, certKeyType, keyPassword, paramIndex);
757+
try (ClientX509Util clientX509Util = new ClientX509Util()) {
758+
ZKConfig zkConfig = new ZKConfig();
759+
zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), SslContextSupplier.class.getName());
760+
// A supplied SSLContext carries its own key material, so no key store must be required.
761+
zkConfig.setProperty(clientX509Util.getSslKeystoreLocationProperty(), "");
762+
// Disable hostname verification so the JdkSslContext is not wrapped in a DelegatingSslContext.
763+
zkConfig.setProperty(clientX509Util.getSslHostnameVerificationEnabledProperty(), "false");
764+
765+
SslContext sslContext = clientX509Util.createNettySslContextForServer(zkConfig);
766+
767+
assertTrue(sslContext instanceof JdkSslContext);
768+
assertEquals(SSLContext.getDefault(), ((JdkSslContext) sslContext).context());
769+
assertTrue(sslContext.isServer());
770+
}
771+
}
772+
773+
@ParameterizedTest
774+
@MethodSource("data")
775+
public void testCreateNettySslContext_customSSLContextClassRejectsNonJdkProvider(
776+
X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
777+
throws Exception {
778+
init(caKeyType, certKeyType, keyPassword, paramIndex);
779+
try (ClientX509Util clientX509Util = new ClientX509Util()) {
780+
ZKConfig zkConfig = new ZKConfig();
781+
zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), SslContextSupplier.class.getName());
782+
zkConfig.setProperty(clientX509Util.getSslProviderProperty(), "OPENSSL");
783+
784+
assertThrows(X509Exception.SSLContextException.class,
785+
() -> clientX509Util.createNettySslContextForClient(zkConfig));
786+
}
787+
}
788+
728789
@ParameterizedTest
729790
@MethodSource("data")
730791
public void testCreateSSLContext_ocspWithJreProvider(

0 commit comments

Comments
 (0)