Skip to content

Commit

Permalink
Using supplier interface instead of custom interface, and renaming pr…
Browse files Browse the repository at this point in the history 
…operty
  • Loading branch information
@arankin-irl
arankin-irl committed Jan 17, 2019
1 parent ec27260 commit 874529b
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 95 deletions.
24 changes: 13 additions & 11 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -160,8 +161,8 @@ public String getSslTruststoreTypeProperty() {
return sslTruststoreTypeProperty;
}

public String getSslClientContextProperty() {
return sslClientContextProperty;
public String getSslContextSupplierClassProperty() {
return sslContextSupplierClassProperty;
}

public String getSslHostnameVerificationEnabledProperty() {
Expand Down Expand Up @@ -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<SSLContext> sslContextSupplier = (Supplier<SSLContext>) 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);
Expand Down
35 changes: 0 additions & 35 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKClientSSLContext.java
View file

This file was deleted.

16 changes: 8 additions & 8 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
52 changes: 40 additions & 12 deletions zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<SSLContext> {

@Override
public SSLContext get() {
return null;
}

}

public static class SslContextSupplier implements Supplier<SSLContext> {

@Override
public SSLContext get() {
try {
return SSLContext.getDefault();
} catch (NoSuchAlgorithmException e) {
return null;
}
}

}

}
29 changes: 0 additions & 29 deletions zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTestClientSSLContext.java
View file

This file was deleted.

0 comments on commit 874529b

Please sign in to comment.