drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j..@apache.org
Subject [04/13] drill git commit: DRILL-5663: Drillbit fails to start when only keystore path is provided without keystore password.
Date Tue, 15 Aug 2017 13:44:16 GMT
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


Mime
View raw message