From 874529ba0bcb21eead0b723d0d2534529fc657b3 Mon Sep 17 00:00:00 2001 From: Alex Rankin Date: Thu, 17 Jan 2019 19:34:53 +0000 Subject: [PATCH] Using supplier interface instead of custom interface, and renaming property --- .../org/apache/zookeeper/common/X509Util.java | 24 +++++---- .../zookeeper/common/ZKClientSSLContext.java | 35 ------------- .../org/apache/zookeeper/common/ZKConfig.java | 16 +++--- .../apache/zookeeper/common/X509UtilTest.java | 52 ++++++++++++++----- .../common/ZKTestClientSSLContext.java | 29 ----------- 5 files changed, 61 insertions(+), 95 deletions(-) delete mode 100644 zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java delete mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java index f2082559bb9..c42dc4ea115 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java @@ -53,6 +53,7 @@ import java.security.cert.X509CertSelector; import java.util.Arrays; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; /** * Utility code for X509 handling @@ -104,7 +105,7 @@ public abstract class X509Util implements Closeable, AutoCloseable { private String sslTruststoreLocationProperty = getConfigPrefix() + "trustStore.location"; private String sslTruststorePasswdProperty = getConfigPrefix() + "trustStore.password"; private String sslTruststoreTypeProperty = getConfigPrefix() + "trustStore.type"; - private String sslClientContextProperty = getConfigPrefix() + "client.context"; + private String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class"; private String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification"; private String sslCrlEnabledProperty = getConfigPrefix() + "crl"; private String sslOcspEnabledProperty = getConfigPrefix() + "ocsp"; @@ -160,8 +161,8 @@ public String getSslTruststoreTypeProperty() { return sslTruststoreTypeProperty; } - public String getSslClientContextProperty() { - return sslClientContextProperty; + public String getSslContextSupplierClassProperty() { + return sslContextSupplierClassProperty; } public String getSslHostnameVerificationEnabledProperty() { @@ -238,18 +239,19 @@ public int getSslHandshakeTimeoutMillis() { return result; } + @SuppressWarnings("unchecked") public SSLContext createSSLContext(ZKConfig config) throws SSLContextException { - if (config.getProperty(sslClientContextProperty) != null) { - LOG.debug("Loading SSLContext from property '" + sslClientContextProperty + "'"); - String sslClientContextClass = config.getProperty(sslClientContextProperty); + if (config.getProperty(sslContextSupplierClassProperty) != null) { + LOG.debug("Loading SSLContext supplier from property '" + sslContextSupplierClassProperty + "'"); + String supplierContextClassName = config.getProperty(sslContextSupplierClassProperty); try { - Class sslContextClass = Class.forName(sslClientContextClass); - ZKClientSSLContext sslContext = (ZKClientSSLContext) sslContextClass.getConstructor().newInstance(); - return sslContext.getSSLContext(); + Class sslContextClass = Class.forName(supplierContextClassName); + Supplier sslContextSupplier = (Supplier) sslContextClass.getConstructor().newInstance(); + return sslContextSupplier.get(); } catch (ClassNotFoundException | ClassCastException | NoSuchMethodException | InvocationTargetException | InstantiationException | IllegalAccessException e) { - throw new SSLContextException("Could not retrieve the SSLContext from source '" + sslClientContextClass + - "' provided in the property '" + sslClientContextProperty + "'", e); + throw new SSLContextException("Could not retrieve the SSLContext from supplier source '" + supplierContextClassName + + "' provided in the property '" + sslContextSupplierClassProperty + "'", e); } } else { return createSSLContextFromConfig(config); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java deleted file mode 100644 index aec72ca1ec0..00000000000 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java +++ /dev/null @@ -1,35 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.zookeeper.common; - -import javax.net.ssl.SSLContext; - -/** - * An interface for providing a custom {@link SSLContext} object to {@link X509Util} using {@link X509Util#getSslClientContextProperty()} - */ -public interface ZKClientSSLContext { - - /** - * Returns an {@link SSLContext} for use within the {@link X509Util} - * - * @return {@link SSLContext} for use within {@link X509Util} - * @throws X509Exception.SSLContextException if {@link SSLContext} cannot be created - */ - SSLContext getSSLContext() throws X509Exception.SSLContextException; - -} diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java index 7422f24686d..658afb79471 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java @@ -18,6 +18,12 @@ package org.apache.zookeeper.common; +import org.apache.zookeeper.Environment; +import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException; +import org.apache.zookeeper.server.util.VerifyingFileFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -26,12 +32,6 @@ import java.util.Map.Entry; import java.util.Properties; -import org.apache.zookeeper.Environment; -import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException; -import org.apache.zookeeper.server.util.VerifyingFileFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * This class is a base class for the configurations of both client and server. * It supports reading client configuration from both system properties and @@ -127,8 +127,8 @@ private void putSSLProperties(X509Util x509Util) { System.getProperty(x509Util.getSslTruststorePasswdProperty())); properties.put(x509Util.getSslTruststoreTypeProperty(), System.getProperty(x509Util.getSslTruststoreTypeProperty())); - properties.put(x509Util.getSslClientContextProperty(), - System.getProperty(x509Util.getSslClientContextProperty())); + properties.put(x509Util.getSslContextSupplierClassProperty(), + System.getProperty(x509Util.getSslContextSupplierClassProperty())); properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty())); properties.put(x509Util.getSslCrlEnabledProperty(), diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java index 38763dfa394..0b26609b4c7 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java @@ -40,6 +40,7 @@ import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; +import java.security.NoSuchAlgorithmException; import java.security.Security; import java.util.Collection; import java.util.concurrent.Callable; @@ -48,10 +49,10 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; @RunWith(Parameterized.class) public class X509UtilTest extends BaseX509ParameterizedTestCase { @@ -406,26 +407,30 @@ public void testGetSslHandshakeDetectionTimeoutMillisProperty() { } } + @Test(expected = X509Exception.SSLContextException.class) + public void testCreateSSLContext_invalidCustomSSLContextClass() throws Exception { + ZKConfig zkConfig = new ZKConfig(); + ClientX509Util clientX509Util = new ClientX509Util(); + zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), String.class.getCanonicalName()); + clientX509Util.createSSLContext(zkConfig); + } + @Test - public void testCreateSSLContext_invalidCustomSSLContextClass() { + public void testCreateSSLContext_validNullCustomSSLContextClass() throws X509Exception.SSLContextException { ZKConfig zkConfig = new ZKConfig(); ClientX509Util clientX509Util = new ClientX509Util(); - zkConfig.setProperty(clientX509Util.getSslClientContextProperty(), String.class.getCanonicalName()); - try { - clientX509Util.createSSLContext(zkConfig); - fail("SSLContextException expected."); - } catch (X509Exception.SSLContextException e) { - assertTrue(e.getMessage().contains(clientX509Util.getSslClientContextProperty())); - } + zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), X509UtilTest.class.getCanonicalName() + "$" + NullSslContextSupplier.class.getSimpleName()); + final SSLContext sslContext = clientX509Util.createSSLContext(zkConfig); + assertNull(sslContext); } @Test public void testCreateSSLContext_validCustomSSLContextClass() throws X509Exception.SSLContextException { ZKConfig zkConfig = new ZKConfig(); ClientX509Util clientX509Util = new ClientX509Util(); - zkConfig.setProperty(clientX509Util.getSslClientContextProperty(), ZKTestClientSSLContext.class.getCanonicalName()); + zkConfig.setProperty(clientX509Util.getSslContextSupplierClassProperty(), X509UtilTest.class.getCanonicalName() + "$" + SslContextSupplier.class.getSimpleName()); final SSLContext sslContext = clientX509Util.createSSLContext(zkConfig); - assertNull(sslContext); + assertNotNull(sslContext); } private static void forceClose(Socket s) { @@ -513,4 +518,27 @@ private void setCustomCipherSuites() { x509Util.close(); // remember to close old instance before replacing it x509Util = new ClientX509Util(); } + + public static class NullSslContextSupplier implements Supplier { + + @Override + public SSLContext get() { + return null; + } + + } + + public static class SslContextSupplier implements Supplier { + + @Override + public SSLContext get() { + try { + return SSLContext.getDefault(); + } catch (NoSuchAlgorithmException e) { + return null; + } + } + + } + } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java deleted file mode 100644 index b91dd298bb4..00000000000 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.zookeeper.common; - -import javax.net.ssl.SSLContext; - -public class ZKTestClientSSLContext implements ZKClientSSLContext { - - @Override - public SSLContext getSSLContext() { - return null; - } - -}