Skip to content

Commit 2ed0c0a

Browse files
tellisonsrowen
authored andcommitted
[SPARK-7756] [CORE] More robust SSL options processing.
Subset the enabled algorithms in an SSLOptions to the elements that are supported by the protocol provider. Update the list of ciphers in the sample config to include modern algorithms, and specify both Oracle and IBM names. In practice the user would either specify their own chosen cipher suites, or specify none, and delegate the decision to the provider. Author: Tim Ellison <t.p.ellison@gmail.com> Closes apache#7043 from tellison/SSLEnhancements and squashes the following commits: 034efa5 [Tim Ellison] Ensure Java imports are grouped and ordered by package. 3797f8b [Tim Ellison] Remove unnecessary use of Option to improve clarity, and fix import style ordering. 4b5c89f [Tim Ellison] More robust SSL options processing.
1 parent 5452457 commit 2ed0c0a

File tree

4 files changed

+85
-23
lines changed

4 files changed

+85
-23
lines changed

core/src/main/scala/org/apache/spark/SSLOptions.scala

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717

1818
package org.apache.spark
1919

20-
import java.io.File
20+
import java.io.{File, FileInputStream}
21+
import java.security.{KeyStore, NoSuchAlgorithmException}
22+
import javax.net.ssl.{KeyManager, KeyManagerFactory, SSLContext, TrustManager, TrustManagerFactory}
2123

2224
import com.typesafe.config.{Config, ConfigFactory, ConfigValueFactory}
2325
import org.eclipse.jetty.util.ssl.SslContextFactory
@@ -38,7 +40,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory
3840
* @param trustStore a path to the trust-store file
3941
* @param trustStorePassword a password to access the trust-store file
4042
* @param protocol SSL protocol (remember that SSLv3 was compromised) supported by Java
41-
* @param enabledAlgorithms a set of encryption algorithms to use
43+
* @param enabledAlgorithms a set of encryption algorithms that may be used
4244
*/
4345
private[spark] case class SSLOptions(
4446
enabled: Boolean = false,
@@ -48,7 +50,8 @@ private[spark] case class SSLOptions(
4850
trustStore: Option[File] = None,
4951
trustStorePassword: Option[String] = None,
5052
protocol: Option[String] = None,
51-
enabledAlgorithms: Set[String] = Set.empty) {
53+
enabledAlgorithms: Set[String] = Set.empty)
54+
extends Logging {
5255

5356
/**
5457
* Creates a Jetty SSL context factory according to the SSL settings represented by this object.
@@ -63,7 +66,7 @@ private[spark] case class SSLOptions(
6366
trustStorePassword.foreach(sslContextFactory.setTrustStorePassword)
6467
keyPassword.foreach(sslContextFactory.setKeyManagerPassword)
6568
protocol.foreach(sslContextFactory.setProtocol)
66-
sslContextFactory.setIncludeCipherSuites(enabledAlgorithms.toSeq: _*)
69+
sslContextFactory.setIncludeCipherSuites(supportedAlgorithms.toSeq: _*)
6770

6871
Some(sslContextFactory)
6972
} else {
@@ -94,14 +97,44 @@ private[spark] case class SSLOptions(
9497
.withValue("akka.remote.netty.tcp.security.protocol",
9598
ConfigValueFactory.fromAnyRef(protocol.getOrElse("")))
9699
.withValue("akka.remote.netty.tcp.security.enabled-algorithms",
97-
ConfigValueFactory.fromIterable(enabledAlgorithms.toSeq))
100+
ConfigValueFactory.fromIterable(supportedAlgorithms.toSeq))
98101
.withValue("akka.remote.netty.tcp.enable-ssl",
99102
ConfigValueFactory.fromAnyRef(true)))
100103
} else {
101104
None
102105
}
103106
}
104107

108+
/*
109+
* The supportedAlgorithms set is a subset of the enabledAlgorithms that
110+
* are supported by the current Java security provider for this protocol.
111+
*/
112+
private val supportedAlgorithms: Set[String] = {
113+
var context: SSLContext = null
114+
try {
115+
context = SSLContext.getInstance(protocol.orNull)
116+
/* The set of supported algorithms does not depend upon the keys, trust, or
117+
rng, although they will influence which algorithms are eventually used. */
118+
context.init(null, null, null)
119+
} catch {
120+
case npe: NullPointerException =>
121+
logDebug("No SSL protocol specified")
122+
context = SSLContext.getDefault
123+
case nsa: NoSuchAlgorithmException =>
124+
logDebug(s"No support for requested SSL protocol ${protocol.get}")
125+
context = SSLContext.getDefault
126+
}
127+
128+
val providerAlgorithms = context.getServerSocketFactory.getSupportedCipherSuites.toSet
129+
130+
// Log which algorithms we are discarding
131+
(enabledAlgorithms &~ providerAlgorithms).foreach { cipher =>
132+
logDebug(s"Discarding unsupported cipher $cipher")
133+
}
134+
135+
enabledAlgorithms & providerAlgorithms
136+
}
137+
105138
/** Returns a string representation of this SSLOptions with all the passwords masked. */
106139
override def toString: String = s"SSLOptions{enabled=$enabled, " +
107140
s"keyStore=$keyStore, keyStorePassword=${keyStorePassword.map(_ => "xxx")}, " +

core/src/test/scala/org/apache/spark/SSLOptionsSuite.scala

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.spark
1919

2020
import java.io.File
21+
import javax.net.ssl.SSLContext
2122

2223
import com.google.common.io.Files
2324
import org.apache.spark.util.Utils
@@ -29,16 +30,24 @@ class SSLOptionsSuite extends SparkFunSuite with BeforeAndAfterAll {
2930
val keyStorePath = new File(this.getClass.getResource("/keystore").toURI).getAbsolutePath
3031
val trustStorePath = new File(this.getClass.getResource("/truststore").toURI).getAbsolutePath
3132

33+
// Pick two cipher suites that the provider knows about
34+
val sslContext = SSLContext.getInstance("TLSv1.2")
35+
sslContext.init(null, null, null)
36+
val algorithms = sslContext
37+
.getServerSocketFactory
38+
.getDefaultCipherSuites
39+
.take(2)
40+
.toSet
41+
3242
val conf = new SparkConf
3343
conf.set("spark.ssl.enabled", "true")
3444
conf.set("spark.ssl.keyStore", keyStorePath)
3545
conf.set("spark.ssl.keyStorePassword", "password")
3646
conf.set("spark.ssl.keyPassword", "password")
3747
conf.set("spark.ssl.trustStore", trustStorePath)
3848
conf.set("spark.ssl.trustStorePassword", "password")
39-
conf.set("spark.ssl.enabledAlgorithms",
40-
"TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_CBC_SHA")
41-
conf.set("spark.ssl.protocol", "SSLv3")
49+
conf.set("spark.ssl.enabledAlgorithms", algorithms.mkString(","))
50+
conf.set("spark.ssl.protocol", "TLSv1.2")
4251

4352
val opts = SSLOptions.parse(conf, "spark.ssl")
4453

@@ -52,9 +61,8 @@ class SSLOptionsSuite extends SparkFunSuite with BeforeAndAfterAll {
5261
assert(opts.trustStorePassword === Some("password"))
5362
assert(opts.keyStorePassword === Some("password"))
5463
assert(opts.keyPassword === Some("password"))
55-
assert(opts.protocol === Some("SSLv3"))
56-
assert(opts.enabledAlgorithms ===
57-
Set("TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_RSA_WITH_AES_256_CBC_SHA"))
64+
assert(opts.protocol === Some("TLSv1.2"))
65+
assert(opts.enabledAlgorithms === algorithms)
5866
}
5967

6068
test("test resolving property with defaults specified ") {

core/src/test/scala/org/apache/spark/SSLSampleConfigs.scala

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ object SSLSampleConfigs {
2525
this.getClass.getResource("/untrusted-keystore").toURI).getAbsolutePath
2626
val trustStorePath = new File(this.getClass.getResource("/truststore").toURI).getAbsolutePath
2727

28+
val enabledAlgorithms =
29+
// A reasonable set of TLSv1.2 Oracle security provider suites
30+
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, " +
31+
"TLS_RSA_WITH_AES_256_CBC_SHA256, " +
32+
"TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, " +
33+
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, " +
34+
"TLS_DHE_RSA_WITH_AES_128_CBC_SHA256, " +
35+
// and their equivalent names in the IBM Security provider
36+
"SSL_ECDHE_RSA_WITH_AES_256_CBC_SHA384, " +
37+
"SSL_RSA_WITH_AES_256_CBC_SHA256, " +
38+
"SSL_DHE_RSA_WITH_AES_256_CBC_SHA256, " +
39+
"SSL_ECDHE_RSA_WITH_AES_128_CBC_SHA256, " +
40+
"SSL_DHE_RSA_WITH_AES_128_CBC_SHA256"
41+
2842
def sparkSSLConfig(): SparkConf = {
2943
val conf = new SparkConf(loadDefaults = false)
3044
conf.set("spark.ssl.enabled", "true")
@@ -33,9 +47,8 @@ object SSLSampleConfigs {
3347
conf.set("spark.ssl.keyPassword", "password")
3448
conf.set("spark.ssl.trustStore", trustStorePath)
3549
conf.set("spark.ssl.trustStorePassword", "password")
36-
conf.set("spark.ssl.enabledAlgorithms",
37-
"SSL_RSA_WITH_RC4_128_SHA, SSL_RSA_WITH_DES_CBC_SHA")
38-
conf.set("spark.ssl.protocol", "TLSv1")
50+
conf.set("spark.ssl.enabledAlgorithms", enabledAlgorithms)
51+
conf.set("spark.ssl.protocol", "TLSv1.2")
3952
conf
4053
}
4154

@@ -47,9 +60,8 @@ object SSLSampleConfigs {
4760
conf.set("spark.ssl.keyPassword", "password")
4861
conf.set("spark.ssl.trustStore", trustStorePath)
4962
conf.set("spark.ssl.trustStorePassword", "password")
50-
conf.set("spark.ssl.enabledAlgorithms",
51-
"SSL_RSA_WITH_RC4_128_SHA, SSL_RSA_WITH_DES_CBC_SHA")
52-
conf.set("spark.ssl.protocol", "TLSv1")
63+
conf.set("spark.ssl.enabledAlgorithms", enabledAlgorithms)
64+
conf.set("spark.ssl.protocol", "TLSv1.2")
5365
conf
5466
}
5567

core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,17 @@ class SecurityManagerSuite extends SparkFunSuite {
127127

128128
test("ssl on setup") {
129129
val conf = SSLSampleConfigs.sparkSSLConfig()
130+
val expectedAlgorithms = Set(
131+
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
132+
"TLS_RSA_WITH_AES_256_CBC_SHA256",
133+
"TLS_DHE_RSA_WITH_AES_256_CBC_SHA256",
134+
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
135+
"TLS_DHE_RSA_WITH_AES_128_CBC_SHA256",
136+
"SSL_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
137+
"SSL_RSA_WITH_AES_256_CBC_SHA256",
138+
"SSL_DHE_RSA_WITH_AES_256_CBC_SHA256",
139+
"SSL_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
140+
"SSL_DHE_RSA_WITH_AES_128_CBC_SHA256")
130141

131142
val securityManager = new SecurityManager(conf)
132143

@@ -143,9 +154,8 @@ class SecurityManagerSuite extends SparkFunSuite {
143154
assert(securityManager.fileServerSSLOptions.trustStorePassword === Some("password"))
144155
assert(securityManager.fileServerSSLOptions.keyStorePassword === Some("password"))
145156
assert(securityManager.fileServerSSLOptions.keyPassword === Some("password"))
146-
assert(securityManager.fileServerSSLOptions.protocol === Some("TLSv1"))
147-
assert(securityManager.fileServerSSLOptions.enabledAlgorithms ===
148-
Set("SSL_RSA_WITH_RC4_128_SHA", "SSL_RSA_WITH_DES_CBC_SHA"))
157+
assert(securityManager.fileServerSSLOptions.protocol === Some("TLSv1.2"))
158+
assert(securityManager.fileServerSSLOptions.enabledAlgorithms === expectedAlgorithms)
149159

150160
assert(securityManager.akkaSSLOptions.trustStore.isDefined === true)
151161
assert(securityManager.akkaSSLOptions.trustStore.get.getName === "truststore")
@@ -154,9 +164,8 @@ class SecurityManagerSuite extends SparkFunSuite {
154164
assert(securityManager.akkaSSLOptions.trustStorePassword === Some("password"))
155165
assert(securityManager.akkaSSLOptions.keyStorePassword === Some("password"))
156166
assert(securityManager.akkaSSLOptions.keyPassword === Some("password"))
157-
assert(securityManager.akkaSSLOptions.protocol === Some("TLSv1"))
158-
assert(securityManager.akkaSSLOptions.enabledAlgorithms ===
159-
Set("SSL_RSA_WITH_RC4_128_SHA", "SSL_RSA_WITH_DES_CBC_SHA"))
167+
assert(securityManager.akkaSSLOptions.protocol === Some("TLSv1.2"))
168+
assert(securityManager.akkaSSLOptions.enabledAlgorithms === expectedAlgorithms)
160169
}
161170

162171
test("ssl off setup") {

0 commit comments

Comments
 (0)