From 379b4d1a409f0980ca57735a22b94e9ce30afe23 Mon Sep 17 00:00:00 2001 From: Aneel Nazareth Date: Tue, 13 Oct 2020 15:49:10 -0500 Subject: [PATCH] SEC-1473 Allow no Truststore password --- .../java/io/confluent/rest/Application.java | 4 +- .../io/confluent/rest/ApplicationServer.java | 10 ++-- .../java/io/confluent/rest/RestConfig.java | 2 +- .../test/java/io/confluent/rest/SslTest.java | 51 ++++++++++++++++++- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/io/confluent/rest/Application.java b/core/src/main/java/io/confluent/rest/Application.java index cf5b41435d..1570114ee4 100644 --- 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 index 0ea98fcff2..4fa402c342 100644 --- 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 index b6053fd060..9bfa3481f8 100644 --- 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 index 78715dfe6e..cb497a38e1 100644 --- 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();