You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jacek-lewandowski <gi...@git.apache.org> on 2014/10/10 01:31:53 UTC

[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

GitHub user jacek-lewandowski opened a pull request:

    https://github.com/apache/spark/pull/2739

    SPARK-3883 SSL support for HttpServer and Akka

     - Introduced SSLOptions object
     - SSLOptions is created by SecurityManager
     - SSLOptions configures Akka and Jetty to use SSL
     - SSLOptions uses property file which is node-local to set SSL settings
     - Provided utility methods to determine the proper Akka protocol for Akka requests and to configure SSL socket factory for URL connections
     - Added tests cases for AkkaUtils, FileServer, SSLOptions and SecurityManager

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jacek-lewandowski/spark SPARK-3883

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/2739.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 #2739
    
----
commit fcc3d29866b73f1c3553ae1861ff8cc9c7829258
Author: Jacek Lewandowski <le...@gmail.com>
Date:   2014-10-08T23:41:05Z

    SPARK-3883 SSL support for HttpServer and Akka
    
     - Introduced SSLOptions object
     - SSLOptions is created by SecurityManager
     - SSLOptions configures Akka and Jetty to use SSL
     - SSLOptions uses property file which is node-local to set SSL settings
     - Provided utility methods to determine the proper Akka protocol for Akka requests and to configure SSL socket factory for URL connections
     - Added tests cases for AkkaUtils, FileServer, SSLOptions and SecurityManager

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19250068
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    That's the tricky part on anything non-Yarn (to be frank, I'm not 100% familiar with how those others work).
    
    Basically what you need is a two step process:
    - launcher talks to cluster manager, makes needed resources available somehow.
    - executors are launched and either those resources are available, or it gets them from the launcher.
    
    On Yarn, this works using the distributed cache. The Yarn client will upload whatever files you ask to Yarn (see ClientBase.scala, I think the method is `prepareLocalResources` or something), and they'll be available for the executors when they launch. So there's very little to do here: executors pick needed files (trust store, cert) from their working dir, and SSL options are passed to the executors on the command line (like other akka options today).
    
    If something like the distributed cache is not available, it does become a little trickier. Basically, the executor needs to launch, download needed resources from the launcher, and only then start Akka. You can pass ssl options on the command line of the executor if needed, but no files, so it should be able to connect back to the HTTP server running in the launcher to download resources.
    
    If that's not possible (or too hard today), I'm okay with punting that to a different task and requiring the trust store / cert to be available on all hosts for non-Yarn modes. But I really think we should support automatically distributing the SSL conf in Yarn mode, since it should be pretty straight-forward.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18975378
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    +
    +  private[spark] val (sslSocketFactory, hostnameVerifier) = if (sslOptions.enabled) {
    +    val trustStoreManagers =
    +      for (trustStore <- sslOptions.trustStore) yield {
    +        val ks = KeyStore.getInstance(KeyStore.getDefaultType)
    +        ks.load(FileUtils.openInputStream(sslOptions.trustStore.get),
    +          sslOptions.trustStorePassword.get.toCharArray)
    +
    +        val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
    +        tmf.init(ks)
    +        tmf.getTrustManagers
    +      }
    +
    +    lazy val credulousTrustStoreManagers = Array({
    +      logWarning("Using 'accept-all' trust manager for SSL connections.")
    +      new X509TrustManager {
    +        override def getAcceptedIssuers: Array[X509Certificate] = null
    +
    +        override def checkClientTrusted(x509Certificates: Array[X509Certificate], s: String) {}
    +
    +        override def checkServerTrusted(x509Certificates: Array[X509Certificate], s: String) {}
    +      }: TrustManager
    +    })
    +
    +    val sslContext = SSLContext.getInstance(sslOptions.protocol.getOrElse("Default"))
    +    sslContext.init(null, trustStoreManagers getOrElse credulousTrustStoreManagers, null)
    +
    +    val hostVerifier = new HostnameVerifier {
    +      override def verify(s: String, sslSession: SSLSession): Boolean = true
    +    }
    +
    +    (Some(sslContext.getSocketFactory), Some(hostVerifier))
    +  } else {
    +    val sslContext = SSLContext.getDefault
    --- End diff --
    
    Is this doing anything?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-58594031
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18973129
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    --- End diff --
    
    Shouldn't this be a list? I'm pretty sure it's allowed to have multiple protocols enabled.
    
    Also, given POODLE, it's probably a good idea to expose the setting to disable specific protocols, and have it include "SSLv3" by default.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60177556
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22051/consoleFull) for   PR 2739 at commit [`fcc3d29`](https://github.com/apache/spark/commit/fcc3d29866b73f1c3553ae1861ff8cc9c7829258).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-58594021
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18972815
  
    --- Diff: conf/ssl.conf.template ---
    @@ -0,0 +1,27 @@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    Not an expert at Apache licensing, but other config files in `conf/` don't have a license header.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19166051
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    Ah. Another thing. I haven't looked at your code to see if it's handled, but akka requires executors to open a listening socket, all the related files (such as the trust store) also need to somehow be sent to the executors... it would kinda suck to force everybody to copy the files manually to all possible places where executors may run.
    
    In Yarn mode that should be easy (using the distributed cache), but no idea whether that's easy or even possible in standalone or mesos...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19252007
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    The problem is that in standalone mode, Spark Master + Spark Workers play a role of the cluster manager. Launcher cannot talk to them without setting up SSL connection. Moreover, Spark Master cannot talk to Spark Workers without setting up SSL connection. 
    
    I think that we are talking about two different things :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18972906
  
    --- Diff: conf/ssl.conf.template ---
    @@ -0,0 +1,27 @@
    +#
    +# 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.
    +#
    +
    +# Spark SSL settings
    +
    +# ssl.enabled               true
    +# ssl.keyStore              /path/to/your/keyStore
    +# ssl.keyStorePassword      password
    +# ssl.keyPassword           password
    +# ssl.trustStore            /path/to/your/trustStore
    +# ssl.trustStorePassword    password
    +# ssl.enabledAlgorithms     [TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_CBC_SHA]
    +# ssl.protocol              SSLv3
    --- End diff --
    
    This is probably the wrong example to use now after POODLE. :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60182904
  
    Ah, also, about the branch thing... it would probably be better to have this go into master first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60175872
  
    btw. @vanzin how to make Jenkins run the tests on this branch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60177158
  
    @vanzin This PR doesn't secure data transfers anyway, because Spark uses raw communication to exchange the real data. This is intended to secure mainly control messages, JARS, application settings, like command line arguments and Spark configuration, because they may include passwords to access third party data sources from executors. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18975328
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    --- End diff --
    
    You declared the parameters in the opposite order and then use named arguments to call that method... that's rather confusing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60312679
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22080/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-58915520
  
    #1980 is a PR to add SSL to the web UI, which might benefit from SSLOptions.  Do you want to comment on that PR's strategy for configuration, etc?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18975351
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -18,7 +18,11 @@
     package org.apache.spark
     
     import java.net.{Authenticator, PasswordAuthentication}
    +import java.security.KeyStore
    +import java.security.cert.X509Certificate
    +import javax.net.ssl._
     
    +import org.apache.commons.io.FileUtils
    --- End diff --
    
    Please avoid commons-io since it's not stable across the Hadoop versions we have to support. Prefer Guava instead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60182360
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22051/consoleFull) for   PR 2739 at commit [`fcc3d29`](https://github.com/apache/spark/commit/fcc3d29866b73f1c3553ae1861ff8cc9c7829258).
     * This patch **fails** unit tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19139861
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -18,7 +18,11 @@
     package org.apache.spark
     
     import java.net.{Authenticator, PasswordAuthentication}
    +import java.security.KeyStore
    +import java.security.cert.X509Certificate
    +import javax.net.ssl._
     
    +import org.apache.commons.io.FileUtils
    --- End diff --
    
    Ok


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60176829
  
    I suppose that this is because this pr is not against the master branch


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60182365
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22051/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18974920
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    +   * - SPARK_CONF_DIR/ssl.conf
    +   * - SPARK_HOME/conf/ssl.conf
    +   */
    +  def defaultConfigFile: Option[File] = {
    +    val specifiedFile = Option(System.getenv("SPARK_SSL_CONFIG_FILE")).map(new File(_))
    +    val sparkConfDir = Option(System.getenv("SPARK_CONF_DIR")).map(new File(_))
    +    val sparkHomeConfDir = Option(System.getenv("SPARK_HOME"))
    +      .map(new File(_, "conf"))
    +    val defaultFile = (sparkConfDir orElse sparkHomeConfDir).map(new File(_, "ssl.conf"))
    +
    +    specifiedFile orElse defaultFile
    +  }
    +
    +  /**
    +   * Loads the given properties file with failover to empty Properties object.
    +   */
    +  def load(configFile: File): Properties = {
    +    logInfo(s"Loading SSL configuration from $configFile")
    +    try {
    +      val props = new Properties()
    +      val reader = new FileReader(configFile)
    +      try {
    +        props.load(reader)
    +        props.put("sslConfigurationFileLocation", configFile.getAbsolutePath)
    +        props
    +      } finally {
    +        reader.close()
    +      }
    +    } catch {
    +      case ex: Throwable =>
    +        logWarning(s"The SSL configuration file ${configFile.getAbsolutePath} " +
    +          s"could not be loaded. The underlying exception was: ${ex.getMessage}")
    +        new Properties
    +    }
    +  }
    +
    +  /**
    +   * Resolves SSLOptions settings from a given Spark configuration object at a given namespace.
    +   * If SSL settings were loaded from the configuration file, ``sslConfigurationFileLocation``
    +   * property is present in the Spark configuration. The parent directory of that location is used
    +   * as a base directory to resolve relative paths to keystore and truststore.
    +   */
    +  def parse(conf: SparkConf, ns: String): SSLOptions = {
    +    val configFilePath = conf.getOption("sslConfigurationFileLocation")
    --- End diff --
    
    This should follow the convention of other spark options (i.e. "spark.something.other").


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18975159
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    +   * - SPARK_CONF_DIR/ssl.conf
    +   * - SPARK_HOME/conf/ssl.conf
    +   */
    +  def defaultConfigFile: Option[File] = {
    +    val specifiedFile = Option(System.getenv("SPARK_SSL_CONFIG_FILE")).map(new File(_))
    +    val sparkConfDir = Option(System.getenv("SPARK_CONF_DIR")).map(new File(_))
    +    val sparkHomeConfDir = Option(System.getenv("SPARK_HOME"))
    +      .map(new File(_, "conf"))
    +    val defaultFile = (sparkConfDir orElse sparkHomeConfDir).map(new File(_, "ssl.conf"))
    +
    +    specifiedFile orElse defaultFile
    +  }
    +
    +  /**
    +   * Loads the given properties file with failover to empty Properties object.
    +   */
    +  def load(configFile: File): Properties = {
    +    logInfo(s"Loading SSL configuration from $configFile")
    +    try {
    +      val props = new Properties()
    +      val reader = new FileReader(configFile)
    +      try {
    +        props.load(reader)
    +        props.put("sslConfigurationFileLocation", configFile.getAbsolutePath)
    +        props
    +      } finally {
    +        reader.close()
    +      }
    +    } catch {
    +      case ex: Throwable =>
    +        logWarning(s"The SSL configuration file ${configFile.getAbsolutePath} " +
    +          s"could not be loaded. The underlying exception was: ${ex.getMessage}")
    +        new Properties
    +    }
    +  }
    +
    +  /**
    +   * Resolves SSLOptions settings from a given Spark configuration object at a given namespace.
    +   * If SSL settings were loaded from the configuration file, ``sslConfigurationFileLocation``
    +   * property is present in the Spark configuration. The parent directory of that location is used
    +   * as a base directory to resolve relative paths to keystore and truststore.
    +   */
    +  def parse(conf: SparkConf, ns: String): SSLOptions = {
    +    val configFilePath = conf.getOption("sslConfigurationFileLocation")
    +
    +    def makeFile(pathString: String): File = {
    +      val path = Paths.get(pathString)
    +
    +      if (path.isAbsolute || configFilePath.isEmpty) {
    +        path.toFile
    +      } else {
    +        new File(FilenameUtils.concat(new File(configFilePath.get).getParent, pathString))
    +      }
    +    }
    +
    +    val enabled = conf.getBoolean(s"$ns.enabled", defaultValue = false)
    +    val keyStore = Try(conf.get(s"$ns.keyStore")).toOption.map(makeFile)
    +    val keyStorePassword = Try(conf.get(s"$ns.keyStorePassword")).toOption
    +    val keyPassword = Try(conf.get(s"$ns.keyPassword")).toOption
    +    val trustStore = Try(conf.get(s"$ns.trustStore")).toOption.map(makeFile)
    +    val trustStorePassword = Try(conf.get(s"$ns.trustStorePassword")).toOption
    +    val protocol = Try(conf.get(s"$ns.protocol")).toOption
    +    val enabledAlgorithms = Try(conf.get(s"$ns.enabledAlgorithms")).toOption
    +      .map(_.trim.dropWhile(_ == '[')
    +      .takeWhile(_ != ']')).map(_.split(",").map(_.trim).toSet)
    +      .getOrElse(Set.empty)
    +
    +    new SSLOptions(enabled, keyStore, keyStorePassword, keyPassword, trustStore, trustStorePassword,
    +      protocol, enabledAlgorithms)
    +  }
    +
    +  /**
    +   * Loads the SSL configuration file. If ``spark.ssl.configFile`` property is in the system
    +   * properties, it is assumed it contains the SSL configuration file location to be used.
    +   * Otherwise, it uses the location returned by [[SSLOptions.defaultConfigFile]].
    +   */
    +  def load(): Properties  = {
    --- End diff --
    
    I'm a little confused about all the different ways to load configuration here. Why do you need more than one way? Why isn't everything configured in the already existing Spark configuration file that is processed by spark-submit and friends?
    
    If you need different SSL configurations for different subsystems, you can do that using namespaces (as I think is what you were aiming at by the method at L140), but I don't see the point of having 2 or 3 or more methods just dealing with loading this configuration in different ways.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19251701
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    What you refer to as "workers" is generally referred to as "executors" inside Spark, to avoid confusion with the "Worker" daemons in the standalone cluster manager. Still, that doesn't change anything I've written so far.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60297509
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22079/consoleFull) for   PR 2739 at commit [`77591cb`](https://github.com/apache/spark/commit/77591cb020fd538bef349f56fa485f8a15c5b645).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19253270
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    @vanzin I understand what you mean. I just thought that you didn't understand me :)
    
    To be more precise, there is one more type of communication:
    Spark Master talking to Spark Worker, this needs to be set up independently from any launcher
    
    For this one, you need SSL configured on all the nodes prior to running any application. My understanding of SSL is that it is not intended to authenticate particular clients (applications) but rather authenticating hosts. In this regard, a single SSL configuration per host is enough. At the end, we want to encrypt the data which are exchanged and a single SSL configuration per host does the job. 
    
    We can introduce further improvements later, though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-59407360
  
    Hi @jacek-lewandowski, I like this because it's trying to support more than just the Web UI, but I think the configuration handling is sort of confusing and overengineered. Feels like a simpler approach based on "all configuration goes through SparkConf" would be much simpler to handle and maintain (both in the code and by admins deploying Spark). What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-59431848
  
    Thanks for review @vanzin 
    I will read all your comments soon, probably during a weekend... currently a little bit overloaded


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19246619
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    +   * - SPARK_CONF_DIR/ssl.conf
    +   * - SPARK_HOME/conf/ssl.conf
    +   */
    +  def defaultConfigFile: Option[File] = {
    +    val specifiedFile = Option(System.getenv("SPARK_SSL_CONFIG_FILE")).map(new File(_))
    +    val sparkConfDir = Option(System.getenv("SPARK_CONF_DIR")).map(new File(_))
    +    val sparkHomeConfDir = Option(System.getenv("SPARK_HOME"))
    +      .map(new File(_, "conf"))
    +    val defaultFile = (sparkConfDir orElse sparkHomeConfDir).map(new File(_, "ssl.conf"))
    +
    +    specifiedFile orElse defaultFile
    +  }
    +
    +  /**
    +   * Loads the given properties file with failover to empty Properties object.
    +   */
    +  def load(configFile: File): Properties = {
    +    logInfo(s"Loading SSL configuration from $configFile")
    +    try {
    +      val props = new Properties()
    +      val reader = new FileReader(configFile)
    +      try {
    +        props.load(reader)
    +        props.put("sslConfigurationFileLocation", configFile.getAbsolutePath)
    +        props
    +      } finally {
    +        reader.close()
    +      }
    +    } catch {
    +      case ex: Throwable =>
    +        logWarning(s"The SSL configuration file ${configFile.getAbsolutePath} " +
    +          s"could not be loaded. The underlying exception was: ${ex.getMessage}")
    +        new Properties
    +    }
    +  }
    +
    +  /**
    +   * Resolves SSLOptions settings from a given Spark configuration object at a given namespace.
    +   * If SSL settings were loaded from the configuration file, ``sslConfigurationFileLocation``
    +   * property is present in the Spark configuration. The parent directory of that location is used
    +   * as a base directory to resolve relative paths to keystore and truststore.
    +   */
    +  def parse(conf: SparkConf, ns: String): SSLOptions = {
    +    val configFilePath = conf.getOption("sslConfigurationFileLocation")
    +
    +    def makeFile(pathString: String): File = {
    +      val path = Paths.get(pathString)
    +
    +      if (path.isAbsolute || configFilePath.isEmpty) {
    +        path.toFile
    +      } else {
    +        new File(FilenameUtils.concat(new File(configFilePath.get).getParent, pathString))
    +      }
    +    }
    +
    +    val enabled = conf.getBoolean(s"$ns.enabled", defaultValue = false)
    +    val keyStore = Try(conf.get(s"$ns.keyStore")).toOption.map(makeFile)
    +    val keyStorePassword = Try(conf.get(s"$ns.keyStorePassword")).toOption
    +    val keyPassword = Try(conf.get(s"$ns.keyPassword")).toOption
    +    val trustStore = Try(conf.get(s"$ns.trustStore")).toOption.map(makeFile)
    +    val trustStorePassword = Try(conf.get(s"$ns.trustStorePassword")).toOption
    +    val protocol = Try(conf.get(s"$ns.protocol")).toOption
    +    val enabledAlgorithms = Try(conf.get(s"$ns.enabledAlgorithms")).toOption
    +      .map(_.trim.dropWhile(_ == '[')
    +      .takeWhile(_ != ']')).map(_.split(",").map(_.trim).toSet)
    +      .getOrElse(Set.empty)
    +
    +    new SSLOptions(enabled, keyStore, keyStorePassword, keyPassword, trustStore, trustStorePassword,
    +      protocol, enabledAlgorithms)
    +  }
    +
    +  /**
    +   * Loads the SSL configuration file. If ``spark.ssl.configFile`` property is in the system
    +   * properties, it is assumed it contains the SSL configuration file location to be used.
    +   * Otherwise, it uses the location returned by [[SSLOptions.defaultConfigFile]].
    +   */
    +  def load(): Properties  = {
    --- End diff --
    
    There is a one method of loading the configuration. If you mean the config location resolving sequence - I think it is quite obvious - firstly it take the system property, then env variable, then config directory and then home/conf directory. 
    
    For the namespaces - yes, my aim was to make it possible to store multiple SSL configurations in a single config file, under different namespaces. In particular, different settings for data, control messages, web UI. 
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65279317
  
    @vanzin already did that. Now I'm running tests - is there a new procedure for testing? Or just sbt clean assembly test?
    
    I can see one test failure which I can reproduce on master as well. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65356699
  
    Here is the new PR https://github.com/apache/spark/pull/3571


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65330260
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24059/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19139391
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    Having the path to `ssl.conf` file in `SparkConf` is pointless, because the executor needs to use the SSL configuration prior to receiving the configuration. This is entirely host-based setting, not the application based setting.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60176191
  
    Let me see if I can trigger tests for you - otherwise an admin will have to intervene.
    
    Also, let me think about the configuration thing some more. To be frank, I'm not really that concerned about the Master/Worker configuration, I think those are not that interesting; I'm way more interested in applications using SSL, since that's when you're passing potentially sensitive data around.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60312673
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22080/consoleFull) for   PR 2739 at commit [`f24d854`](https://github.com/apache/spark/commit/f24d854704101d781ab6b15c49caf51c9b4ba9ce).
     * This patch **fails** unit tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19245926
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    Actually executor hosts have to be configured to work with SSL. Admin needs to create keystores and truststores, so why wouldn't he generate ssl config file?
    
    For the other comment - the only way I can see to make it possible to put these settings into Spark config is to add them as -Dspark.xxx=blablabla strings to the `spark.executor.extraJavaOptions` config option. It is resolved and used before an executor is spawned by a worker. 
    
    Still, I cannot see the reason for doing it this way. Remember that SSL works for all Akka communication, also between Driver, Workers and Master. So, just to run workers you need to provide the SSL configuration on each host which you want to use, because they are unable to connect to Master otherwise. 
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60297784
  
    @JoshRosen will it be retested automatically after commit ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19246989
  
    --- Diff: conf/ssl.conf.template ---
    @@ -0,0 +1,27 @@
    +#
    +# 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.
    +#
    +
    +# Spark SSL settings
    +
    +# ssl.enabled               true
    +# ssl.keyStore              /path/to/your/keyStore
    +# ssl.keyStorePassword      password
    +# ssl.keyPassword           password
    +# ssl.trustStore            /path/to/your/trustStore
    +# ssl.trustStorePassword    password
    +# ssl.enabledAlgorithms     [TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_CBC_SHA]
    +# ssl.protocol              SSLv3
    --- End diff --
    
    Yeah, that's right, I'll change the default protocol


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60592413
  
    Please also update the documentation.  docs/security.md and the big comment header at the top of SecurityManager.scala


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19139794
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    --- End diff --
    
    Good point


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65322917
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24059/consoleFull) for   PR 2739 at commit [`f24d854`](https://github.com/apache/spark/commit/f24d854704101d781ab6b15c49caf51c9b4ba9ce).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65356790
  
    So - can I close this one?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18974767
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    I'm not a fan of using env variables. We should be trying to discourage their use, not add new ones. I'd suggest instead an approach based on SparkConf. Have a "spark.ssl.config" option, and if it's not set, load it with `SSLOptions.getClass.getResourceAsStream("/ssl.conf")` (the conf dir is added to the classpath by the scripts in bin/).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19246425
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    I know you need the configuration. But I think it's both an unwanted burden and too restricting to force all machines in a cluster to have a static SSL configuration. Think of maintaing that in a cluster with 1000 machines. Also, that would make it really hard to have different users use different trust stores / certificates.
    
    What I'm suggesting is looking at an approach where the launcher distributes the configuration and needed files somehow. That way, the configuration and other files only need to exist on the launcher node - which may not even be part of the cluster itself. That's trivial in Yarn, since Spark uses its distributed cache to distribute files to all workers before launching the executors (or the AM), but it might be tricky in other modes where such distribution mechanism does not exist.
    
    Still, my main point is that requiring all these files to be kept in sync in a large cluster is a little too much to ask, and is not required by anything else in Spark.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19138979
  
    --- Diff: conf/ssl.conf.template ---
    @@ -0,0 +1,27 @@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    Ok, I'll remove it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65323531
  
    Oh yeah - this is still against 1.1. @jacek-lewandowski can you open a new PR and close this one?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19165851
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    I see. Still, by using this env variable, you're assuming the hosts where the executors will run actually have that file available. I don't think we should make that assumption.
    
    Instead, I'd prefer an approach such as this: https://github.com/apache/spark/blob/master/yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnableUtil.scala#L69
    
    Basically, any options needed for the executor to connect back to the driver should be passed on the executor's command line. It also fits well into the "everything should go into SparkConf" model (which requires config options to start with `spark.`, at least when you want system properties to be loaded automatically).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60298244
  
    @vanzin fyi:
    ```
    =========================================================================
    Running Apache RAT checks
    =========================================================================
    Attempting to fetch rat
    Launching rat from /home/jenkins/workspace/SparkPullRequestBuilder/lib/apache-rat-0.10.jar
    Could not find Apache license headers in the following files:
     !????? /home/jenkins/workspace/SparkPullRequestBuilder/conf/ssl.conf.template
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-62344790
  
    @jacek-lewandowski do you have any sense of when you'll be able to do this? It would be great to get this into master soon!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-58932802
  
    I'll go through the discussion and changes in that ticket tomorrow morning, thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65330247
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24059/consoleFull) for   PR 2739 at commit [`f24d854`](https://github.com/apache/spark/commit/f24d854704101d781ab6b15c49caf51c9b4ba9ce).
     * This patch **fails** unit tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60275209
  
    Hi @jacek-lewandowski,
    
    Now that I finally noticed you built this on top of branch-1.1, some of the choices you made make a lot more sense. (I always assume people are working on master, since it's generally preferable to add new features to master first.)
    
    One huge difference in master, which lead to a lot of my comments, is SPARK-2098. That fix added the ability of all daemons - including Master and Worker - to read the spark-defaults.conf file. So, if you build on top of that, you need zero code dealing with loading config data, and can rely on SparkConf for everything. Then, you could have something like:
    
        class SSLOptions(conf: SparkConf, module: String)
    
    That would load options like this:
    
        sslEnabled = conf.getOption(s"spark.$module.ssl.enabled")
          .orElse(conf.getOption("spark.ssl.enabled"))
          .getOrElse(false)
    
    Then you have module-specific configuration and a global fallback. What do you think?
    
    On the subject of distributing the configuration, I think it's sort of ok to rely on that, for the time being, for standalone mode. Long term, it would be better to allow each job to be able to distribute its own configuration, so that it's easy for admins and users to use different certificates for the daemons and for the jobs, for example.
    
    On Yarn, I still believe we should not have this requirement - since when using Spark-on-Yarn, Spark is kind of a client-side thing and shouldn't require any changes in the cluster . The needed files should be distributed automatically by Spark and made available to executors. That should be doable by disabling certificate validation (so that the hostnames don't matter) or using wildcard certificates (assuming everything is in the same sub-domain). If that's not enough to cover all user cases, we can leave other enhancements for later.
    
    I'm not familiar enough with mesos to be able to suggest anything.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65322205
  
    Jenkins, test this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19139582
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    +   * - SPARK_CONF_DIR/ssl.conf
    +   * - SPARK_HOME/conf/ssl.conf
    +   */
    +  def defaultConfigFile: Option[File] = {
    +    val specifiedFile = Option(System.getenv("SPARK_SSL_CONFIG_FILE")).map(new File(_))
    +    val sparkConfDir = Option(System.getenv("SPARK_CONF_DIR")).map(new File(_))
    +    val sparkHomeConfDir = Option(System.getenv("SPARK_HOME"))
    +      .map(new File(_, "conf"))
    +    val defaultFile = (sparkConfDir orElse sparkHomeConfDir).map(new File(_, "ssl.conf"))
    +
    +    specifiedFile orElse defaultFile
    +  }
    +
    +  /**
    +   * Loads the given properties file with failover to empty Properties object.
    +   */
    +  def load(configFile: File): Properties = {
    +    logInfo(s"Loading SSL configuration from $configFile")
    +    try {
    +      val props = new Properties()
    +      val reader = new FileReader(configFile)
    +      try {
    +        props.load(reader)
    +        props.put("sslConfigurationFileLocation", configFile.getAbsolutePath)
    +        props
    +      } finally {
    +        reader.close()
    +      }
    +    } catch {
    +      case ex: Throwable =>
    +        logWarning(s"The SSL configuration file ${configFile.getAbsolutePath} " +
    +          s"could not be loaded. The underlying exception was: ${ex.getMessage}")
    +        new Properties
    +    }
    +  }
    +
    +  /**
    +   * Resolves SSLOptions settings from a given Spark configuration object at a given namespace.
    +   * If SSL settings were loaded from the configuration file, ``sslConfigurationFileLocation``
    +   * property is present in the Spark configuration. The parent directory of that location is used
    +   * as a base directory to resolve relative paths to keystore and truststore.
    +   */
    +  def parse(conf: SparkConf, ns: String): SSLOptions = {
    +    val configFilePath = conf.getOption("sslConfigurationFileLocation")
    --- End diff --
    
    I can do it. However, not using the convention `spark.something.other` was intentional here, because this is a kind of "internal" setting, which shouldn't be confused with the application settings which follow the convention. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60176777
  
    (Seems like that only works for my own PRs, so this will probably need an admin to trigger tests for you...)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60302418
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22080/consoleFull) for   PR 2739 at commit [`f24d854`](https://github.com/apache/spark/commit/f24d854704101d781ab6b15c49caf51c9b4ba9ce).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19244643
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    +
    +  private[spark] val (sslSocketFactory, hostnameVerifier) = if (sslOptions.enabled) {
    +    val trustStoreManagers =
    +      for (trustStore <- sslOptions.trustStore) yield {
    +        val ks = KeyStore.getInstance(KeyStore.getDefaultType)
    +        ks.load(FileUtils.openInputStream(sslOptions.trustStore.get),
    +          sslOptions.trustStorePassword.get.toCharArray)
    +
    +        val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
    +        tmf.init(ks)
    +        tmf.getTrustManagers
    +      }
    +
    +    lazy val credulousTrustStoreManagers = Array({
    +      logWarning("Using 'accept-all' trust manager for SSL connections.")
    +      new X509TrustManager {
    +        override def getAcceptedIssuers: Array[X509Certificate] = null
    +
    +        override def checkClientTrusted(x509Certificates: Array[X509Certificate], s: String) {}
    +
    +        override def checkServerTrusted(x509Certificates: Array[X509Certificate], s: String) {}
    +      }: TrustManager
    +    })
    +
    +    val sslContext = SSLContext.getInstance(sslOptions.protocol.getOrElse("Default"))
    +    sslContext.init(null, trustStoreManagers getOrElse credulousTrustStoreManagers, null)
    +
    +    val hostVerifier = new HostnameVerifier {
    +      override def verify(s: String, sslSession: SSLSession): Boolean = true
    +    }
    +
    +    (Some(sslContext.getSocketFactory), Some(hostVerifier))
    +  } else {
    +    val sslContext = SSLContext.getDefault
    --- End diff --
    
    Good catch, I accidentally left it there


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60300403
  
    You may want to add that file to `.rat-excludes` (unless you plan to rework this on top of master and get rid of the file altogether :-)).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18973684
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    --- End diff --
    
    There are a few options in SslContextFactory that are not covered here, such as:
    
    - setAllowRenegotiate
    - setNeedClientAuth
    - setSslSessionCacheSize
    - setSslSessionTimeout
    - setTrustAll
    - setValidateCerts
    - setValidatePeerCerts
    - setWantClientAuth
    
    How hard would it be to have some sort of bypass from the configuration so that those who want to try these options can? I've done that in the past using BeanUtils, it's a little ugly but it mostly works. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19251575
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    Am I right saying that you understand worker = executor?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60182463
  
    There's still sensitive data that may go in control messages; e.g., IIRC broadcasts go through akka, and those may include things like Hadoop job configuration and delegation tokens. Anyway, I know there are more channels that might need securing, but it's ok to treat those separately. Having a common SSL configuration infrastructure is a good first start.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65278543
  
    @jacek-lewandowski please work on top of master. We can work on backporting it to branch-1.2 if there's a strong desire for it, but new features should always be checked into master first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60297608
  
    @vanzin I'll thing about this for a while.
    
    Still, I don't know why the test fail - I successfully run them locally. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60176200
  
    Jenkins, test this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65280862
  
    @vanzin yeah, thats right
    I tried running the real jobs before rebasing and they worked :)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65314387
  
    I still have got one test failing: 
    ```
    [info] InputOutputMetricsSuite:
    [info] - input metrics when reading text file with single split (34 milliseconds)
    [info] - input metrics when reading text file with multiple splits *** FAILED *** (5 seconds, 250 milliseconds)
    ...  had length 3438 instead of expected length 2 (InputOutputMetricsSuite.scala:78)
    ```
    It fails on master and on my branch - do you know about it - it looks to me as a broken test case. Can you confirm?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65321102
  
    If you don't believe it's your fault, it will be much easier to help if you create the new PR and an admin triggers a jenkins job to test it. Then we can see whether it's a flaky test or a result of your code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60297862
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22079/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19139237
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    --- End diff --
    
    The set of options is the intersection of the options supported by SslContextFactory and Akka transport.
    
    Setting other options, such as the options mentioned by you, would be inconsistent and would make the users feel they enabled them for Spark, not only for Jetty based connections. If this is that important, I can add these options. Nevertheless, I'd display warning message when they are explicitly set. 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60499825
  
    Hi @jacek-lewandowski,
    
    Do you mind re-opening this pull request against the `master` branch, since this is targeted for 1.2?
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19247143
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    --- End diff --
    
    AFAIK, you can select a single protocol, but you can provide multiple encryption algorithms.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60177228
  
    Jenkins, this is ok to test.  Jenkins, retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-60297858
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22079/consoleFull) for   PR 2739 at commit [`77591cb`](https://github.com/apache/spark/commit/77591cb020fd538bef349f56fa485f8a15c5b645).
     * This patch **fails** unit tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-63716956
  
    @jacek-lewandowski are you still working on this? If you don't plan to continue working on this I'd like to pick it up. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19252433
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    So backtrack a bit. I think I might be seeing what you mean. There are two different things:
    
    1. Launcher talking to Spark Master
    
    Other than enabling SSL, I don't see why both need to share *the same* configuration. You can still use different trust stores and certs and even certain different configs, as long as things end up matching (e.g. if the Master is set up to validate certs, launchers will need a certificate recognized by the Master when trying to launch jobs - but not necessarily the *same* certificate).
    
    2. Driver talking to Executors
    
    That's what I've been talking about. I think it's not reasonable to require the SSL configuration to be manually distributed before you can launch jobs, for the reasons I already pointed out.
    
    You may go down the path of having different Akka configs to talk to the Master and for the driver <-> executor communication, but that would require multiple actor systems in the same VM and it would probably require changes in other places where it's assumed both use the same actor system.
    
    So, that being said, do you understand what I'm getting at?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r18975459
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    +
    +  private[spark] val (sslSocketFactory, hostnameVerifier) = if (sslOptions.enabled) {
    +    val trustStoreManagers =
    +      for (trustStore <- sslOptions.trustStore) yield {
    +        val ks = KeyStore.getInstance(KeyStore.getDefaultType)
    +        ks.load(FileUtils.openInputStream(sslOptions.trustStore.get),
    +          sslOptions.trustStorePassword.get.toCharArray)
    +
    +        val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
    +        tmf.init(ks)
    +        tmf.getTrustManagers
    +      }
    +
    +    lazy val credulousTrustStoreManagers = Array({
    --- End diff --
    
    Why lazy if you're using it in the next few lines anyway? Also, shouldn't this be keyed off some option? Some people won't want to disable cert verification.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65216596
  
    Back to working on this... I've rebased against branch-1.2 and then I'll rebase against master if you want.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19249681
  
    --- Diff: core/src/main/scala/org/apache/spark/SSLOptions.scala ---
    @@ -0,0 +1,188 @@
    +/*
    + * 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.spark
    +
    +import java.io.{FileReader, File}
    +import java.nio.file.Paths
    +import java.util.Properties
    +
    +import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
    +import org.apache.commons.io.FilenameUtils
    +import org.eclipse.jetty.util.ssl.SslContextFactory
    +
    +import scala.util.Try
    +
    +case class SSLOptions(enabled: Boolean = false,
    +                      keyStore: Option[File] = None,
    +                      keyStorePassword: Option[String] = None,
    +                      keyPassword: Option[String] = None,
    +                      trustStore: Option[File] = None,
    +                      trustStorePassword: Option[String] = None,
    +                      protocol: Option[String] = None,
    +                      enabledAlgorithms: Set[String] = Set.empty) {
    +
    +  /**
    +   * Creates a Jetty SSL context factory according to the SSL settings represented by this object.
    +   */
    +  def createJettySslContextFactory(): Option[SslContextFactory] = {
    +    if (enabled) {
    +      val sslContextFactory = new SslContextFactory()
    +
    +      keyStore.foreach(file => sslContextFactory.setKeyStorePath(file.getAbsolutePath))
    +      trustStore.foreach(file => sslContextFactory.setTrustStore(file.getAbsolutePath))
    +      keyStorePassword.foreach(sslContextFactory.setKeyStorePassword)
    +      trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
    +      keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
    +      protocol.foreach(sslContextFactory.setProtocol)
    +      sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
    +
    +      Some(sslContextFactory)
    +    } else {
    +      None
    +    }
    +  }
    +
    +  /**
    +   * Creates an Akka configuration object which contains all the SSL settings represented by this
    +   * object. It can be used then to compose the ultimate Akka configuration.
    +   */
    +  def createAkkaConfig: Option[Config] = {
    +    import scala.collection.JavaConversions._
    +    if (enabled) {
    +      Some(ConfigFactory.empty()
    +        .withValue("akka.remote.netty.tcp.security.key-store",
    +          ConfigValueFactory.fromAnyRef(keyStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-store-password",
    +          ConfigValueFactory.fromAnyRef(keyStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store",
    +          ConfigValueFactory.fromAnyRef(trustStore.map(_.getAbsolutePath).getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.trust-store-password",
    +          ConfigValueFactory.fromAnyRef(trustStorePassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.key-password",
    +          ConfigValueFactory.fromAnyRef(keyPassword.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.random-number-generator",
    +          ConfigValueFactory.fromAnyRef(""))
    +        .withValue("akka.remote.netty.tcp.security.protocol",
    +          ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
    +        .withValue("akka.remote.netty.tcp.security.enabled-algorithms",
    +          ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
    +        .withValue("akka.remote.netty.tcp.enable-ssl",
    +          ConfigValueFactory.fromAnyRef(true)))
    +    } else {
    +      None
    +    }
    +  }
    +
    +}
    +
    +object SSLOptions extends Logging {
    +
    +  /**
    +   * Resolves the SSL configuration file location by checking:
    +   * - SPARK_SSL_CONFIG_FILE env variable
    --- End diff --
    
    @vanzin - but how launcher would deal with the workers? Worker do need the SSL stuff before any application is started. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/2739#issuecomment-65280103
  
    I think `dev/run-tests` will run more comprehensive checks. Also, always a good idea to try such a thing yourself on real jobs, aside from the unit tests. :-)
    
    You'll probably need to open a new PR, I don't think Github allows you to change the target branch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3883 SSL support for HttpServer and Akka

Posted by jacek-lewandowski <gi...@git.apache.org>.
Github user jacek-lewandowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2739#discussion_r19244806
  
    --- Diff: core/src/main/scala/org/apache/spark/SecurityManager.scala ---
    @@ -192,6 +196,44 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging {
         )
       }
     
    +  private[spark] val sslOptions = SSLOptions.parse(ns = "ssl", conf = sparkConf)
    +
    +  private[spark] val (sslSocketFactory, hostnameVerifier) = if (sslOptions.enabled) {
    +    val trustStoreManagers =
    +      for (trustStore <- sslOptions.trustStore) yield {
    +        val ks = KeyStore.getInstance(KeyStore.getDefaultType)
    +        ks.load(FileUtils.openInputStream(sslOptions.trustStore.get),
    +          sslOptions.trustStorePassword.get.toCharArray)
    +
    +        val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
    +        tmf.init(ks)
    +        tmf.getTrustManagers
    +      }
    +
    +    lazy val credulousTrustStoreManagers = Array({
    --- End diff --
    
    Because it is conditionally evaluated - in the case when `trustStoreManagers` is undefined


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org