SEC-1473 Allow no Truststore password

confluentinc · Oct 13, 2020 · 379b4d1 · 379b4d1
1 parent 1a1d53b
commit 379b4d1
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 7 deletions.
diff --git a/core/src/main/java/io/confluent/rest/Application.java b/core/src/main/java/io/confluent/rest/Application.java
@@ -602,7 +602,9 @@ public void join() throws InterruptedException {
    * @throws Exception If the application fails to stop
    */
   public void stop() throws Exception {
-    server.stop();
+    if (server != null) {
+      server.stop();
+    }
   }
 
   final void doShutdown() {

diff --git a/core/src/main/java/io/confluent/rest/ApplicationServer.java b/core/src/main/java/io/confluent/rest/ApplicationServer.java
@@ -283,7 +283,9 @@ private Path getWatchLocation(RestConfig config) {
     return keystorePath;
   }
 
+  // CHECKSTYLE_RULES.OFF: CyclomaticComplexity|NPathComplexity
   private SslContextFactory createSslContextFactory(RestConfig config) {
+    // CHECKSTYLE_RULES.ON: CyclomaticComplexity|NPathComplexity
     SslContextFactory sslContextFactory = new SslContextFactory.Server();
     if (!config.getString(RestConfig.SSL_KEYSTORE_LOCATION_CONFIG).isEmpty()) {
       sslContextFactory.setKeyStorePath(
@@ -341,9 +343,11 @@ private SslContextFactory createSslContextFactory(RestConfig config) {
       sslContextFactory.setTrustStorePath(
               config.getString(RestConfig.SSL_TRUSTSTORE_LOCATION_CONFIG)
       );
-      sslContextFactory.setTrustStorePassword(
-              config.getPassword(RestConfig.SSL_TRUSTSTORE_PASSWORD_CONFIG).value()
-      );
+      if (config.getPassword(RestConfig.SSL_TRUSTSTORE_PASSWORD_CONFIG) != null) {
+        sslContextFactory.setTrustStorePassword(
+            config.getPassword(RestConfig.SSL_TRUSTSTORE_PASSWORD_CONFIG).value()
+        );
+      }
       sslContextFactory.setTrustStoreType(
               config.getString(RestConfig.SSL_TRUSTSTORE_TYPE_CONFIG)
       );

diff --git a/core/src/main/java/io/confluent/rest/RestConfig.java b/core/src/main/java/io/confluent/rest/RestConfig.java
@@ -168,7 +168,7 @@ public class RestConfig extends AbstractConfig {
   public static final String SSL_TRUSTSTORE_PASSWORD_CONFIG = "ssl.truststore.password";
   protected static final String SSL_TRUSTSTORE_PASSWORD_DOC =
       "The store password for the trust store file.";
-  protected static final String SSL_TRUSTSTORE_PASSWORD_DEFAULT = "";
+  protected static final String SSL_TRUSTSTORE_PASSWORD_DEFAULT = null;
   public static final String SSL_TRUSTSTORE_TYPE_CONFIG = "ssl.truststore.type";
   protected static final String SSL_TRUSTSTORE_TYPE_DOC =
       "The type of trust store file.";

diff --git a/core/src/test/java/io/confluent/rest/SslTest.java b/core/src/test/java/io/confluent/rest/SslTest.java
@@ -49,7 +49,6 @@
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLException;
-import javax.net.ssl.SSLHandshakeException;
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
 import javax.ws.rs.Produces;
@@ -73,7 +72,7 @@ public class SslTest {
 
   public static final String SSL_PASSWORD = "test1234";
   public static final String EXPECTED_200_MSG = "Response status must be 200.";
-  public static final int CERT_RELOAD_WAIT_TIME = 20000;
+  public static final int CERT_RELOAD_WAIT_TIME = 30000;
 
   @Before
   public void setUp() throws Exception {
@@ -116,6 +115,15 @@ private void configServerTruststore(Properties props) {
     props.put(RestConfig.SSL_TRUSTSTORE_PASSWORD_CONFIG, SSL_PASSWORD);
   }
 
+  private void configServerTruststore(Properties props, String password) {
+    props.put(RestConfig.SSL_TRUSTSTORE_LOCATION_CONFIG, trustStore.getAbsolutePath());
+    props.put(RestConfig.SSL_TRUSTSTORE_PASSWORD_CONFIG, password);
+  }
+
+  private void configServerNoTruststorePassword(Properties props) {
+    props.put(RestConfig.SSL_TRUSTSTORE_LOCATION_CONFIG, trustStore.getAbsolutePath());
+  }
+
   private void enableSslClientAuth(Properties props) {
     props.put(RestConfig.SSL_CLIENT_AUTH_CONFIG, true);
   }
@@ -271,6 +279,45 @@ public void testHttpsWithNoClientCertAndNoServerTruststore() throws Exception {
     }
   }
 
+  @Test(expected = IOException.class)
+  public void testHttpsWithEmptyStringTruststorePassword() throws Exception {
+    Properties props = new Properties();
+    String uri = "https://localhost:8080";
+    props.put(RestConfig.LISTENERS_CONFIG, uri);
+    configServerKeystore(props);
+    configServerTruststore(props, "");
+    TestRestConfig config = new TestRestConfig(props);
+    SslTestApplication app = new SslTestApplication(config);
+    try {
+      // Empty string is a valid password, but it's not the password the truststore uses
+      // The app should fail at startup with:
+      // java.io.IOException: Keystore was tampered with, or password was incorrect
+      app.start();
+    } finally {
+      app.stop();
+    }
+  }
+
+  @Test
+  public void testHttpsWithNoTruststorePassword() throws Exception {
+    Properties props = new Properties();
+    String uri = "https://localhost:8080";
+    props.put(RestConfig.LISTENERS_CONFIG, uri);
+    configServerKeystore(props);
+    configServerNoTruststorePassword(props);
+    TestRestConfig config = new TestRestConfig(props);
+    SslTestApplication app = new SslTestApplication(config);
+    try {
+      // With no password set (null), verification of the truststore is disabled
+      app.start();
+
+      int statusCode = makeGetRequest(uri + "/test");
+      assertEquals(EXPECTED_200_MSG, 200, statusCode);
+    } finally {
+      app.stop();
+    }
+  }
+
   @Test(expected = SocketException.class)
   public void testHttpsWithAuthAndBadClientCert() throws Exception {
     Properties props = new Properties();