You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2020/04/23 18:26:28 UTC

[GitHub] [zeppelin] alexott commented on a change in pull request #3747: [ZEPPELIN-4774] TLS - create Keystores with PEM files

alexott commented on a change in pull request #3747:
URL: https://github.com/apache/zeppelin/pull/3747#discussion_r414018198



##########
File path: conf/zeppelin-site.xml.template
##########
@@ -456,6 +456,38 @@
 </property>
 -->
 
+<!--
+<property>
+  <name>zeppelin.ssl.pem.key</name>
+  <value></value>
+  <description>Path to a TLS key file saved as pem.</description>
+</property>
+-->
+
+<!--
+<property>
+  <name>zeppelin.ssl.pem.key.password</name>
+  <value></value>
+  <description>Password of the TLS key file saved as pem.</description>
+</property>
+-->
+
+<!--
+<property>
+  <name>zeppelin.ssl.pem.cert</name>
+  <value></value>
+  <description>Path to a TLS full certificate chain file saved as pem.</description>
+</property>
+-->
+
+<!--
+<property>
+  <name>zeppelin.ssl.pem.ca</name>
+  <value></value>
+  <description>Path to a TLS certificate file containing all client certificates</description>

Review comment:
       Is it client certificate? not the Certificate Authority?

##########
File path: docs/setup/operation/configuration.md
##########
@@ -173,6 +173,30 @@ If both are defined, then the **environment variables** will take priority.
     <td></td>
     <td></td>
   </tr>
+  <tr>
+    <td><h6 class="properties">ZEPPELIN_SSL_PEM_KEY</h6></td>
+    <td><h6 class="properties">zeppelin.ssl.pem.key</h6></td>
+    <td></td>
+    <td></td>
+  </tr>
+  <tr>
+    <td><h6 class="properties">ZEPPELIN_SSL_PEM_KEY_PASSWORD</h6></td>
+    <td><h6 class="properties">zeppelin.ssl.pem.key.password</h6></td>
+    <td></td>
+    <td></td>
+  </tr>
+  <tr>
+    <td><h6 class="properties">ZEPPELIN_SSL_PEM_CERT</h6></td>
+    <td><h6 class="properties">zeppelin.ssl.pem.cert</h6></td>
+    <td></td>
+    <td></td>
+  </tr>
+  <tr>
+    <td><h6 class="properties">ZEPPELIN_SSL_PEM_CA</h6></td>
+    <td><h6 class="properties">zeppelin.ssl.pem.ca</h6></td>
+    <td></td>
+    <td></td>

Review comment:
       Should we add description of what this mean?

##########
File path: zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
##########
@@ -438,19 +440,38 @@ private static void setupClusterManagerServer(ServiceLocator serviceLocator) {
   private static SslContextFactory getSslContextFactory(ZeppelinConfiguration conf) {
     SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
 
-    // Set keystore
-    sslContextFactory.setKeyStorePath(conf.getKeyStorePath());
-    sslContextFactory.setKeyStoreType(conf.getKeyStoreType());
-    sslContextFactory.setKeyStorePassword(conf.getKeyStorePassword());
-    sslContextFactory.setKeyManagerPassword(conf.getKeyManagerPassword());
-
-    if (conf.useClientAuth()) {
-      sslContextFactory.setNeedClientAuth(conf.useClientAuth());
-
-      // Set truststore
-      sslContextFactory.setTrustStorePath(conf.getTrustStorePath());
-      sslContextFactory.setTrustStoreType(conf.getTrustStoreType());
-      sslContextFactory.setTrustStorePassword(conf.getTrustStorePassword());
+    // Check for PEM files
+    if (StringUtils.isNoneBlank(conf.getPemKeyFile(), conf.getPemCertFile())) {
+      File pemKey = new File(conf.getPemKeyFile());
+      File pemCert = new File(conf.getPemCertFile());
+      try {
+        String password = conf.getPemKeyPassword();
+        sslContextFactory.setKeyStore(PEMImporter.loadKeyStore(pemCert, pemKey, password));
+        sslContextFactory.setKeyStoreType("JKS");
+        sslContextFactory.setKeyStorePassword(password);
+        if (conf.useClientAuth() && StringUtils.isNotBlank(conf.getPemCAFile())) {
+          File pemCA = new File(conf.getPemCAFile());

Review comment:
       here as well

##########
File path: zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
##########
@@ -438,19 +440,38 @@ private static void setupClusterManagerServer(ServiceLocator serviceLocator) {
   private static SslContextFactory getSslContextFactory(ZeppelinConfiguration conf) {
     SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
 
-    // Set keystore
-    sslContextFactory.setKeyStorePath(conf.getKeyStorePath());
-    sslContextFactory.setKeyStoreType(conf.getKeyStoreType());
-    sslContextFactory.setKeyStorePassword(conf.getKeyStorePassword());
-    sslContextFactory.setKeyManagerPassword(conf.getKeyManagerPassword());
-
-    if (conf.useClientAuth()) {
-      sslContextFactory.setNeedClientAuth(conf.useClientAuth());
-
-      // Set truststore
-      sslContextFactory.setTrustStorePath(conf.getTrustStorePath());
-      sslContextFactory.setTrustStoreType(conf.getTrustStoreType());
-      sslContextFactory.setTrustStorePassword(conf.getTrustStorePassword());
+    // Check for PEM files
+    if (StringUtils.isNoneBlank(conf.getPemKeyFile(), conf.getPemCertFile())) {
+      File pemKey = new File(conf.getPemKeyFile());
+      File pemCert = new File(conf.getPemCertFile());

Review comment:
       I would be make explicit check for presence of files, for easier identification of errors...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org