DRILL-5663: Drillbit fails to start when only keystore path is provided without keystore password.
Added comments for the drill-override-example.conf file close #874 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/f1e1dfe0 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/f1e1dfe0 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/f1e1dfe0 Branch: refs/heads/master Commit: f1e1dfe09c3e57bdeb313d01940275b3fc83535f Parents: 109b2c0 Author: Sindhuri Rayavaram <[email protected]> Authored: Thu Jul 13 16:07:53 2017 -0700 Committer: Jinfeng Ni <[email protected]> Committed: Mon Aug 14 22:19:24 2017 -0700 ---------------------------------------------------------------------- .../src/resources/drill-override-example.conf | 19 ++-- .../org/apache/drill/exec/ExecConstants.java | 8 +- .../java/org/apache/drill/exec/SSLConfig.java | 69 +++++++++++++ .../drill/exec/server/rest/WebServer.java | 30 +++--- .../src/main/resources/drill-module.conf | 14 ++- .../org/apache/drill/exec/TestSSLConfig.java | 100 +++++++++++++++++++ 6 files changed, 216 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/distribution/src/resources/drill-override-example.conf ---------------------------------------------------------------------- diff --git a/distribution/src/resources/drill-override-example.conf b/distribution/src/resources/drill-override-example.conf index 8010f85..0b66f68 100644 --- a/distribution/src/resources/drill-override-example.conf +++ b/distribution/src/resources/drill-override-example.conf @@ -93,6 +93,17 @@ drill.exec: { credentials: true } }, + # Below SSL parameters need to be set for custom transport layer settings. + ssl: { + #If not provided then the default value is java system property javax.net.ssl.keyStore value + keyStorePath: "/keystore.file", + #If not provided then the default value is java system property javax.net.ssl.keyStorePassword value + keyStorePassword: "ks_passwd", + #If not provided then the default value is java system property javax.net.ssl.trustStore value + trustStorePath: "/truststore.file", + #If not provided then the default value is java system property javax.net.ssl.trustStorePassword value + trustStorePassword: "ts_passwd" + }, functions: ["org.apache.drill.expr.fn.impl"], network: { start: 35000 @@ -213,11 +224,5 @@ drill.exec: { default_temporary_workspace: "dfs.tmp" } -# Below SSL parameters need to be set for custom transport layer settings. -javax.net.ssl { - keyStore: "/keystore.file", - keyStorePassword: "ks_passwd", - trustStore: "/truststore.file", - trustStorePassword: "ts_passwd" -} + http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java index 97cb321..a88875f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java @@ -122,10 +122,10 @@ public interface ExecConstants { String HTTP_SESSION_MEMORY_RESERVATION = "drill.exec.http.session.memory.reservation"; String HTTP_SESSION_MEMORY_MAXIMUM = "drill.exec.http.session.memory.maximum"; String HTTP_SESSION_MAX_IDLE_SECS = "drill.exec.http.session_max_idle_secs"; - String HTTP_KEYSTORE_PATH = "javax.net.ssl.keyStore"; - String HTTP_KEYSTORE_PASSWORD = "javax.net.ssl.keyStorePassword"; - String HTTP_TRUSTSTORE_PATH = "javax.net.ssl.trustStore"; - String HTTP_TRUSTSTORE_PASSWORD = "javax.net.ssl.trustStorePassword"; + String HTTP_KEYSTORE_PATH = "drill.exec.ssl.keyStorePath"; + String HTTP_KEYSTORE_PASSWORD = "drill.exec.ssl.keyStorePassword"; + String HTTP_TRUSTSTORE_PATH = "drill.exec.ssl.trustStorePath"; + String HTTP_TRUSTSTORE_PASSWORD = "drill.exec.ssl.trustStorePassword"; String SYS_STORE_PROVIDER_CLASS = "drill.exec.sys.store.provider.class"; String SYS_STORE_PROVIDER_LOCAL_PATH = "drill.exec.sys.store.provider.local.path"; String SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE = "drill.exec.sys.store.provider.local.write"; http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java b/exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java new file mode 100644 index 0000000..c6d6374 --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec; + +import com.typesafe.config.Config; +import org.apache.drill.common.exceptions.DrillException; + +public class SSLConfig { + + private final String keystorePath; + + private final String keystorePassword; + + private final String truststorePath; + + private final String truststorePassword; + + + public SSLConfig(Config config) throws DrillException { + + keystorePath = config.getString(ExecConstants.HTTP_KEYSTORE_PATH).trim(); + + keystorePassword = config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD).trim(); + + truststorePath = config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH).trim(); + + truststorePassword = config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD).trim(); + + /*If keystorePath or keystorePassword is provided in the configuration file use that*/ + if (!keystorePath.isEmpty() || !keystorePassword.isEmpty()) { + if (keystorePath.isEmpty()) { + throw new DrillException(" *.ssl.keyStorePath in the configuration file is empty, but *.ssl.keyStorePassword is set"); + } + else if (keystorePassword.isEmpty()){ + throw new DrillException(" *.ssl.keyStorePassword in the configuration file is empty, but *.ssl.keyStorePath is set "); + } + + } + } + + public boolean isSslValid() { return !keystorePath.isEmpty() && !keystorePassword.isEmpty(); } + + public String getKeyStorePath() { return keystorePath; } + + public String getKeyStorePassword() { return keystorePassword; } + + public boolean hasTrustStorePath() { return !truststorePath.isEmpty(); } + + public boolean hasTrustStorePassword() { return !truststorePassword.isEmpty(); } + + public String getTrustStorePath() { return truststorePath; } + + public String getTrustStorePassword() { return truststorePassword; } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java index b3fb692..1706b71 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java @@ -24,8 +24,11 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.drill.common.exceptions.DrillException; import org.apache.drill.common.config.DrillConfig; import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.SSLConfig; +import org.apache.drill.exec.exception.DrillbitStartupException; import org.apache.drill.exec.rpc.security.plain.PlainFactory; import org.apache.drill.exec.server.BootStrapContext; import org.apache.drill.exec.server.rest.auth.DrillRestLoginService; @@ -139,7 +142,12 @@ public class WebServer implements AutoCloseable { final ServerConnector serverConnector; if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) { - serverConnector = createHttpsConnector(); + try { + serverConnector = createHttpsConnector(); + } + catch(DrillException e){ + throw new DrillbitStartupException(e.getMessage(), e); + } } else { serverConnector = createHttpConnector(); } @@ -263,18 +271,16 @@ public class WebServer implements AutoCloseable { logger.info("Setting up HTTPS connector for web server"); final SslContextFactory sslContextFactory = new SslContextFactory(); - - if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) && - !Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) { + SSLConfig ssl = new SSLConfig(config); + if(ssl.isSslValid()){ logger.info("Using configured SSL settings for web server"); - sslContextFactory.setKeyStorePath(config.getString(ExecConstants.HTTP_KEYSTORE_PATH)); - sslContextFactory.setKeyStorePassword(config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD)); - - // TrustStore and TrustStore password are optional - if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PATH)) { - sslContextFactory.setTrustStorePath(config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH)); - if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)) { - sslContextFactory.setTrustStorePassword(config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)); + + sslContextFactory.setKeyStorePath(ssl.getKeyStorePath()); + sslContextFactory.setKeyStorePassword(ssl.getKeyStorePassword()); + if(ssl.hasTrustStorePath()){ + sslContextFactory.setTrustStorePath(ssl.getTrustStorePath()); + if(ssl.hasTrustStorePassword()){ + sslContextFactory.setTrustStorePassword(ssl.getTrustStorePassword()); } } } else { http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/exec/java-exec/src/main/resources/drill-module.conf ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/resources/drill-module.conf b/exec/java-exec/src/main/resources/drill-module.conf index 146df1f..437862e 100644 --- a/exec/java-exec/src/main/resources/drill-module.conf +++ b/exec/java-exec/src/main/resources/drill-module.conf @@ -51,7 +51,12 @@ drill.client: { // By default ${DRILL_TMP_DIR} is used if set or ${drill.tmp-dir} if it's been overridden. drill.tmp-dir: "/tmp" drill.tmp-dir: ${?DRILL_TMP_DIR} - +javax.net.ssl: { + keyStore = "", + keyStorePassword = "", + trustStore = "", + trustStorePassword = "" +}, drill.exec: { cluster-id: "drillbits1" rpc: { @@ -134,6 +139,13 @@ drill.exec: { } } }, + //setting javax variables for ssl configurations is being deprecated. + ssl: { + keyStorePath = ${?javax.net.ssl.keyStore} + keyStorePassword = ${?javax.net.ssl.keyStorePassword} + trustStorePath = ${?javax.net.ssl.trustStore} + trustStorePassword = ${?javax.net.ssl.trustStorePassword} + }, network: { start: 35000 }, http://git-wip-us.apache.org/repos/asf/drill/blob/f1e1dfe0/exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java b/exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java new file mode 100644 index 0000000..e83fc05 --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java @@ -0,0 +1,100 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.drill.exec; + + +import org.apache.drill.common.exceptions.DrillException; +import org.apache.drill.test.ConfigBuilder; +import org.junit.Test; +import static junit.framework.TestCase.fail; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class TestSSLConfig { + + @Test + public void testMissingKeystorePath() throws Exception { + + ConfigBuilder config = new ConfigBuilder(); + config.put(ExecConstants.HTTP_KEYSTORE_PATH, ""); + config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "root"); + try { + SSLConfig sslv = new SSLConfig(config.build()); + fail(); + //Expected + } catch (Exception e) { + assertTrue(e instanceof DrillException); + } + } + + @Test + public void testMissingKeystorePassword() throws Exception { + + ConfigBuilder config = new ConfigBuilder(); + config.put(ExecConstants.HTTP_KEYSTORE_PATH, "/root"); + config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, ""); + try { + SSLConfig sslv = new SSLConfig(config.build()); + fail(); + //Expected + } catch (Exception e) { + assertTrue(e instanceof DrillException); + } + } + + @Test + public void testForKeystoreConfig() throws Exception { + + ConfigBuilder config = new ConfigBuilder(); + config.put(ExecConstants.HTTP_KEYSTORE_PATH, "/root"); + config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "root"); + try { + SSLConfig sslv = new SSLConfig(config.build()); + assertEquals("/root", sslv.getKeyStorePath()); + assertEquals("root", sslv.getKeyStorePassword()); + } catch (Exception e) { + fail(); + + } + } + + @Test + public void testForBackwardCompatability() throws Exception { + + ConfigBuilder config = new ConfigBuilder(); + config.put("javax.net.ssl.keyStore", "/root"); + config.put("javax.net.ssl.keyStorePassword", "root"); + SSLConfig sslv = new SSLConfig(config.build()); + assertEquals("/root",sslv.getKeyStorePath()); + assertEquals("root", sslv.getKeyStorePassword()); + } + + @Test + public void testForTrustStore() throws Exception { + + ConfigBuilder config = new ConfigBuilder(); + config.put(ExecConstants.HTTP_TRUSTSTORE_PATH, "/root"); + config.put(ExecConstants.HTTP_TRUSTSTORE_PASSWORD, "root"); + SSLConfig sslv = new SSLConfig(config.build()); + assertEquals(true, sslv.hasTrustStorePath()); + assertEquals(true,sslv.hasTrustStorePassword()); + assertEquals("/root",sslv.getTrustStorePath()); + assertEquals("root",sslv.getTrustStorePassword()); + } +} \ No newline at end of file
