Browse Source

KAFKA-8241; Handle configs without truststore for broker keystore update (#6585)

Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
pull/6594/head
Rajini Sivaram 6 years ago committed by GitHub
parent
commit
7f9b9a60da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 41
      clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
  2. 67
      clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java

41
clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java

@ -161,7 +161,8 @@ public class SslFactory implements Reconfigurable { @@ -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 { @@ -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 { @@ -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 { @@ -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);

67
clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java

@ -25,6 +25,7 @@ import javax.net.ssl.SSLContext; @@ -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;
@ -134,6 +135,72 @@ public class SslFactoryTest { @@ -134,6 +135,72 @@ public class SslFactoryTest {
assertSame("SSL context recreated unnecessarily", sslContext, sslFactory.sslContext());
}
@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");

Loading…
Cancel
Save