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 <sindhurirayavaram@srayava-E422.local>
Authored: Thu Jul 13 16:07:53 2017 -0700
Committer: Jinfeng Ni <jni@apache.org>
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
|