kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rsiva...@apache.org
Subject [kafka] branch trunk updated: KAFKA-8241; Handle configs without truststore for broker keystore update (#6585)
Date Wed, 17 Apr 2019 17:17:54 GMT
This is an automated email from the ASF dual-hosted git repository.

rsivaram pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 7f9b9a6  KAFKA-8241; Handle configs without truststore for broker keystore update
(#6585)
7f9b9a6 is described below

commit 7f9b9a60dab990efb79df89a6603e78d9ce5a34f
Author: Rajini Sivaram <rajinisivaram@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.reddy@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,


Mime
View raw message