[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16127230#comment-16127230 ] ASF GitHub Bot commented on DRILL-5663: --- Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/874 > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > Labels: ready-to-commit > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124304#comment-16124304
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132802715
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -93,6 +93,13 @@ drill.exec: {
credentials: true
}
},
+ # Below SSL parameters need to be set for custom transport layer
settings.
+ ssl: {
+keyStorePath: "/keystore.file",
+keyStorePassword: "ks_passwd",
+trustStorePath: "/truststore.file",
+trustStorePassword: "ts_passwd"
--- End diff --
Comments as suggested in previous comment?
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
> Labels: ready-to-commit
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124302#comment-16124302 ] ASF GitHub Bot commented on DRILL-5663: --- Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/874 Changes made and added another commit! > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124260#comment-16124260
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800051
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,77 @@
+/*
+ * 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;
+ }
--- End diff --
Very minor suggestion: these simple returns can also be one-liners like the
"has" methods.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124256#comment-16124256
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800512
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,108 @@
+/**
+ * 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 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());
+ }
+}
--- End diff --
Thanks for the tests!
Maybe delete the extra lines at the end of the file...
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drill
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124255#comment-16124255
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800101
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -139,7 +142,13 @@ public void start() throws Exception {
final ServerConnector serverConnector;
if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) {
- serverConnector = createHttpsConnector();
+ try {
+serverConnector = createHttpsConnector();
+ }
+ catch(DrillException e){
+throw new DrillbitStartupException(e.getMessage(),e);
--- End diff --
Space after comma.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124259#comment-16124259
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800092
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -139,7 +142,13 @@ public void start() throws Exception {
final ServerConnector serverConnector;
if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) {
- serverConnector = createHttpsConnector();
+ try {
+serverConnector = createHttpsConnector();
+ }
+ catch(DrillException e){
--- End diff --
{code}
catch (DrillbitException e) {
{code}
(note spaces)
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124258#comment-16124258
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800128
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -263,18 +272,17 @@ private ServerConnector createHttpsConnector() throws
Exception {
logger.info("Setting up HTTPS connector for web server");
final SslContextFactory sslContextFactory = new SslContextFactory();
+SSLConfig ssl = new SSLConfig(config);
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
+if(ssl.isSslValid()){
--- End diff --
{code}
if (...) {
{code}
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124254#comment-16124254
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132799898
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -93,6 +93,13 @@ drill.exec: {
credentials: true
}
},
+ # Below SSL parameters need to be set for custom transport layer
settings.
+ ssl{
+keyStorePath: "/keystore.file",
+keyStorePassword: "ks_passwd",
+trustStorePath: "/truststore.file",
+trustStorePassword: "ts_passwd"
--- End diff --
ssl{ --> ssl: {
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16124257#comment-16124257
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132800461
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -52,6 +52,14 @@ drill.client: {
drill.tmp-dir: "/tmp"
drill.tmp-dir: ${?DRILL_TMP_DIR}
+javax.net.ssl :
+ {
+ keyStore = "",
+ keyStorePassword = "",
+ trustStore = "",
+ trustStorePassword = ""
+ },
--- End diff --
{code}
javax.net.ssl: {
keyStore = "",
{code}
That is, adjust formatting of the header line and indent the values.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16122208#comment-16122208 ] ASF GitHub Bot commented on DRILL-5663: --- Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/874 Created a separate pull request for DRILL-5712. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16121989#comment-16121989 ] ASF GitHub Bot commented on DRILL-5663: --- Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/874 LGTM. Are you going to put the fix from DRILL_5712 into a separate PR? > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16120752#comment-16120752
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r132323524
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -93,6 +93,13 @@ drill.exec: {
credentials: true
}
},
+ # Below SSL parameters need to be set for custom transport layer
settings.
+ ssl{
+keyStore: "/keystore.file",
--- End diff --
The names in this example file are not consistent with the names uses in
ExecConstants. keyStore and trustStore instead of keyStorePath, trustStorePath.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16116834#comment-16116834 ] ASF GitHub Bot commented on DRILL-5663: --- Github user parthchandra commented on a diff in the pull request: https://github.com/apache/drill/pull/874#discussion_r131703561 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -122,10 +122,10 @@ 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"; --- End diff -- Hmm, not entirely convinced, but I'm fine as long as you have provided backward compatibility for existing users. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16113349#comment-16113349 ] ASF GitHub Bot commented on DRILL-5663: --- Github user sindhurirayavaram commented on a diff in the pull request: https://github.com/apache/drill/pull/874#discussion_r131238139 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -122,10 +122,10 @@ 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"; --- End diff -- Using the _javax_ property name for Drill property might be confusing for users, since changing that value inside Drill configuration will not reflect on the actual System property. So to avoid that confusion this property was renamed taking example from [Hadoop](https://hadoop.apache.org/docs/r2.7.2/hadoop-mapreduce-client/hadoop-mapreduce-client-core/EncryptedShuffle.html) and [IBM](https://www.ibm.com/support/knowledgecenter/en/SSZH4A_5.0.6/com.ibm.worklight.help.doc/admin/r_ssl_certificate_keystore_setup.html). Though Hadoop provides separate parameter for client/server whereas Drill has only server side config, these properties can be renamed to include _.server_ if needed. But making this name change will not affect Drill users already using the older property since backward compatibility is provided here. It will work seamlessly for them. All the below scenarios are tested. 1) Setting javax.ssl.* properties in drill-override.conf 2) Setting javax.ssl.* properties as system options with -D option. 3) Setting ssl.* properties in drill-override.conf So if a user provide the property values only as system property, then both Drill and JSSE will still be consuming it and work as expected. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16113187#comment-16113187 ] ASF GitHub Bot commented on DRILL-5663: --- Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/874 Hadoop uses different property names for configuring ssl properties. We can provide backward compatibility where we can still use the java properties in drill-override.conf. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16111806#comment-16111806 ] ASF GitHub Bot commented on DRILL-5663: --- Github user parthchandra commented on a diff in the pull request: https://github.com/apache/drill/pull/874#discussion_r130993406 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -122,10 +122,10 @@ 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"; --- End diff -- These are predefined system property names and we should continue to use these. System.getProperties will get these values correctly even if the user does not provide them in the config file. Also, JSSE looks for these properties. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16111807#comment-16111807
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r131009531
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void secondTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void thirdTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ SSLConfig sslv = new SSLConfig(config);
+ assertEquals("root", sslv.getkeystorePassword());
+ assertEquals("/root", sslv.getkeystorePath());
+}
+ }
+}
--- End diff --
@paul-rogers No, truststore path and truststore password are optional. The
patch correctly handles the creation of the SSLContext by setting these params
only if not empty.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16111346#comment-16111346
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user sindhurirayavaram commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130939205
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -214,7 +214,7 @@ drill.exec: {
}
# Below SSL parameters need to be set for custom transport layer settings.
-javax.net.ssl {
+ssl{
keyStore: "/keystore.file",
--- End diff --
Can you look into this paul? This an example I got.
SSL certificate keystore location.
Default: conf/default.keystore.
ssl.keystore.path can be configured in 2 ways:
By using the relative path, for example,
ssl.keystore.path=conf/default.keystore, where the root directory is \server\.
By using the absolute path, for example,
ssl.keystore.path=/opt/wl/default.keystore for UNIX, or
C:\\wl\\default.keystore for Windows.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105652#comment-16105652
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130183909
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
--- End diff --
Good to give these names that suggest the test case:
`testMissingKeystorePath` for example.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105657#comment-16105657
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130183742
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
--- End diff --
`// Expected`
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105670#comment-16105670
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184855
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
+
+ private final String keystorePath;
--- End diff --
We call this `keyStorePath` in the config, (upper-case the "S") so might as
well use the same convention here.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105668#comment-16105668
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130181769
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
+
+ private final String keystorePath;
+
+ private final String keystorePassword;
+
+ private final String truststorePassword;
+
+ private final String truststorePath;
+
+ public boolean isValid = false;
+
+ 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() || keystorePassword.isEmpty()) {
+throw new DrillException(" *.ssl.keyStorePath and/or
*.ssl.keyStorePassword in the configuration file can't be empty.");
--- End diff --
For message, use the constant so the user has exact information:
```
...( ExecConstants.HTTP_KEYSTORE_PATH + " and/or " ...
```
Even better, help the user:
* Keystore path is provided, but password is missing.
* Password is provided, but keystore path is not set.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105658#comment-16105658
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130180741
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -214,7 +214,7 @@ drill.exec: {
}
# Below SSL parameters need to be set for custom transport layer settings.
-javax.net.ssl {
+ssl{
--- End diff --
`ssl: {` under `drill.exec`
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105666#comment-16105666
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184027
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void secondTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void thirdTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ SSLConfig sslv = new SSLConfig(config);
+ assertEquals("root", sslv.getkeystorePassword());
+ assertEquals("/root", sslv.getkeystorePath());
+}
+ }
+}
+
--- End diff --
Delete extra lines
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105660#comment-16105660
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184346
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void secondTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void thirdTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
--- End diff --
Again, just a nit. Since all you need is the config, you can use the
`ConfigBuilder` directly:
```
DrillConfig config = new ConfigBuilder()
.put(...)
.build();
```
In this case, no need for the try/catch block and the operator fixture
since we are not using the memory allocator.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105669#comment-16105669
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184655
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
+
+ private final String keystorePath;
+
+ private final String keystorePassword;
+
+ private final String truststorePassword;
+
+ private final String truststorePath;
+
+ public boolean isValid = false;
+
+ 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() || keystorePassword.isEmpty()) {
+throw new DrillException(" *.ssl.keyStorePath and/or
*.ssl.keyStorePassword in the configuration file can't be empty.");
+ }
+ isValid = true;
+}
+ }
+
+ public String getkeystorePath() {
+return keystorePath;
+ }
+
+ public String getkeystorePassword() {
+return keystorePassword;
+ }
+
+ public String gettruststorePath() {
+return truststorePath;
+ }
+
+ public String gettruststorePassword() {
--- End diff --
`getTrustStorePassword` That is, use `functionCase`... Here and above.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105651#comment-16105651
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130183662
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
--- End diff --
Not a big deal, but handy shortcut "fluent" style:
```
OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder()
.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "root")
.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
```
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105659#comment-16105659
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130183310
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -338,3 +338,11 @@ drill.exec: {
drill.jdbc: {
batch_queue_throttling_threshold: 100
}
+javax.net.ssl.keyStore = ""
+ssl.keyStorePath = ${?javax.net.ssl.keyStore}
+javax.net.ssl.keyStorePassword = ""
+ssl.keyStorePassword = ${?javax.net.ssl.keyStorePassword}
+javax.net.ssl.trustStore = ""
+ssl.trustStorePath = ${?javax.net.ssl.trustStore}
+javax.net.ssl.trustStorePassword = ""
+ssl.trustStorePassword = ${?javax.net.ssl.trustStorePassword}
--- End diff --
Add a comment about the `javax` versions being legacy and deprecated.
```
javax.net.ssl: {
keyStorePath = ...
...
}
drill.exec.ssl {
keyStorePath = "",
...
}
```
Better, move the `drill.exec` stuff under the existing `drill.exec` section.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105665#comment-16105665
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130181979
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -139,7 +142,13 @@ public void start() throws Exception {
final ServerConnector serverConnector;
if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) {
- serverConnector = createHttpsConnector();
+ try {
+serverConnector = createHttpsConnector();
+ }
+ catch(DrillException e){
+throw new DrillbitStartupException(e.getMessage(),e.getCause());
--- End diff --
`e.getCause()` --> `e` else you'll pass `null` to the startup exception.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105661#comment-16105661
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184900
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
--- End diff --
Not used, so can be deleted.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105667#comment-16105667
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130184493
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,89 @@
+/**
+ * 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 org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.SSLConfig;
+import org.apache.drill.test.OperatorFixture;
+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 firstTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void secondTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ try {
+SSLConfig sslv = new SSLConfig(config);
+fail();
+ } catch (Exception e) {
+assertTrue(e instanceof DrillException);
+ }
+}
+ }
+
+ @Test
+ public void thirdTest() throws Exception {
+
+OperatorFixture.OperatorFixtureBuilder builder =
OperatorFixture.builder();
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PASSWORD,
"root");
+builder.configBuilder().put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+try (OperatorFixture fixture = builder.build()) {
+ DrillConfig config = fixture.config();
+ SSLConfig sslv = new SSLConfig(config);
+ assertEquals("root", sslv.getkeystorePassword());
+ assertEquals("/root", sslv.getkeystorePath());
+}
+ }
+}
--- End diff --
Do we need to test the trust store options? Are they subject to the same
"neither or both" constraints?
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105655#comment-16105655 ] ASF GitHub Bot commented on DRILL-5663: --- Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/874#discussion_r130181300 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -122,10 +122,10 @@ 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 = "ssl.keyStorePath"; --- End diff -- `drill.exec.ssl.keyStorePath` and below. > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105654#comment-16105654
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130182034
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -263,18 +272,17 @@ private ServerConnector createHttpsConnector() throws
Exception {
logger.info("Setting up HTTPS connector for web server");
final SslContextFactory sslContextFactory = new SslContextFactory();
+SSLConfig ssl = new SSLConfig(config);
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
+if(ssl.isValid){
--- End diff --
`if (ssl.isValid) {`
Here, `isValid` means `isSslEnabled`.
Better: `ssl.isEnabled()`
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105662#comment-16105662
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130182919
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -341,6 +349,7 @@ private ServerConnector createHttpsConnector() throws
Exception {
sslConnector.setPort(config.getInt(ExecConstants.HTTP_PORT));
return sslConnector;
+
--- End diff --
Remove blank line.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105664#comment-16105664
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130182809
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -263,18 +272,17 @@ private ServerConnector createHttpsConnector() throws
Exception {
logger.info("Setting up HTTPS connector for web server");
final SslContextFactory sslContextFactory = new SslContextFactory();
+SSLConfig ssl = new SSLConfig(config);
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
+if(ssl.isValid){
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.gettruststorePath().isEmpty()){
--- End diff --
Better: `ssl.hasTrustStore()`
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105663#comment-16105663
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130182610
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
+
+ private final String keystorePath;
+
+ private final String keystorePassword;
+
+ private final String truststorePassword;
+
+ private final String truststorePath;
+
+ public boolean isValid = false;
+
+ 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() || keystorePassword.isEmpty()) {
+throw new DrillException(" *.ssl.keyStorePath and/or
*.ssl.keyStorePassword in the configuration file can't be empty.");
+ }
+ isValid = true;
--- End diff --
Confused by this one. Don't we have three cases?
* Neither property set. No SSL.
* Both properties set, use SSL.
* Incorrectly set, exception, this object is not created.
So `isValid` in the sense of "is the data valid" is unnecessary: the object
is not created if invalid. But, "isSslEnabled" is useful. Rather than exposing
a member variable, perhaps expose a function `isSslEnabled()`.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105653#comment-16105653
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130180974
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -214,7 +214,7 @@ drill.exec: {
}
# Below SSL parameters need to be set for custom transport layer settings.
-javax.net.ssl {
+ssl{
keyStore: "/keystore.file",
--- End diff --
This is an existing property. Generally, a leading slash means an absolute
path. So, do we expect the keystore to be in the Linux root directory? This
would be very unusual. Or, do we mean "keystore.file" relative to some implied
root directory? What is that root?
If we do not have a root, should we look relative to the $DRILL_CONF
directory (normally /usr/drill or $DRILL_HOME/conf or given by the --site
option.)
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16105656#comment-16105656
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r130182211
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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 static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(SSLConfig.class);
+
+ private final String keystorePath;
+
+ private final String keystorePassword;
+
+ private final String truststorePassword;
+
+ private final String truststorePath;
+
+ public boolean isValid = false;
--- End diff --
`final`
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16087847#comment-16087847
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r127529165
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws
Exception {
final SslContextFactory sslContextFactory = new SslContextFactory();
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
- 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));
-}
+final boolean hasPath =
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
--- End diff --
To avoid this double-checking, standard practice is:
* Use a blank value to indicate the value is unset.
* Provide a default (blank) value in `drill-module.conf`
Then, the code can just be:
```
String path = config.getString(ExecConstants.HTTP_KEYSTORE_PATH).trim();
if (! path.isEmpty()) {
// do stuff
```
Also, if the user is expected to set this key, please provide an example in
`drill-override-example.conf`.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
> Fix For: 1.11.0
>
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16087849#comment-16087849
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r127530311
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -289,30 +297,19 @@ private ServerConnector createHttpsConnector() throws
Exception {
final DateTime now = DateTime.now();
// Create builder for certificate attributes
- final X500NameBuilder nameBuilder =
- new X500NameBuilder(BCStyle.INSTANCE)
- .addRDN(BCStyle.OU, "Apache Drill (auth-generated)")
- .addRDN(BCStyle.O, "Apache Software Foundation
(auto-generated)")
- .addRDN(BCStyle.CN,
workManager.getContext().getEndpoint().getAddress());
+ final X500NameBuilder nameBuilder = new
X500NameBuilder(BCStyle.INSTANCE).addRDN(BCStyle.OU, "Apache Drill
(auth-generated)").addRDN(BCStyle.O, "Apache Software Foundation
(auto-generated)").addRDN(BCStyle.CN,
workManager.getContext().getEndpoint().getAddress());
--- End diff --
Can we keep the previous one-argument-per-line, indented style? Much easier
to read.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
> Fix For: 1.11.0
>
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16087846#comment-16087846
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r127529372
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws
Exception {
final SslContextFactory sslContextFactory = new SslContextFactory();
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
- 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));
-}
+final boolean hasPath =
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
+final boolean hasPassword =
config.hasPath(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+// Check if both keypath and password are present or not
+if (hasPath && hasPassword) {
+ final String pathValue =
config.getString(ExecConstants.HTTP_KEYSTORE_PATH);
+ final String passwordValue =
config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+ // checking if any one of them is null or empty
+ if (!Strings.isNullOrEmpty(pathValue) &&
!Strings.isNullOrEmpty(passwordValue)) {
--- End diff --
Path can't be null here, so `! pathValue.isEmpty()` works also.
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
> Fix For: 1.11.0
>
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16087848#comment-16087848
]
ASF GitHub Bot commented on DRILL-5663:
---
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/874#discussion_r127530151
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
---
@@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws
Exception {
final SslContextFactory sslContextFactory = new SslContextFactory();
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
- 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));
-}
+final boolean hasPath =
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
+final boolean hasPassword =
config.hasPath(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+// Check if both keypath and password are present or not
+if (hasPath && hasPassword) {
+ final String pathValue =
config.getString(ExecConstants.HTTP_KEYSTORE_PATH);
+ final String passwordValue =
config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD);
+
+ // checking if any one of them is null or empty
+ if (!Strings.isNullOrEmpty(pathValue) &&
!Strings.isNullOrEmpty(passwordValue)) {
+sslContextFactory.setKeyStorePath(pathValue);
+sslContextFactory.setKeyStorePassword(passwordValue);
+ }
+
+ // Throwing an exception if anyone of them is null or empty
+ else {
+throw new DrillbitStartupException("keystorepath and/or
keystorepassword can't be empty.");
--- End diff --
Please provide a bit of a clearer message. Also, provide the actual path
name. This will be the critical message for folks that made mistake, the
Drillbit won't start, and they have to figure out what's what.
Maybe,
```
"To enable web UI security, both " + ExecConstants.HTTP_KEYSTORE_PATH + "
and " + ... + " are required.";
```
Also, this is just for the Web UI? In that case, maybe just disable the web
UI, but don't fail the Drillbit.
```
"Web UI misconfiguration: web UI is disabled."
```
> Drillbit fails to start when only keystore path is provided without keystore
> password.
> --
>
> Key: DRILL-5663
> URL: https://issues.apache.org/jira/browse/DRILL-5663
> Project: Apache Drill
> Issue Type: Bug
>Reporter: Sorabh Hamirwasia
>Assignee: Sindhuri Ramanarayan Rayavaram
> Fix For: 1.11.0
>
>
> When we configure keystore path without keystore password inside
> drill-override.conf for WebServer, then Drillbit fails to start. We should
> explicitly check for either both being present or both being absent. If any
> one of them is only present then throw startup exception for Drill.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5663) Drillbit fails to start when only keystore path is provided without keystore password.
[ https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16084919#comment-16084919 ] ASF GitHub Bot commented on DRILL-5663: --- GitHub user sindhurirayavaram opened a pull request: https://github.com/apache/drill/pull/874 DRILL-5663: Drillbit fails to start when only keystore path is provided without keystore password. When we configure keystore path without keystore password inside drill-override.conf for WebServer, then Drillbit fails to start. We should explicitly check for either both being present or both being absent. If any one of them is only present then we should throw startup exception for Drill. In the code change, added checks for both keystorepath and keystorepasword. You can merge this pull request into a Git repository by running: $ git pull https://github.com/sindhurirayavaram/drill master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/874.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #874 commit b9c6ee1cddffaf1472235adba7ac5597959f9c8e Author: Sindhuri Rayavaram Date: 2017-07-10T23:59:36Z DRILL-5663:added check for keystorePassword commit ca1f56776911c2c312212e3546264defe812c7c2 Author: Sindhuri Rayavaram Date: 2017-07-11T20:26:51Z DRILL-5663: Added a check for keystorePassword commit 59b581d5402da104bcaee6ffa16050c5ecf3a90b Author: Sindhuri Rayavaram Date: 2017-07-12T22:50:52Z DRILL-5663: Added check for keystorepassword commit e3cc91bea31921a8c540e981db2b7cbccae9ff83 Author: Sindhuri Rayavaram Date: 2017-07-12T23:35:20Z DRILL-5663: Added check for keystorepassword > Drillbit fails to start when only keystore path is provided without keystore > password. > -- > > Key: DRILL-5663 > URL: https://issues.apache.org/jira/browse/DRILL-5663 > Project: Apache Drill > Issue Type: Bug >Reporter: Sorabh Hamirwasia >Assignee: Sindhuri Ramanarayan Rayavaram > Fix For: 1.11.0 > > > When we configure keystore path without keystore password inside > drill-override.conf for WebServer, then Drillbit fails to start. We should > explicitly check for either both being present or both being absent. If any > one of them is only present then throw startup exception for Drill. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
