This is an automated email from the ASF dual-hosted git repository.
rsivaram pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 10de7c9 KAFKA-8241; Handle configs without truststore for broker
keystore update (#6585)
10de7c9 is described below
commit 10de7c9512341794bdc0d9bc18b85f79ef9fc3b4
Author: Rajini Sivaram <rajinisiva...@googlemail.com>
AuthorDate: Wed Apr 17 18:17:40 2019 +0100
KAFKA-8241; Handle configs without truststore for broker keystore update
(#6585)
Reviewers: Manikumar Reddy <manikumar.re...@gmail.com>
---
.../kafka/common/security/ssl/SslFactory.java | 41 ++++++++-----
.../kafka/common/security/ssl/SslFactoryTest.java | 67 ++++++++++++++++++++++
2 files changed, 93 insertions(+), 15 deletions(-)
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
index 0c5094d..a03d1bc 100644
--- a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
+++ b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
@@ -161,7 +161,8 @@ public class SslFactory implements Reconfigurable {
createSSLContext(keystore, truststore);
}
} catch (Exception e) {
- throw new ConfigException("Validation of dynamic config update
failed", e);
+ log.debug("Validation of dynamic config update of SSL
keystore/truststore failed", e);
+ throw new ConfigException("Validation of dynamic config update of
SSL keystore/truststore failed: " + e);
}
}
@@ -178,21 +179,24 @@ public class SslFactory implements Reconfigurable {
this.keystore = keystore;
this.truststore = truststore;
} catch (Exception e) {
- throw new ConfigException("Reconfiguration of SSL
keystore/truststore failed", e);
+ log.debug("Reconfiguration of SSL keystore/truststore failed",
e);
+ throw new ConfigException("Reconfiguration of SSL
keystore/truststore failed: " + e);
}
}
}
private SecurityStore maybeCreateNewKeystore(Map<String, ?> configs) {
- boolean keystoreChanged =
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG),
keystore.type) ||
-
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG),
keystore.path) ||
-
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG),
keystore.password) ||
-
!Objects.equals(configs.get(SslConfigs.SSL_KEY_PASSWORD_CONFIG),
keystore.keyPassword);
-
- if (!keystoreChanged) {
- keystoreChanged = keystore.modified();
+ boolean keystoreChanged = false;
+ if (keystore != null) {
+ keystoreChanged =
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG),
keystore.type) ||
+
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG),
keystore.path) ||
+
!Objects.equals(configs.get(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG),
keystore.password) ||
+
!Objects.equals(configs.get(SslConfigs.SSL_KEY_PASSWORD_CONFIG),
keystore.keyPassword);
+ if (!keystoreChanged) {
+ keystoreChanged = keystore.modified();
+ }
}
- if (keystoreChanged) {
+ if (keystoreChanged || keystore == null) {
return createKeystore((String)
configs.get(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG),
(String)
configs.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG),
(Password)
configs.get(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG),
@@ -202,14 +206,18 @@ public class SslFactory implements Reconfigurable {
}
private SecurityStore maybeCreateNewTruststore(Map<String, ?> configs) {
- boolean truststoreChanged =
!Objects.equals(configs.get(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG),
truststore.type) ||
+ boolean truststoreChanged = false;
+ if (truststore != null) {
+ truststoreChanged =
!Objects.equals(configs.get(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG),
truststore.type) ||
!Objects.equals(configs.get(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG),
truststore.path) ||
!Objects.equals(configs.get(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG),
truststore.password);
- if (!truststoreChanged) {
- truststoreChanged = truststore.modified();
+ if (!truststoreChanged) {
+ truststoreChanged = truststore.modified();
+ }
}
- if (truststoreChanged) {
+
+ if (truststoreChanged || truststore == null) {
return createTruststore((String)
configs.get(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG),
(String)
configs.get(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG),
(Password)
configs.get(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG));
@@ -244,8 +252,11 @@ public class SslFactory implements Reconfigurable {
boolean verifyKeystore = keystore != null && keystore != this.keystore;
boolean verifyTruststore = truststore != null && truststore !=
this.truststore;
if (verifyKeystore || verifyTruststore) {
- if (this.keystore == null)
+ if (this.keystore == null && keystore != null)
throw new ConfigException("Cannot add SSL keystore to an
existing listener for which no keystore was configured.");
+ if (this.truststore == null && truststore != null)
+ throw new ConfigException("Cannot add SSL truststore to an
existing listener for which no truststore was configured.");
+
if (keystoreVerifiableUsingTruststore) {
SSLConfigValidatorEngine.validate(this, sslContext,
this.sslContext);
SSLConfigValidatorEngine.validate(this, this.sslContext,
sslContext);
diff --git
a/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
b/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
index 6c4a239..11035d0 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
@@ -25,6 +25,7 @@ import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLHandshakeException;
+import org.apache.kafka.common.config.ConfigException;
import org.apache.kafka.common.config.SslConfigs;
import org.apache.kafka.common.config.types.Password;
import org.apache.kafka.test.TestSslUtils;
@@ -135,6 +136,72 @@ public class SslFactoryTest {
}
@Test
+ public void testReconfigurationWithoutTruststore() throws Exception {
+ File trustStoreFile = File.createTempFile("truststore", ".jks");
+ Map<String, Object> sslConfig = TestSslUtils
+ .createSslConfig(false, true, Mode.SERVER, trustStoreFile,
"server");
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG);
+ SslFactory sslFactory = new SslFactory(Mode.SERVER);
+ sslFactory.configure(sslConfig);
+ SSLContext sslContext = sslFactory.sslContext();
+ assertNotNull("SSL context not created", sslContext);
+ assertSame("SSL context recreated unnecessarily", sslContext,
sslFactory.sslContext());
+ assertFalse(sslFactory.createSslEngine("localhost",
0).getUseClientMode());
+
+ trustStoreFile = File.createTempFile("truststore", ".jks");
+ sslConfig = TestSslUtils.createSslConfig(false, true, Mode.SERVER,
trustStoreFile, "server");
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_LOCATION_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_TRUSTSTORE_TYPE_CONFIG);
+ sslFactory.reconfigure(sslConfig);
+ assertNotSame("SSL context not recreated", sslContext,
sslFactory.sslContext());
+
+ trustStoreFile = File.createTempFile("truststore", ".jks");
+ sslConfig = TestSslUtils.createSslConfig(false, true, Mode.SERVER,
trustStoreFile, "server");
+ try {
+ sslFactory.reconfigure(sslConfig);
+ fail("Truststore configured dynamically for listener without
previous truststore");
+ } catch (ConfigException e) {
+ // Expected exception
+ }
+ }
+
+ @Test
+ public void testReconfigurationWithoutKeystore() throws Exception {
+ File trustStoreFile = File.createTempFile("truststore", ".jks");
+ Map<String, Object> sslConfig = TestSslUtils
+ .createSslConfig(false, true, Mode.SERVER, trustStoreFile,
"server");
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG);
+ SslFactory sslFactory = new SslFactory(Mode.SERVER);
+ sslFactory.configure(sslConfig);
+ SSLContext sslContext = sslFactory.sslContext();
+ assertNotNull("SSL context not created", sslContext);
+ assertSame("SSL context recreated unnecessarily", sslContext,
sslFactory.sslContext());
+ assertFalse(sslFactory.createSslEngine("localhost",
0).getUseClientMode());
+
+ trustStoreFile = File.createTempFile("truststore", ".jks");
+ sslConfig = TestSslUtils.createSslConfig(false, true, Mode.SERVER,
trustStoreFile, "server");
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_PASSWORD_CONFIG);
+ sslConfig.remove(SslConfigs.SSL_KEYSTORE_TYPE_CONFIG);
+ sslFactory.reconfigure(sslConfig);
+ assertNotSame("SSL context not recreated", sslContext,
sslFactory.sslContext());
+
+ trustStoreFile = File.createTempFile("truststore", ".jks");
+ sslConfig = TestSslUtils.createSslConfig(false, true, Mode.SERVER,
trustStoreFile, "server");
+ try {
+ sslFactory.reconfigure(sslConfig);
+ fail("Keystore configured dynamically for listener without
previous keystore");
+ } catch (ConfigException e) {
+ // Expected exception
+ }
+ }
+
+ @Test
public void testKeyStoreTrustStoreValidation() throws Exception {
File trustStoreFile = File.createTempFile("truststore", ".jks");
Map<String, Object> serverSslConfig =
TestSslUtils.createSslConfig(false, true,
