You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "aratno (via GitHub)" <gi...@apache.org> on 2024/01/19 16:53:12 UTC
[PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
aratno opened a new pull request, #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907
Jira: https://issues.apache.org/jira/browse/CASSANDRA-19180
This PR includes keystore reloading for `DefautlSslEngineFactory`, implemented by checking the contents of the keystore file at a periodic interval. This is intended to be useful in environments where mTLS is enabled, application instances restart infrequently, and certificates are short-lived. The existing default behavior (no reloading at all) would cause application re-connections to fail after the client certificate in the original keystore has expired, requiring an application restart to establish a new session to pick up the updated contents of the keystore file.
This PR does not include a mechanism for updating the contents of a keystore file in different environments. In some environments where client certificates are automatically renewed, an application may have to load those certificates into their keystore with a tool like fsnotify or a cronjob to keep identity certificates and the client-referenced keystore file in sync.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1470651460
##########
core/src/main/resources/reference.conf:
##########
@@ -790,6 +790,13 @@ datastax-java-driver {
// truststore-password = password123
// keystore-path = /path/to/client.keystore
// keystore-password = password123
+
+ # The duration between attempts to reload the keystore from the contents of the file specified
+ # by `keystore-path`. This is mainly relevant in environments where certificates have short
+ # lifetimes and applications are restarted infrequently, since an expired client certificate
+ # will prevent new connections from being established until the application is restarted. If
+ # not set, defaults to not reload the keystore.
+ // keystore-reload-interval = 30 minutes
Review Comment:
As discussed in the DefaultSslEngineFactory code above this would no longer be commented out and would have a value of 0 minutes.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java:
##########
@@ -54,6 +55,7 @@
* truststore-password = password123
* keystore-path = /path/to/client.keystore
* keystore-password = password123
+ * keystore-reload-interval = 30 minutes
Review Comment:
Following on from the conversation below it might be worth a comment here saying something like "We enable keystore reloading by specifying a non-zero interval here" or something similar.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java:
##########
@@ -159,8 +150,22 @@ protected SSLContext buildContext(DriverExecutionProfile config) throws Exceptio
}
}
+ private ReloadingKeyManagerFactory buildReloadingKeyManagerFactory(DriverExecutionProfile config)
+ throws Exception {
+ Path keystorePath = Paths.get(config.getString(DefaultDriverOption.SSL_KEYSTORE_PATH));
+ String password =
+ config.isDefined(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
+ ? config.getString(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
+ : null;
Review Comment:
DriverExecutionProfile includes getters with default values so config.getString(DefaultDriverOption.SSL_KEYSTORE_PASSWORD, null) is probably a cleaner way to express this.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,253 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval)
+ throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+ CertificateException, IOException {
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ }
+ kmf.init(ks, keystorePassword.toCharArray());
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+
+ // Ensure that reload is called once synchronously, to make sure the file exists etc.
+ reload();
+
+ if (!reloadInterval.isZero()) {
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setName(String.format("%s-%%d", this.getClass().getSimpleName()));
+ t.setDaemon(true);
+ return t;
+ });
+ this.executor.scheduleWithFixedDelay(
+ this::reload,
+ reloadInterval.toMillis(),
+ reloadInterval.toMillis(),
+ TimeUnit.MILLISECONDS);
+ }
+ }
+
+ @VisibleForTesting
+ void reload() {
+ try {
+ reload0();
+ } catch (Exception e) {
+ String msg =
+ "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
+ + "certificates and fail to re-establish connections to Cassandra hosts.";
Review Comment:
Nit: this should be " certificates" in order to make the spacing of the full sentence work out.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,253 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval)
+ throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+ CertificateException, IOException {
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ }
+ kmf.init(ks, keystorePassword.toCharArray());
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+
+ // Ensure that reload is called once synchronously, to make sure the file exists etc.
+ reload();
+
+ if (!reloadInterval.isZero()) {
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setName(String.format("%s-%%d", this.getClass().getSimpleName()));
+ t.setDaemon(true);
+ return t;
+ });
+ this.executor.scheduleWithFixedDelay(
+ this::reload,
+ reloadInterval.toMillis(),
+ reloadInterval.toMillis(),
+ TimeUnit.MILLISECONDS);
+ }
+ }
+
+ @VisibleForTesting
+ void reload() {
+ try {
+ reload0();
+ } catch (Exception e) {
+ String msg =
+ "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
+ + "certificates and fail to re-establish connections to Cassandra hosts.";
+ logger.warn(msg, e);
+ }
+ }
+
+ private synchronized void reload0()
+ throws NoSuchAlgorithmException, IOException, KeyStoreException, CertificateException,
+ UnrecoverableKeyException {
+ logger.debug("Checking KeyStore file {} for updates", keystorePath);
+
+ final byte[] keyStoreBytes = Files.readAllBytes(keystorePath);
+ final byte[] newDigest = digest(keyStoreBytes);
+ if (lastDigest != null && Arrays.equals(lastDigest, digest(keyStoreBytes))) {
+ logger.debug("KeyStore file content has not changed; skipping update");
+ return;
+ }
+
+ final KeyStore keyStore = KeyStore.getInstance(KEYSTORE_TYPE);
+ try (InputStream inputStream = new ByteArrayInputStream(keyStoreBytes)) {
+ keyStore.load(inputStream, keystorePassword.toCharArray());
+ }
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ kmf.init(keyStore, keystorePassword.toCharArray());
+ logger.info("Detected updates to KeyStore file {}", keystorePath);
+
+ this.spi.keyManager.set((X509ExtendedKeyManager) kmf.getKeyManagers()[0]);
+ this.lastDigest = newDigest;
+ }
Review Comment:
Seems like this operation should generalize nicely to a CAS op which would allow you to avoid the synchronized usage here.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java:
##########
@@ -159,8 +150,22 @@ protected SSLContext buildContext(DriverExecutionProfile config) throws Exceptio
}
}
+ private ReloadingKeyManagerFactory buildReloadingKeyManagerFactory(DriverExecutionProfile config)
+ throws Exception {
+ Path keystorePath = Paths.get(config.getString(DefaultDriverOption.SSL_KEYSTORE_PATH));
+ String password =
+ config.isDefined(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
+ ? config.getString(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
+ : null;
+ Duration reloadInterval =
+ config.isDefined(DefaultDriverOption.SSL_KEYSTORE_RELOAD_INTERVAL)
+ ? config.getDuration(DefaultDriverOption.SSL_KEYSTORE_RELOAD_INTERVAL)
+ : Duration.ZERO;
Review Comment:
The convention in the driver config is for configs that have a legit default value to be included in reference.conf; the commented-out configs are intended to provide examples of what the user could do. In this case I'd argue the keystore-reload-interval should be included directly in reference.conf (without being commented out) with a value of 0 minutes. That's actually the default we want here... it's on the user to override that with their app config if they wish to do so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#issuecomment-1989597253
With two +1's from committers (mine + one from driftx on the JIRA ticket) this ticket was considered eligible to merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce merged PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#issuecomment-1931068838
FTR these changes got a clean run on DataStax Jenkins
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1464032000
##########
core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java:
##########
@@ -255,6 +255,12 @@ public enum DefaultDriverOption implements DriverOption {
* <p>Value-type: {@link String}
*/
SSL_KEYSTORE_PASSWORD("advanced.ssl-engine-factory.keystore-password"),
+ /**
+ * The duration between attempts to reload the keystore.
+ *
+ * <p>Value-type: {@link java.time.Duration}
+ */
+ SSL_KEYSTORE_RELOAD_INTERVAL("advanced.ssl-engine-factory.keystore-reload-interval"),
Review Comment:
it was surprising to me that the revapi checker doesn't like this:
```
[ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.10.5:check (default) on project java-driver-core: The following API problems caused the build to fail:
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.ADDRESS_TRANSLATOR_CLASS: The enum constant was defined on position 61 but is now on 62. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.CLOUD_SECURE_CONNECT_BUNDLE: The enum constant was defined on position 122 but is now on 123. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.COALESCER_INTERVAL: The enum constant was defined on position 116 but is now on 117. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.COALESCER_MAX_RUNS: The enum constant was defined on position 115 but is now on 116. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.CONNECTION_CONNECT_TIMEOUT: The enum constant was defined on position 124 but is now on 125. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.CONTROL_CONNECTION_AGREEMENT_INTERVAL: The enum constant was defined on position 98 but is now on 99. The user code can break if it relies on the return value of the "ordinal()" method.
[ERROR] java.field.enumConstantOrderChanged: field com.datastax.oss.driver.api.core.config.DefaultDriverOption.CONTROL_CONNECTION_AGREEMENT_TIMEOUT: The enum constant was defined on position 99 but is now on 100. The user code can break if it relies on the return value of the "ordinal()" method.
```
I guess changing ordinal positioning could break code that depends on the ordering, although that seems pretty fragile to depend on
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1464035185
##########
core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java:
##########
@@ -255,6 +255,12 @@ public enum DefaultDriverOption implements DriverOption {
* <p>Value-type: {@link String}
*/
SSL_KEYSTORE_PASSWORD("advanced.ssl-engine-factory.keystore-password"),
+ /**
+ * The duration between attempts to reload the keystore.
+ *
+ * <p>Value-type: {@link java.time.Duration}
+ */
+ SSL_KEYSTORE_RELOAD_INTERVAL("advanced.ssl-engine-factory.keystore-reload-interval"),
Review Comment:
Yeah I'd prefer to keep the driver options ordered into groups, like keeping all the SSL options together. I'd be surprised if anyone was serializing these options based on their enum order. I'll fix this up though - no need to rock the boat.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#issuecomment-1908284283
I ran the entire integration suite this PR and seemed to run pretty good;
Since i'm on a M1 mac and need your [CASSANDRA-19292](https://issues.apache.org/jira/browse/CASSANDRA-19292) patch to run against 4.1 I ran against a local C* with a newer JNA patched in and the entire suite ran clean, with exception to DefaultReactiveResultSetIT (think that may be somewhat flaky, taking a look that).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1476658241
##########
core/src/main/resources/reference.conf:
##########
@@ -790,6 +790,13 @@ datastax-java-driver {
// truststore-password = password123
// keystore-path = /path/to/client.keystore
// keystore-password = password123
+
+ # The duration between attempts to reload the keystore from the contents of the file specified
+ # by `keystore-path`. This is mainly relevant in environments where certificates have short
+ # lifetimes and applications are restarted infrequently, since an expired client certificate
+ # will prevent new connections from being established until the application is restarted. If
+ # not set, defaults to not reload the keystore.
+ // keystore-reload-interval = 30 minutes
Review Comment:
We talked about this and a few other things on Slack - decided to keep the commented-out config as a way to disable reloading by default. It's possible we enable it by default in the future, when mTLS is more popular, but that's for another day.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,253 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval)
+ throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+ CertificateException, IOException {
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ }
+ kmf.init(ks, keystorePassword.toCharArray());
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+
+ // Ensure that reload is called once synchronously, to make sure the file exists etc.
+ reload();
+
+ if (!reloadInterval.isZero()) {
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setName(String.format("%s-%%d", this.getClass().getSimpleName()));
+ t.setDaemon(true);
+ return t;
+ });
+ this.executor.scheduleWithFixedDelay(
+ this::reload,
+ reloadInterval.toMillis(),
+ reloadInterval.toMillis(),
+ TimeUnit.MILLISECONDS);
+ }
+ }
+
+ @VisibleForTesting
+ void reload() {
+ try {
+ reload0();
+ } catch (Exception e) {
+ String msg =
+ "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
+ + "certificates and fail to re-establish connections to Cassandra hosts.";
Review Comment:
Thanks for noticing this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1476669128
##########
upgrade_guide/README.md:
##########
@@ -19,6 +19,17 @@ under the License.
## Upgrade guide
+### NEW VERSION PLACEHOLDER
Review Comment:
@absurdfarce Let me know how you prefer this to be handled - can keep it as an area for other contributors to put their notes, then replace the version placeholder as part of the next release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1464029541
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
Review Comment:
Good feedback; pushed updates
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1463852818
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
Review Comment:
despite the comment are you keeping this `volatile` in case that ever changes i'm guessing?
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
Review Comment:
very tiny nit: one slight difference with previous implementation I just noticed is that previously these would be thrown as checked exceptions in `DefaultSslEngineFactory.buildContext`; these are now wrapped in `RuntimeException` which is unchecked.
I noticed that the `buildContext` is called in `DefaultSslEngineFactory`s constructor and any exceptions thrown will be wrapped in `IllegalStateException`. In this case it would probably be better to not wrap in RuntimeException and just propagate the exceptions as is since they'll be wrapped anyways.
```java
try {
this.sslContext = buildContext(config);
} catch (Exception e) {
throw new IllegalStateException("Cannot initialize SSL Context", e);
}
```
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
Review Comment:
just occurred to be me that in theory we could avoid creating the executor (and thus a thread) if the reload interval is zero right?
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
Review Comment:
from some local testing at least from what I can tell this never creates a thread if nothing is ever scheduled, so maybe this isn't needed (fine with it either way)
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setDaemon(true);
Review Comment:
can you set a name on the thread as well?
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setDaemon(true);
Review Comment:
right now it shows up as `pool-1-thread-1` (although may be different depending on if there are other thread pools using `defaultThreadFactory`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1463975280
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setDaemon(true);
Review Comment:
Great find, thanks for mentioning
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1478784272
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,253 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval)
+ throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
+ CertificateException, IOException {
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ }
+ kmf.init(ks, keystorePassword.toCharArray());
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+
+ // Ensure that reload is called once synchronously, to make sure the file exists etc.
+ reload();
+
+ if (!reloadInterval.isZero()) {
+ this.executor =
+ Executors.newScheduledThreadPool(
+ 1,
+ runnable -> {
+ Thread t = Executors.defaultThreadFactory().newThread(runnable);
+ t.setName(String.format("%s-%%d", this.getClass().getSimpleName()));
+ t.setDaemon(true);
+ return t;
+ });
+ this.executor.scheduleWithFixedDelay(
+ this::reload,
+ reloadInterval.toMillis(),
+ reloadInterval.toMillis(),
+ TimeUnit.MILLISECONDS);
+ }
+ }
+
+ @VisibleForTesting
+ void reload() {
+ try {
+ reload0();
+ } catch (Exception e) {
+ String msg =
+ "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
+ + "certificates and fail to re-establish connections to Cassandra hosts.";
+ logger.warn(msg, e);
+ }
+ }
+
+ private synchronized void reload0()
+ throws NoSuchAlgorithmException, IOException, KeyStoreException, CertificateException,
+ UnrecoverableKeyException {
+ logger.debug("Checking KeyStore file {} for updates", keystorePath);
+
+ final byte[] keyStoreBytes = Files.readAllBytes(keystorePath);
+ final byte[] newDigest = digest(keyStoreBytes);
+ if (lastDigest != null && Arrays.equals(lastDigest, digest(keyStoreBytes))) {
+ logger.debug("KeyStore file content has not changed; skipping update");
+ return;
+ }
+
+ final KeyStore keyStore = KeyStore.getInstance(KEYSTORE_TYPE);
+ try (InputStream inputStream = new ByteArrayInputStream(keyStoreBytes)) {
+ keyStore.load(inputStream, keystorePassword.toCharArray());
+ }
+ KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ kmf.init(keyStore, keystorePassword.toCharArray());
+ logger.info("Detected updates to KeyStore file {}", keystorePath);
+
+ this.spi.keyManager.set((X509ExtendedKeyManager) kmf.getKeyManagers()[0]);
+ this.lastDigest = newDigest;
+ }
Review Comment:
Agreed that it's not in any way essential; I think what you have here will work fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1478776793
##########
upgrade_guide/README.md:
##########
@@ -19,6 +19,17 @@ under the License.
## Upgrade guide
+### NEW VERSION PLACEHOLDER
Review Comment:
Your suggestion seems like a pretty reasonable approach to me @aratno . In the past we'd usually just set the placeholder to whatever we thought the next version would be (knowing full well it might be changed as things moved along) but I have no objection to just leaving a placeholder in the doc. Part of the release checklist could then become "update the placeholder to the correct version string".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1463983979
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
+
+ /**
+ * Create a new {@link ReloadingKeyManagerFactory} with the given keystore file and password,
+ * reloading from the file's content at the given interval. This function will do an initial
+ * reload before returning, to confirm that the file exists and is readable.
+ *
+ * @param keystorePath the keystore file to reload
+ * @param keystorePassword the keystore password
+ * @param reloadInterval the duration between reload attempts. Set to {@link
+ * java.time.Duration#ZERO} to disable scheduled reloading.
+ * @return
+ */
+ public static ReloadingKeyManagerFactory create(
+ Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ KeyManagerFactory kmf;
+ try {
+ kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+
+ KeyStore ks;
+ try (InputStream ksf = Files.newInputStream(keystorePath)) {
+ ks = KeyStore.getInstance(KEYSTORE_TYPE);
+ ks.load(ksf, keystorePassword.toCharArray());
+ } catch (IOException | CertificateException | KeyStoreException | NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ try {
+ kmf.init(ks, keystorePassword.toCharArray());
+ } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+ throw new RuntimeException(e);
+ }
+
+ ReloadingKeyManagerFactory reloadingKeyManagerFactory = new ReloadingKeyManagerFactory(kmf);
+ reloadingKeyManagerFactory.start(keystorePath, keystorePassword, reloadInterval);
+ return reloadingKeyManagerFactory;
+ }
+
+ @VisibleForTesting
+ protected ReloadingKeyManagerFactory(KeyManagerFactory initial) {
+ this(
+ new Spi((X509ExtendedKeyManager) initial.getKeyManagers()[0]),
+ initial.getProvider(),
+ initial.getAlgorithm());
+ }
+
+ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm) {
+ super(spi, provider, algorithm);
+ this.spi = spi;
+ }
+
+ private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+ this.keystorePath = keystorePath;
+ this.keystorePassword = keystorePassword;
+ this.executor =
Review Comment:
I'll avoid constructing the executor if possible, worth doing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19180: Support reloading keystore in cassandra-java-driver [cassandra-java-driver]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #1907:
URL: https://github.com/apache/cassandra-java-driver/pull/1907#discussion_r1463974845
##########
core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java:
##########
@@ -0,0 +1,257 @@
+/*
+ * 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 com.datastax.oss.driver.internal.core.ssl;
+
+import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.Socket;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
+import java.security.PrivateKey;
+import java.security.Provider;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.KeyManagerFactorySpi;
+import javax.net.ssl.ManagerFactoryParameters;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509ExtendedKeyManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ReloadingKeyManagerFactory extends KeyManagerFactory implements AutoCloseable {
+ private static final Logger logger = LoggerFactory.getLogger(ReloadingKeyManagerFactory.class);
+ private static final String KEYSTORE_TYPE = "JKS";
+ private Path keystorePath;
+ private String keystorePassword;
+ private ScheduledExecutorService executor;
+ private final Spi spi;
+
+ // We're using a single thread executor so this shouldn't need to be volatile, since all updates
+ // to lastDigest should come from the same thread
+ private volatile byte[] lastDigest;
Review Comment:
I was intending to keep it volatile. It's less prone to misuse, and the cost is really low. This variable should be set very infrequently. The cost of errors here is low as well, but I feel best about keeping it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org