This is an automated email from the ASF dual-hosted git repository.
ifesdjeen pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new cbf4da4 CASSANDRA-14991 Follow-up: clean up SSL Hot Reloading code
cbf4da4 is described below
commit cbf4da4397c2cec34d6a240b0e917a847c46b3d0
Author: Dinesh A. Joshi <dinesh.jo...@apple.com>
AuthorDate: Mon Feb 11 17:29:44 2019 -0800
CASSANDRA-14991 Follow-up: clean up SSL Hot Reloading code
patch by Dinesh Joshi, reviewed by Alex Petrov for CASSANDRA-15018
---
.../cassandra/config/DatabaseDescriptor.java | 3 +-
.../apache/cassandra/config/EncryptionOptions.java | 30 ++++++++++---
.../apache/cassandra/net/async/NettyFactory.java | 4 +-
.../cassandra/net/async/OptionalSslHandler.java | 2 +-
.../org/apache/cassandra/security/SSLFactory.java | 30 +++++++++----
.../org/apache/cassandra/transport/Server.java | 2 +-
.../apache/cassandra/transport/SimpleClient.java | 5 +--
.../apache/cassandra/security/SSLFactoryTest.java | 52 ++++++++++------------
8 files changed, 76 insertions(+), 52 deletions(-)
diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
index 9b7a6e5..5c70c9a 100644
--- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
+++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
@@ -905,7 +905,8 @@ public class DatabaseDescriptor
try
{
SSLFactory.initHotReloading(conf.server_encryption_options,
conf.client_encryption_options, false);
- } catch(IOException e)
+ }
+ catch(IOException e)
{
throw new ConfigurationException("Failed to initialize SSL hot
reloading", e);
}
diff --git a/src/java/org/apache/cassandra/config/EncryptionOptions.java
b/src/java/org/apache/cassandra/config/EncryptionOptions.java
index 45579fb..9524cec 100644
--- a/src/java/org/apache/cassandra/config/EncryptionOptions.java
+++ b/src/java/org/apache/cassandra/config/EncryptionOptions.java
@@ -57,6 +57,10 @@ public class EncryptionOptions
optional = options.optional;
}
+ /**
+ * The method is being mainly used to cache SslContexts therefore, we only
consider
+ * fields that would make a difference when the TrustStore or KeyStore
files are updated
+ */
@Override
public boolean equals(Object o)
{
@@ -66,23 +70,37 @@ public class EncryptionOptions
return false;
EncryptionOptions opt = (EncryptionOptions)o;
- return Objects.equals(keystore, opt.keystore) &&
+ return enabled == opt.enabled &&
+ optional == opt.optional &&
+ require_client_auth == opt.require_client_auth &&
+ require_endpoint_verification ==
opt.require_endpoint_verification &&
+ Objects.equals(keystore, opt.keystore) &&
+ Objects.equals(keystore_password, opt.keystore_password) &&
Objects.equals(truststore, opt.truststore) &&
- Objects.equals(algorithm, opt.algorithm) &&
+ Objects.equals(truststore_password, opt.truststore_password) &&
Objects.equals(protocol, opt.protocol) &&
- Arrays.equals(cipher_suites, opt.cipher_suites) &&
- require_client_auth == opt.require_client_auth &&
- require_endpoint_verification ==
opt.require_endpoint_verification;
+ Objects.equals(algorithm, opt.algorithm) &&
+ Objects.equals(store_type, opt.store_type) &&
+ Arrays.equals(cipher_suites, opt.cipher_suites);
}
+ /**
+ * The method is being mainly used to cache SslContexts therefore, we only
consider
+ * fields that would make a difference when the TrustStore or KeyStore
files are updated
+ */
@Override
public int hashCode()
{
int result = 0;
result += 31 * (keystore == null ? 0 : keystore.hashCode());
+ result += 31 * (keystore_password == null ? 0 :
keystore_password.hashCode());
result += 31 * (truststore == null ? 0 : truststore.hashCode());
- result += 31 * (algorithm == null ? 0 : algorithm.hashCode());
+ result += 31 * (truststore_password == null ? 0 :
truststore_password.hashCode());
result += 31 * (protocol == null ? 0 : protocol.hashCode());
+ result += 31 * (algorithm == null ? 0 : algorithm.hashCode());
+ result += 31 * (store_type == null ? 0 : store_type.hashCode());
+ result += 31 * Boolean.hashCode(enabled);
+ result += 31 * Boolean.hashCode(optional);
result += 31 * Arrays.hashCode(cipher_suites);
result += 31 * Boolean.hashCode(require_client_auth);
result += 31 * Boolean.hashCode(require_endpoint_verification);
diff --git a/src/java/org/apache/cassandra/net/async/NettyFactory.java
b/src/java/org/apache/cassandra/net/async/NettyFactory.java
index 3752927..346a067 100644
--- a/src/java/org/apache/cassandra/net/async/NettyFactory.java
+++ b/src/java/org/apache/cassandra/net/async/NettyFactory.java
@@ -292,7 +292,7 @@ public final class NettyFactory
}
else
{
- SslContext sslContext =
SSLFactory.getSslContext(encryptionOptions, true, SSLFactory.SocketType.SERVER);
+ SslContext sslContext =
SSLFactory.getOrCreateSslContext(encryptionOptions, true,
SSLFactory.SocketType.SERVER);
InetSocketAddress peer =
encryptionOptions.require_endpoint_verification ? channel.remoteAddress() :
null;
SslHandler sslHandler = newSslHandler(channel, sslContext,
peer);
logger.trace("creating inbound netty SslContext:
context={}, engine={}", sslContext.getClass().getName(),
sslHandler.engine().getClass().getName());
@@ -369,7 +369,7 @@ public final class NettyFactory
// order of handlers: ssl -> logger -> handshakeHandler
if (params.encryptionOptions != null)
{
- SslContext sslContext =
SSLFactory.getSslContext(params.encryptionOptions, true,
SSLFactory.SocketType.CLIENT);
+ SslContext sslContext =
SSLFactory.getOrCreateSslContext(params.encryptionOptions, true,
SSLFactory.SocketType.CLIENT);
// for some reason channel.remoteAddress() will return null
InetAddressAndPort address = params.connectionId.remote();
InetSocketAddress peer =
params.encryptionOptions.require_endpoint_verification ? new
InetSocketAddress(address.address, address.port) : null;
diff --git a/src/java/org/apache/cassandra/net/async/OptionalSslHandler.java
b/src/java/org/apache/cassandra/net/async/OptionalSslHandler.java
index 3b4f794..3fb8562 100644
--- a/src/java/org/apache/cassandra/net/async/OptionalSslHandler.java
+++ b/src/java/org/apache/cassandra/net/async/OptionalSslHandler.java
@@ -51,7 +51,7 @@ public class OptionalSslHandler extends ByteToMessageDecoder
if (SslHandler.isEncrypted(in))
{
// Connection uses SSL/TLS, replace the detection handler with a
SslHandler and so use encryption.
- SslContext sslContext =
SSLFactory.getSslContext(encryptionOptions, true, SSLFactory.SocketType.SERVER);
+ SslContext sslContext =
SSLFactory.getOrCreateSslContext(encryptionOptions, true,
SSLFactory.SocketType.SERVER);
Channel channel = ctx.channel();
InetSocketAddress peer =
encryptionOptions.require_endpoint_verification ? (InetSocketAddress)
channel.remoteAddress() : null;
SslHandler sslHandler = NettyFactory.newSslHandler(channel,
sslContext, peer);
diff --git a/src/java/org/apache/cassandra/security/SSLFactory.java
b/src/java/org/apache/cassandra/security/SSLFactory.java
index 700142d..7519875 100644
--- a/src/java/org/apache/cassandra/security/SSLFactory.java
+++ b/src/java/org/apache/cassandra/security/SSLFactory.java
@@ -129,6 +129,15 @@ public final class SSLFactory
lastModTime = curModTime;
return result;
}
+
+ @Override
+ public String toString()
+ {
+ return "HotReloadableFile{" +
+ "file=" + file +
+ ", lastModTime=" + lastModTime +
+ '}';
+ }
}
/**
@@ -221,18 +230,18 @@ public final class SSLFactory
/**
* get a netty {@link SslContext} instance
*/
- public static SslContext getSslContext(EncryptionOptions options, boolean
buildTruststore,
- SocketType socketType) throws
IOException
+ public static SslContext getOrCreateSslContext(EncryptionOptions options,
boolean buildTruststore,
+ SocketType socketType)
throws IOException
{
- return getSslContext(options, buildTruststore, socketType,
OpenSsl.isAvailable());
+ return getOrCreateSslContext(options, buildTruststore, socketType,
OpenSsl.isAvailable());
}
/**
* Get a netty {@link SslContext} instance.
*/
@VisibleForTesting
- static SslContext getSslContext(EncryptionOptions options, boolean
buildTruststore,
- SocketType socketType, boolean useOpenSsl)
throws IOException
+ static SslContext getOrCreateSslContext(EncryptionOptions options, boolean
buildTruststore,
+ SocketType socketType, boolean
useOpenSsl) throws IOException
{
CacheKey key = new CacheKey(options, socketType);
SslContext sslContext;
@@ -302,7 +311,7 @@ public final class SSLFactory
if (!isHotReloadingInitialized)
throw new IllegalStateException("Hot reloading functionality has
not been initialized.");
- logger.trace("Checking whether certificates have been updated");
+ logger.debug("Checking whether certificates have been updated {}",
hotReloadableFiles);
if
(hotReloadableFiles.stream().anyMatch(HotReloadableFile::shouldReload))
{
@@ -311,7 +320,8 @@ public final class SSLFactory
{
validateSslCerts(serverOpts, clientOpts);
cachedSslContexts.clear();
- } catch(Exception e)
+ }
+ catch(Exception e)
{
logger.error("Failed to hot reload the SSL Certificates!
Please check the certificate files.", e);
}
@@ -378,7 +388,8 @@ public final class SSLFactory
createNettySslContext(serverOpts, true, SocketType.SERVER,
OpenSsl.isAvailable());
createNettySslContext(serverOpts, true, SocketType.CLIENT,
OpenSsl.isAvailable());
}
- } catch (Exception e)
+ }
+ catch (Exception e)
{
throw new IOException("Failed to create SSL context using
server_encryption_options!", e);
}
@@ -391,7 +402,8 @@ public final class SSLFactory
createNettySslContext(clientOpts,
clientOpts.require_client_auth, SocketType.SERVER, OpenSsl.isAvailable());
createNettySslContext(clientOpts,
clientOpts.require_client_auth, SocketType.CLIENT, OpenSsl.isAvailable());
}
- } catch (Exception e)
+ }
+ catch (Exception e)
{
throw new IOException("Failed to create SSL context using
client_encryption_options!", e);
}
diff --git a/src/java/org/apache/cassandra/transport/Server.java
b/src/java/org/apache/cassandra/transport/Server.java
index 056a4a0..33cd0fb 100644
--- a/src/java/org/apache/cassandra/transport/Server.java
+++ b/src/java/org/apache/cassandra/transport/Server.java
@@ -406,7 +406,7 @@ public class Server implements CassandraDaemon.Server
protected final SslHandler createSslHandler(ByteBufAllocator
allocator) throws IOException
{
- SslContext sslContext =
SSLFactory.getSslContext(encryptionOptions,
encryptionOptions.require_client_auth, SSLFactory.SocketType.SERVER);
+ SslContext sslContext =
SSLFactory.getOrCreateSslContext(encryptionOptions,
encryptionOptions.require_client_auth, SSLFactory.SocketType.SERVER);
return sslContext.newHandler(allocator);
}
}
diff --git a/src/java/org/apache/cassandra/transport/SimpleClient.java
b/src/java/org/apache/cassandra/transport/SimpleClient.java
index 0db9136..9ed4272 100644
--- a/src/java/org/apache/cassandra/transport/SimpleClient.java
+++ b/src/java/org/apache/cassandra/transport/SimpleClient.java
@@ -25,7 +25,6 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.SynchronousQueue;
@@ -292,8 +291,8 @@ public class SimpleClient implements Closeable
protected void initChannel(Channel channel) throws Exception
{
super.initChannel(channel);
- SslContext sslContext =
SSLFactory.getSslContext(encryptionOptions,
encryptionOptions.require_client_auth,
-
SSLFactory.SocketType.CLIENT);
+ SslContext sslContext =
SSLFactory.getOrCreateSslContext(encryptionOptions,
encryptionOptions.require_client_auth,
+
SSLFactory.SocketType.CLIENT);
channel.pipeline().addFirst("ssl",
sslContext.newHandler(channel.alloc()));
}
}
diff --git a/test/unit/org/apache/cassandra/security/SSLFactoryTest.java
b/test/unit/org/apache/cassandra/security/SSLFactoryTest.java
index b253c59..5fdbe7b 100644
--- a/test/unit/org/apache/cassandra/security/SSLFactoryTest.java
+++ b/test/unit/org/apache/cassandra/security/SSLFactoryTest.java
@@ -96,7 +96,7 @@ public class SSLFactoryTest
}
EncryptionOptions options = addKeystoreOptions(encryptionOptions);
- SslContext sslContext = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, true);
+ SslContext sslContext = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, true);
Assert.assertNotNull(sslContext);
Assert.assertTrue(sslContext instanceof OpenSslContext);
}
@@ -105,7 +105,7 @@ public class SSLFactoryTest
public void getSslContext_JdkSsl() throws IOException
{
EncryptionOptions options = addKeystoreOptions(encryptionOptions);
- SslContext sslContext = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, false);
+ SslContext sslContext = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, false);
Assert.assertNotNull(sslContext);
Assert.assertTrue(sslContext instanceof JdkSslContext);
Assert.assertEquals(Arrays.asList(encryptionOptions.cipher_suites),
sslContext.cipherSuites());
@@ -174,16 +174,16 @@ public class SSLFactoryTest
SSLFactory.initHotReloading((ServerEncryptionOptions) options,
options, true);
- SslContext oldCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ SslContext oldCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
File keystoreFile = new File(options.keystore);
SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);
- Thread.sleep(5000);
- FileUtils.touch(keystoreFile);
+
+ keystoreFile.setLastModified(System.currentTimeMillis() + 15000);
SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);;
- SslContext newCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ SslContext newCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
Assert.assertNotSame(oldCtx, newCtx);
@@ -209,36 +209,31 @@ public class SSLFactoryTest
}
@Test
- public void
testSslFactoryHotReload_BadPassword_DoesNotClearExistingSslContext() throws
IOException,
-
InterruptedException
+ public void
testSslFactoryHotReload_BadPassword_DoesNotClearExistingSslContext() throws
IOException
{
try
{
addKeystoreOptions(encryptionOptions);
- EncryptionOptions options = new
ServerEncryptionOptions(encryptionOptions);
+ ServerEncryptionOptions options = new
ServerEncryptionOptions(encryptionOptions);
options.enabled = true;
- SSLFactory.initHotReloading((ServerEncryptionOptions) options,
options, true);
- SslContext oldCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ SSLFactory.initHotReloading(options, options, true);
+ SslContext oldCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
File keystoreFile = new File(options.keystore);
- SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);
- Thread.sleep(5000);
- FileUtils.touch(keystoreFile);
+ SSLFactory.checkCertFilesForHotReloading(options, options);
+ keystoreFile.setLastModified(System.currentTimeMillis() + 5000);
- options.keystore_password = "bad password";
- SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);;
- SslContext newCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ ServerEncryptionOptions modOptions = new
ServerEncryptionOptions(options);
+ modOptions.keystore_password = "bad password";
+ SSLFactory.checkCertFilesForHotReloading(modOptions, modOptions);
+ SslContext newCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
Assert.assertSame(oldCtx, newCtx);
}
- catch (Exception e)
- {
- throw e;
- }
finally
{
DatabaseDescriptor.loadConfig();
@@ -261,16 +256,15 @@ public class SSLFactoryTest
options.enabled = true;
SSLFactory.initHotReloading((ServerEncryptionOptions) options,
options, true);
- SslContext oldCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ SslContext oldCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);
- Thread.sleep(5000);
- FileUtils.touch(testKeystoreFile);
+ testKeystoreFile.setLastModified(System.currentTimeMillis() +
15000);
FileUtils.forceDelete(testKeystoreFile);
SSLFactory.checkCertFilesForHotReloading((ServerEncryptionOptions)
options, options);;
- SslContext newCtx = SSLFactory.getSslContext(options, true,
SSLFactory.SocketType.CLIENT, OpenSsl
+ SslContext newCtx = SSLFactory.getOrCreateSslContext(options,
true, SSLFactory.SocketType.CLIENT, OpenSsl
.isAvailable());
Assert.assertSame(oldCtx, newCtx);
@@ -293,16 +287,16 @@ public class SSLFactoryTest
options.enabled = true;
options.cipher_suites = new String[]{
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" };
- SslContext ctx1 = SSLFactory.getSslContext(options, true,
-
SSLFactory.SocketType.SERVER, OpenSsl.isAvailable());
+ SslContext ctx1 = SSLFactory.getOrCreateSslContext(options, true,
+
SSLFactory.SocketType.SERVER, OpenSsl.isAvailable());
Assert.assertTrue(ctx1.isServer());
Assert.assertArrayEquals(ctx1.cipherSuites().toArray(),
options.cipher_suites);
options.cipher_suites = new String[]{
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" };
- SslContext ctx2 = SSLFactory.getSslContext(options, true,
-
SSLFactory.SocketType.CLIENT, OpenSsl.isAvailable());
+ SslContext ctx2 = SSLFactory.getOrCreateSslContext(options, true,
+
SSLFactory.SocketType.CLIENT, OpenSsl.isAvailable());
Assert.assertTrue(ctx2.isClient());
Assert.assertArrayEquals(ctx2.cipherSuites().toArray(),
options.cipher_suites);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
